Skip to content

Commit

Permalink
fix(semantic): correctly resolve identifiers inside parameter initial…
Browse files Browse the repository at this point in the history
…izers (#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.
  • Loading branch information
Boshen committed Apr 21, 2024
1 parent ee1c0e5 commit 1f7033e
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 18 deletions.
52 changes: 38 additions & 14 deletions crates/oxc_semantic/src/builder.rs
Expand Up @@ -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
}

Expand All @@ -313,18 +325,22 @@ impl<'a> SemanticBuilder<'a> {
.drain()
.collect::<Vec<(_, Vec<_>)>>();

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<ReferenceId>) {
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);
}
}

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions crates/oxc_semantic/tests/scopes.rs
Expand Up @@ -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();
}
17 changes: 13 additions & 4 deletions crates/oxc_syntax/src/node.rs
Expand Up @@ -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)
}
}

0 comments on commit 1f7033e

Please sign in to comment.