refactor(semantic): ScopeTree::rename_binding remove old binding fi…

…rst (#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

@@ -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());
         });
     }
 

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`.