Skip to content

Commit

Permalink
feat(semantic): check for abstract initializations and implementations (
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac authored Jul 8, 2024
1 parent 365d9ba commit c4ee9f8
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 13 deletions.
6 changes: 6 additions & 0 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_semantic/src/checker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
84 changes: 83 additions & 1 deletion crates/oxc_semantic/src/checker/typescript.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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));
}
}
94 changes: 89 additions & 5 deletions tasks/coverage/parser_babel.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
87 changes: 81 additions & 6 deletions tasks/coverage/parser_typescript.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {}
Expand Down Expand Up @@ -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 │
Expand Down

0 comments on commit c4ee9f8

Please sign in to comment.