diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index a321e8fe40..7742cb946d 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -36,7 +36,7 @@ use crate::{ node::{Ancestry, AstNodeStore, AstNodeStoreKind}, scoping::{Bindings, Scoping}, stats::Stats, - unresolved_stack::UnresolvedReferences, + unresolved_stack::{UnresolvedReference, UnresolvedReferences}, }; #[cfg(feature = "jsdoc")] use oxc_jsdoc::JSDocBuilder; @@ -97,9 +97,6 @@ pub struct SemanticBuilder<'a> { pub(crate) scoping: Scoping, pub(crate) unresolved_references: UnresolvedReferences<'a>, - /// Checkpoint for early resolution of function parameter / catch parameter references. - /// Tracks the start index in the flat unresolved references list. - unresolved_references_checkpoint: usize, unused_labels: UnusedLabels<'a>, #[cfg(feature = "jsdoc")] @@ -159,7 +156,6 @@ impl<'a> SemanticBuilder<'a> { hoisting_variables: FxHashMap::default(), scoping, unresolved_references: UnresolvedReferences::new(), - unresolved_references_checkpoint: 0, unused_labels: UnusedLabels::default(), #[cfg(feature = "jsdoc")] jsdoc: JSDocBuilder::default(), @@ -610,8 +606,13 @@ impl<'a> SemanticBuilder<'a> { name: Ident<'a>, reference: Reference, ) -> ReferenceId { + let lookup_scope_id = reference.scope_id(); let reference_id = self.scoping.create_reference(reference); - self.unresolved_references.push(name, reference_id); + self.unresolved_references.push(UnresolvedReference { + name, + reference_id, + lookup_scope_id, + }); reference_id } @@ -640,27 +641,35 @@ impl<'a> SemanticBuilder<'a> { /// Walk-up is faster because it only does hashmap lookups (no drain+insert), /// and reference creation is a simple Vec push instead of a hashmap insert. fn resolve_all_references(&mut self) { + let root_scope_id = self.scoping.root_scope_id(); let refs = self.unresolved_references.take(); - for (name, reference_id) in refs { - if !self.walk_up_resolve_reference(name, reference_id) { - self.scoping.add_root_unresolved_reference(name, reference_id); + for unresolved in refs { + if !self.walk_up_resolve_reference(unresolved, root_scope_id) { + self.scoping + .add_root_unresolved_reference(unresolved.name, unresolved.reference_id); } } } - /// Walk up the scope chain trying to resolve a reference. - /// Returns `true` if resolved. + /// Walk up the scope chain through `last_scope_id`. #[expect(clippy::inline_always, reason = "Hot path — called for every reference resolution")] #[inline(always)] - fn walk_up_resolve_reference(&mut self, name: Ident<'a>, reference_id: ReferenceId) -> bool { - let mut scope_id = Some(self.scoping.references[reference_id].scope_id()); - while let Some(sid) = scope_id { - if let Some(symbol_id) = self.scoping.get_binding(sid, name) - && self.try_resolve_reference(reference_id, symbol_id) + fn walk_up_resolve_reference( + &mut self, + unresolved: UnresolvedReference<'a>, + last_scope_id: ScopeId, + ) -> bool { + let mut current_scope_id = Some(unresolved.lookup_scope_id); + while let Some(scope_id) = current_scope_id { + if let Some(symbol_id) = self.scoping.get_binding(scope_id, unresolved.name) + && self.try_resolve_reference(unresolved.reference_id, symbol_id) { return true; } - scope_id = self.scoping.scope_parent_id(sid); + if scope_id == last_scope_id { + return false; + } + current_scope_id = self.scoping.scope_parent_id(scope_id); } false } @@ -715,34 +724,58 @@ impl<'a> SemanticBuilder<'a> { true } - /// Early-resolve references collected since the checkpoint by walking up the - /// full scope chain. Used for function parameters and catch parameters where - /// references must be resolved before entering the function body, to avoid - /// binding to variables declared inside the body (which share the same scope). + /// Early-resolve references collected while visiting the current function or catch parameter + /// scope. + /// + /// Function parameters and bodies currently share one scope. To emulate the separate + /// parameter environment, unresolved parameter references resume from the parent scope during + /// final resolution. Nested parameter resolution advances this boundary one function at a time, + /// so a nested function parameter can still resolve to a later declaration in an enclosing + /// function body while skipping declarations in its own body. + /// + /// This is a workaround until function bodies have separate scopes: + /// . /// /// Resolved references are removed. Unresolved references stay in the flat /// list for later resolution by `resolve_all_references` (which handles /// forward references to declarations not yet visited). - fn resolve_references_for_current_scope(&mut self) { + fn resolve_references_for_current_scope(&mut self, unresolved_start: usize) { // Process in-place using a retain-style write-cursor — no temporary - // `Vec`. Reads each `(name, reference_id)` by value out of the flat - // list (both fields are `Copy`), so calling `walk_up_resolve_reference` - // (which takes `&mut self`) doesn't conflict with the index read. - let checkpoint = self.unresolved_references_checkpoint; + // `Vec`. Reads each entry by value out of the flat list (all fields are + // `Copy`), so calling `try_resolve_reference` (which takes `&mut self`) + // doesn't conflict with the index read. let end = self.unresolved_references.len(); - if end <= checkpoint { + if end <= unresolved_start { return; } - let mut write_idx = checkpoint; - for read_idx in checkpoint..end { - let (name, reference_id) = self.unresolved_references.get(read_idx); - if !self.walk_up_resolve_reference(name, reference_id) { - // Keep in the flat list — may resolve later via forward declarations. - if write_idx != read_idx { - self.unresolved_references.set(write_idx, name, reference_id); - } + let mut write_idx = unresolved_start; + for read_idx in unresolved_start..end { + let mut unresolved = self.unresolved_references.get(read_idx); + + // Parameter decorators are visited in an outer class scope. Leave those references + // for final resolution because the current function is not on their scope chain. + let is_in_current_scope = unresolved.lookup_scope_id == self.current_scope_id + || self + .scoping + .scope_is_descendant_of(unresolved.lookup_scope_id, self.current_scope_id); + if !is_in_current_scope { + self.unresolved_references.set(write_idx, unresolved); write_idx += 1; + continue; } + + if self.walk_up_resolve_reference(unresolved, self.current_scope_id) { + continue; + } + + // Skip this function body during final resolution. An enclosing parameter resolution + // may still resolve the reference before advancing the boundary again. + unresolved.lookup_scope_id = self + .scoping + .scope_parent_id(self.current_scope_id) + .expect("function and catch parameter scopes always have a parent"); + self.unresolved_references.set(write_idx, unresolved); + write_idx += 1; } self.unresolved_references.truncate(write_idx); } @@ -2057,9 +2090,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { )); /* cfg */ - // Save checkpoint before visiting type params/params/return type - let saved_checkpoint = self.unresolved_references_checkpoint; - self.unresolved_references_checkpoint = self.unresolved_references.checkpoint(); + let unresolved_start = self.unresolved_references.len(); if let Some(type_parameters) = &func.type_parameters { self.visit_ts_type_parameter_declaration(type_parameters); @@ -2080,9 +2111,8 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { // Parameter initializers must be resolved after all parameters have been declared. // Param types and return type must be resolved after type parameters have been declared. // In both cases, need to avoid binding to variables/types declared inside the function body. - self.resolve_references_for_current_scope(); + self.resolve_references_for_current_scope(unresolved_start); } - self.unresolved_references_checkpoint = saved_checkpoint; if let Some(body) = &func.body { self.visit_function_body(body); @@ -2141,9 +2171,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { &expr.scope_id, ); - // Save checkpoint before visiting type params/params/return type - let saved_checkpoint = self.unresolved_references_checkpoint; - self.unresolved_references_checkpoint = self.unresolved_references.checkpoint(); + let unresolved_start = self.unresolved_references.len(); if let Some(parameters) = &expr.type_parameters { self.visit_ts_type_parameter_declaration(parameters); @@ -2171,9 +2199,8 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { // Parameter initializers must be resolved after all parameters have been declared. // Param types and return type must be resolved after type parameters have been declared. // In both cases, need to avoid binding to variables/types declared inside the function body. - self.resolve_references_for_current_scope(); + self.resolve_references_for_current_scope(unresolved_start); } - self.unresolved_references_checkpoint = saved_checkpoint; self.visit_arrow_function_body(&expr.body); @@ -2376,16 +2403,14 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { self.enter_node(kind); param.bind(self); - let saved_checkpoint = self.unresolved_references_checkpoint; - self.unresolved_references_checkpoint = self.unresolved_references.checkpoint(); + let unresolved_start = self.unresolved_references.len(); self.visit_span(¶m.span); self.visit_binding_pattern(¶m.pattern); if let Some(type_annotation) = ¶m.type_annotation { self.visit_ts_type_annotation(type_annotation); } - self.resolve_references_for_current_scope(); - self.unresolved_references_checkpoint = saved_checkpoint; + self.resolve_references_for_current_scope(unresolved_start); self.leave_node(kind); } diff --git a/crates/oxc_semantic/src/unresolved_stack.rs b/crates/oxc_semantic/src/unresolved_stack.rs index 4cfd503783..89710f7c69 100644 --- a/crates/oxc_semantic/src/unresolved_stack.rs +++ b/crates/oxc_semantic/src/unresolved_stack.rs @@ -1,5 +1,13 @@ use oxc_str::Ident; -use oxc_syntax::reference::ReferenceId; +use oxc_syntax::{reference::ReferenceId, scope::ScopeId}; + +#[derive(Clone, Copy)] +pub struct UnresolvedReference<'a> { + pub name: Ident<'a>, + pub reference_id: ReferenceId, + /// Scope where the next lookup should begin. + pub lookup_scope_id: ScopeId, +} /// Flat list of unresolved references collected during AST traversal. /// @@ -7,8 +15,9 @@ use oxc_syntax::reference::ReferenceId; /// references are collected flat and resolved in a single pass after traversal (walk-up). /// This eliminates all hashmap drain+insert operations during scope exit. pub struct UnresolvedReferences<'a> { - /// Flat list of (name, reference_id) pairs collected during traversal. - references: Vec<(Ident<'a>, ReferenceId)>, + /// The lookup scope initially matches the reference's recorded scope and advances past + /// function bodies which are not visible to parameter references. + references: Vec>, } impl<'a> UnresolvedReferences<'a> { @@ -26,19 +35,13 @@ impl<'a> UnresolvedReferences<'a> { /// Push an unresolved reference to the flat list. #[inline] - pub(crate) fn push(&mut self, name: Ident<'a>, reference_id: ReferenceId) { - self.references.push((name, reference_id)); - } - - /// Get the current length, used as a checkpoint for early resolution. - #[inline] - pub(crate) fn checkpoint(&self) -> usize { - self.references.len() + pub(crate) fn push(&mut self, reference: UnresolvedReference<'a>) { + self.references.push(reference); } /// Take all collected references, leaving the list empty. O(1) pointer swap. #[inline] - pub(crate) fn take(&mut self) -> Vec<(Ident<'a>, ReferenceId)> { + pub(crate) fn take(&mut self) -> Vec> { std::mem::take(&mut self.references) } @@ -51,14 +54,14 @@ impl<'a> UnresolvedReferences<'a> { /// Read a reference by index, by value. /// /// Used by [`crate::SemanticBuilder::resolve_references_for_current_scope`] - /// to process the list in-place without allocating a temporary `Vec`. Both - /// `Ident<'a>` and `ReferenceId` are `Copy`, so this hands the caller an - /// owned pair that's detached from the underlying borrow. + /// to process the list in-place without allocating a temporary `Vec`. All + /// entry fields are `Copy`, so this hands the caller an owned value that's + /// detached from the underlying borrow. /// /// # Panics /// Panics if `idx >= self.len()`. #[inline] - pub(crate) fn get(&self, idx: usize) -> (Ident<'a>, ReferenceId) { + pub(crate) fn get(&self, idx: usize) -> UnresolvedReference<'a> { self.references[idx] } @@ -68,8 +71,8 @@ impl<'a> UnresolvedReferences<'a> { /// # Panics /// Panics if `idx >= self.len()`. #[inline] - pub(crate) fn set(&mut self, idx: usize, name: Ident<'a>, reference_id: ReferenceId) { - self.references[idx] = (name, reference_id); + pub(crate) fn set(&mut self, idx: usize, reference: UnresolvedReference<'a>) { + self.references[idx] = reference; } /// Truncate the list to `len`, removing references at the end. diff --git a/crates/oxc_semantic/tests/integration/symbols.rs b/crates/oxc_semantic/tests/integration/symbols.rs index 645650cc11..9738be8b92 100644 --- a/crates/oxc_semantic/tests/integration/symbols.rs +++ b/crates/oxc_semantic/tests/integration/symbols.rs @@ -2,6 +2,29 @@ use oxc_semantic::{Reference, SymbolFlags}; use crate::util::SemanticTester; +fn assert_root_and_single_local_reference_counts( + tester: &SemanticTester<'_>, + expected: &[(&str, usize, usize)], +) { + let semantic = tester.build(); + let scoping = semantic.scoping(); + let root_scope_id = scoping.root_scope_id(); + + for &(name, root_count, local_count) in expected { + let root_symbol = scoping.get_binding(root_scope_id, name.into()).unwrap(); + let local_symbol = scoping + .symbol_ids() + .find(|&symbol_id| { + scoping.symbol_name(symbol_id) == name + && scoping.symbol_scope_id(symbol_id) != root_scope_id + }) + .unwrap(); + + assert_eq!(scoping.get_resolved_references(root_symbol).count(), root_count, "{name}"); + assert_eq!(scoping.get_resolved_references(local_symbol).count(), local_count, "{name}"); + } +} + #[test] fn test_class_simple() { SemanticTester::js("export class Foo {};") @@ -85,6 +108,100 @@ fn test_var_read_write() { .test(); } +// https://github.com/rolldown/rolldown/issues/10722 +#[test] +fn test_forward_outer_binding_in_nested_default_parameter() { + let tester = SemanticTester::js( + "let var_n = 0, let_n = 0, fn_n = 0; + function outer() { + const read_var = function(a = var_n) { return a }; + const read_let = function(a = let_n) { return a }; + const read_fn = function(a = fn_n) { return a }; + var var_n = 1; + let let_n = 1; + function fn_n() {} + return [read_var, read_let, read_fn]; + } + use(var_n, let_n, fn_n, outer);", + ); + assert_root_and_single_local_reference_counts( + &tester, + &[("var_n", 1, 1), ("let_n", 1, 1), ("fn_n", 1, 1)], + ); +} + +#[test] +fn test_nested_default_parameter_does_not_see_own_function_body() { + let tester = SemanticTester::js( + "let n = 0; + function outer() { + const read = function(a = n) { + var n = 1; + return a; + }; + return read; + } + use(n, outer);", + ); + assert_root_and_single_local_reference_counts(&tester, &[("n", 2, 0)]); +} + +#[test] +fn test_nested_default_parameter_in_outer_parameter_skips_outer_function_body() { + let tester = SemanticTester::js( + "let n = 0; + function outer(read = function(a = n) { return a }) { + var n = 1; + return read; + } + use(n, outer);", + ); + assert_root_and_single_local_reference_counts(&tester, &[("n", 2, 0)]); +} + +#[test] +fn test_nested_default_parameter_sees_later_outer_parameter() { + SemanticTester::js( + "function outer(read = function(a = n) { return a }, n) { + return read; + } + use(outer);", + ) + .has_some_symbol("n") + .has_number_of_references(1) + .test(); +} + +// https://github.com/oxc-project/oxc/issues/20427 +// https://github.com/oxc-project/oxc/issues/22783 +#[test] +fn test_function_signature_does_not_see_function_body_bindings() { + let tester = SemanticTester::ts( + "type T = number; + let value = 0; + function f(a: T): typeof value { + type T = string; + var value = 1; + return value; + } + use(value, f);", + ); + assert_root_and_single_local_reference_counts(&tester, &[("T", 1, 0), ("value", 2, 1)]); +} + +#[test] +fn test_arrow_parameter_does_not_see_arrow_body_binding() { + let tester = SemanticTester::js( + "let n = 0; + const read = (a = n) => { + let n = 1; + return a; + }; + use(n, read);", + ); + assert_root_and_single_local_reference_counts(&tester, &[("n", 2, 0)]); +} + #[test] fn test_types_simple() { let test = SemanticTester::ts( diff --git a/crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs b/crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs index 1661d0bcff..a75c3fb3c7 100644 --- a/crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs +++ b/crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs @@ -116,6 +116,17 @@ fn typeof_define() { ); } +// https://github.com/rolldown/rolldown/issues/10779 +#[test] +fn typeof_define_in_default_parameter_is_not_shadowed_by_function_body_var() { + let config = config(&[("typeof window", "'undefined'")]); + test_define_only( + "export function load(value = typeof window !== 'undefined' ? import('browser') : null) { var window; return value; }", + "export function load(value = 'undefined' !== 'undefined' ? import('browser') : null) { var window; return value; }", + &config, + ); +} + #[test] fn typeof_define_is_exact() { let config = config(&[("typeof window", "'undefined'"), ("typeof process.env", "'object'")]); diff --git a/tasks/coverage/snapshots/semantic_typescript.snap b/tasks/coverage/snapshots/semantic_typescript.snap index e5214bc322..e277d07611 100644 --- a/tasks/coverage/snapshots/semantic_typescript.snap +++ b/tasks/coverage/snapshots/semantic_typescript.snap @@ -2,7 +2,7 @@ commit: b465fdbf semantic_typescript Summary: AST Parsed : 4720/4720 (100.00%) -Positive Passed: 3490/4720 (73.94%) +Positive Passed: 3489/4720 (73.92%) semantic Error: tasks/coverage/typescript/tests/cases/compiler/APISample_Watch.ts Bindings mismatch: after transform: ScopeId(0): ["console", "formatHost", "os", "process", "reportDiagnostic", "reportWatchStatusChanged", "ts", "watchMain"] @@ -17577,6 +17577,17 @@ Symbol redeclarations mismatch for "f": after transform: SymbolId(0): [Span { start: 9, end: 10 }, Span { start: 38, end: 39 }] rebuilt : SymbolId(0): [] +semantic Error: tasks/coverage/typescript/tests/cases/conformance/functions/functionParameterObjectRestAndInitializers.ts +Symbol reference IDs mismatch for "a": +after transform: SymbolId(1): [ReferenceId(0)] +rebuilt : SymbolId(6): [] +Reference symbol mismatch for "a": +after transform: SymbolId(1) "a" +rebuilt : +Unresolved references mismatch: +after transform: ["require"] +rebuilt : ["a", "require"] + semantic Error: tasks/coverage/typescript/tests/cases/conformance/generators/generatorYieldContextualType.ts Bindings mismatch: after transform: ScopeId(0): ["Directive", "StepResult", "_wrapAsyncGenerator", "canPickStepContinue", "createPickStep", "f1", "f2", "showStep"] diff --git a/tasks/transform_conformance/snapshots/babel.snap.md b/tasks/transform_conformance/snapshots/babel.snap.md index 8d0f079ace..77f894156e 100644 --- a/tasks/transform_conformance/snapshots/babel.snap.md +++ b/tasks/transform_conformance/snapshots/babel.snap.md @@ -1,6 +1,6 @@ commit: 1eac4481 -Passed: 732/1162 +Passed: 731/1162 # All Passed: * babel-plugin-transform-logical-assignment-operators @@ -924,7 +924,7 @@ x Output mismatch x Output mismatch -# babel-plugin-transform-object-rest-spread (29/39) +# babel-plugin-transform-object-rest-spread (28/39) * object-rest/for-x/input.js x Output mismatch @@ -946,6 +946,47 @@ x Output mismatch * object-rest/object-ref-computed/input.js x Output mismatch +* object-rest/parameters-object-rest-used-in-default/input.js +Symbol reference IDs mismatch for "R": +after transform: SymbolId(0): [ReferenceId(0)] +rebuilt : SymbolId(3): [] +Symbol reference IDs mismatch for "Y": +after transform: SymbolId(2): [ReferenceId(1)] +rebuilt : SymbolId(6): [] +Symbol reference IDs mismatch for "R": +after transform: SymbolId(6): [ReferenceId(2)] +rebuilt : SymbolId(10): [] +Symbol reference IDs mismatch for "R": +after transform: SymbolId(7): [ReferenceId(3)] +rebuilt : SymbolId(16): [] +Symbol reference IDs mismatch for "R": +after transform: SymbolId(13): [ReferenceId(6)] +rebuilt : SymbolId(20): [] +Symbol reference IDs mismatch for "R": +after transform: SymbolId(15): [ReferenceId(7)] +rebuilt : SymbolId(23): [] +Reference symbol mismatch for "R": +after transform: SymbolId(0) "R" +rebuilt : +Reference symbol mismatch for "Y": +after transform: SymbolId(2) "Y" +rebuilt : +Reference symbol mismatch for "R": +after transform: SymbolId(6) "R" +rebuilt : +Reference symbol mismatch for "R": +after transform: SymbolId(7) "R" +rebuilt : +Reference symbol mismatch for "R": +after transform: SymbolId(13) "R" +rebuilt : +Reference symbol mismatch for "R": +after transform: SymbolId(15) "R" +rebuilt : +Unresolved references mismatch: +after transform: ["b", "babelHelpers", "f", "q"] +rebuilt : ["R", "Y", "b", "babelHelpers", "f", "q"] + * object-rest/symbol/input.js x Output mismatch diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index b001fbc17c..8548ef9387 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -488,7 +488,7 @@ Bindings mismatch: after transform: ScopeId(3): ["Cls2"] rebuilt : ScopeId(4): [] Symbol reference IDs mismatch for "dec": -after transform: SymbolId(0): [ReferenceId(4), ReferenceId(0), ReferenceId(1), ReferenceId(3)] +after transform: SymbolId(0): [ReferenceId(0), ReferenceId(1), ReferenceId(3), ReferenceId(4)] rebuilt : SymbolId(0): [ReferenceId(1), ReferenceId(10)] Symbol scope ID mismatch for "Cls": after transform: SymbolId(4): ScopeId(1)