refactor(traverse)!: remove TraverseCtx::clone_identifier_reference (…

…#7266)

Remove `TraverseCtx::clone_identifier_reference`.

This API encourages unnecessary repeated lookups of the `SymbolId` for a reference. It is preferable to use `MaybeBoundIdentifier` which only looks up the `SymbolId` once.

crates/oxc_transformer/src/es2020/nullish_coalescing_operator.rs

@@ -33,7 +33,7 @@ use oxc_ast::{ast::*, NONE};
 use oxc_semantic::{ReferenceFlags, ScopeFlags, SymbolFlags};
 use oxc_span::SPAN;
 use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator};
-use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
+use oxc_traverse::{Ancestor, MaybeBoundIdentifier, Traverse, TraverseCtx};
 
 use crate::TransformCtx;
 
@@ -142,9 +142,10 @@ impl<'a, 'ctx> Traverse<'a> for NullishCoalescingOperator<'a, 'ctx> {
 impl<'a, 'ctx> NullishCoalescingOperator<'a, 'ctx> {
     fn clone_expression(expr: &Expression<'a>, ctx: &mut TraverseCtx<'a>) -> Expression<'a> {
         match expr {
-            Expression::Identifier(ident) => Expression::Identifier(
-                ctx.ast.alloc(ctx.clone_identifier_reference(ident, ReferenceFlags::Read)),
-            ),
+            Expression::Identifier(ident) => {
+                let binding = MaybeBoundIdentifier::from_identifier_reference(ident, ctx);
+                binding.create_spanned_expression(ident.span, ReferenceFlags::Read, ctx)
+            }
             _ => expr.clone_in(ctx.ast.allocator),
         }
     }

crates/oxc_transformer/src/es2021/logical_assignment_operators.rs

@@ -59,7 +59,7 @@ use oxc_ast::ast::*;
 use oxc_semantic::{ReferenceFlags, SymbolFlags};
 use oxc_span::SPAN;
 use oxc_syntax::operator::{AssignmentOperator, LogicalOperator};
-use oxc_traverse::{BoundIdentifier, Traverse, TraverseCtx};
+use oxc_traverse::{BoundIdentifier, MaybeBoundIdentifier, Traverse, TraverseCtx};
 
 use crate::TransformCtx;
 
@@ -293,14 +293,15 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
 
     /// Clone an expression
     ///
-    /// If it is an identifier, clone the identifier by [TraverseCtx::clone_identifier_reference], otherwise, use [CloneIn].
+    /// If it is an identifier, clone the identifier via [MaybeBoundIdentifier], otherwise, use [CloneIn].
     ///
     /// TODO: remove this once <https://github.com/oxc-project/oxc/issues/4804> is resolved.
     fn clone_expression(expr: &Expression<'a>, ctx: &mut TraverseCtx<'a>) -> Expression<'a> {
         match expr {
-            Expression::Identifier(ident) => Expression::Identifier(
-                ctx.ast.alloc(ctx.clone_identifier_reference(ident, ReferenceFlags::Read)),
-            ),
+            Expression::Identifier(ident) => {
+                let binding = MaybeBoundIdentifier::from_identifier_reference(ident, ctx);
+                binding.create_spanned_expression(ident.span, ReferenceFlags::Read, ctx)
+            }
             _ => expr.clone_in(ctx.ast.allocator),
         }
     }

crates/oxc_traverse/src/context/maybe_bound_identifier.rs

@@ -26,9 +26,7 @@ use super::BoundIdentifier;
 ///   it for later use.
 /// * `MaybeBoundIdentifier` re-uses the same `Atom` for all `BindingIdentifier` / `IdentifierReference`s
 ///   created from it.
-/// * `MaybeBoundIdentifier` looks up the `SymbolId` for the reference only once,
-///   rather than `TraverseCtx::clone_identifier_reference` which looks it up every time you create
-///   an `IdentifierReference`.
+/// * `MaybeBoundIdentifier` looks up the `SymbolId` for the reference only once.
 #[derive(Debug, Clone)]
 pub struct MaybeBoundIdentifier<'a> {
     pub name: Atom<'a>,

crates/oxc_traverse/src/context/mod.rs

@@ -530,21 +530,6 @@ impl<'a> TraverseCtx<'a> {
         self.scoping.delete_reference_for_identifier(ident);
     }
 
-    /// Clone `IdentifierReference` based on the original reference's `SymbolId` and name.
-    ///
-    /// This method makes a lookup of the `SymbolId` for the reference. If you need to create multiple
-    /// `IdentifierReference`s for the same binding, it is better to look up the `SymbolId` only once,
-    /// and generate `IdentifierReference`s with `TraverseCtx::create_reference_id`.
-    pub fn clone_identifier_reference(
-        &mut self,
-        ident: &IdentifierReference<'a>,
-        flags: ReferenceFlags,
-    ) -> IdentifierReference<'a> {
-        let reference = self.symbols().get_reference(ident.reference_id());
-        let symbol_id = reference.symbol_id();
-        self.create_reference_id(ident.span, ident.name.clone(), symbol_id, flags)
-    }
-
     /// Determine whether evaluating the specific input `node` is a consequenceless reference.
     ///
     /// i.e. evaluating it won't result in potentially arbitrary code from being run.