Skip to content

Commit 81a28d4

Browse files
authored
fix(parser): forbid abstract class members with implementation in parser instead of semantic (#14325)
The following four cases are now errors in parser instead of semantic errors. [TS Playground](https://www.typescriptlang.org/play/?#code/IYIwzgLgTsDGEAJYBthjAgYge2wg3gFAIKiQzwIAOU2VCAvAgMwDchxp40ciAtgFMIAC2wATABQBKAggC+HEmR6UA5kITqIEAVGkFOJKEICuUAHYIAjOxILOyiojAaX23RIBuwZCYEAuBHMTPhBdGSISEk4FOSA) for reference. Closes #14013 ```typescript abstract class Foo { abstract prop = 3; abstract method() { } abstract get getter() { return 1; } abstract set setter(value: number) { } } ```
1 parent 07193c2 commit 81a28d4

File tree

8 files changed

+138
-112
lines changed

8 files changed

+138
-112
lines changed

crates/oxc_parser/src/diagnostics.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,33 @@ pub fn index_signature_type_annotation(span: Span) -> OxcDiagnostic {
691691
ts_error("1021", "An index signature must have a type annotation.").with_label(span)
692692
}
693693

694+
#[cold]
695+
pub fn abstract_method_cannot_have_implementation(name: &str, span: Span) -> OxcDiagnostic {
696+
ts_error(
697+
"1245",
698+
format!("Method '{name}' cannot have an implementation because it is marked abstract."),
699+
)
700+
.with_label(span)
701+
}
702+
703+
#[cold]
704+
pub fn abstract_property_cannot_have_initializer(name: &str, span: Span) -> OxcDiagnostic {
705+
ts_error(
706+
"1267",
707+
format!("Property '{name}' cannot have an initializer because it is marked abstract."),
708+
)
709+
.with_label(span)
710+
}
711+
712+
#[cold]
713+
pub fn abstract_accessor_cannot_have_implementation(name: &str, span: Span) -> OxcDiagnostic {
714+
ts_error(
715+
"1318",
716+
format!("Accessor '{name}' cannot have an implementation because it is marked abstract."),
717+
)
718+
.with_label(span)
719+
}
720+
694721
#[cold]
695722
pub fn unexpected_export(span: Span) -> OxcDiagnostic {
696723
OxcDiagnostic::error("Unexpected export.").with_label(span)

crates/oxc_parser/src/js/class.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,13 @@ impl<'a> ParserImpl<'a> {
614614
self.error(diagnostics::static_prototype(span));
615615
}
616616
}
617+
if r#abstract && initializer.is_some() {
618+
let (name, span) = name.prop_name().unwrap_or_else(|| {
619+
let span = name.span();
620+
(&self.source_text[span], span)
621+
});
622+
self.error(diagnostics::abstract_property_cannot_have_initializer(name, span));
623+
}
617624
self.ast.class_element_property_definition(
618625
self.end_span(span),
619626
r#type,
@@ -690,5 +697,22 @@ impl<'a> ParserImpl<'a> {
690697
self.error(diagnostics::ts_constructor_type_parameter(type_sig.span));
691698
}
692699
}
700+
if method.r#type.is_abstract() && method.value.body.is_some() {
701+
let (name, span) = method.key.prop_name().unwrap_or_else(|| {
702+
let span = method.key.span();
703+
(&self.source_text[span], span)
704+
});
705+
match method.kind {
706+
MethodDefinitionKind::Method => {
707+
self.error(diagnostics::abstract_method_cannot_have_implementation(name, span));
708+
}
709+
MethodDefinitionKind::Get | MethodDefinitionKind::Set => {
710+
self.error(diagnostics::abstract_accessor_cannot_have_implementation(
711+
name, span,
712+
));
713+
}
714+
MethodDefinitionKind::Constructor => {}
715+
}
716+
}
693717
}
694718
}

crates/oxc_semantic/src/checker/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ pub fn check<'a>(kind: AstKind<'a>, ctx: &SemanticBuilder<'a>) {
8181
AstKind::MethodDefinition(method) => {
8282
ts::check_method_definition(method, ctx);
8383
}
84-
AstKind::PropertyDefinition(prop) => ts::check_property_definition(prop, ctx),
8584
AstKind::ObjectProperty(prop) => {
8685
ts::check_object_property(prop, ctx);
8786
}

crates/oxc_semantic/src/checker/typescript.rs

Lines changed: 1 addition & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_hash::FxHashMap;
55

66
use oxc_ast::{AstKind, ast::*};
77
use oxc_diagnostics::OxcDiagnostic;
8-
use oxc_ecmascript::{BoundNames, PropName};
8+
use oxc_ecmascript::BoundNames;
99
use oxc_span::{Atom, GetSpan, Span};
1010

1111
use crate::{builder::SemanticBuilder, diagnostics::redeclaration};
@@ -289,48 +289,6 @@ fn reserved_type_name(span: Span, reserved_name: &str, syntax_name: &str) -> Oxc
289289
ts_error("2414", format!("{syntax_name} name cannot be '{reserved_name}'")).with_label(span)
290290
}
291291

292-
fn abstract_element_cannot_have_initializer(
293-
code: &'static str,
294-
elem_name: &str,
295-
prop_name: &str,
296-
span: Span,
297-
init_or_impl: &str,
298-
) -> OxcDiagnostic {
299-
ts_error(
300-
code,
301-
format!(
302-
"{elem_name} '{prop_name}' cannot have an {init_or_impl} because it is marked abstract."
303-
),
304-
)
305-
.with_label(span)
306-
}
307-
308-
/// TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
309-
fn abstract_method_cannot_have_implementation(method_name: &str, span: Span) -> OxcDiagnostic {
310-
abstract_element_cannot_have_initializer("1245", "Method", method_name, span, "implementation")
311-
}
312-
313-
/// TS(1267): Property 'foo' cannot have an initializer because it is marked abstract.
314-
fn abstract_property_cannot_have_initializer(prop_name: &str, span: Span) -> OxcDiagnostic {
315-
abstract_element_cannot_have_initializer("1267", "Property", prop_name, span, "initializer")
316-
}
317-
318-
/// TS(1318): Accessor 'foo' cannot have an implementation because it is marked abstract.
319-
///
320-
/// Applies to getters/setters
321-
///
322-
/// > TS's original message, `An abstract accessor cannot have an
323-
/// > implementation.`, is less helpful than the one provided here.
324-
fn abstract_accessor_cannot_have_implementation(accessor_name: &str, span: Span) -> OxcDiagnostic {
325-
abstract_element_cannot_have_initializer(
326-
"1318",
327-
"Accessor",
328-
accessor_name,
329-
span,
330-
"implementation",
331-
)
332-
}
333-
334292
/// 'abstract' modifier can only appear on a class, method, or property declaration. (1242)
335293
fn illegal_abstract_modifier(span: Span) -> OxcDiagnostic {
336294
ts_error(
@@ -372,23 +330,6 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic
372330
// constructors cannot be abstract, no matter what
373331
if method.kind.is_constructor() {
374332
ctx.error(illegal_abstract_modifier(method.key.span()));
375-
} else if method.value.body.is_some() {
376-
// abstract class elements cannot have bodies or initializers
377-
let (method_name, span) = method.key.prop_name().unwrap_or_else(|| {
378-
let key_span = method.key.span();
379-
(&ctx.source_text[key_span], key_span)
380-
});
381-
match method.kind {
382-
MethodDefinitionKind::Method => {
383-
ctx.error(abstract_method_cannot_have_implementation(method_name, span));
384-
}
385-
MethodDefinitionKind::Get | MethodDefinitionKind::Set => {
386-
ctx.error(abstract_accessor_cannot_have_implementation(method_name, span));
387-
}
388-
// abstract classes can have concrete methods. Constructors cannot
389-
// have abstract modifiers, but this gets checked during parsing
390-
MethodDefinitionKind::Constructor => {}
391-
}
392333
}
393334
}
394335

@@ -408,16 +349,6 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic
408349
}
409350
}
410351

411-
pub fn check_property_definition<'a>(prop: &PropertyDefinition<'a>, ctx: &SemanticBuilder<'a>) {
412-
if prop.r#type.is_abstract() && prop.value.is_some() {
413-
let (prop_name, span) = prop.key.prop_name().unwrap_or_else(|| {
414-
let key_span = prop.key.span();
415-
(&ctx.source_text[key_span], key_span)
416-
});
417-
ctx.error(abstract_property_cannot_have_initializer(prop_name, span));
418-
}
419-
}
420-
421352
pub fn check_object_property(prop: &ObjectProperty, ctx: &SemanticBuilder<'_>) {
422353
if let Expression::FunctionExpression(func) = &prop.value
423354
&& prop.kind.is_accessor()
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
abstract class Foo {
2+
abstract prop = 3;
3+
4+
abstract method() { }
5+
6+
abstract get getter() {
7+
return 1;
8+
}
9+
10+
abstract set setter(value: number) {
11+
12+
}
13+
}

tasks/coverage/snapshots/parser_babel.snap

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12630,6 +12630,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1263012630
2 │ }
1263112631
╰────
1263212632

12633+
× TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
12634+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:5:13]
12635+
4 │ static *c() {}
12636+
5 │ abstract *d() {}
12637+
· ─
12638+
6 │ readonly *e() {}
12639+
╰────
12640+
1263312641
× TS(1024): 'readonly' modifier can only appear on a property declaration or index signature.
1263412642
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:6:3]
1263512643
5 │ abstract *d() {}
@@ -12646,14 +12654,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1264612654
8 │ protected *g() {}
1264712655
╰────
1264812656

12649-
× TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
12650-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:5:13]
12651-
4 │ static *c() {}
12652-
5 │ abstract *d() {}
12653-
· ─
12654-
6 │ readonly *e() {}
12655-
╰────
12656-
1265712657
× TS(1244): Abstract methods can only appear within an abstract class.
1265812658
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/generator-method-with-modifiers/input.ts:5:13]
1265912659
4 │ static *c() {}
@@ -12766,6 +12766,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1276612766
4 │ }
1276712767
╰────
1276812768

12769+
× TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
12770+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:2:13]
12771+
1 │ class C {
12772+
2 │ abstract *d?() { }
12773+
· ─
12774+
3 │ readonly *e?() { }
12775+
╰────
12776+
1276912777
× TS(1024): 'readonly' modifier can only appear on a property declaration or index signature.
1277012778
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:3:3]
1277112779
2 │ abstract *d?() { }
@@ -12782,6 +12790,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1278212790
5 │ }
1278312791
╰────
1278412792

12793+
× TS(1245): Method 'd?.d' cannot have an implementation because it is marked abstract.
12794+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:8:14]
12795+
7 │ class A {
12796+
8 │ abstract *[d?.d]?() { }
12797+
· ────
12798+
9 │ readonly *[e?.e]?() { }
12799+
╰────
12800+
1278512801
× TS(1024): 'readonly' modifier can only appear on a property declaration or index signature.
1278612802
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:9:3]
1278712803
8 │ abstract *[d?.d]?() { }
@@ -12798,14 +12814,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1279812814
11 │ }
1279912815
╰────
1280012816

12801-
× TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
12802-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:2:13]
12803-
1 │ class C {
12804-
2 │ abstract *d?() { }
12805-
· ─
12806-
3 │ readonly *e?() { }
12807-
╰────
12808-
1280912817
× TS(1244): Abstract methods can only appear within an abstract class.
1281012818
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:2:13]
1281112819
1 │ class C {
@@ -12814,14 +12822,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1281412822
3 │ readonly *e?() { }
1281512823
╰────
1281612824

12817-
× TS(1245): Method 'd?.d' cannot have an implementation because it is marked abstract.
12818-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:8:14]
12819-
7 │ class A {
12820-
8 │ abstract *[d?.d]?() { }
12821-
· ────
12822-
9 │ readonly *[e?.e]?() { }
12823-
╰────
12824-
1282512825
× TS(1244): Abstract methods can only appear within an abstract class.
1282612826
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/optional-generator-method-with-invalid-modifiers/input.ts:8:14]
1282712827
7 │ class A {

tasks/coverage/snapshots/parser_misc.snap

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
parser_misc Summary:
22
AST Parsed : 49/49 (100.00%)
33
Positive Passed: 49/49 (100.00%)
4-
Negative Passed: 91/91 (100.00%)
4+
Negative Passed: 92/92 (100.00%)
55

66
× Cannot assign to 'arguments' in strict mode
77
╭─[misc/fail/arguments-eval.ts:1:10]
@@ -2851,6 +2851,38 @@ Negative Passed: 91/91 (100.00%)
28512851
· ──
28522852
╰────
28532853
2854+
× TS(1267): Property 'prop' cannot have an initializer because it is marked abstract.
2855+
╭─[misc/fail/oxc-14013.ts:2:12]
2856+
1 │ abstract class Foo {
2857+
2abstract prop = 3;
2858+
· ────
2859+
3
2860+
╰────
2861+
2862+
× TS(1245): Method 'method' cannot have an implementation because it is marked abstract.
2863+
╭─[misc/fail/oxc-14013.ts:4:12]
2864+
3
2865+
4abstract method() { }
2866+
· ──────
2867+
5
2868+
╰────
2869+
2870+
× TS(1318): Accessor 'getter' cannot have an implementation because it is marked abstract.
2871+
╭─[misc/fail/oxc-14013.ts:6:16]
2872+
5
2873+
6abstract get getter() {
2874+
· ──────
2875+
7 │ return 1;
2876+
╰────
2877+
2878+
× TS(1318): Accessor 'setter' cannot have an implementation because it is marked abstract.
2879+
╭─[misc/fail/oxc-14013.ts:10:16]
2880+
9 │
2881+
10 │ abstract set setter(value: number) {
2882+
· ──────
2883+
11
2884+
╰────
2885+
28542886
× Classes may not have a field named 'constructor'
28552887
╭─[misc/fail/oxc-14014.ts:2:12]
28562888
1class Bar {

tasks/coverage/snapshots/parser_typescript.snap

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14637,14 +14637,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
1463714637
30 │
1463814638
╰────
1463914639

14640-
× TS(1244): Abstract methods can only appear within an abstract class.
14641-
╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:2:14]
14642-
1 │ class A {
14643-
2 │ abstract foo();
14644-
· ───
14645-
3 │ }
14646-
╰────
14647-
1464814640
× TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
1464914641
╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14]
1465014642
5 │ class B {
@@ -14653,6 +14645,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
1465314645
7 │ }
1465414646
╰────
1465514647

14648+
× TS(1244): Abstract methods can only appear within an abstract class.
14649+
╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:2:14]
14650+
1 │ class A {
14651+
2 │ abstract foo();
14652+
· ───
14653+
3 │ }
14654+
╰────
14655+
1465614656
× TS(1244): Abstract methods can only appear within an abstract class.
1465714657
╭─[typescript/tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14]
1465814658
5 │ class B {
@@ -16141,6 +16141,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
1614116141
29 │ }
1614216142
╰────
1614316143

16144+
× TS(1267): Property '#quux' cannot have an initializer because it is marked abstract.
16145+
╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:32:14]
16146+
31 │ abstract class B {
16147+
32 │ abstract #quux = 3; // Error
16148+
· ─────
16149+
33 │ }
16150+
╰────
16151+
1614416152
× Getters and setters must have an implementation.
1614516153
╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:25:17]
1614616154
24 │ readonly set #quxProp(value: number) { } // Error
@@ -16157,14 +16165,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
1615716165
27 │ async get #asyncProp() { return 1; } // Error
1615816166
╰────
1615916167

16160-
× TS(1267): Property '#quux' cannot have an initializer because it is marked abstract.
16161-
╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesIncompatibleModifiers.ts:32:14]
16162-
31 │ abstract class B {
16163-
32 │ abstract #quux = 3; // Error
16164-
· ─────
16165-
33 │ }
16166-
╰────
16167-
1616816168
× Private identifier '#prop' is not allowed outside class bodies
1616916169
╭─[typescript/tests/cases/conformance/classes/members/privateNames/privateNamesInterfaceExtendingClass.ts:10:7]
1617016170
9 │ function func(x: I) {

0 commit comments

Comments
 (0)