feat(semantic): check for abstract initializations and implementations (#4125)

DonIsaac · web-flow · commit c4ee9f8ec6a6 · 2024-07-08T17:30:16.000-04:00
diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
@@ -1271,6 +1271,12 @@ impl<'a> ClassElement<'a> {
     }
 }
 
+impl PropertyDefinitionType {
+    pub fn is_abstract(&self) -> bool {
+        matches!(self, Self::TSAbstractPropertyDefinition)
+    }
+}
+
 impl MethodDefinitionKind {
     pub fn is_constructor(&self) -> bool {
         matches!(self, Self::Constructor)
diff --git a/crates/oxc_semantic/src/checker/mod.rs b/crates/oxc_semantic/src/checker/mod.rs
@@ -71,7 +71,11 @@ pub fn check<'a>(node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) {
         AstKind::Class(class) => {
             js::check_class(class, node, ctx);
         }
-        AstKind::MethodDefinition(method) => js::check_method_definition(method, ctx),
+        AstKind::MethodDefinition(method) => {
+            js::check_method_definition(method, ctx);
+            ts::check_method_definition(method, ctx);
+        }
+        AstKind::PropertyDefinition(prop) => ts::check_property_definition(prop, ctx),
         AstKind::ObjectProperty(prop) => js::check_object_property(prop, ctx),
         AstKind::Super(sup) => js::check_super(sup, node, ctx),
 
diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs
@@ -1,4 +1,4 @@
-use oxc_ast::syntax_directed_operations::BoundNames;
+use oxc_ast::syntax_directed_operations::{BoundNames, PropName};
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::{ast::*, AstKind};
 use oxc_diagnostics::OxcDiagnostic;
@@ -184,3 +184,85 @@ pub fn check_ts_import_equals_declaration<'a>(
         ctx.error(import_alias_cannot_use_import_type(decl.span));
     }
 }
+
+fn abstract_element_cannot_have_initializer(
+    code: u32,
+    elem_name: &str,
+    prop_name: &str,
+    span: Span,
+    init_or_impl: &str,
+) -> OxcDiagnostic {
+    OxcDiagnostic::error(
+        format!("TS({code}): {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 {
+    OxcDiagnostic::error("TS(1242): 'abstract' modifier can only appear on a class, method, or property declaration.")
+        .with_label(span)
+}
+
+pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &SemanticBuilder<'a>) {
+    if method.r#type.is_abstract() {
+        // 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 => {}
+            }
+            ctx.error(abstract_method_cannot_have_implementation(method_name, span));
+        }
+    }
+}
+
+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));
+    }
+}
diff --git a/tasks/coverage/parser_babel.snap b/tasks/coverage/parser_babel.snap
@@ -3,7 +3,7 @@ commit: 12619ffe
 parser_babel Summary:
 AST Parsed     : 2093/2101 (99.62%)
 Positive Passed: 2083/2101 (99.14%)
-Negative Passed: 1373/1501 (91.47%)
+Negative Passed: 1377/1501 (91.74%)
 Expect Syntax Error: "annex-b/disabled/1.1-html-comments-close/input.js"
 Expect Syntax Error: "annex-b/disabled/3.1-sloppy-labeled-functions/input.js"
 Expect Syntax Error: "annex-b/disabled/3.1-sloppy-labeled-functions-if-body/input.js"
@@ -45,9 +45,6 @@ Expect Syntax Error: "typescript/cast/unparenthesized-type-assertion-and-assign/
 Expect Syntax Error: "typescript/class/abstract-method-in-non-abstract-class-1/input.ts"
 Expect Syntax Error: "typescript/class/abstract-method-in-non-abstract-class-2/input.ts"
 Expect Syntax Error: "typescript/class/abstract-method-in-non-abstract-class-3/input.ts"
-Expect Syntax Error: "typescript/class/abstract-method-with-body/input.ts"
-Expect Syntax Error: "typescript/class/abstract-method-with-body-computed/input.ts"
-Expect Syntax Error: "typescript/class/abstract-property-initializer/input.ts"
 Expect Syntax Error: "typescript/class/constructor-with-invalid-order-modifiers-1/input.ts"
 Expect Syntax Error: "typescript/class/constructor-with-invalid-order-modifiers-2/input.ts"
 Expect Syntax Error: "typescript/class/constructor-with-invalid-order-modifiers-3/input.ts"
@@ -58,7 +55,6 @@ Expect Syntax Error: "typescript/class/declare-field-initializer/input.ts"
 Expect Syntax Error: "typescript/class/declare-initializer/input.ts"
 Expect Syntax Error: "typescript/class/declare-method/input.ts"
 Expect Syntax Error: "typescript/class/declare-readonly-field-initializer-w-annotation/input.ts"
-Expect Syntax Error: "typescript/class/generator-method-with-modifiers/input.ts"
 Expect Syntax Error: "typescript/class/index-signature-errors/input.ts"
 Expect Syntax Error: "typescript/class/invalid-modifiers-order/input.ts"
 Expect Syntax Error: "typescript/class/method-readonly/input.ts"
@@ -9975,6 +9971,78 @@ Expect to Parse: "typescript/types/const-type-parameters-babel-7/input.ts"
  2 │ func<T>(a: T);
    ╰────
 
+  × TS(1245): Method 'method' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/abstract-method-with-body/input.ts:2:12]
+ 1 │ abstract class Foo {
+ 2 │   abstract method() {}
+   ·            ──────
+ 3 │ }
+   ╰────
+
+  × TS(1245): Method 'method' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/abstract-method-with-body/input.ts:2:12]
+ 1 │ abstract class Foo {
+ 2 │   abstract method() {}
+   ·            ──────
+ 3 │ }
+   ╰────
+
+  × TS(1245): Method 'foo()' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/abstract-method-with-body-computed/input.ts:2:13]
+ 1 │ abstract class Foo {
+ 2 │   abstract [foo()]() {}
+   ·             ─────
+ 3 │ }
+   ╰────
+
+  × TS(1245): Method 'foo()' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/abstract-method-with-body-computed/input.ts:2:13]
+ 1 │ abstract class Foo {
+ 2 │   abstract [foo()]() {}
+   ·             ─────
+ 3 │ }
+   ╰────
+
+  × TS(1267): Property 'prop' cannot have an initializer because it is marked abstract.
+   ╭─[typescript/class/abstract-property-initializer/input.ts:2:12]
+ 1 │ abstract class Foo {
+ 2 │   abstract prop = 1
+   ·            ────
+ 3 │   abstract [Bar.foo] = 2
+   ╰────
+
+  × TS(1267): Property 'Bar.foo' cannot have an initializer because it is marked abstract.
+   ╭─[typescript/class/abstract-property-initializer/input.ts:3:13]
+ 2 │   abstract prop = 1
+ 3 │   abstract [Bar.foo] = 2
+   ·             ───────
+ 4 │   abstract [Bar] = 3
+   ╰────
+
+  × TS(1267): Property 'Bar' cannot have an initializer because it is marked abstract.
+   ╭─[typescript/class/abstract-property-initializer/input.ts:4:13]
+ 3 │   abstract [Bar.foo] = 2
+ 4 │   abstract [Bar] = 3
+   ·             ───
+ 5 │   abstract 2 = 4
+   ╰────
+
+  × TS(1267): Property '2' cannot have an initializer because it is marked abstract.
+   ╭─[typescript/class/abstract-property-initializer/input.ts:5:12]
+ 4 │   abstract [Bar] = 3
+ 5 │   abstract 2 = 4
+   ·            ─
+ 6 │   abstract "c" = 5
+   ╰────
+
+  × TS(1267): Property 'c' cannot have an initializer because it is marked abstract.
+   ╭─[typescript/class/abstract-property-initializer/input.ts:6:12]
+ 5 │   abstract 2 = 4
+ 6 │   abstract "c" = 5
+   ·            ───
+ 7 │ }
+   ╰────
+
   × An accessibility modifier cannot be used with a private identifier.
    ╭─[typescript/class/accessor-invalid/input.ts:3:3]
  2 │   declare accessor prop7: number;
@@ -10036,6 +10104,22 @@ Expect to Parse: "typescript/types/const-type-parameters-babel-7/input.ts"
  2 │ }
    ╰────
 
+  × TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/generator-method-with-modifiers/input.ts:5:13]
+ 4 │   static *c() {}
+ 5 │   abstract *d() {}
+   ·             ─
+ 6 │   readonly *e() {}
+   ╰────
+
+  × TS(1245): Method 'd' cannot have an implementation because it is marked abstract.
+   ╭─[typescript/class/generator-method-with-modifiers/input.ts:5:13]
+ 4 │   static *c() {}
+ 5 │   abstract *d() {}
+   ·             ─
+ 6 │   readonly *e() {}
+   ╰────
+
   × Unexpected token
    ╭─[typescript/class/implements-empty/input.ts:1:22]
  1 │ class Foo implements {
diff --git a/tasks/coverage/parser_typescript.snap b/tasks/coverage/parser_typescript.snap
@@ -3,7 +3,7 @@ commit: d8086f14
 parser_typescript Summary:
 AST Parsed     : 5279/5283 (99.92%)
 Positive Passed: 5272/5283 (99.79%)
-Negative Passed: 1085/4875 (22.26%)
+Negative Passed: 1090/4875 (22.36%)
 Expect Syntax Error: "compiler/ClassDeclaration10.ts"
 Expect Syntax Error: "compiler/ClassDeclaration11.ts"
 Expect Syntax Error: "compiler/ClassDeclaration13.ts"
@@ -1952,10 +1952,8 @@ Expect Syntax Error: "conformance/async/es6/functionDeclarations/asyncFunctionDe
 Expect Syntax Error: "conformance/async/es6/functionDeclarations/asyncFunctionDeclaration8_es6.ts"
 Expect Syntax Error: "conformance/asyncGenerators/asyncGeneratorParameterEvaluation.ts"
 Expect Syntax Error: "conformance/classes/awaitAndYieldInProperty.ts"
-Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAccessor.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAssignabilityConstructorFunction.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractClinterfaceAssignability.ts"
-Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractConstructor.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractConstructorAssignability.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractDeclarations.d.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractExtends.ts"
@@ -1967,8 +1965,6 @@ Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractInheritance2.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractInstantiations1.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractInstantiations2.ts"
-Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts"
-Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodWithImplementation.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMixedWithModifiers.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractOverloads.ts"
 Expect Syntax Error: "conformance/classes/classDeclarations/classAbstractKeyword/classAbstractOverrideWithAbstract.ts"
@@ -2118,7 +2114,6 @@ Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/accessorsOv
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty3.ts"
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty4.ts"
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty6.ts"
-Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty7.ts"
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationES2022.ts"
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts"
 Expect Syntax Error: "conformance/classes/propertyMemberDeclarations/autoAccessor1.ts"
@@ -11074,6 +11069,46 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  47 │ }
     ╰────
 
+  × TS(1318): Accessor 'aa' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAccessor.ts:3:17]
+ 2 │    abstract get a();
+ 3 │    abstract get aa() { return 1; } // error
+   ·                 ──
+ 4 │    abstract set b(x: string);
+   ╰────
+
+  × TS(1245): Method 'aa' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAccessor.ts:3:17]
+ 2 │    abstract get a();
+ 3 │    abstract get aa() { return 1; } // error
+   ·                 ──
+ 4 │    abstract set b(x: string);
+   ╰────
+
+  × TS(1318): Accessor 'bb' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAccessor.ts:5:17]
+ 4 │    abstract set b(x: string);
+ 5 │    abstract set bb(x: string) {} // error
+   ·                 ──
+ 6 │ }
+   ╰────
+
+  × TS(1245): Method 'bb' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractAccessor.ts:5:17]
+ 4 │    abstract set b(x: string);
+ 5 │    abstract set bb(x: string) {} // error
+   ·                 ──
+ 6 │ }
+   ╰────
+
+  × TS(1242): 'abstract' modifier can only appear on a class, method, or property declaration.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractConstructor.ts:2:14]
+ 1 │ abstract class A {
+ 2 │     abstract constructor() {}
+   ·              ───────────
+ 3 │ }
+   ╰────
+
   × Unexpected token
    ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractCrashedOnce.ts:8:5]
  7 │         this.
@@ -11114,6 +11149,38 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  18 │ 
     ╰────
 
+  × TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14]
+ 5 │ class B {
+ 6 │     abstract foo() {}
+   ·              ───
+ 7 │ }
+   ╰────
+
+  × TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodInNonAbstractClass.ts:6:14]
+ 5 │ class B {
+ 6 │     abstract foo() {}
+   ·              ───
+ 7 │ }
+   ╰────
+
+  × TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodWithImplementation.ts:2:14]
+ 1 │ abstract class A {
+ 2 │     abstract foo() {}
+   ·              ───
+ 3 │ }
+   ╰────
+
+  × TS(1245): Method 'foo' cannot have an implementation because it is marked abstract.
+   ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMethodWithImplementation.ts:2:14]
+ 1 │ abstract class A {
+ 2 │     abstract foo() {}
+   ·              ───
+ 3 │ }
+   ╰────
+
   × 'abstract' modifier cannot be used here.
    ╭─[conformance/classes/classDeclarations/classAbstractKeyword/classAbstractWithInterface.ts:1:1]
  1 │ abstract interface I {}
@@ -12361,6 +12428,14 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
     ╰────
   help: Remove the duplicate modifier.
 
+  × TS(1267): Property 'p' cannot have an initializer because it is marked abstract.
+   ╭─[conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty7.ts:2:14]
+ 1 │ abstract class A {
+ 2 │     abstract p = 'yep'
+   ·              ─
+ 3 │ }
+   ╰────
+
   × Identifier `accessor` has already been declared
     ╭─[conformance/classes/propertyMemberDeclarations/autoAccessor11.ts:5:12]
   4 │ 

Original file line number	Diff line number	Diff line change
`@@ -1271,6 +1271,12 @@ impl<'a> ClassElement<'a> {`
`1271`	`1271`	`}`
`1272`	`1272`	`}`
`1273`	`1273`
	`1274`	`+impl PropertyDefinitionType {`
	`1275`	`+ pub fn is_abstract(&self) -> bool {`
	`1276`	`+ matches!(self, Self::TSAbstractPropertyDefinition)`
	`1277`	`+ }`
	`1278`	`+}`
	`1279`	`+`
`1274`	`1280`	`impl MethodDefinitionKind {`
`1275`	`1281`	`pub fn is_constructor(&self) -> bool {`
`1276`	`1282`	`matches!(self, Self::Constructor)`