From f1adf9fa18de5c305b9bec967933286762b9f0d1 Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Fri, 20 Dec 2024 02:38:30 +0000
Subject: [PATCH] refactor(semantic): `ScopeTree::rename_binding` remove old
 binding first (#8020)

#8019 changed scope bindings from an ordered `IndexMap` to an unordered `FxHashMap`.

`ScopeTree::rename_binding` no longer needs to preserve order, so can remove the old binding before adding the new one. This makes it less likely that the hash map will need to reallocate.

Also remove comments about preserving ordering of bindings, which are now outdated.
---
 crates/oxc_semantic/src/scope.rs           | 12 +++---------
 crates/oxc_traverse/src/context/scoping.rs |  2 --
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs
index 9d7927292310c..cb89f89280ace 100644
--- a/crates/oxc_semantic/src/scope.rs
+++ b/crates/oxc_semantic/src/scope.rs
@@ -380,8 +380,6 @@ impl ScopeTree {
 
     /// Rename a binding to a new name.
     ///
-    /// Preserves order of bindings.
-    ///
     /// The following must be true for successful operation:
     /// * Binding exists in specified scope for `old_name`.
     /// * Existing binding is for specified `symbol_id`.
@@ -396,16 +394,12 @@ impl ScopeTree {
         new_name: &str,
     ) {
         self.cell.with_dependent_mut(|allocator, inner| {
-            let new_name = allocator.alloc_str(new_name);
             let bindings = &mut inner.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.remove(old_name);
             debug_assert_eq!(old_symbol_id, Some(symbol_id));
+            let new_name = allocator.alloc_str(new_name);
+            let existing_symbol_id = bindings.insert(new_name, symbol_id);
+            debug_assert!(existing_symbol_id.is_none());
         });
     }
 
diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs
index d8e150c13d4c2..5ec763a87689d 100644
--- a/crates/oxc_traverse/src/context/scoping.rs
+++ b/crates/oxc_traverse/src/context/scoping.rs
@@ -365,8 +365,6 @@ impl TraverseScoping {
 
     /// Rename symbol.
     ///
-    /// Preserves original order of bindings for scope.
-    ///
     /// The following must be true for successful operation:
     /// * Binding exists in specified scope for `symbol_id`.
     /// * No binding already exists in scope for `new_name`.