Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
77 changes: 69 additions & 8 deletions src/js_parser/parse/parse_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,6 @@
// "export declare class Foo {}"
opts.is_export = true;
opts.lexical_decl = LexicalDecl::AllowAll;
opts.is_typescript_declare = true;
return p.parse_stmt(opts);
}
}
Expand Down Expand Up @@ -1090,6 +1089,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 +1139,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 +1776,38 @@
}
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 {}"
if opts.is_export {
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);
}
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() {
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);
}
Comment thread
robobun marked this conversation as resolved.
}
js_lexer::TypescriptStmtKeyword::TsStmtGlobal => {
// "declare module 'fs' { global { namespace NodeJS {} } }"
Expand All @@ -1791,6 +1822,15 @@
}
}
js_lexer::TypescriptStmtKeyword::TsStmtDeclare => {
if p.lexer.has_newline_before {
if opts.ts_decorators.is_some() {
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 All @@ -1817,8 +1857,29 @@
}

// "declare const x: any"
let after_declare_range = p.lexer.range();
let scope_index = p.scopes_in_order.len();
let stmt = p.parse_stmt(opts)?;
// Anything that we don't expect is a syntax error ("declare foo",
// "declare interface \n Foo {}", "declare type \n Foo = number").
// esbuild rewinds its lexer and calls `Unexpected()` here; we
// point at the token range captured before recursing instead.

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

View check run for this annotation

Claude / Claude Code Review

New comment exceeds CLAUDE.md 3-line max

nit: this new comment is 4 lines, one over CLAUDE.md rule 13's 3-line max. Easily tightened — e.g. drop the parenthetical example list (already covered by the tests) or merge the last two lines into `// esbuild rewinds its lexer here; we point at the range captured before recursing instead.`
Comment thread
robobun marked this conversation as resolved.
Outdated
if !matches!(
&stmt.data,
js_ast::StmtData::STypeScript(_)
| js_ast::StmtData::SLocal(_)
| js_ast::StmtData::SEmpty(_)
) {
p.log().add_range_error_fmt(
Some(p.source),
after_declare_range,
format_args!(
"Unexpected {}",
bun_core::fmt::quote(p.source.text_for_range(after_declare_range))
),
);
return Err(crate::Error::SyntaxError);
}
if let Some(decs) = &opts.ts_decorators {
p.discard_scopes_up_to(decs.scope_index);
} else {
Expand Down
11 changes: 4 additions & 7 deletions test/bundler/transpiler/scope-mismatch-panic.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import { join } from "path";

describe("scope mismatch panic regression test", () => {
test("should not panic with scope mismatch when arrow function is followed by array literal", async () => {
Expand Down Expand Up @@ -28,7 +29,7 @@ const Layout = () => {
// With the fix, it should fail with a normal ReferenceError for 'app'
await using proc = Bun.spawn({
cmd: [bunExe(), "index.tsx"],
env: bunEnv,
env: { ...bunEnv, NODE_PATH: join(import.meta.dir, "..", "..", "node_modules") },
cwd: String(dir),
stderr: "pipe",
stdout: "pipe",
Expand Down Expand Up @@ -101,13 +102,9 @@ describe("TypeScript 'declare' statements discard scopes of dropped statements",
// Each of these parses a statement after "declare" that records scopes during the
// parse pass and is then dropped. The recorded scopes used to be left behind, so
// visiting the following class statement hit "Scope mismatch while visiting".
// Two earlier cases ("declare foo: bar" and "declare module : es2015") have since
// become parse errors (matching esbuild) and are covered in transpiler.test.js.
Comment thread
robobun marked this conversation as resolved.
Outdated
const cases: [name: string, source: string, expected: string[]][] = [
[
"declare module with an invalid name followed by a class",
"declare module : es2015\nclass Foo {}\n",
["class Foo"],
],
["declare followed by a labeled statement and a class", "declare foo: bar\nclass Foo {}\n", ["class Foo"]],
[
"declare const with an arrow function initializer followed by a class",
"declare const x = () => {};\nclass Foo {}\n",
Expand Down
90 changes: 90 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,96 @@ 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 X" where X is not a valid ambient declaration is rejected, so a
// newline-split keyword cannot leave the remainder as live runtime code.
err("declare interface\nFoo\n{ sideEffect() }", 'Unexpected "interface"');
err("declare abstract\nclass Foo {}", 'Unexpected "abstract"');
err("declare type\nFoo = number", 'Unexpected "type"');
err("declare namespace\nFoo { sideEffect() }", 'Unexpected "namespace"');
err("declare module\nFoo { sideEffect() }", 'Unexpected "module"');
err("declare declare\nlet x = 1", 'Unexpected "declare"');
err("declare foo", 'Unexpected "foo"');
err("declare foo: bar", 'Unexpected "foo"');
err("declare module : es2015", 'Unexpected "module"');
err("export declare interface\nFoo {}", 'Unexpected "interface"');
err("export declare abstract\nclass Foo {}", 'Unexpected "abstract"');
// All valid "declare X" forms still emit nothing.
exp("declare function f(): void", "");
exp("declare class C {}", "");
exp("declare enum E { A }", "");
exp("declare namespace N { let x: number }", "");
exp("declare abstract class C {}", "");
exp("export declare function f(): void", "");
exp("export declare const x: number", "");
// "export abstract \n class" and "export declare \n class" fall through silently like esbuild.
exp("export abstract\nclass Foo {}\nnew Foo", "abstract;\n\nclass Foo {\n}\nnew Foo;\n");
exp("export declare\nclass Foo {}\nnew Foo", "declare;\n\nclass Foo {\n}\nnew Foo;\n");
exp("export declare\nlet x = 1\nuse(x)", "declare;\nlet x = 1;\nuse(x);\n");
// Inside an ambient body the flag is propagated for body semantics, but the whole
// block is erased regardless, so newline-split keywords in the body are harmless.
exp("declare namespace N { abstract\nclass Foo {} }", "");
exp("declare namespace N { declare\nlet x: number }", "");
exp('declare module "m" { abstract\n class Foo {} }', "");
exp("declare global { abstract\nclass Foo {} }\nexport {}", "export {};\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