Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(semantic): correctly resolve identifiers inside parameter initializers #3046

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
}
}