Skip to content

Commit

Permalink
fix(semantic): resolve reference incorrectly when a parameter referen…
Browse files Browse the repository at this point in the history
…ces 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.
  • Loading branch information
Dunqing committed Jul 2, 2024
1 parent 0fe22a8 commit d995f94
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 36 deletions.
3 changes: 3 additions & 0 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
39 changes: 15 additions & 24 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

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

Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_semantic/tests/integration/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 0 additions & 6 deletions crates/oxc_syntax/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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)
}
}

0 comments on commit d995f94

Please sign in to comment.