Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
72 changes: 71 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,73 @@ 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",
)
}

pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &SemanticBuilder<'a>) {
if method.r#type.is_abstract() && method.value.body.is_some() {
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(1245): Method 'constructor' cannot have an implementation because it is marked abstract.
╭─[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
Loading