From 6d46700523bbbdd281071063ac6fb0f2c729e17c Mon Sep 17 00:00:00 2001 From: Cameron Clark Date: Mon, 23 Dec 2024 19:59:42 +0000 Subject: [PATCH] fix(parser): report syntax errors for missing constructor implementations --- crates/oxc_semantic/src/builder.rs | 8 ++ crates/oxc_semantic/src/checker/typescript.rs | 34 +++++++-- .../coverage/snapshots/parser_typescript.snap | 74 ++++++++++++++++--- 3 files changed, 100 insertions(+), 16 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index b1959b9f839f9..9503630f0f2ff 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -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) { diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs index ea4f2ed4502da..a523412d5aba2 100644 --- a/crates/oxc_semantic/src/checker/typescript.rs +++ b/crates/oxc_semantic/src/checker/typescript.rs @@ -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 @@ -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)` diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index 50e1d61e74026..ddeb428b6a434 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -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 @@ -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 @@ -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 @@ -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 { @@ -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 { @@ -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 {