From c8682e9fcbec3db2da2b5de530684e76557fceb9 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Tue, 24 Sep 2024 08:32:05 +0000 Subject: [PATCH] fix(semantic,codegen,transformer): handle definite `!` operator in variable declarator (#6019) closes #5999 --- crates/oxc_codegen/src/gen.rs | 31 ++++++++++--- .../oxc_isolated_declarations/src/module.rs | 2 +- crates/oxc_semantic/src/checker/typescript.rs | 46 +++++++++++++++---- crates/oxc_transformer/src/lib.rs | 8 ++++ .../src/typescript/annotations.rs | 8 ++++ crates/oxc_transformer/src/typescript/mod.rs | 8 ++++ .../coverage/snapshots/parser_typescript.snap | 27 ++++++++++- 7 files changed, 110 insertions(+), 20 deletions(-) diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs index 7ed2ab60584e0..87bc69ef18c0a 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -588,7 +588,18 @@ impl<'a> Gen for VariableDeclaration<'a> { impl<'a> Gen for VariableDeclarator<'a> { fn gen(&self, p: &mut Codegen, ctx: Context) { - self.id.print(p, ctx); + self.id.kind.print(p, ctx); + if self.definite { + p.print_char(b'!'); + } + if self.id.optional { + p.print_str("?"); + } + if let Some(type_annotation) = &self.id.type_annotation { + p.print_colon(); + p.print_soft_space(); + type_annotation.print(p, ctx); + } if let Some(init) = &self.init { p.print_soft_space(); p.print_equal(); @@ -2585,12 +2596,7 @@ impl<'a> Gen for PrivateIdentifier<'a> { impl<'a> Gen for BindingPattern<'a> { fn gen(&self, p: &mut Codegen, ctx: Context) { - match &self.kind { - BindingPatternKind::BindingIdentifier(ident) => ident.print(p, ctx), - BindingPatternKind::ObjectPattern(pattern) => pattern.print(p, ctx), - BindingPatternKind::ArrayPattern(pattern) => pattern.print(p, ctx), - BindingPatternKind::AssignmentPattern(pattern) => pattern.print(p, ctx), - } + self.kind.print(p, ctx); if self.optional { p.print_str("?"); } @@ -2602,6 +2608,17 @@ impl<'a> Gen for BindingPattern<'a> { } } +impl<'a> Gen for BindingPatternKind<'a> { + fn gen(&self, p: &mut Codegen, ctx: Context) { + match self { + BindingPatternKind::BindingIdentifier(ident) => ident.print(p, ctx), + BindingPatternKind::ObjectPattern(pattern) => pattern.print(p, ctx), + BindingPatternKind::ArrayPattern(pattern) => pattern.print(p, ctx), + BindingPatternKind::AssignmentPattern(pattern) => pattern.print(p, ctx), + } + } +} + impl<'a> Gen for ObjectPattern<'a> { fn gen(&self, p: &mut Codegen, ctx: Context) { p.add_source_mapping(self.span.start); diff --git a/crates/oxc_isolated_declarations/src/module.rs b/crates/oxc_isolated_declarations/src/module.rs index cfe3a72116597..dbe12767bc3b2 100644 --- a/crates/oxc_isolated_declarations/src/module.rs +++ b/crates/oxc_isolated_declarations/src/module.rs @@ -67,7 +67,7 @@ impl<'a> IsolatedDeclarations<'a> { let id = self.ast.binding_pattern(id, type_annotation, false); let declarations = - self.ast.vec1(self.ast.variable_declarator(SPAN, kind, id, None, true)); + self.ast.vec1(self.ast.variable_declarator(SPAN, kind, id, None, false)); Some(( Some(self.ast.variable_declaration( diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs index 4290b996cbf6a..9e26b01d20995 100644 --- a/crates/oxc_semantic/src/checker/typescript.rs +++ b/crates/oxc_semantic/src/checker/typescript.rs @@ -54,26 +54,52 @@ fn unexpected_optional(span: Span, type_annotation: Option<&str>) -> OxcDiagnost } } -#[allow(clippy::cast_possible_truncation)] +#[expect(clippy::cast_possible_truncation)] +fn find_char(span: Span, source_text: &str, c: char) -> Option { + let Some(offset) = span.source_text(source_text).find(c) else { + debug_assert!( + false, + "Flag {c} not found in source text. This is likely indicates a bug in the parser.", + ); + return None; + }; + let offset = span.start + offset as u32; + Some(Span::new(offset, offset)) +} + pub fn check_variable_declarator(decl: &VariableDeclarator, ctx: &SemanticBuilder<'_>) { // Check for `let x?: number;` if decl.id.optional { // NOTE: BindingPattern spans cover the identifier _and_ the type annotation. - let start = decl.id.span().start; - let Some(offset) = ctx.source_text[start as usize..].find('?') else { - debug_assert!(false, "Optional flag not found in source text. This is likely indicates a bug in the parser."); - return; - }; - let offset = start + offset as u32; - let span = Span::new(offset, offset); let ty = decl .id .type_annotation .as_ref() .map(|ty| ty.type_annotation.span()) .map(|span| &ctx.source_text[span]); - - ctx.error(unexpected_optional(span, ty)); + if let Some(span) = find_char(decl.span, ctx.source_text, '?') { + ctx.error(unexpected_optional(span, ty)); + } + } + if decl.definite { + // Check for `let x!: number = 1;` + // ^ + let Some(span) = find_char(decl.span, ctx.source_text, '!') else { return }; + if decl.init.is_some() { + let error = ts_error( + "1263", + "Declarations with initializers cannot also have definite assignment assertions.", + ) + .with_label(span); + ctx.error(error); + } else if decl.id.type_annotation.is_none() { + let error = ts_error( + "1264", + "Declarations with definite assignment assertions must also have type annotations.", + ) + .with_label(span); + ctx.error(error); + } } } diff --git a/crates/oxc_transformer/src/lib.rs b/crates/oxc_transformer/src/lib.rs index d22ac9f4b4b9a..539da05fad605 100644 --- a/crates/oxc_transformer/src/lib.rs +++ b/crates/oxc_transformer/src/lib.rs @@ -148,6 +148,14 @@ impl<'a> Traverse<'a> for Transformer<'a> { self.x0_typescript.enter_arrow_function_expression(arrow, ctx); } + fn enter_variable_declarator( + &mut self, + decl: &mut VariableDeclarator<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + self.x0_typescript.enter_variable_declarator(decl, ctx); + } + fn enter_binding_pattern(&mut self, pat: &mut BindingPattern<'a>, ctx: &mut TraverseCtx<'a>) { self.x0_typescript.enter_binding_pattern(pat, ctx); } diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index d113affa3b030..29650080df383 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -165,6 +165,14 @@ impl<'a> Traverse<'a> for TypeScriptAnnotations<'a> { expr.return_type = None; } + fn enter_variable_declarator( + &mut self, + decl: &mut VariableDeclarator<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { + decl.definite = false; + } + fn enter_binding_pattern(&mut self, pat: &mut BindingPattern<'a>, _ctx: &mut TraverseCtx<'a>) { pat.type_annotation = None; diff --git a/crates/oxc_transformer/src/typescript/mod.rs b/crates/oxc_transformer/src/typescript/mod.rs index 609a62f118f04..3249610c77b14 100644 --- a/crates/oxc_transformer/src/typescript/mod.rs +++ b/crates/oxc_transformer/src/typescript/mod.rs @@ -94,6 +94,14 @@ impl<'a> Traverse<'a> for TypeScript<'a> { self.annotations.enter_arrow_function_expression(expr, ctx); } + fn enter_variable_declarator( + &mut self, + decl: &mut VariableDeclarator<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + self.annotations.enter_variable_declarator(decl, ctx); + } + fn enter_binding_pattern(&mut self, pat: &mut BindingPattern<'a>, ctx: &mut TraverseCtx<'a>) { self.annotations.enter_binding_pattern(pat, ctx); } diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index 601ddd2c60b65..64a6037b4b45b 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -3,7 +3,7 @@ commit: a709f989 parser_typescript Summary: AST Parsed : 6469/6479 (99.85%) Positive Passed: 6456/6479 (99.65%) -Negative Passed: 1225/5715 (21.43%) +Negative Passed: 1226/5715 (21.45%) Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration10.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration11.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration13.ts @@ -2542,7 +2542,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFl Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/controlFlowOptionalChain3.tsx Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/controlFlowTypeofObject.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/controlFlowWhileStatement.ts -Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/definiteAssignmentAssertions.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/exhaustiveSwitchStatements1.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/controlFlow/neverReturningFunctions1.ts @@ -14917,6 +14916,30 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private 164 │ get p2(): asserts this is string; ╰──── + × TS(1264): Declarations with definite assignment assertions must also have type annotations. + ╭─[typescript/tests/cases/conformance/controlFlow/definiteAssignmentAssertions.ts:69:10] + 68 │ function f4() { + 69 │ let a!; + · ▲ + 70 │ let b! = 1; + ╰──── + + × TS(1263): Declarations with initializers cannot also have definite assignment assertions. + ╭─[typescript/tests/cases/conformance/controlFlow/definiteAssignmentAssertions.ts:70:10] + 69 │ let a!; + 70 │ let b! = 1; + · ▲ + 71 │ let c!: number = 1; + ╰──── + + × TS(1263): Declarations with initializers cannot also have definite assignment assertions. + ╭─[typescript/tests/cases/conformance/controlFlow/definiteAssignmentAssertions.ts:71:10] + 70 │ let b! = 1; + 71 │ let c!: number = 1; + · ▲ + 72 │ } + ╰──── + × Expected `,` but found `!` ╭─[typescript/tests/cases/conformance/controlFlow/definiteAssignmentAssertionsWithObjectShortHand.ts:2:16] 1 │ const a: string | undefined = 'ff';