From e6ef75ece5727335eff9b1154525cd85d3176811 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Tue, 2 Sep 2025 20:57:02 +0200 Subject: [PATCH 1/3] fix(parser): improve diagnostics around modifier checks --- crates/oxc_parser/src/diagnostics.rs | 27 ++++- crates/oxc_parser/src/js/class.rs | 32 +++++ crates/oxc_parser/src/modifiers.rs | 5 +- crates/oxc_parser/src/ts/statement.rs | 18 ++- crates/oxc_parser/src/ts/types.rs | 14 ++- tasks/coverage/misc/fail/oxc-11713-13.ts | 5 +- tasks/coverage/misc/fail/oxc-11713-23.ts | 3 + tasks/coverage/misc/fail/oxc-11713-24.ts | 3 + tasks/coverage/misc/fail/oxc-11713-25.ts | 3 + tasks/coverage/misc/fail/oxc-11713-26.ts | 1 + tasks/coverage/misc/fail/oxc-11713-27.ts | 1 + tasks/coverage/snapshots/parser_babel.snap | 112 ++++++++++++++++-- tasks/coverage/snapshots/parser_misc.snap | 80 +++++++------ .../coverage/snapshots/parser_typescript.snap | 54 ++++++--- 14 files changed, 280 insertions(+), 78 deletions(-) create mode 100644 tasks/coverage/misc/fail/oxc-11713-23.ts create mode 100644 tasks/coverage/misc/fail/oxc-11713-24.ts create mode 100644 tasks/coverage/misc/fail/oxc-11713-25.ts create mode 100644 tasks/coverage/misc/fail/oxc-11713-26.ts create mode 100644 tasks/coverage/misc/fail/oxc-11713-27.ts diff --git a/crates/oxc_parser/src/diagnostics.rs b/crates/oxc_parser/src/diagnostics.rs index a7bc4ee0d0813..28ef47ae0d598 100644 --- a/crates/oxc_parser/src/diagnostics.rs +++ b/crates/oxc_parser/src/diagnostics.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use oxc_diagnostics::OxcDiagnostic; use oxc_span::Span; -use crate::modifiers::Modifier; +use crate::modifiers::{Modifier, ModifierKind}; #[inline] fn ts_error(code: C, message: M) -> OxcDiagnostic @@ -536,6 +536,20 @@ pub fn modifier_cannot_be_used_here(modifier: &Modifier) -> OxcDiagnostic { .with_label(modifier.span) } +#[cold] +pub fn modifier_only_on_property_declaration_or_index_signature( + modifier: &Modifier, +) -> OxcDiagnostic { + ts_error( + "1024", + format!( + "'{}' modifier can only appear on a property declaration or index signature.", + modifier.kind + ), + ) + .with_label(modifier.span) +} + #[cold] pub fn accessibility_modifier_already_seen(modifier: &Modifier) -> OxcDiagnostic { ts_error("1028", "Accessibility modifier already seen.") @@ -544,8 +558,15 @@ pub fn accessibility_modifier_already_seen(modifier: &Modifier) -> OxcDiagnostic } #[cold] -pub fn export_modifier_must_precede_declare(modifier: &Modifier) -> OxcDiagnostic { - ts_error("1029", "'export' modifier must precede 'declare' modifier.").with_label(modifier.span) +pub fn modifier_must_precede_other_modifier( + modifier: &Modifier, + other_modifier: ModifierKind, +) -> OxcDiagnostic { + ts_error( + "1029", + format!("'{}' modifier must precede '{}' modifier.", modifier.kind, other_modifier), + ) + .with_label(modifier.span) } #[cold] diff --git a/crates/oxc_parser/src/js/class.rs b/crates/oxc_parser/src/js/class.rs index e6201b021d905..7e8723133d0f4 100644 --- a/crates/oxc_parser/src/js/class.rs +++ b/crates/oxc_parser/src/js/class.rs @@ -278,6 +278,23 @@ impl<'a> ParserImpl<'a> { for decorator in decorators { self.error(diagnostics::decorators_are_not_valid_here(decorator.span)); } + + let mut has_seen_readonly_modifier = false; + for modifier in modifiers.iter() { + match modifier.kind { + ModifierKind::Readonly => has_seen_readonly_modifier = true, + ModifierKind::Static => { + if has_seen_readonly_modifier { + self.error(diagnostics::modifier_must_precede_other_modifier( + modifier, + ModifierKind::Readonly, + )); + } + } + _ => self.error(diagnostics::cannot_appear_on_an_index_signature(modifier)), + } + } + return ClassElement::TSIndexSignature( self.parse_index_signature_declaration(span, &modifiers), ); @@ -476,6 +493,21 @@ impl<'a> ParserImpl<'a> { let optional = optional_span.is_some(); if generator || matches!(self.cur_kind(), Kind::LParen | Kind::LAngle) { + for modifier in modifiers.iter() { + match modifier.kind { + ModifierKind::Declare => { + self.error(diagnostics::cannot_appear_on_class_elements(modifier)); + } + ModifierKind::Readonly => { + self.error( + diagnostics::modifier_only_on_property_declaration_or_index_signature( + modifier, + ), + ); + } + _ => {} + } + } return self.parse_method_declaration( span, r#type, generator, name, computed, optional, modifiers, decorators, ); diff --git a/crates/oxc_parser/src/modifiers.rs b/crates/oxc_parser/src/modifiers.rs index 3a1d8ae8247ce..f28694639e056 100644 --- a/crates/oxc_parser/src/modifiers.rs +++ b/crates/oxc_parser/src/modifiers.rs @@ -309,7 +309,10 @@ impl<'a> ParserImpl<'a> { self.bump_any(); let modifier = self.modifier(kind, self.end_span(span)); if modifier.kind == ModifierKind::Export { - self.error(diagnostics::export_modifier_must_precede_declare(&modifier)); + self.error(diagnostics::modifier_must_precede_other_modifier( + &modifier, + ModifierKind::Declare, + )); } self.check_for_duplicate_modifiers(flags, &modifier); flags.set(modifier_flags, true); diff --git a/crates/oxc_parser/src/ts/statement.rs b/crates/oxc_parser/src/ts/statement.rs index 7bdc75514eb98..336201cee3ff2 100644 --- a/crates/oxc_parser/src/ts/statement.rs +++ b/crates/oxc_parser/src/ts/statement.rs @@ -244,6 +244,18 @@ impl<'a> ParserImpl<'a> { /* permit_const_as_modifier */ true, /* stop_on_start_of_class_static_block */ false, ); + + if self.is_index_signature() { + self.verify_modifiers( + &modifiers, + ModifierFlags::READONLY, + diagnostics::cannot_appear_on_an_index_signature, + ); + return TSSignature::TSIndexSignature( + self.parse_index_signature_declaration(span, &modifiers), + ); + } + self.verify_modifiers( &modifiers, ModifierFlags::READONLY, @@ -258,12 +270,6 @@ impl<'a> ParserImpl<'a> { return self.parse_getter_setter_signature_member(span, TSMethodSignatureKind::Set); } - if self.is_index_signature() { - return TSSignature::TSIndexSignature( - self.parse_index_signature_declaration(span, &modifiers), - ); - } - self.parse_property_or_method_signature(span, &modifiers) } diff --git a/crates/oxc_parser/src/ts/types.rs b/crates/oxc_parser/src/ts/types.rs index 518e54f37e959..56c4e96949774 100644 --- a/crates/oxc_parser/src/ts/types.rs +++ b/crates/oxc_parser/src/ts/types.rs @@ -1178,6 +1178,15 @@ impl<'a> ParserImpl<'a> { let optional = self.eat(Kind::Question); if self.at(Kind::LParen) || self.at(Kind::LAngle) { + for modifier in modifiers.iter() { + if modifier.kind == ModifierKind::Readonly { + self.error( + diagnostics::modifier_only_on_property_declaration_or_index_signature( + modifier, + ), + ); + } + } let type_parameters = self.parse_ts_type_parameters(); let (this_param, params) = self .parse_formal_parameters(FunctionKind::Declaration, FormalParameterKind::Signature); @@ -1213,11 +1222,6 @@ impl<'a> ParserImpl<'a> { span: u32, modifiers: &Modifiers<'a>, ) -> Box<'a, TSIndexSignature<'a>> { - self.verify_modifiers( - modifiers, - ModifierFlags::READONLY | ModifierFlags::STATIC, - diagnostics::cannot_appear_on_an_index_signature, - ); self.expect(Kind::LBrack); let (params, comma_span) = self.parse_delimited_list( Kind::RBrack, diff --git a/tasks/coverage/misc/fail/oxc-11713-13.ts b/tasks/coverage/misc/fail/oxc-11713-13.ts index 0565fe2a8f8f9..4e07309ce2c16 100644 --- a/tasks/coverage/misc/fail/oxc-11713-13.ts +++ b/tasks/coverage/misc/fail/oxc-11713-13.ts @@ -1,2 +1,3 @@ -const [index: string]: number -} +interface Foo { + const [index: string]: number +} \ No newline at end of file diff --git a/tasks/coverage/misc/fail/oxc-11713-23.ts b/tasks/coverage/misc/fail/oxc-11713-23.ts new file mode 100644 index 0000000000000..f28bdeb442f8a --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-11713-23.ts @@ -0,0 +1,3 @@ +interface Foo { + readonly method(); +} diff --git a/tasks/coverage/misc/fail/oxc-11713-24.ts b/tasks/coverage/misc/fail/oxc-11713-24.ts new file mode 100644 index 0000000000000..bad13b0ba0196 --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-11713-24.ts @@ -0,0 +1,3 @@ +class Foo { + declare method() {} +} diff --git a/tasks/coverage/misc/fail/oxc-11713-25.ts b/tasks/coverage/misc/fail/oxc-11713-25.ts new file mode 100644 index 0000000000000..48cd98cf9ac63 --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-11713-25.ts @@ -0,0 +1,3 @@ +class Foo { + readonly method() {} +} diff --git a/tasks/coverage/misc/fail/oxc-11713-26.ts b/tasks/coverage/misc/fail/oxc-11713-26.ts new file mode 100644 index 0000000000000..d3231c30751c8 --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-11713-26.ts @@ -0,0 +1 @@ +function foo(readonly parameter) {} diff --git a/tasks/coverage/misc/fail/oxc-11713-27.ts b/tasks/coverage/misc/fail/oxc-11713-27.ts new file mode 100644 index 0000000000000..fe5085465e3fb --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-11713-27.ts @@ -0,0 +1 @@ +class Foo { method(readonly parameter) {} } diff --git a/tasks/coverage/snapshots/parser_babel.snap b/tasks/coverage/snapshots/parser_babel.snap index 473f6387ac65a..36f840d2b30b5 100644 --- a/tasks/coverage/snapshots/parser_babel.snap +++ b/tasks/coverage/snapshots/parser_babel.snap @@ -3,7 +3,7 @@ commit: 41d96516 parser_babel Summary: AST Parsed : 2407/2423 (99.34%) Positive Passed: 2385/2423 (98.43%) -Negative Passed: 1646/1755 (93.79%) +Negative Passed: 1649/1755 (93.96%) Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/core/categorized/invalid-startindex-and-startline-specified-without-startcolumn/input.js Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/core/categorized/startline-and-startcolumn-specified/input.js @@ -86,16 +86,10 @@ Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/ty Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-initializer/input.ts -Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-method/input.ts - Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-readonly-field-initializer-w-annotation/input.ts -Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/index-signature-errors/input.ts - Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/invalid-modifiers-order/input.ts -Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/method-readonly/input.ts - Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/modifiers-incompatible/input.ts Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/modifiers-invalid-order/input.ts @@ -12022,6 +12016,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc · ─── ╰──── + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/estree/typescript/invalid-class-method-empty-body/input.js:4:3] + 3 │ #method(); + 4 │ declare method(); + · ─────── + 5 │ } + ╰──── + × Function implementation is missing or not immediately following the declaration. ╭─[babel/packages/babel-parser/test/fixtures/estree/typescript/invalid-class-method-empty-body/input.js:2:3] 1 │ class C { @@ -12046,6 +12048,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 5 │ } ╰──── + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/estree/typescript/invalid-class-method-empty-body-babel-7/input.js:4:3] + 3 │ #method(); + 4 │ declare method(); + · ─────── + 5 │ } + ╰──── + × Function implementation is missing or not immediately following the declaration. ╭─[babel/packages/babel-parser/test/fixtures/estree/typescript/invalid-class-method-empty-body-babel-7/input.js:2:3] 1 │ class C { @@ -12562,6 +12572,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 4 │ } ╰──── + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/declare-method/input.ts:2:3] + 1 │ class A { + 2 │ declare foo() {} + · ─────── + 3 │ } + ╰──── + × Expected a semicolon or an implicit semicolon after a statement, but found none ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/declare-new-line-abstract/input.ts:1:8] 1 │ declare abstract @@ -12660,6 +12678,22 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 2 │ } ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:6:3] + 5 │ abstract *d() {} + 6 │ readonly *e() {} + · ──────── + 7 │ declare *f() {} + ╰──── + + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:7:3] + 6 │ readonly *e() {} + 7 │ declare *f() {} + · ─────── + 8 │ protected *g() {} + ╰──── + × TS(1245): Method 'd' cannot have an implementation because it is marked abstract. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:5:13] 4 │ static *c() {} @@ -12691,6 +12725,22 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 2 │ } ╰──── + × TS(1029): 'static' modifier must precede 'readonly' modifier. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/index-signature-errors/input.ts:2:14] + 1 │ class C { + 2 │ readonly static [x: string]: any; + · ────── + 3 │ } + ╰──── + + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/method-readonly/input.ts:2:5] + 1 │ class C { + 2 │ readonly m() {} + · ──────── + 3 │ } + ╰──── + × The keyword 'private' is reserved ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/modifier-name-parameters/input.ts:2:15] 1 │ class Foo { @@ -12772,6 +12822,38 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 4 │ } ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:3:3] + 2 │ abstract *d?() { } + 3 │ readonly *e?() { } + · ──────── + 4 │ declare *f?() { } + ╰──── + + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:4:3] + 3 │ readonly *e?() { } + 4 │ declare *f?() { } + · ─────── + 5 │ } + ╰──── + + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:9:3] + 8 │ abstract *[d?.d]?() { } + 9 │ readonly *[e?.e]?() { } + · ──────── + 10 │ declare *[f?.f]?() { } + ╰──── + + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:10:3] + 9 │ readonly *[e?.e]?() { } + 10 │ declare *[f?.f]?() { } + · ─────── + 11 │ } + ╰──── + × TS(1245): Method 'd' cannot have an implementation because it is marked abstract. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:2:13] 1 │ class C { @@ -13433,6 +13515,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 8 │ readonly g(); ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-method/input.ts:8:3] + 7 │ abstract f(); + 8 │ readonly g(); + · ──────── + 9 │ } + ╰──── + × TS(1070): 'private' modifier cannot appear on a type member. ╭─[babel/packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-method-babel-7/input.ts:2:3] 1 │ interface Foo { @@ -13481,6 +13571,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 8 │ readonly g(); ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[babel/packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-method-babel-7/input.ts:8:3] + 7 │ abstract f(); + 8 │ readonly g(); + · ──────── + 9 │ } + ╰──── + × TS(1070): 'private' modifier cannot appear on a type member. ╭─[babel/packages/babel-parser/test/fixtures/typescript/interface/invalid-modifiers-property/input.ts:2:3] 1 │ interface Foo { diff --git a/tasks/coverage/snapshots/parser_misc.snap b/tasks/coverage/snapshots/parser_misc.snap index d1fae1c473f3b..47f4a0e40091b 100644 --- a/tasks/coverage/snapshots/parser_misc.snap +++ b/tasks/coverage/snapshots/parser_misc.snap @@ -1,7 +1,7 @@ parser_misc Summary: AST Parsed : 49/49 (100.00%) Positive Passed: 49/49 (100.00%) -Negative Passed: 78/78 (100.00%) +Negative Passed: 83/83 (100.00%) × Cannot assign to 'arguments' in strict mode ╭─[misc/fail/arguments-eval.ts:1:10] @@ -197,26 +197,12 @@ Negative Passed: 78/78 (100.00%) 3 │ } ╰──── - × Missing initializer in destructuring declaration - ╭─[misc/fail/oxc-11713-13.ts:1:7] - 1 │ const [index: string]: number - · ─────────────────────── - 2 │ } - ╰──── - - × Unexpected token - ╭─[misc/fail/oxc-11713-13.ts:2:1] - 1 │ const [index: string]: number - 2 │ } - · ─ - ╰──── - - × TS(1070): 'const' modifier cannot appear on a type member. - ╭─[misc/fail/oxc-11713-14.ts:2:2] - 1 │ const foo: { - 2 │ const [index: string] : string - · ───── - 3 │ } = {}; + × TS(1071): 'const' modifier cannot appear on an index signature. + ╭─[misc/fail/oxc-11713-13.ts:2:3] + 1 │ interface Foo { + 2 │ const [index: string]: number + · ───── + 3 │ } ╰──── × TS(1071): 'const' modifier cannot appear on an index signature. @@ -259,14 +245,6 @@ Negative Passed: 78/78 (100.00%) 3 │ } ╰──── - × TS(1070): 'default' modifier cannot appear on a type member. - ╭─[misc/fail/oxc-11713-19.ts:2:2] - 1 │ interface Foo { - 2 │ default [index: string]: number - · ─────── - 3 │ } - ╰──── - × TS(1071): 'default' modifier cannot appear on an index signature. ╭─[misc/fail/oxc-11713-19.ts:2:2] 1 │ interface Foo { @@ -283,14 +261,6 @@ Negative Passed: 78/78 (100.00%) 3 │ } ╰──── - × TS(1070): 'default' modifier cannot appear on a type member. - ╭─[misc/fail/oxc-11713-20.ts:2:2] - 1 │ const foo: { - 2 │ default [index: string] : string - · ─────── - 3 │ } = {}; - ╰──── - × TS(1071): 'default' modifier cannot appear on an index signature. ╭─[misc/fail/oxc-11713-20.ts:2:2] 1 │ const foo: { @@ -315,6 +285,42 @@ Negative Passed: 78/78 (100.00%) 3 │ } ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[misc/fail/oxc-11713-23.ts:2:3] + 1 │ interface Foo { + 2 │ readonly method(); + · ──────── + 3 │ } + ╰──── + + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[misc/fail/oxc-11713-24.ts:2:3] + 1 │ class Foo { + 2 │ declare method() {} + · ─────── + 3 │ } + ╰──── + + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[misc/fail/oxc-11713-25.ts:2:3] + 1 │ class Foo { + 2 │ readonly method() {} + · ──────── + 3 │ } + ╰──── + + × A parameter property is only allowed in a constructor implementation. + ╭─[misc/fail/oxc-11713-26.ts:1:14] + 1 │ function foo(readonly parameter) {} + · ────────────────── + ╰──── + + × A parameter property is only allowed in a constructor implementation. + ╭─[misc/fail/oxc-11713-27.ts:1:20] + 1 │ class Foo { method(readonly parameter) {} } + · ────────────────── + ╰──── + × TS(1090): 'private' modifier cannot appear on a parameter. ╭─[misc/fail/oxc-11713-3.ts:1:14] 1 │ function foo(private parameter) {} diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index ac9277903f2b4..c0455902a1ace 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -3,7 +3,7 @@ commit: 261630d6 parser_typescript Summary: AST Parsed : 8814/8816 (99.98%) Positive Passed: 8803/8816 (99.85%) -Negative Passed: 1435/3535 (40.59%) +Negative Passed: 1437/3535 (40.65%) Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment7.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment8.ts @@ -2270,8 +2270,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/classes/p Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts -Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/classes/propertyMemberDeclarations/derivedUninitializedPropertyDeclaration.ts - Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/classes/propertyMemberDeclarations/initializerReferencingConstructorLocals.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/classes/propertyMemberDeclarations/initializerReferencingConstructorParameters.ts @@ -3608,8 +3606,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ec Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration3.ts -Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts - Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/MemberVariableDeclarations/parserMemberVariableDeclaration3.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ModuleDeclarations/parserModuleDeclaration1.d.ts @@ -9972,14 +9968,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 6 │ }; ╰──── - × TS(1070): 'public' modifier cannot appear on a type member. - ╭─[typescript/tests/cases/compiler/modifiersOnInterfaceIndexSignature1.ts:2:3] - 1 │ interface I { - 2 │ public [a: string]: number; - · ────── - 3 │ } - ╰──── - × TS(1071): 'public' modifier cannot appear on an index signature. ╭─[typescript/tests/cases/compiler/modifiersOnInterfaceIndexSignature1.ts:2:3] 1 │ interface I { @@ -16088,6 +16076,22 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 11 │ readonly #quxMethod() { return 3; } // Error ╰──── + × TS(1024): 'readonly' modifier can only appear on a property declaration or index signature. + ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:11:5] + 10 │ protected #bazMethod() { return 3; } // Error + 11 │ readonly #quxMethod() { return 3; } // Error + · ──────── + 12 │ declare #whatMethod() // Error + ╰──── + + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:12:5] + 11 │ readonly #quxMethod() { return 3; } // Error + 12 │ declare #whatMethod() // Error + · ─────── + 13 │ async #asyncMethod() { return 1; } //OK + ╰──── + × TS(18010): An accessibility modifier cannot be used with a private identifier. ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:17:5] 16 │ @@ -16437,6 +16441,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 12 │ }; ╰──── + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/derivedUninitializedPropertyDeclaration.ts:15:5] + 14 │ class BOther extends A { + 15 │ declare m() { return 2 } // not allowed on methods + · ─────── + 16 │ declare nonce: any; // ok, even though it's not in the base + ╰──── + × Function implementation is missing or not immediately following the declaration. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/memberFunctionDeclarations/memberFunctionOverloadMixingStaticAndInstance.ts:3:12] 2 │ foo(); @@ -16593,7 +16605,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 9 │ } ╰──── - × TS(1070): 'static' modifier cannot appear on a type member. + × TS(1071): 'static' modifier cannot appear on an index signature. ╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature4.ts:12:5] 11 │ interface IB { 12 │ static [s: string]: number; @@ -16601,7 +16613,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 13 │ static [s: number]: 42 | 233; ╰──── - × TS(1070): 'static' modifier cannot appear on a type member. + × TS(1071): 'static' modifier cannot appear on an index signature. ╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature4.ts:13:5] 12 │ static [s: string]: number; 13 │ static [s: number]: 42 | 233; @@ -16609,7 +16621,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 14 │ } ╰──── - × TS(1070): 'static' modifier cannot appear on a type member. + × TS(1071): 'static' modifier cannot appear on an index signature. ╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature5.ts:7:5] 6 │ interface I { 7 │ static readonly [s: string]: number; @@ -16617,7 +16629,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 8 │ static readonly [s: number]: 42 | 233 ╰──── - × TS(1070): 'static' modifier cannot appear on a type member. + × TS(1071): 'static' modifier cannot appear on an index signature. ╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature5.ts:8:5] 7 │ static readonly [s: string]: number; 8 │ static readonly [s: number]: 42 | 233 @@ -23542,6 +23554,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── + × TS(1031): 'declare' modifier cannot appear on class elements of this kind. + ╭─[typescript/tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts:2:5] + 1 │ class C { + 2 │ declare Foo() { } + · ─────── + 3 │ } + ╰──── + × Identifier `public` has already been declared ╭─[typescript/tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclarationAmbiguities1.ts:2:3] 1 │ class C { From 070828aef5bb7ebc6dde69839ee9a4f24b19cb13 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Mon, 8 Sep 2025 20:19:42 +0200 Subject: [PATCH 2/3] branch off the more specific TS(1028) diagnostic from the general TS(1030) diagnostic like TypeScript does --- crates/oxc_parser/src/modifiers.rs | 14 ++++---- tasks/coverage/snapshots/parser_babel.snap | 12 +++---- .../coverage/snapshots/parser_typescript.snap | 32 +++++++++---------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/oxc_parser/src/modifiers.rs b/crates/oxc_parser/src/modifiers.rs index f28694639e056..f59af5c472769 100644 --- a/crates/oxc_parser/src/modifiers.rs +++ b/crates/oxc_parser/src/modifiers.rs @@ -471,14 +471,14 @@ impl<'a> ParserImpl<'a> { } fn check_for_duplicate_modifiers(&mut self, seen_flags: ModifierFlags, modifier: &Modifier) { - if seen_flags.contains(modifier.kind.into()) - || (matches!( - modifier.kind, - ModifierKind::Public | ModifierKind::Protected | ModifierKind::Private - ) && seen_flags.intersects( - ModifierFlags::PUBLIC | ModifierFlags::PROTECTED | ModifierFlags::PRIVATE, - )) + if matches!( + modifier.kind, + ModifierKind::Public | ModifierKind::Protected | ModifierKind::Private + ) && seen_flags + .intersects(ModifierFlags::PUBLIC | ModifierFlags::PROTECTED | ModifierFlags::PRIVATE) { + self.error(diagnostics::accessibility_modifier_already_seen(modifier)); + } else if seen_flags.contains(modifier.kind.into()) { self.error(diagnostics::modifier_already_seen(modifier)); } } diff --git a/tasks/coverage/snapshots/parser_babel.snap b/tasks/coverage/snapshots/parser_babel.snap index 36f840d2b30b5..bab61b35bbe73 100644 --- a/tasks/coverage/snapshots/parser_babel.snap +++ b/tasks/coverage/snapshots/parser_babel.snap @@ -12606,7 +12606,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:2:10] 1 │ class Foo { 2 │ public public a; @@ -12615,7 +12615,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:3:11] 2 │ public public a; 3 │ private public b; @@ -12624,7 +12624,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'private' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:4:13] 3 │ private public b; 4 │ protected private c; @@ -12633,7 +12633,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'protected' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:5:10] 4 │ protected private c; 5 │ public protected d; @@ -12642,7 +12642,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'protected' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:6:10] 5 │ public protected d; 6 │ public protected private e; @@ -12651,7 +12651,7 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'private' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/duplicates-accessibility/input.ts:6:20] 5 │ public protected d; 6 │ public protected private e; diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index c0455902a1ace..ff71113812e38 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -6742,7 +6742,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/compiler/constructorArgsErrors3.ts:2:25] 1 │ class foo { 2 │ constructor (public public a: number) { @@ -6751,7 +6751,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/compiler/constructorArgsErrors4.ts:2:26] 1 │ class foo { 2 │ constructor (private public a: number) { @@ -10222,7 +10222,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 2 │ .then((role: Role) => ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/compiler/multipleClassPropertyModifiersErrors.ts:2:9] 1 │ class C { 2 │ public public p1; @@ -10231,7 +10231,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'private' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/compiler/multipleClassPropertyModifiersErrors.ts:3:10] 2 │ public public p1; 3 │ private private p2; @@ -16243,7 +16243,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/accessibilityModifiers.ts:39:13] 38 │ class E { 39 │ private public protected property; @@ -16252,7 +16252,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'protected' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/accessibilityModifiers.ts:39:20] 38 │ class E { 39 │ private public protected property; @@ -16261,7 +16261,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'protected' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/accessibilityModifiers.ts:40:12] 39 │ private public protected property; 40 │ public protected method() { } @@ -16270,7 +16270,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'protected' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/accessibilityModifiers.ts:41:13] 40 │ public protected method() { } 41 │ private protected get getter() { return 0; } @@ -16279,7 +16279,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/classes/propertyMemberDeclarations/accessibilityModifiers.ts:42:12] 41 │ private protected get getter() { return 0; } 42 │ public public set setter(a: number) { } @@ -22470,7 +22470,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration6.ts:2:10] 1 │ class C { 2 │ public public constructor() { } @@ -22479,7 +22479,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'private' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration7.ts:2:10] 1 │ class C { 2 │ public private constructor() { } @@ -23510,7 +23510,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration7.ts:2:12] 1 │ class C { 2 │ public public get Foo() { } @@ -23528,7 +23528,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 3 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration1.ts:2:12] 1 │ class C { 2 │ public public Foo() { } @@ -23618,7 +23618,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 13 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/MemberVariableDeclarations/parserMemberVariableDeclaration1.ts:2:10] 1 │ class C { 2 │ public public Foo; @@ -23831,7 +23831,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 2 │ } ╰──── - × TS(1030): 'public' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/Protected/Protected4.ts:2:13] 1 │ class C { 2 │ protected public m() { } @@ -23840,7 +23840,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc ╰──── help: Remove the duplicate modifier. - × TS(1030): 'private' modifier already seen. + × TS(1028): Accessibility modifier already seen. ╭─[typescript/tests/cases/conformance/parser/ecmascript5/Protected/Protected7.ts:2:13] 1 │ class C { 2 │ protected private m() { } From ac3e7d843eb1892adb06de63adaeb4ec99292070 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Mon, 8 Sep 2025 21:08:26 +0200 Subject: [PATCH 3/3] remove incorrect code to parse modifiers on typescript this parameter --- crates/oxc_parser/src/js/function.rs | 4 +- crates/oxc_parser/src/ts/statement.rs | 7 +-- crates/oxc_parser/src/ts/types.rs | 87 +-------------------------- 3 files changed, 4 insertions(+), 94 deletions(-) diff --git a/crates/oxc_parser/src/js/function.rs b/crates/oxc_parser/src/js/function.rs index 21fe0335490b3..f1d0c825e2355 100644 --- a/crates/oxc_parser/src/js/function.rs +++ b/crates/oxc_parser/src/js/function.rs @@ -49,9 +49,7 @@ impl<'a> ParserImpl<'a> { self.expect(Kind::LParen); let this_param = if self.is_ts && self.at(Kind::This) { let param = self.parse_ts_this_parameter(); - if !self.at(Kind::RParen) { - self.expect(Kind::Comma); - } + self.bump(Kind::Comma); Some(param) } else { None diff --git a/crates/oxc_parser/src/ts/statement.rs b/crates/oxc_parser/src/ts/statement.rs index 336201cee3ff2..e9106f9786491 100644 --- a/crates/oxc_parser/src/ts/statement.rs +++ b/crates/oxc_parser/src/ts/statement.rs @@ -561,14 +561,11 @@ impl<'a> ParserImpl<'a> { pub(crate) fn parse_ts_this_parameter(&mut self) -> TSThisParameter<'a> { let span = self.start_span(); - self.parse_class_element_modifiers(true); - - let this_span = self.start_span(); self.bump_any(); - let this = self.end_span(this_span); + let this_span = self.end_span(span); let type_annotation = self.parse_ts_type_annotation(); - self.ast.ts_this_parameter(self.end_span(span), this, type_annotation) + self.ast.ts_this_parameter(self.end_span(span), this_span, type_annotation) } pub(crate) fn at_start_of_ts_declaration(&mut self) -> bool { diff --git a/crates/oxc_parser/src/ts/types.rs b/crates/oxc_parser/src/ts/types.rs index 56c4e96949774..5c04a117077b8 100644 --- a/crates/oxc_parser/src/ts/types.rs +++ b/crates/oxc_parser/src/ts/types.rs @@ -6,7 +6,7 @@ use oxc_syntax::operator::UnaryOperator; use crate::{ Context, ParserImpl, diagnostics, lexer::Kind, - modifiers::{Modifier, ModifierFlags, ModifierKind, Modifiers}, + modifiers::{ModifierFlags, ModifierKind, Modifiers}, }; use super::{super::js::FunctionKind, statement::CallOrConstructorSignature}; @@ -1274,91 +1274,6 @@ impl<'a> ParserImpl<'a> { } } - pub(crate) fn parse_class_element_modifiers( - &mut self, - is_constructor_parameter: bool, - ) -> Modifiers<'a> { - if !self.is_ts { - return Modifiers::empty(); - } - - let mut flags = ModifierFlags::empty(); - let mut modifiers = None; - - loop { - if !self.is_at_modifier(is_constructor_parameter) { - break; - } - - if let Ok(kind) = ModifierKind::try_from(self.cur_kind()) { - let modifier = Modifier { kind, span: self.cur_token().span() }; - let new_flag = ModifierFlags::from(kind); - if flags.contains(new_flag) { - self.error(diagnostics::accessibility_modifier_already_seen(&modifier)); - } else { - flags.insert(new_flag); - modifiers.get_or_insert_with(|| self.ast.vec()).push(modifier); - } - } else { - break; - } - - self.bump_any(); - } - - Modifiers::new(modifiers, flags) - } - - fn is_at_modifier(&mut self, is_constructor_parameter: bool) -> bool { - if !matches!( - self.cur_kind(), - Kind::Public - | Kind::Protected - | Kind::Private - | Kind::Static - | Kind::Abstract - | Kind::Readonly - | Kind::Declare - | Kind::Override - | Kind::Export - ) { - return false; - } - - let checkpoint = self.checkpoint(); - self.bump_any(); - let next = self.cur_token(); - - if next.is_on_new_line() { - self.rewind(checkpoint); - return false; - } - - let followed_by_any_member = matches!(next.kind(), Kind::PrivateIdentifier | Kind::LBrack) - || next.kind().is_literal_property_name(); - if followed_by_any_member { - self.rewind(checkpoint); - return true; - } - - let followed_by_class_member = !is_constructor_parameter && next.kind() == Kind::Star; - if followed_by_class_member { - self.rewind(checkpoint); - return true; - } - - // allow `...` for error recovery - let followed_by_parameter = is_constructor_parameter - && matches!(next.kind(), Kind::LCurly | Kind::LBrack | Kind::Dot3); - if followed_by_parameter { - self.rewind(checkpoint); - return true; - } - - self.rewind(checkpoint); - false - } - fn parse_js_doc_unknown_or_nullable_type(&mut self) -> TSType<'a> { let span = self.start_span(); self.bump_any(); // bump `?`