From 1f7033e7ab590c52c9182fc6626415e52eed4411 Mon Sep 17 00:00:00 2001 From: Boshen Date: Sun, 21 Apr 2024 23:38:31 +0800 Subject: [PATCH] fix(semantic): correctly resolve identifiers inside parameter initializers (#3046) close: #2644 This fixes function parameters. I think we need an extra AST node to fix catch parameters, which will be the next PR. --- crates/oxc_semantic/src/builder.rs | 52 +++++++++++++++++++++-------- crates/oxc_semantic/tests/scopes.rs | 15 +++++++++ crates/oxc_syntax/src/node.rs | 17 +++++++--- 3 files changed, 66 insertions(+), 18 deletions(-) diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index e53d8447148c..c9873149bc40 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -284,10 +284,22 @@ impl<'a> SemanticBuilder<'a> { Some(symbol_id) } - pub fn declare_reference(&mut self, reference: Reference) -> ReferenceId { + pub fn declare_reference( + &mut self, + reference: Reference, + add_unresolved_reference: bool, + ) -> ReferenceId { let reference_name = reference.name().clone(); let reference_id = self.symbols.create_reference(reference); - self.scope.add_unresolved_reference(self.current_scope_id, reference_name, reference_id); + 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]); + } reference_id } @@ -313,18 +325,22 @@ impl<'a> SemanticBuilder<'a> { .drain() .collect::)>>(); + for (name, reference_ids) in all_references { + self.resolve_reference_ids(name, reference_ids); + } + } + + fn resolve_reference_ids(&mut self, name: CompactStr, reference_ids: Vec) { let parent_scope_id = self.scope.get_parent_id(self.current_scope_id).unwrap_or(self.current_scope_id); - for (name, reference_ids) in all_references { - if let Some(symbol_id) = self.scope.get_binding(self.current_scope_id, &name) { - for reference_id in &reference_ids { - self.symbols.references[*reference_id].set_symbol_id(symbol_id); - } - self.symbols.resolved_references[symbol_id].extend(reference_ids); - } else { - self.scope.extend_unresolved_reference(parent_scope_id, name, reference_ids); + if let Some(symbol_id) = self.scope.get_binding(self.current_scope_id, &name) { + for reference_id in &reference_ids { + self.symbols.references[*reference_id].set_symbol_id(symbol_id); } + self.symbols.resolved_references[symbol_id].extend(reference_ids); + } else { + self.scope.extend_unresolved_reference(parent_scope_id, name, reference_ids); } } @@ -1710,6 +1726,7 @@ impl<'a> SemanticBuilder<'a> { element.bind(self); } AstKind::FormalParameters(_) => { + self.current_node_flags |= NodeFlags::Parameter; self.remove_export_flag(); } AstKind::FormalParameter(param) => { @@ -1815,6 +1832,9 @@ impl<'a> SemanticBuilder<'a> { AstKind::ArrowFunctionExpression(_) => { self.function_stack.pop(); } + AstKind::FormalParameters(_) => { + self.current_node_flags -= NodeFlags::Parameter; + } AstKind::TSModuleBlock(_) => { self.namespace_stack.pop(); } @@ -1864,9 +1884,13 @@ impl<'a> SemanticBuilder<'a> { fn reference_identifier(&mut self, ident: &IdentifierReference) { let flag = self.resolve_reference_usages(); - let reference = - Reference::new(ident.span, ident.name.to_compact_str(), self.current_node_id, flag); - let reference_id = self.declare_reference(reference); + let name = ident.name.to_compact_str(); + let reference = Reference::new(ident.span, name.clone(), 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); ident.reference_id.set(Some(reference_id)); } @@ -1897,7 +1921,7 @@ impl<'a> SemanticBuilder<'a> { self.current_node_id, ReferenceFlag::read(), ); - self.declare_reference(reference); + self.declare_reference(reference, true); } fn is_not_expression_statement_parent(&self) -> bool { diff --git a/crates/oxc_semantic/tests/scopes.rs b/crates/oxc_semantic/tests/scopes.rs index d05abe66a12a..d77f6aa6eb0f 100644 --- a/crates/oxc_semantic/tests/scopes.rs +++ b/crates/oxc_semantic/tests/scopes.rs @@ -106,3 +106,18 @@ fn test_switch_case() { .has_number_of_references(1) .test(); } + +#[test] +fn test_function_parameters() { + SemanticTester::js( + " + const foo = 2; + function func(a = foo, b = a) { + const foo = 0; + } + ", + ) + .has_root_symbol("foo") + .has_number_of_references(1) + .test(); +} diff --git a/crates/oxc_syntax/src/node.rs b/crates/oxc_syntax/src/node.rs index 6c0182d1ec0f..8864ee767102 100644 --- a/crates/oxc_syntax/src/node.rs +++ b/crates/oxc_syntax/src/node.rs @@ -13,29 +13,38 @@ export type NodeFlags = { JSDoc: 1, Class: 2, HasYield: 4 + Parameter: 8 }; "#; bitflags! { #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct NodeFlags: u8 { - 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 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 } } impl NodeFlags { + #[inline] pub fn has_jsdoc(&self) -> bool { self.contains(Self::JSDoc) } + #[inline] pub fn has_class(&self) -> bool { self.contains(Self::Class) } + #[inline] pub fn has_yield(&self) -> bool { self.contains(Self::HasYield) } + + #[inline] + pub fn has_parameter(&self) -> bool { + self.contains(Self::Parameter) + } }