Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/js_parser/parse/parse_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
T::TOpenBracket
| T::TNumericLiteral
| T::TStringLiteral
| T::TAsterisk
| T::TPrivateIdentifier
);
)
|| (p.lexer.token == T::TAsterisk
&& (opts.is_async || (raw != b"get" && raw != b"set")));

// If so, check for a modifier keyword
if could_be_modifier_keyword {
Expand Down Expand Up @@ -417,6 +418,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// https://github.com/oven-sh/bun/issues/1907
if opts.is_class
&& Self::IS_TYPESCRIPT_ENABLED
&& !p.lexer.has_newline_before
&& raw == b"declare"
{
let scope_index = p.scopes_in_order.len();
Expand All @@ -440,6 +442,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
PropertyModifierKeyword::PAbstract => {
if opts.is_class
&& Self::IS_TYPESCRIPT_ENABLED
&& !p.lexer.has_newline_before
&& !opts.is_ts_abstract
&& raw == b"abstract"
{
Expand All @@ -464,6 +467,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
PropertyModifierKeyword::PAccessor => {
// "accessor" keyword for auto-accessor fields (TC39 standard decorators)
if opts.is_class
&& !p.lexer.has_newline_before
&& p.options.features.standard_decorators
&& PropertyModifierKeyword::find(raw)
== Some(PropertyModifierKeyword::PAccessor)
Expand Down
56 changes: 49 additions & 7 deletions src/js_parser/parse/parse_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,7 @@
&& is_identifier
&& (p.lexer.token == T::TClass || opts.ts_decorators.is_some())
&& name == b"abstract"
&& !p.lexer.has_newline_before
Comment thread
robobun marked this conversation as resolved.
&& matches!(expr.data, js_ast::ExprData::EIdentifier(_))
{
let mut stmt_opts = ParseStatementOptions {
Expand Down Expand Up @@ -1139,7 +1140,18 @@
}

// "@decorator export default abstract = 1"
// "@decorator export default abstract \n class Foo {}"
if opts.ts_decorators.is_some() {
if is_identifier
&& name == b"abstract"
&& p.lexer.has_newline_before
&& matches!(expr.data, js_ast::ExprData::EIdentifier(_))
{
let r = js_lexer::range_of_identifier(p.source, expr.loc);
p.log()
.add_range_error(Some(p.source), r, b"Unexpected \"abstract\"");
return Err(crate::Error::SyntaxError);
}
p.lexer.expected(T::TClass)?;
}

Expand Down Expand Up @@ -1765,18 +1777,39 @@
}
js_lexer::TypescriptStmtKeyword::TsStmtInterface => {
// "interface Foo {}"
let mut stmt_opts = ParseStatementOptions {
is_module_scope: opts.is_module_scope,
..Default::default()
};
// "export default interface Foo {}"
// "export default interface \n Foo {}"
if !p.lexer.has_newline_before || opts.is_name_optional {
let mut stmt_opts = ParseStatementOptions {
is_module_scope: opts.is_module_scope,
..Default::default()
};

p.skip_type_script_interface_stmt(&mut stmt_opts)?;
return Ok(Some(p.s(S::TypeScript {}, loc)));
p.skip_type_script_interface_stmt(&mut stmt_opts)?;
return Ok(Some(p.s(S::TypeScript {}, loc)));
}
// "interface \n Foo {}"
// "export interface \n Foo {}"
// "declare interface \n Foo {}"
if opts.is_export || opts.is_typescript_declare {
let r = js_lexer::range_of_identifier(p.source, loc);
p.log()
.add_range_error(Some(p.source), r, b"Unexpected \"interface\"");
return Err(crate::Error::SyntaxError);

Check warning on line 1798 in src/js_parser/parse/parse_stmt.rs

View check run for this annotation

Claude / Claude Code Review

534bc365's per-arm fix misses sibling arms TsStmtType/TsStmtNamespace/TsStmtModule

🟡 nit: 534bc365 added the `is_typescript_declare` error fallthrough here (and to `TsStmtAbstract`/`TsStmtDeclare`) so `declare interface\n…` / `declare abstract\n…` reject instead of leaking live code, but the sibling `TsStmtType` (~1752) and `TsStmtNamespace`/`TsStmtModule` (~1764) arms — which already had `!has_newline_before` gates — were not given the same check. `declare type\nFoo = number` still emits a live `Foo = number;` assignment and `declare namespace\nFoo\n{ sideEffect() }` emits `F
Comment thread
robobun marked this conversation as resolved.
Outdated
}
Comment thread
robobun marked this conversation as resolved.
}
js_lexer::TypescriptStmtKeyword::TsStmtAbstract => {
if p.lexer.token == T::TClass || opts.ts_decorators.is_some() {
if !p.lexer.has_newline_before
&& (p.lexer.token == T::TClass || opts.ts_decorators.is_some())
{
return Ok(Some(p.parse_class_stmt(loc, opts)?));
}
if opts.ts_decorators.is_some() || opts.is_typescript_declare {
let r = js_lexer::range_of_identifier(p.source, loc);
p.log()
.add_range_error(Some(p.source), r, b"Unexpected \"abstract\"");
return Err(crate::Error::SyntaxError);
}

Check warning on line 1812 in src/js_parser/parse/parse_stmt.rs

View check run for this annotation

Claude / Claude Code Review

534bc365's is_typescript_declare guard is over-broad: rejects export declare\n and keyword\n inside ambient bodies

534bc365's new `opts.is_typescript_declare` guard here (and in `TsStmtDeclare` at line 1828) is over-broad: that flag is also set by `t_export`'s `SDeclare` arm before it re-parses the same `declare` token, and by `parse_type_script_namespace_stmt` / the `declare global {}` handler for every body statement. So `export declare\nclass Foo {}` and `declare namespace N { abstract\nclass Foo {} }` / `declare global { abstract\nclass Foo {} }` now error, while esbuild (and this PR's own tested `export
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Outdated
}
js_lexer::TypescriptStmtKeyword::TsStmtGlobal => {
// "declare module 'fs' { global { namespace NodeJS {} } }"
Expand All @@ -1791,6 +1824,15 @@
}
}
js_lexer::TypescriptStmtKeyword::TsStmtDeclare => {
if p.lexer.has_newline_before {
if opts.ts_decorators.is_some() || opts.is_typescript_declare {
let r = js_lexer::range_of_identifier(p.source, loc);
p.log()
.add_range_error(Some(p.source), r, b"Unexpected \"declare\"");
return Err(crate::Error::SyntaxError);
}
return Ok(None);
}
opts.lexical_decl = LexicalDecl::AllowAll;
opts.is_typescript_declare = true;

Expand Down
68 changes: 68 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,74 @@ describe("Bun.Transpiler", () => {
exp("declare class Foo {}", "");
});

it("contextual keywords followed by a newline apply ASI instead of acting as modifiers", () => {
const exp = ts.expectPrinted_;
const err = ts.expectParseError;

// Statement-level "declare": a newline splits into "declare;" + the following declaration.
exp("declare\nfunction foo() { return 1 }\nfoo()", "declare;\nfunction foo() {\n return 1;\n}\nfoo();\n");
exp("declare\nlet x = 1\nuse(x)", "declare;\nlet x = 1;\nuse(x);\n");
exp("declare\nclass Foo {}\nnew Foo", "declare;\n\nclass Foo {\n}\nnew Foo;\n");
exp("declare function foo(): void", "");
exp("declare let x: number", "");

// Statement-level "abstract": a newline splits into "abstract;" + "class Foo {}".
exp("abstract\nclass Foo {}\nnew Foo", "abstract;\n\nclass Foo {\n}\nnew Foo;\n");
exp("abstract class Foo { abstract bar(): void }\nnew Foo", "class Foo {\n}\nnew Foo;\n");

// Statement-level "interface": a newline splits into three statements.
exp("interface\nFoo\n{ sideEffect() }", "interface;\nFoo;\n{\n sideEffect();\n}");
exp("interface Foo { x: number }", "");

// "export interface \n Foo {}" is a syntax error, matching esbuild.
err("export interface\nFoo {}", 'Unexpected "interface"');
// "export default interface \n Foo {}" is allowed (the interface name can be on the next line).
exp("export default interface\nFoo {}", "");
exp("export default interface Foo {}", "");

// "export default abstract \n class A {}" exports the identifier `abstract` and declares A separately.
exp(
"export default abstract\nclass A { foo() { return 1 } }\nnew A",
"export default abstract;\n\nclass A {\n foo() {\n return 1;\n }\n}\nnew A;\n",
);
exp("export default abstract class A {}", "export default class A {\n}");

// Class body "declare": a newline makes it a field named "declare" followed by a method.
exp(
"class Foo { declare\n foo() { return 1 } }\nnew Foo().foo()",
"class Foo {\n declare;\n foo() {\n return 1;\n }\n}\nnew Foo().foo();\n",
);
exp("class Foo { declare foo: number }", "class Foo {\n}");

// Class body "abstract": a newline makes it a field named "abstract" followed by a method.
exp("abstract class A { abstract\n foo() {} }\nnew A", "class A {\n abstract;\n foo() {}\n}\nnew A;\n");
exp("abstract class A { abstract foo(): void }\nnew A", "class A {\n}\nnew A;\n");

// Class body "accessor": a newline makes it a field named "accessor" followed by a field.
exp("class A { accessor\n x = 1 }\nnew A", "class A {\n accessor;\n x = 1;\n}\nnew A;\n");

// Class body "get"/"set" followed by "*": the asterisk starts a generator; the prior word is a field.
exp("class A { get\n *x() {} }\nnew A", "class A {\n get;\n *x() {}\n}\nnew A;\n");
exp("class A { set\n *x() {} }\nnew A", "class A {\n set;\n *x() {}\n}\nnew A;\n");
// "get"/"set" without the generator star still bind to the next key across a newline.
exp("class A { get\n x() { return 1 } }", "class A {\n get x() {\n return 1;\n }\n}");

// "declare <keyword>" with a newline cannot fall through to SExpr because that would
// leave the remainder as live runtime code. Match esbuild and reject instead.
err("declare interface\nFoo\n{ sideEffect() }", 'Unexpected "interface"');
err("declare abstract\nclass Foo {}", 'Unexpected "abstract"');
err("export declare interface\nFoo {}", 'Unexpected "interface"');
err("export declare abstract\nclass Foo {}", 'Unexpected "abstract"');
err("declare declare\nlet x = 1", 'Unexpected "declare"');
// "export abstract \n class" falls through silently like esbuild (export is discarded).
exp("export abstract\nclass Foo {}\nnew Foo", "abstract;\n\nclass Foo {\n}\nnew Foo;\n");

// Decorators before "declare"/"abstract" with a newline must still demand a class.
err("function dec(c){return c}\n@dec declare\nclass Foo {}", 'Unexpected "declare"');
err("function dec(c){return c}\n@dec abstract\nclass Foo {}", 'Unexpected "abstract"');
err("function dec(c){return c}\n@dec export default abstract\nclass Foo {}", 'Unexpected "abstract"');
});

it("does not crash when export default abstract is an expression followed by a class", () => {
const exp = ts.expectPrinted_;
const err = ts.expectParseError;
Expand Down
Loading