From ecc9151f99cd75c2cdebad505f21c65bbb3ca831 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 23 Oct 2024 03:36:30 +0000 Subject: [PATCH] refactor(ast, parser, transformer, traverse)!: remove `IdentifierReference::new` methods (#6785) --- crates/oxc_ast/src/ast_impl/js.rs | 16 ----- crates/oxc_parser/src/js/expression.rs | 2 +- crates/oxc_parser/src/js/grammar.rs | 2 +- crates/oxc_parser/src/ts/types.rs | 2 +- crates/oxc_transformer/src/react/jsx.rs | 2 +- .../src/typescript/annotations.rs | 4 +- crates/oxc_traverse/src/context/mod.rs | 31 +++++----- crates/oxc_traverse/src/context/scoping.rs | 61 +------------------ 8 files changed, 22 insertions(+), 98 deletions(-) diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index 06134c8df..18061db29 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -307,22 +307,6 @@ impl<'a> fmt::Display for IdentifierName<'a> { } impl<'a> IdentifierReference<'a> { - #[allow(missing_docs)] - #[inline] - pub fn new(span: Span, name: Atom<'a>) -> Self { - Self { span, name, reference_id: Cell::default() } - } - - #[inline] - #[allow(missing_docs)] - pub fn new_with_reference_id( - span: Span, - name: Atom<'a>, - reference_id: Option, - ) -> Self { - Self { span, name, reference_id: Cell::new(reference_id) } - } - #[inline] #[allow(missing_docs)] pub fn reference_id(&self) -> Option { diff --git a/crates/oxc_parser/src/js/expression.rs b/crates/oxc_parser/src/js/expression.rs index 60ac1be30..2dc3a507e 100644 --- a/crates/oxc_parser/src/js/expression.rs +++ b/crates/oxc_parser/src/js/expression.rs @@ -67,7 +67,7 @@ impl<'a> ParserImpl<'a> { } let (span, name) = self.parse_identifier_kind(Kind::Ident); self.check_identifier(span, &name); - Ok(IdentifierReference::new(span, name)) + Ok(self.ast.identifier_reference(span, name)) } /// `BindingIdentifier` : Identifier diff --git a/crates/oxc_parser/src/js/grammar.rs b/crates/oxc_parser/src/js/grammar.rs index 65fb88beb..3703df466 100644 --- a/crates/oxc_parser/src/js/grammar.rs +++ b/crates/oxc_parser/src/js/grammar.rs @@ -154,7 +154,7 @@ impl<'a> CoverGrammar<'a, ObjectProperty<'a>> for AssignmentTargetProperty<'a> { let binding = match property.key { PropertyKey::StaticIdentifier(ident) => { let ident = ident.unbox(); - IdentifierReference::new(ident.span, ident.name) + p.ast.identifier_reference(ident.span, ident.name) } _ => return Err(p.unexpected()), }; diff --git a/crates/oxc_parser/src/ts/types.rs b/crates/oxc_parser/src/ts/types.rs index 70e5d38f3..49374dd6c 100644 --- a/crates/oxc_parser/src/ts/types.rs +++ b/crates/oxc_parser/src/ts/types.rs @@ -758,7 +758,7 @@ impl<'a> ParserImpl<'a> { pub(crate) fn parse_ts_type_name(&mut self) -> Result> { let span = self.start_span(); let ident = self.parse_identifier_name()?; - let ident = IdentifierReference::new(ident.span, ident.name); + let ident = self.ast.identifier_reference(ident.span, ident.name); let mut left = TSTypeName::IdentifierReference(self.ast.alloc(ident)); while self.eat(Kind::Dot) { let right = self.parse_identifier_name()?; diff --git a/crates/oxc_transformer/src/react/jsx.rs b/crates/oxc_transformer/src/react/jsx.rs index 593be8e76..9426b625a 100644 --- a/crates/oxc_transformer/src/react/jsx.rs +++ b/crates/oxc_transformer/src/react/jsx.rs @@ -1033,7 +1033,7 @@ fn get_read_identifier_reference<'a>( ) -> IdentifierReference<'a> { let reference_id = ctx.create_reference_in_current_scope(name.to_compact_str(), ReferenceFlags::Read); - IdentifierReference::new_with_reference_id(span, name, Some(reference_id)) + ctx.ast.identifier_reference_with_reference_id(span, name, reference_id) } fn create_static_member_expression<'a>( diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index d385b0b49..7b448618d 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -622,10 +622,10 @@ impl<'a> Assignment<'a> { // Creates `this.name = name` fn create_this_property_assignment(&self, ctx: &mut TraverseCtx<'a>) -> Statement<'a> { let reference_id = ctx.create_bound_reference(self.symbol_id, ReferenceFlags::Read); - let id = IdentifierReference::new_with_reference_id( + let id = ctx.ast.identifier_reference_with_reference_id( self.span, self.name.clone(), - Some(reference_id), + reference_id, ); ctx.ast.statement_expression( diff --git a/crates/oxc_traverse/src/context/mod.rs b/crates/oxc_traverse/src/context/mod.rs index 47b940932..8bb4e8227 100644 --- a/crates/oxc_traverse/src/context/mod.rs +++ b/crates/oxc_traverse/src/context/mod.rs @@ -405,9 +405,6 @@ impl<'a> TraverseCtx<'a> { } /// Create an `IdentifierReference` bound to a `SymbolId`. - /// - /// This is a shortcut for `ctx.scoping.create_bound_reference_id`. - #[inline] pub fn create_bound_reference_id( &mut self, span: Span, @@ -415,7 +412,8 @@ impl<'a> TraverseCtx<'a> { symbol_id: SymbolId, flags: ReferenceFlags, ) -> IdentifierReference<'a> { - self.scoping.create_bound_reference_id(span, name, symbol_id, flags) + let reference_id = self.create_bound_reference(symbol_id, flags); + self.ast.identifier_reference_with_reference_id(span, name, reference_id) } /// Create an unbound reference. @@ -431,16 +429,14 @@ impl<'a> TraverseCtx<'a> { } /// Create an unbound `IdentifierReference`. - /// - /// This is a shortcut for `ctx.scoping.create_unbound_reference_id`. - #[inline] pub fn create_unbound_reference_id( &mut self, span: Span, name: Atom<'a>, flags: ReferenceFlags, ) -> IdentifierReference<'a> { - self.scoping.create_unbound_reference_id(span, name, flags) + let reference_id = self.create_unbound_reference(name.to_compact_str(), flags); + self.ast.identifier_reference_with_reference_id(span, name, reference_id) } /// Create a reference optionally bound to a `SymbolId`. @@ -463,9 +459,6 @@ impl<'a> TraverseCtx<'a> { /// /// If you know if there's a `SymbolId` or not, prefer `TraverseCtx::create_bound_reference_id` /// or `TraverseCtx::create_unbound_reference_id`. - /// - /// This is a shortcut for `ctx.scoping.create_reference_id`. - #[inline] pub fn create_reference_id( &mut self, span: Span, @@ -473,7 +466,11 @@ impl<'a> TraverseCtx<'a> { symbol_id: Option, flags: ReferenceFlags, ) -> IdentifierReference<'a> { - self.scoping.create_reference_id(span, name, symbol_id, flags) + if let Some(symbol_id) = symbol_id { + self.create_bound_reference_id(span, name, symbol_id, flags) + } else { + self.create_unbound_reference_id(span, name, flags) + } } /// Create reference in current scope, looking up binding for `name`, @@ -509,15 +506,17 @@ impl<'a> TraverseCtx<'a> { /// 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`. - /// - /// This is a shortcut for `ctx.scoping.clone_identifier_reference`. - #[inline] pub fn clone_identifier_reference( &mut self, ident: &IdentifierReference<'a>, flags: ReferenceFlags, ) -> IdentifierReference<'a> { - self.scoping.clone_identifier_reference(ident, flags) + let reference = + self.symbols().get_reference(ident.reference_id.get().unwrap_or_else(|| { + unreachable!("IdentifierReference must have a 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. diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index 629b779e5..6ef30004c 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -7,7 +7,7 @@ use rustc_hash::FxHashSet; #[allow(clippy::wildcard_imports)] use oxc_ast::{ast::*, visit::Visit}; use oxc_semantic::{NodeId, Reference, ScopeTree, SymbolTable}; -use oxc_span::{Atom, CompactStr, Span}; +use oxc_span::CompactStr; use oxc_syntax::{ reference::{ReferenceFlags, ReferenceId}, scope::{ScopeFlags, ScopeId}, @@ -266,18 +266,6 @@ impl TraverseScoping { reference_id } - /// Create an `IdentifierReference` bound to a `SymbolId` - pub fn create_bound_reference_id<'a>( - &mut self, - span: Span, - name: Atom<'a>, - symbol_id: SymbolId, - flags: ReferenceFlags, - ) -> IdentifierReference<'a> { - let reference_id = self.create_bound_reference(symbol_id, flags); - IdentifierReference::new_with_reference_id(span, name, Some(reference_id)) - } - /// Create an unbound reference pub fn create_unbound_reference( &mut self, @@ -290,17 +278,6 @@ impl TraverseScoping { reference_id } - /// Create an unbound `IdentifierReference` - pub fn create_unbound_reference_id<'a>( - &mut self, - span: Span, - name: Atom<'a>, - flags: ReferenceFlags, - ) -> IdentifierReference<'a> { - let reference_id = self.create_unbound_reference(name.to_compact_str(), flags); - IdentifierReference::new_with_reference_id(span, name, Some(reference_id)) - } - /// Create a reference optionally bound to a `SymbolId`. /// /// If you know if there's a `SymbolId` or not, prefer `TraverseCtx::create_bound_reference` @@ -318,24 +295,6 @@ impl TraverseScoping { } } - /// Create an `IdentifierReference` optionally bound to a `SymbolId`. - /// - /// If you know if there's a `SymbolId` or not, prefer `TraverseCtx::create_bound_reference_id` - /// or `TraverseCtx::create_unbound_reference_id`. - pub fn create_reference_id<'a>( - &mut self, - span: Span, - name: Atom<'a>, - symbol_id: Option, - flags: ReferenceFlags, - ) -> IdentifierReference<'a> { - if let Some(symbol_id) = symbol_id { - self.create_bound_reference_id(span, name, symbol_id, flags) - } else { - self.create_unbound_reference_id(span, name, flags) - } - } - /// Create reference in current scope, looking up binding for `name` pub fn create_reference_in_current_scope( &mut self, @@ -365,24 +324,6 @@ impl TraverseScoping { self.delete_reference(ident.reference_id().unwrap(), &ident.name); } - /// 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 `TraverseScoping::create_reference_id`. - pub fn clone_identifier_reference<'a>( - &mut self, - ident: &IdentifierReference<'a>, - flags: ReferenceFlags, - ) -> IdentifierReference<'a> { - let reference = - self.symbols().get_reference(ident.reference_id.get().unwrap_or_else(|| { - unreachable!("IdentifierReference must have a 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 ran. The following are