refactor(semantic): change Bindings to a plain FxHashMap (#8019)

`IndexMap` was needed for the insertion order requirement in mangler.

Bindings (symbol ids) are monotonically increasing by binding
positions, which means we can get insertion order by sorting the symbol ids in
mangler.

Previous attempt: #4228

crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs

@@ -43,18 +43,19 @@ 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().get_bindings(node.scope_id()).values()
+                    {
                         if let AstKind::TSInterfaceDeclaration(scope_interface) =
-                            get_symbol_kind(symbol_id, ctx)
+                            get_symbol_kind(*symbol_id, ctx)
                         {
                             check_and_diagnostic(ident, &scope_interface.id, ctx);
                         }
                     }
                 }
             }
             AstKind::TSInterfaceDeclaration(decl) => {
-                for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) {
-                    if let AstKind::Class(scope_class) = get_symbol_kind(symbol_id, ctx) {
+                for symbol_id in ctx.semantic().scopes().get_bindings(node.scope_id()).values() {
+                    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);
                         }

crates/oxc_mangler/src/lib.rs

@@ -110,8 +110,10 @@ impl Mangler {
             let mut slot = parent_max_slot;
 
             if !bindings.is_empty() {
-                // `bindings` are stored in order, traverse and increment slot
-                for symbol_id in bindings.values().copied() {
+                // Sort `bindings` in declaration order.
+                let mut bindings = bindings.values().copied().collect::<Vec<_>>();
+                bindings.sort_unstable();
+                for symbol_id in bindings {
                     slots[symbol_id] = slot;
                     slot += 1;
                 }

crates/oxc_semantic/Cargo.toml

@@ -30,16 +30,13 @@ 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 }
 
 [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 }

crates/oxc_semantic/src/builder.rs

@@ -23,18 +23,9 @@ use oxc_syntax::{
 };
 
 use crate::{
-    binder::Binder,
-    checker,
-    class::ClassTableBuilder,
-    diagnostics::redeclaration,
-    jsdoc::JSDocBuilder,
-    label::UnusedLabels,
-    node::AstNodes,
-    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, scope::ScopeTree, stats::Stats,
+    symbol::SymbolTable, unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic,
 };
 
 macro_rules! control_flow {
@@ -713,10 +704,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::<Bindings>();
-                parent_bindings.values().for_each(|&symbol_id| {
+                let parent_bindings = mem::take(parent_bindings);
+                for &symbol_id in parent_bindings.values() {
                     self.symbols.set_scope_id(symbol_id, self.current_scope_id);
-                });
+                }
                 *self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings;
             }
         }

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<K, V> = IndexMap<K, V, FxBuildHasher>;
-
-pub(crate) type Bindings = FxIndexMap<CompactStr, SymbolId>;
+pub(crate) type Bindings = FxHashMap<CompactStr, SymbolId>;
 pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<ReferenceId>>;
 
 /// 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].contains_key(name)
     }
 
     /// Get the symbol bound to an identifier name in a scope.
@@ -330,13 +327,13 @@ impl ScopeTree {
 
     /// 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].remove(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) {
+        if let Some((name, symbol_id)) = from_map.remove_entry(name) {
             self.bindings[to].insert(name, symbol_id);
         }
     }
@@ -365,7 +362,7 @@ impl ScopeTree {
         // 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);
+        let old_symbol_id = bindings.remove(old_name);
         debug_assert_eq!(old_symbol_id, Some(symbol_id));
     }
 

crates/oxc_semantic/tests/main.rs

@@ -46,7 +46,8 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
             .as_str(),
         );
         result.push_str("\"symbols\": ");
-        let bindings = scope_tree.get_bindings(scope_id);
+        let mut bindings = scope_tree.get_bindings(scope_id).iter().collect::<Vec<_>>();
+        bindings.sort_unstable_by_key(|&(_, symbol_id)| symbol_id);
         result.push('[');
         bindings.iter().enumerate().for_each(|(index, (name, &symbol_id))| {
             if index != 0 {

crates/oxc_wasm/src/lib.rs

@@ -387,10 +387,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)| {
+                    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("}");
                 }
             }

tasks/transform_checker/src/lib.rs

@@ -346,10 +346,12 @@ impl PostTransformChecker<'_, '_> {
             if binding_names.is_mismatch() {
                 self.errors.push_mismatch("Bindings mismatch", scope_ids, binding_names);
             } else {
-                let symbol_ids = self.get_pair(scope_ids, |scoping, scope_id| {
+                let mut symbol_ids = self.get_pair(scope_ids, |scoping, scope_id| {
                     scoping.scopes.get_bindings(scope_id).values().copied().collect::<Vec<_>>()
                 });
                 if self.remap_symbol_ids_sets(&symbol_ids).is_mismatch() {
+                    symbol_ids.after_transform.sort_unstable();
+                    symbol_ids.rebuilt.sort_unstable();
                     self.errors.push_mismatch("Binding symbols mismatch", scope_ids, symbol_ids);
                 }
             }