From 57d0d623c18a50e22b95b5564f8f6f5d40da366a Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Fri, 3 Oct 2025 19:38:57 +0200 Subject: [PATCH 1/2] fix(parser): forbid abstract class members with implementation in parser instead of semantic --- crates/oxc_parser/src/diagnostics.rs | 27 +++++++ crates/oxc_parser/src/js/class.rs | 24 +++++++ crates/oxc_semantic/src/checker/mod.rs | 1 - crates/oxc_semantic/src/checker/typescript.rs | 71 +------------------ tasks/coverage/misc/fail/oxc-14013.ts | 13 ++++ tasks/coverage/snapshots/parser_babel.snap | 48 ++++++------- tasks/coverage/snapshots/parser_misc.snap | 34 ++++++++- .../coverage/snapshots/parser_typescript.snap | 32 ++++----- 8 files changed, 138 insertions(+), 112 deletions(-) create mode 100644 tasks/coverage/misc/fail/oxc-14013.ts diff --git a/crates/oxc_parser/src/diagnostics.rs b/crates/oxc_parser/src/diagnostics.rs index 9783f989b4cc8..a8bc5a3657a6c 100644 --- a/crates/oxc_parser/src/diagnostics.rs +++ b/crates/oxc_parser/src/diagnostics.rs @@ -691,6 +691,33 @@ pub fn index_signature_type_annotation(span: Span) -> OxcDiagnostic { ts_error("1021", "An index signature must have a type annotation.").with_label(span) } +#[cold] +pub fn abstract_method_cannot_have_implementation(name: &str, span: Span) -> OxcDiagnostic { + ts_error( + "1245", + format!("Method '{name}' cannot have an implementation because it is marked abstract."), + ) + .with_label(span) +} + +#[cold] +pub fn abstract_property_cannot_have_initializer(name: &str, span: Span) -> OxcDiagnostic { + ts_error( + "1267", + format!("Property '{name}' cannot have an initializer because it is marked abstract."), + ) + .with_label(span) +} + +#[cold] +pub fn abstract_accessor_cannot_have_implementation(name: &str, span: Span) -> OxcDiagnostic { + ts_error( + "1318", + format!("Accessor '{name}' cannot have an implementation because it is marked abstract."), + ) + .with_label(span) +} + #[cold] pub fn unexpected_export(span: Span) -> OxcDiagnostic { OxcDiagnostic::error("Unexpected export.").with_label(span) diff --git a/crates/oxc_parser/src/js/class.rs b/crates/oxc_parser/src/js/class.rs index 0a7004d2692cd..91b4a4f5e547d 100644 --- a/crates/oxc_parser/src/js/class.rs +++ b/crates/oxc_parser/src/js/class.rs @@ -614,6 +614,13 @@ impl<'a> ParserImpl<'a> { self.error(diagnostics::static_prototype(span)); } } + if r#abstract && initializer.is_some() { + let (name, span) = name.prop_name().unwrap_or_else(|| { + let span = name.span(); + (&self.source_text[span], span) + }); + self.error(diagnostics::abstract_property_cannot_have_initializer(name, span)); + } self.ast.class_element_property_definition( self.end_span(span), r#type, @@ -690,5 +697,22 @@ impl<'a> ParserImpl<'a> { self.error(diagnostics::ts_constructor_type_parameter(type_sig.span)); } } + if method.r#type.is_abstract() && method.value.body.is_some() { + let (name, span) = method.key.prop_name().unwrap_or_else(|| { + let span = method.key.span(); + (&self.source_text[span], span) + }); + match method.kind { + MethodDefinitionKind::Method => { + self.error(diagnostics::abstract_method_cannot_have_implementation(name, span)); + } + MethodDefinitionKind::Get | MethodDefinitionKind::Set => { + self.error(diagnostics::abstract_accessor_cannot_have_implementation( + name, span, + )); + } + MethodDefinitionKind::Constructor => {} + } + } } } diff --git a/crates/oxc_semantic/src/checker/mod.rs b/crates/oxc_semantic/src/checker/mod.rs index ad188d8ec4e9f..9550c5e56b38d 100644 --- a/crates/oxc_semantic/src/checker/mod.rs +++ b/crates/oxc_semantic/src/checker/mod.rs @@ -81,7 +81,6 @@ pub fn check<'a>(kind: AstKind<'a>, ctx: &SemanticBuilder<'a>) { AstKind::MethodDefinition(method) => { ts::check_method_definition(method, ctx); } - AstKind::PropertyDefinition(prop) => ts::check_property_definition(prop, ctx), AstKind::ObjectProperty(prop) => { ts::check_object_property(prop, ctx); } diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs index e56d1deb3d8ed..09c6d2fd03b97 100644 --- a/crates/oxc_semantic/src/checker/typescript.rs +++ b/crates/oxc_semantic/src/checker/typescript.rs @@ -5,7 +5,7 @@ use rustc_hash::FxHashMap; use oxc_ast::{AstKind, ast::*}; use oxc_diagnostics::OxcDiagnostic; -use oxc_ecmascript::{BoundNames, PropName}; +use oxc_ecmascript::BoundNames; use oxc_span::{Atom, GetSpan, Span}; use crate::{builder::SemanticBuilder, diagnostics::redeclaration}; @@ -289,48 +289,6 @@ fn reserved_type_name(span: Span, reserved_name: &str, syntax_name: &str) -> Oxc ts_error("2414", format!("{syntax_name} name cannot be '{reserved_name}'")).with_label(span) } -fn abstract_element_cannot_have_initializer( - code: &'static str, - elem_name: &str, - prop_name: &str, - span: Span, - init_or_impl: &str, -) -> OxcDiagnostic { - ts_error( - code, - format!( - "{elem_name} '{prop_name}' cannot have an {init_or_impl} because it is marked abstract." - ), - ) - .with_label(span) -} - -/// TS(1245): Method 'foo' cannot have an implementation because it is marked abstract. -fn abstract_method_cannot_have_implementation(method_name: &str, span: Span) -> OxcDiagnostic { - abstract_element_cannot_have_initializer("1245", "Method", method_name, span, "implementation") -} - -/// TS(1267): Property 'foo' cannot have an initializer because it is marked abstract. -fn abstract_property_cannot_have_initializer(prop_name: &str, span: Span) -> OxcDiagnostic { - abstract_element_cannot_have_initializer("1267", "Property", prop_name, span, "initializer") -} - -/// TS(1318): Accessor 'foo' cannot have an implementation because it is marked abstract. -/// -/// Applies to getters/setters -/// -/// > TS's original message, `An abstract accessor cannot have an -/// > implementation.`, is less helpful than the one provided here. -fn abstract_accessor_cannot_have_implementation(accessor_name: &str, span: Span) -> OxcDiagnostic { - abstract_element_cannot_have_initializer( - "1318", - "Accessor", - accessor_name, - span, - "implementation", - ) -} - /// 'abstract' modifier can only appear on a class, method, or property declaration. (1242) fn illegal_abstract_modifier(span: Span) -> OxcDiagnostic { ts_error( @@ -372,23 +330,6 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic // constructors cannot be abstract, no matter what if method.kind.is_constructor() { ctx.error(illegal_abstract_modifier(method.key.span())); - } else if method.value.body.is_some() { - // abstract class elements cannot have bodies or initializers - let (method_name, span) = method.key.prop_name().unwrap_or_else(|| { - let key_span = method.key.span(); - (&ctx.source_text[key_span], key_span) - }); - match method.kind { - MethodDefinitionKind::Method => { - ctx.error(abstract_method_cannot_have_implementation(method_name, span)); - } - MethodDefinitionKind::Get | MethodDefinitionKind::Set => { - ctx.error(abstract_accessor_cannot_have_implementation(method_name, span)); - } - // abstract classes can have concrete methods. Constructors cannot - // have abstract modifiers, but this gets checked during parsing - MethodDefinitionKind::Constructor => {} - } } } @@ -408,16 +349,6 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic } } -pub fn check_property_definition<'a>(prop: &PropertyDefinition<'a>, ctx: &SemanticBuilder<'a>) { - if prop.r#type.is_abstract() && prop.value.is_some() { - let (prop_name, span) = prop.key.prop_name().unwrap_or_else(|| { - let key_span = prop.key.span(); - (&ctx.source_text[key_span], key_span) - }); - ctx.error(abstract_property_cannot_have_initializer(prop_name, span)); - } -} - pub fn check_object_property(prop: &ObjectProperty, ctx: &SemanticBuilder<'_>) { if let Expression::FunctionExpression(func) = &prop.value && prop.kind.is_accessor() diff --git a/tasks/coverage/misc/fail/oxc-14013.ts b/tasks/coverage/misc/fail/oxc-14013.ts new file mode 100644 index 0000000000000..f87be388cf641 --- /dev/null +++ b/tasks/coverage/misc/fail/oxc-14013.ts @@ -0,0 +1,13 @@ +abstract class Foo { + abstract prop = 3; + + abstract method() { } + + abstract get accessor() { + return 1; + } + + abstract set accessor(value: number) { + + } +} \ No newline at end of file diff --git a/tasks/coverage/snapshots/parser_babel.snap b/tasks/coverage/snapshots/parser_babel.snap index e4ef1a0d8c0f6..47d7eaeb3eff1 100644 --- a/tasks/coverage/snapshots/parser_babel.snap +++ b/tasks/coverage/snapshots/parser_babel.snap @@ -12630,6 +12630,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 2 │ } ╰──── + × 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() {} + 5 │ abstract *d() {} + · ─ + 6 │ readonly *e() {} + ╰──── + × 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() {} @@ -12646,14 +12654,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 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() {} - 5 │ abstract *d() {} - · ─ - 6 │ readonly *e() {} - ╰──── - × TS(1244): Abstract methods can only appear within an abstract class. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:5:13] 4 │ static *c() {} @@ -12766,6 +12766,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 4 │ } ╰──── + × 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 { + 2 │ abstract *d?() { } + · ─ + 3 │ readonly *e?() { } + ╰──── + × 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?() { } @@ -12782,6 +12790,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 5 │ } ╰──── + × TS(1245): Method 'd?.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:8:14] + 7 │ class A { + 8 │ abstract *[d?.d]?() { } + · ──── + 9 │ readonly *[e?.e]?() { } + ╰──── + × 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]?() { } @@ -12798,14 +12814,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 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 { - 2 │ abstract *d?() { } - · ─ - 3 │ readonly *e?() { } - ╰──── - × TS(1244): Abstract methods can only appear within an abstract class. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:2:13] 1 │ class C { @@ -12814,14 +12822,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc 3 │ readonly *e?() { } ╰──── - × TS(1245): Method 'd?.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:8:14] - 7 │ class A { - 8 │ abstract *[d?.d]?() { } - · ──── - 9 │ readonly *[e?.e]?() { } - ╰──── - × TS(1244): Abstract methods can only appear within an abstract class. ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:8:14] 7 │ class A { diff --git a/tasks/coverage/snapshots/parser_misc.snap b/tasks/coverage/snapshots/parser_misc.snap index 7146273a8066c..7cf7315778bf9 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: 91/91 (100.00%) +Negative Passed: 92/92 (100.00%) × Cannot assign to 'arguments' in strict mode ╭─[misc/fail/arguments-eval.ts:1:10] @@ -2851,6 +2851,38 @@ Negative Passed: 91/91 (100.00%) · ── ╰──── + × TS(1267): Property 'prop' cannot have an initializer because it is marked abstract. + ╭─[misc/fail/oxc-14013.ts:2:12] + 1 │ abstract class Foo { + 2 │ abstract prop = 3; + · ──── + 3 │ + ╰──── + + × TS(1245): Method 'method' cannot have an implementation because it is marked abstract. + ╭─[misc/fail/oxc-14013.ts:4:12] + 3 │ + 4 │ abstract method() { } + · ────── + 5 │ + ╰──── + + × TS(1318): Accessor 'accessor' cannot have an implementation because it is marked abstract. + ╭─[misc/fail/oxc-14013.ts:6:16] + 5 │ + 6 │ abstract get accessor() { + · ──────── + 7 │ return 1; + ╰──── + + × TS(1318): Accessor 'accessor' cannot have an implementation because it is marked abstract. + ╭─[misc/fail/oxc-14013.ts:10:16] + 9 │ + 10 │ abstract set accessor(value: number) { + · ──────── + 11 │ + ╰──── + × Classes may not have a field named 'constructor' ╭─[misc/fail/oxc-14014.ts:2:12] 1 │ class Bar { diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index 6c234651b948f..62fa5fd68e074 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -14637,14 +14637,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 30 │ ╰──── - × TS(1244): Abstract methods can only appear within an abstract class. - ╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:2:14] - 1 │ class A { - 2 │ abstract foo(); - · ─── - 3 │ } - ╰──── - × TS(1245): Method 'foo' cannot have an implementation because it is marked abstract. ╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14] 5 │ class B { @@ -14653,6 +14645,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 7 │ } ╰──── + × TS(1244): Abstract methods can only appear within an abstract class. + ╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:2:14] + 1 │ class A { + 2 │ abstract foo(); + · ─── + 3 │ } + ╰──── + × TS(1244): Abstract methods can only appear within an abstract class. ╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14] 5 │ class B { @@ -16141,6 +16141,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 29 │ } ╰──── + × TS(1267): Property '#quux' cannot have an initializer because it is marked abstract. + ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:32:14] + 31 │ abstract class B { + 32 │ abstract #quux = 3; // Error + · ───── + 33 │ } + ╰──── + × Getters and setters must have an implementation. ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:25:17] 24 │ readonly set #quxProp(value: number) { } // Error @@ -16157,14 +16165,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc 27 │ async get #asyncProp() { return 1; } // Error ╰──── - × TS(1267): Property '#quux' cannot have an initializer because it is marked abstract. - ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:32:14] - 31 │ abstract class B { - 32 │ abstract #quux = 3; // Error - · ───── - 33 │ } - ╰──── - × Private identifier '#prop' is not allowed outside class bodies ╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesInterfaceExtendingClass.ts:10:7] 9 │ function func(x: I) { From e659c9efb7abfb8665294277f6db8f1ee100f852 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Sat, 4 Oct 2025 12:17:02 +0200 Subject: [PATCH 2/2] clearer names --- tasks/coverage/misc/fail/oxc-14013.ts | 4 ++-- tasks/coverage/snapshots/parser_misc.snap | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tasks/coverage/misc/fail/oxc-14013.ts b/tasks/coverage/misc/fail/oxc-14013.ts index f87be388cf641..63e855d33a3ff 100644 --- a/tasks/coverage/misc/fail/oxc-14013.ts +++ b/tasks/coverage/misc/fail/oxc-14013.ts @@ -3,11 +3,11 @@ abstract class Foo { abstract method() { } - abstract get accessor() { + abstract get getter() { return 1; } - abstract set accessor(value: number) { + abstract set setter(value: number) { } } \ No newline at end of file diff --git a/tasks/coverage/snapshots/parser_misc.snap b/tasks/coverage/snapshots/parser_misc.snap index 7cf7315778bf9..ea25b63ef693b 100644 --- a/tasks/coverage/snapshots/parser_misc.snap +++ b/tasks/coverage/snapshots/parser_misc.snap @@ -2867,19 +2867,19 @@ Negative Passed: 92/92 (100.00%) 5 │ ╰──── - × TS(1318): Accessor 'accessor' cannot have an implementation because it is marked abstract. + × TS(1318): Accessor 'getter' cannot have an implementation because it is marked abstract. ╭─[misc/fail/oxc-14013.ts:6:16] 5 │ - 6 │ abstract get accessor() { - · ──────── + 6 │ abstract get getter() { + · ────── 7 │ return 1; ╰──── - × TS(1318): Accessor 'accessor' cannot have an implementation because it is marked abstract. + × TS(1318): Accessor 'setter' cannot have an implementation because it is marked abstract. ╭─[misc/fail/oxc-14013.ts:10:16] 9 │ - 10 │ abstract set accessor(value: number) { - · ──────── + 10 │ abstract set setter(value: number) { + · ────── 11 │ ╰────