From d995f94280da043cfc1a8930889135b2640f8f04 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Tue, 2 Jul 2024 02:12:15 +0000 Subject: [PATCH] fix(semantic): resolve reference incorrectly when a parameter references a parameter that hasn't been defined yet (#4004) close: #3682 The TypeScript code that handles this is [here](https://github.com/Dunqing/TypeScript/blob/d8086f14b6b97c0df34a0cc2f56d4b5926a0c299/src/compiler/utilities.ts#L11515-L11577). It looks complicated. --- crates/oxc_ast/src/ast_impl/js.rs | 3 ++ crates/oxc_semantic/src/builder.rs | 39 +++++++------------ .../oxc_semantic/tests/integration/scopes.rs | 14 ++++--- crates/oxc_syntax/src/node.rs | 6 --- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index 35fb31b01f8f5..e39c488d59d9c 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -1048,6 +1048,9 @@ impl<'a> FormalParameters<'a> { pub fn is_empty(&self) -> bool { self.items.is_empty() } + pub fn has_parameter(&self) -> bool { + !self.is_empty() || self.rest.is_some() + } } impl<'a> FunctionBody<'a> { diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index f8cb9592c5887..3c0c4d8363df8 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -314,22 +314,10 @@ impl<'a> SemanticBuilder<'a> { Some(symbol_id) } - pub fn declare_reference( - &mut self, - reference: Reference, - add_unresolved_reference: bool, - ) -> ReferenceId { + pub fn declare_reference(&mut self, reference: Reference) -> ReferenceId { let reference_name = reference.name().clone(); let reference_id = self.symbols.create_reference(reference); - if add_unresolved_reference { - self.scope.add_unresolved_reference( - self.current_scope_id, - reference_name, - reference_id, - ); - } else { - self.resolve_reference_ids(reference_name.clone(), vec![reference_id]); - } + self.scope.add_unresolved_reference(self.current_scope_id, reference_name, reference_id); reference_id } @@ -1725,14 +1713,12 @@ impl<'a> SemanticBuilder<'a> { element.bind(self); } AstKind::FormalParameters(_) => { - self.current_node_flags |= NodeFlags::Parameter; self.current_symbol_flags -= SymbolFlags::Export; } AstKind::FormalParameter(param) => { param.bind(self); } AstKind::CatchParameter(param) => { - self.current_node_flags |= NodeFlags::Parameter; param.bind(self); } AstKind::TSModuleDeclaration(module_declaration) => { @@ -1843,8 +1829,17 @@ impl<'a> SemanticBuilder<'a> { AstKind::ArrowFunctionExpression(_) => { self.function_stack.pop(); } - AstKind::FormalParameters(_) | AstKind::CatchParameter(_) => { - self.current_node_flags -= NodeFlags::Parameter; + AstKind::FormalParameters(parameters) => { + if parameters.has_parameter() { + // `function foo({bar: identifier_reference}) {}` + // ^^^^^^^^^^^^^^^^^^^^ Parameter initializer must be resolved + // after all parameters have been declared + // to avoid binding to variables inside the scope + self.resolve_references_for_current_scope(); + } + } + AstKind::CatchParameter(_) => { + self.resolve_references_for_current_scope(); } AstKind::TSModuleBlock(_) => { self.namespace_stack.pop(); @@ -1889,11 +1884,7 @@ impl<'a> SemanticBuilder<'a> { let flag = self.resolve_reference_usages(); let name = ident.name.to_compact_str(); let reference = Reference::new(ident.span, name, self.current_node_id, flag); - // `function foo({bar: identifier_reference}) {}` - // ^^^^^^^^^^^^^^^^^^^^ Parameter initializer must be resolved immediately - // to avoid binding to variables inside the scope - let add_unresolved_reference = !self.current_node_flags.has_parameter(); - let reference_id = self.declare_reference(reference, add_unresolved_reference); + let reference_id = self.declare_reference(reference); ident.reference_id.set(Some(reference_id)); } @@ -1922,7 +1913,7 @@ impl<'a> SemanticBuilder<'a> { self.current_node_id, ReferenceFlag::read(), ); - self.declare_reference(reference, true); + self.declare_reference(reference); } fn is_not_expression_statement_parent(&self) -> bool { diff --git a/crates/oxc_semantic/tests/integration/scopes.rs b/crates/oxc_semantic/tests/integration/scopes.rs index f56acd02bb8eb..be50154e2b5d5 100644 --- a/crates/oxc_semantic/tests/integration/scopes.rs +++ b/crates/oxc_semantic/tests/integration/scopes.rs @@ -108,17 +108,19 @@ fn test_switch_case() { #[test] fn test_function_parameters() { - SemanticTester::js( + let tester = SemanticTester::js( " const foo = 2; - function func(a = foo, b = a) { + const c = 0; + function func(a = foo, b = c, c = 0) { const foo = 0; } ", - ) - .has_root_symbol("foo") - .has_number_of_references(1) - .test(); + ); + + tester.has_root_symbol("foo").has_number_of_references(1).test(); + // b = c should reference the third parameter, so root symbol `c`` should have 0 reference + tester.has_root_symbol("c").has_number_of_references(0).test(); } #[test] diff --git a/crates/oxc_syntax/src/node.rs b/crates/oxc_syntax/src/node.rs index 931cc6ebf0cbd..c7d6eaa6894ef 100644 --- a/crates/oxc_syntax/src/node.rs +++ b/crates/oxc_syntax/src/node.rs @@ -30,7 +30,6 @@ bitflags! { const JSDoc = 1 << 0; // If the Node has a JSDoc comment attached const Class = 1 << 1; // If Node is inside a class const HasYield = 1 << 2; // If function has yield statement - const Parameter = 1 << 3; // If Node is inside a parameter } } @@ -49,9 +48,4 @@ impl NodeFlags { pub fn has_yield(&self) -> bool { self.contains(Self::HasYield) } - - #[inline] - pub fn has_parameter(&self) -> bool { - self.contains(Self::Parameter) - } }