Skip to content

Commit

Permalink
fix(parser): report syntax errors for missing constructor implementat…
Browse files Browse the repository at this point in the history 
…ions
  • Loading branch information
@camc314
camc314 committed Dec 23, 2024
1 parent 2736657 commit 6d46700
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 16 deletions.
8 changes: 8 additions & 0 deletions crates/oxc_semantic/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,14 @@ impl<'a> SemanticBuilder<'a> {
self.errors.borrow_mut().push(error);
}

pub(crate) fn in_declare_scope(&self) -> bool {
self.source_type.is_typescript_definition()
|| self
.scope
.ancestors(self.current_scope_id)
.any(|scope_id| self.scope.get_flags(scope_id).is_ts_module_block())
}

fn create_ast_node(&mut self, kind: AstKind<'a>) {
let mut flags = self.current_node_flags;
if self.build_jsdoc && self.jsdoc.retrieve_attached_jsdoc(&kind) {
Expand Down
34 changes: 27 additions & 7 deletions crates/oxc_semantic/src/checker/typescript.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,21 +404,23 @@ fn accessor_without_body(span: Span) -> OxcDiagnostic {
OxcDiagnostic::error("Getters and setters must have an implementation.").with_label(span)
}

fn constructor_implementation_missing(span: Span) -> OxcDiagnostic {
OxcDiagnostic::error("Constructor implementation is missing.").with_label(span)
}

pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &SemanticBuilder<'a>) {
let is_abstract = method.r#type.is_abstract();
let is_declare = ctx.class_table_builder.current_class_id.map_or(
ctx.source_type.is_typescript_definition(),
|id| {
let is_declare =
ctx.class_table_builder.current_class_id.map_or(ctx.in_declare_scope(), |id| {
let node_id = ctx.class_table_builder.classes.declarations[id];
let AstKind::Class(class) = ctx.nodes.get_node(node_id).kind() else {
#[cfg(debug_assertions)]
panic!("current_class_id is set, but does not point to a Class node.");
#[cfg(not(debug_assertions))]
return ctx.source_type.is_typescript_definition();
return ctx.in_declare_scope();
};
class.declare || ctx.source_type.is_typescript_definition()
},
);
class.declare || ctx.in_declare_scope()
});

if is_abstract {
// constructors cannot be abstract, no matter what
Expand Down Expand Up @@ -453,6 +455,24 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic
ctx.error(parameter_property_only_in_constructor_impl(param.span));
}
}

if !is_declare
&& ctx.class_table_builder.current_class_id.map_or(false, |id| {
let node_id = ctx.class_table_builder.classes.declarations[id];
let AstKind::Class(class) = ctx.nodes.get_node(node_id).kind() else {
#[cfg(debug_assertions)]
panic!("current_class_id is set, but does not point to a Class node.");
#[cfg(not(debug_assertions))]
return false;
};
let next = class.body.body.iter().skip_while(|m| m.span() != method.span).nth(1);
next.map_or(true, |m| {
m.method_definition_kind().map_or(true, |c| !c.is_constructor())
})
})
{
ctx.error(constructor_implementation_missing(method.key.span()));
}
}

// Illegal to have `get foo();` or `set foo(a)`
Expand Down
74 changes: 65 additions & 9 deletions tasks/coverage/snapshots/parser_typescript.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ commit: d85767ab
parser_typescript Summary:
AST Parsed : 6494/6503 (99.86%)
Positive Passed: 6483/6503 (99.69%)
Negative Passed: 1240/5747 (21.58%)
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration10.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration11.ts
Negative Passed: 1248/5747 (21.72%)
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration13.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration14.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration15.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration21.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration22.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration24.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration25.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration8.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration9.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment7.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment8.ts
Expand Down Expand Up @@ -3723,11 +3719,8 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ec
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClass1.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClass2.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration1.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration10.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration11.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration12.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration13.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration14.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration15.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration18.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration2.ts
Expand All @@ -3740,7 +3733,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ec
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration5.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration6.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration7.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration8.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration9.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ComputedPropertyNames/parserES5ComputedPropertyName1.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/parser/ecmascript5/ComputedPropertyNames/parserES5ComputedPropertyName10.ts
Expand Down Expand Up @@ -4827,6 +4819,30 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
· ────────────────
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/compiler/ClassDeclaration10.ts:2:4]
1 │ class C {
2 │ constructor();
· ───────────
3 │ foo();
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/compiler/ClassDeclaration11.ts:2:4]
1 │ class C {
2 │ constructor();
· ───────────
3 │ foo() { }
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/compiler/ClassDeclaration14.ts:3:4]
2 │ foo();
3 │ constructor();
· ───────────
4 │ }
╰────

× TS(1248): A class member cannot have the 'const' keyword.
╭─[typescript/tests/cases/compiler/ClassDeclaration26.ts:2:18]
1 │ class C {
Expand All @@ -4845,6 +4861,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
╰────
help: Try insert a semicolon here

× Constructor implementation is missing.
╭─[typescript/tests/cases/compiler/ClassDeclaration8.ts:2:3]
1 │ class C {
2 │ constructor();
· ───────────
3 │ }
╰────

× TS(1248): A class member cannot have the 'const' keyword.
╭─[typescript/tests/cases/compiler/ClassDeclarationWithInvalidConstOnPropertyDeclaration.ts:2:16]
1 │ class AtomicNumbers {
Expand Down Expand Up @@ -20186,6 +20210,38 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
· ╰── `,` expected
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration10.ts:2:4]
1 │ class C {
2 │ constructor();
· ───────────
3 │ foo();
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration11.ts:2:4]
1 │ class C {
2 │ constructor();
· ───────────
3 │ foo() { }
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration14.ts:3:4]
2 │ foo();
3 │ constructor();
· ───────────
4 │ }
╰────

× Constructor implementation is missing.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/ClassDeclarations/parserClassDeclaration8.ts:2:3]
1 │ class C {
2 │ constructor();
· ───────────
3 │ }
╰────

× TS(1164): Computed property names are not allowed in enums.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/ComputedPropertyNames/parserES5ComputedPropertyName6.ts:2:4]
1 │ enum E {
Expand Down

0 comments on commit 6d46700

Please sign in to comment.