diff --git a/crates/oxc_linter/src/rules/eslint/no_else_return.rs b/crates/oxc_linter/src/rules/eslint/no_else_return.rs index 0490c4c66fb46..60d766a45b6b3 100644 --- a/crates/oxc_linter/src/rules/eslint/no_else_return.rs +++ b/crates/oxc_linter/src/rules/eslint/no_else_return.rs @@ -189,7 +189,8 @@ fn is_safe_from_name_collisions( let parent_bindings = scopes.get_bindings(parent_scope_id); if bindings.iter().any(|(name, symbol_id)| { - let Some((parent_name, parent_symbol_id)) = parent_bindings.get_key_value(name) + let Some((parent_name, parent_symbol_id)) = + parent_bindings.iter().find(|(n, _)| n == name) else { return false; }; @@ -495,41 +496,41 @@ fn test() { ), ("function foo10() { if (foo) return bar; else (foo).bar(); }", None), ( - "function foo11() { if (foo) return bar + "function foo11() { if (foo) return bar else { [1, 2, 3].map(foo) } }", None, ), ( - "function foo12() { if (foo) return bar - else { baz() } + "function foo12() { if (foo) return bar + else { baz() } [1, 2, 3].map(foo) }", None, ), ( - "function foo13() { if (foo) return bar; + "function foo13() { if (foo) return bar; else { [1, 2, 3].map(foo) } }", None, ), ( - "function foo14() { if (foo) return bar - else { baz(); } + "function foo14() { if (foo) return bar + else { baz(); } [1, 2, 3].map(foo) }", None, ), ("function foo15() { if (foo) return bar; else { baz() } qaz() }", None), ( - "function foo16() { if (foo) return bar + "function foo16() { if (foo) return bar else { baz() } qaz() }", None, ), ( - "function foo17() { if (foo) return bar - else { baz() } + "function foo17() { if (foo) return bar + else { baz() } qaz() }", None, ), ( - "function foo18() { if (foo) return function() {} + "function foo18() { if (foo) return function() {} else [1, 2, 3].map(bar) }", None, ), @@ -730,24 +731,24 @@ fn test() { None, ), ( - "function foo13() { if (foo) return bar; + "function foo13() { if (foo) return bar; else { [1, 2, 3].map(foo) } }", "function foo13() { if (foo) return bar; [1, 2, 3].map(foo) }", None, ), ( - "function foo14() { if (foo) return bar - else { baz(); } + "function foo14() { if (foo) return bar + else { baz(); } [1, 2, 3].map(foo) }", - "function foo14() { if (foo) return bar\n baz(); + "function foo14() { if (foo) return bar\n baz(); [1, 2, 3].map(foo) }", None, ), ( - "function foo17() { if (foo) return bar - else { baz() } + "function foo17() { if (foo) return bar + else { baz() } qaz() }", - "function foo17() { if (foo) return bar\n baz() + "function foo17() { if (foo) return bar\n baz() qaz() }", None, ), diff --git a/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs b/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs index 4d45d4f4ff72d..272c46a7a500d 100644 --- a/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs +++ b/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs @@ -43,7 +43,7 @@ impl Rule for NoUnsafeDeclarationMerging { match node.kind() { AstKind::Class(decl) => { if let Some(ident) = decl.id.as_ref() { - for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { + for symbol_id in ctx.semantic().scopes().iter_bindings_in(node.scope_id()) { if let AstKind::TSInterfaceDeclaration(scope_interface) = get_symbol_kind(symbol_id, ctx) { @@ -53,7 +53,7 @@ impl Rule for NoUnsafeDeclarationMerging { } } AstKind::TSInterfaceDeclaration(decl) => { - for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { + for symbol_id in ctx.semantic().scopes().iter_bindings_in(node.scope_id()) { if let AstKind::Class(scope_class) = get_symbol_kind(symbol_id, ctx) { if let Some(scope_class_ident) = scope_class.id.as_ref() { check_and_diagnostic(&decl.id, scope_class_ident, ctx); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_else_return.snap b/crates/oxc_linter/src/snapshots/eslint_no_else_return.snap index c8f5024c67145..531cab5a17a53 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_else_return.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_else_return.snap @@ -148,7 +148,7 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo11() { if (foo) return bar + 1 │ ╭─▶ function foo11() { if (foo) return bar · │ ─────┬──── · │ ╰── This consequent block always returns, 2 │ ├─▶ else { [1, 2, 3].map(foo) } } @@ -158,10 +158,10 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo12() { if (foo) return bar + 1 │ ╭─▶ function foo12() { if (foo) return bar · │ ─────┬──── · │ ╰── This consequent block always returns, - 2 │ ├─▶ else { baz() } + 2 │ ├─▶ else { baz() } · ╰──── Making this `else` block unnecessary. 3 │ [1, 2, 3].map(foo) } ╰──── @@ -169,7 +169,7 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo13() { if (foo) return bar; + 1 │ ╭─▶ function foo13() { if (foo) return bar; · │ ─────┬───── · │ ╰── This consequent block always returns, 2 │ ├─▶ else { [1, 2, 3].map(foo) } } @@ -179,10 +179,10 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo14() { if (foo) return bar + 1 │ ╭─▶ function foo14() { if (foo) return bar · │ ─────┬──── · │ ╰── This consequent block always returns, - 2 │ ├─▶ else { baz(); } + 2 │ ├─▶ else { baz(); } · ╰──── Making this `else` block unnecessary. 3 │ [1, 2, 3].map(foo) } ╰──── @@ -199,7 +199,7 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo16() { if (foo) return bar + 1 │ ╭─▶ function foo16() { if (foo) return bar · │ ─────┬──── · │ ╰── This consequent block always returns, 2 │ ├─▶ else { baz() } qaz() } @@ -209,10 +209,10 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo17() { if (foo) return bar + 1 │ ╭─▶ function foo17() { if (foo) return bar · │ ─────┬──── · │ ╰── This consequent block always returns, - 2 │ ├─▶ else { baz() } + 2 │ ├─▶ else { baz() } · ╰──── Making this `else` block unnecessary. 3 │ qaz() } ╰──── @@ -220,7 +220,7 @@ snapshot_kind: text ⚠ eslint(no-else-return): Unnecessary 'else' after 'return'. ╭─[no_else_return.tsx:1:29] - 1 │ ╭─▶ function foo18() { if (foo) return function() {} + 1 │ ╭─▶ function foo18() { if (foo) return function() {} · │ ──────────┬───────── · │ ╰── This consequent block always returns, 2 │ ├─▶ else [1, 2, 3].map(bar) } diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index 1eac2ef473696..7f660bb517542 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -111,8 +111,8 @@ impl Mangler { if !bindings.is_empty() { // `bindings` are stored in order, traverse and increment slot - for symbol_id in bindings.values().copied() { - slots[symbol_id] = slot; + for (_, symbol_id) in bindings { + slots[*symbol_id] = slot; slot += 1; } } @@ -143,7 +143,7 @@ impl Mangler { if !is_keyword(n) && !is_special_name(n) && !root_unresolved_references.contains_key(n) - && (self.options.top_level || !root_bindings.contains_key(n)) + && (self.options.top_level || !root_bindings.iter().any(|(name, _)| name == n)) { break name; } diff --git a/crates/oxc_semantic/Cargo.toml b/crates/oxc_semantic/Cargo.toml index facd8e08fcfa6..1cb358ee1e8b5 100644 --- a/crates/oxc_semantic/Cargo.toml +++ b/crates/oxc_semantic/Cargo.toml @@ -30,7 +30,6 @@ oxc_span = { workspace = true } oxc_syntax = { workspace = true } assert-unchecked = { workspace = true } -indexmap = { workspace = true } itertools = { workspace = true } phf = { workspace = true, features = ["macros"] } rustc-hash = { workspace = true } @@ -40,10 +39,8 @@ tsify = { workspace = true, optional = true } wasm-bindgen = { workspace = true, optional = true } [dev-dependencies] -oxc_parser = { workspace = true } - -indexmap = { workspace = true } insta = { workspace = true, features = ["glob"] } +oxc_parser = { workspace = true } phf = { workspace = true, features = ["macros"] } rustc-hash = { workspace = true } serde_json = { workspace = true } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 0c236d678ce93..f8feba9dbd82b 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -23,19 +23,10 @@ use oxc_syntax::{ }; use crate::{ - binder::Binder, - checker, - class::ClassTableBuilder, - diagnostics::redeclaration, - jsdoc::JSDocBuilder, - label::UnusedLabels, - node::AstNodes, - reference::Reference, - scope::{Bindings, ScopeTree}, - stats::Stats, - symbol::SymbolTable, - unresolved_stack::UnresolvedReferencesStack, - JSDocFinder, Semantic, + binder::Binder, checker, class::ClassTableBuilder, diagnostics::redeclaration, + jsdoc::JSDocBuilder, label::UnusedLabels, node::AstNodes, reference::Reference, + scope::ScopeTree, stats::Stats, symbol::SymbolTable, + unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic, }; macro_rules! control_flow { @@ -474,7 +465,12 @@ impl<'a> SemanticBuilder<'a> { self.current_scope_id, self.current_node_id, ); - self.scope.get_bindings_mut(scope_id).insert(name, symbol_id); + let bindings_mut = self.scope.get_bindings_mut(scope_id); + if let Some((_, sid)) = bindings_mut.iter_mut().find(|(n, _)| *n == name) { + *sid = symbol_id; + } else { + bindings_mut.push((name, symbol_id)); + }; symbol_id } @@ -489,7 +485,9 @@ impl<'a> SemanticBuilder<'a> { // Try to resolve a reference. // If unresolved, transfer it to parent scope's unresolved references. let bindings = self.scope.get_bindings(self.current_scope_id); - if let Some(symbol_id) = bindings.get(name.as_str()).copied() { + if let Some((_, symbol_id)) = bindings.iter().find(|(n, _)| n.as_str() == name.as_str()) + { + let symbol_id = *symbol_id; let symbol_flags = self.symbols.get_flags(symbol_id); references.retain(|&reference_id| { let reference = &mut self.symbols.references[reference_id]; @@ -714,10 +712,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { if self.scope.get_flags(parent_scope_id).is_catch_clause() { let parent_bindings = self.scope.get_bindings_mut(parent_scope_id); if !parent_bindings.is_empty() { - let parent_bindings = parent_bindings.drain(..).collect::(); - parent_bindings.values().for_each(|&symbol_id| { - self.symbols.set_scope_id(symbol_id, self.current_scope_id); - }); + let parent_bindings = std::mem::take(parent_bindings); + for (_, symbol_id) in &parent_bindings { + self.symbols.set_scope_id(*symbol_id, self.current_scope_id); + } *self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings; } } diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index aada47ae49786..ba582b9776ad3 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -1,7 +1,6 @@ use std::mem; -use indexmap::IndexMap; -use rustc_hash::{FxBuildHasher, FxHashMap}; +use rustc_hash::FxHashMap; use oxc_index::IndexVec; use oxc_span::CompactStr; @@ -12,9 +11,7 @@ use oxc_syntax::{ symbol::SymbolId, }; -type FxIndexMap = IndexMap; - -pub(crate) type Bindings = FxIndexMap; +pub(crate) type Bindings = Vec<(CompactStr, SymbolId)>; pub type UnresolvedReferences = FxHashMap>; /// Scope Tree @@ -203,7 +200,7 @@ impl ScopeTree { /// Check if a symbol is declared in a certain scope. pub fn has_binding(&self, scope_id: ScopeId, name: &str) -> bool { - self.bindings[scope_id].get(name).is_some() + self.bindings[scope_id].iter().any(|v| v.0 == name) } /// Get the symbol bound to an identifier name in a scope. @@ -215,7 +212,7 @@ impl ScopeTree { /// /// [`find_binding`]: ScopeTree::find_binding pub fn get_binding(&self, scope_id: ScopeId, name: &str) -> Option { - self.bindings[scope_id].get(name).copied() + self.bindings[scope_id].iter().find_map(|v| (v.0 == name).then_some(v.1)) } /// Find a binding by name in a scope or its ancestors. @@ -224,7 +221,7 @@ impl ScopeTree { /// found. If no binding is found, [`None`] is returned. pub fn find_binding(&self, scope_id: ScopeId, name: &str) -> Option { for scope_id in self.ancestors(scope_id) { - if let Some(&symbol_id) = self.bindings[scope_id].get(name) { + if let Some(symbol_id) = self.get_binding(scope_id, name) { return Some(symbol_id); } } @@ -251,15 +248,15 @@ impl ScopeTree { /// /// [`iter_bindings_in`]: ScopeTree::iter_bindings_in pub fn iter_bindings(&self) -> impl Iterator + '_ { - self.bindings.iter_enumerated().flat_map(|(scope_id, bindings)| { - bindings.iter().map(move |(name, &symbol_id)| (scope_id, symbol_id, name)) - }) + self.bindings + .iter_enumerated() + .flat_map(|(scope_id, bindings)| bindings.iter().map(move |v| (scope_id, v.1, &v.0))) } /// Iterate over bindings declared inside a scope. #[inline] pub fn iter_bindings_in(&self, scope_id: ScopeId) -> impl Iterator + '_ { - self.bindings[scope_id].values().copied() + self.bindings[scope_id].iter().map(|v| v.1) } #[inline] @@ -325,19 +322,23 @@ impl ScopeTree { /// /// [`binding`]: Bindings pub fn add_binding(&mut self, scope_id: ScopeId, name: CompactStr, symbol_id: SymbolId) { - self.bindings[scope_id].insert(name, symbol_id); + if let Some((_, sid)) = self.bindings[scope_id].iter_mut().find(|(n, _s)| *n == name) { + *sid = symbol_id; + } else { + self.bindings[scope_id].push((name, symbol_id)); + } } /// Remove an existing binding from a scope. pub fn remove_binding(&mut self, scope_id: ScopeId, name: &CompactStr) { - self.bindings[scope_id].shift_remove(name); + self.bindings[scope_id].retain(|(n, _)| n != name); } /// Move a binding from one scope to another. pub fn move_binding(&mut self, from: ScopeId, to: ScopeId, name: &str) { - let from_map = &mut self.bindings[from]; - if let Some((name, symbol_id)) = from_map.swap_remove_entry(name) { - self.bindings[to].insert(name, symbol_id); + if let Some(index) = self.bindings[from].iter().position(|(n, _)| n == name) { + let (_, symbol_id) = self.bindings[from].remove(index); + self.add_binding(to, name.into(), symbol_id); } } @@ -358,15 +359,13 @@ impl ScopeTree { old_name: &str, new_name: CompactStr, ) { - let bindings = &mut self.bindings[scope_id]; - // Insert on end - let existing_symbol_id = bindings.insert(new_name, symbol_id); - debug_assert!(existing_symbol_id.is_none()); - // Remove old entry. `swap_remove` swaps the last entry into the place of removed entry. - // We just inserted the new entry as last, so the new entry takes the place of the old. - // Order of entries is same as before, only the key has changed. - let old_symbol_id = bindings.swap_remove(old_name); - debug_assert_eq!(old_symbol_id, Some(symbol_id)); + if let Some((name, _)) = + self.bindings[scope_id].iter_mut().find(|(n, s)| *s == symbol_id && n == old_name) + { + *name = new_name; + } else { + debug_assert!(false, "Invalid conditions."); + } } /// Reserve memory for an `additional` number of scopes. diff --git a/crates/oxc_semantic/tests/main.rs b/crates/oxc_semantic/tests/main.rs index 18f3dee01a107..d019c738fe446 100644 --- a/crates/oxc_semantic/tests/main.rs +++ b/crates/oxc_semantic/tests/main.rs @@ -48,7 +48,8 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator result.push_str("\"symbols\": "); let bindings = scope_tree.get_bindings(scope_id); result.push('['); - bindings.iter().enumerate().for_each(|(index, (name, &symbol_id))| { + bindings.iter().enumerate().for_each(|(index, (name, symbol_id))| { + let symbol_id = *symbol_id; if index != 0 { result.push(','); } diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index 518f4fbe948a4..5abc8b400e6fe 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -386,10 +386,10 @@ impl Oxc { let bindings = self.scopes.get_bindings(scope_id); if !bindings.is_empty() { self.write_line("Bindings: {"); - bindings.iter().for_each(|(name, &symbol_id)| { - let symbol_flags = self.symbols.get_flags(symbol_id); + for (name, symbol_id) in bindings { + let symbol_flags = self.symbols.get_flags(*symbol_id); self.write_line(format!(" {name} ({symbol_id:?} {symbol_flags:?})",)); - }); + } self.write_line("}"); } } diff --git a/tasks/transform_checker/src/lib.rs b/tasks/transform_checker/src/lib.rs index 1d957f2ef838d..03e332a3bd703 100644 --- a/tasks/transform_checker/src/lib.rs +++ b/tasks/transform_checker/src/lib.rs @@ -336,8 +336,12 @@ impl PostTransformChecker<'_, '_> { for scope_ids in self.scope_ids_map.pairs() { // Check bindings are the same fn get_sorted_binding_names(scoping: &Scoping, scope_id: ScopeId) -> Vec { - let mut binding_names = - scoping.scopes.get_bindings(scope_id).keys().cloned().collect::>(); + let mut binding_names = scoping + .scopes + .get_bindings(scope_id) + .iter() + .map(|(n, _)| n.clone()) + .collect::>(); binding_names.sort_unstable(); binding_names } @@ -347,7 +351,12 @@ impl PostTransformChecker<'_, '_> { self.errors.push_mismatch("Bindings mismatch", scope_ids, binding_names); } else { let symbol_ids = self.get_pair(scope_ids, |scoping, scope_id| { - scoping.scopes.get_bindings(scope_id).values().copied().collect::>() + scoping + .scopes + .get_bindings(scope_id) + .iter() + .map(|(_, s)| *s) + .collect::>() }); if self.remap_symbol_ids_sets(&symbol_ids).is_mismatch() { self.errors.push_mismatch("Binding symbols mismatch", scope_ids, symbol_ids);