feat(span): remove From<String> and From<Cow> API because they create memory leak (#2628)

closes #2621

Author: Boshen

crates/oxc_ast/src/ast/js.rs

@@ -1,7 +1,7 @@
 use std::{cell::Cell, fmt, hash::Hash};
 
 use oxc_allocator::{Box, Vec};
-use oxc_span::{Atom, SourceType, Span};
+use oxc_span::{Atom, CompactStr, SourceType, Span};
 use oxc_syntax::{
     operator::{
         AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator, UpdateOperator,
@@ -212,7 +212,7 @@ impl<'a> Expression<'a> {
         }
     }
 
-    pub fn is_specific_member_access(&'a self, object: &str, property: &str) -> bool {
+    pub fn is_specific_member_access(&self, object: &str, property: &str) -> bool {
         match self.get_inner_expression() {
             Expression::MemberExpression(expr) => expr.is_specific_member_access(object, property),
             Expression::ChainExpression(chain) => {
@@ -466,19 +466,22 @@ pub enum PropertyKey<'a> {
 }
 
 impl<'a> PropertyKey<'a> {
-    pub fn static_name(&self) -> Option<Atom<'a>> {
+    pub fn static_name(&self) -> Option<CompactStr> {
         match self {
-            Self::Identifier(ident) => Some(ident.name.clone()),
+            Self::Identifier(ident) => Some(ident.name.to_compact_str()),
             Self::PrivateIdentifier(_) => None,
             Self::Expression(expr) => match expr {
-                Expression::StringLiteral(lit) => Some(lit.value.clone()),
-                Expression::RegExpLiteral(lit) => Some(Atom::from(lit.regex.to_string())),
-                Expression::NumericLiteral(lit) => Some(Atom::from(lit.value.to_string())),
-                Expression::BigintLiteral(lit) => Some(lit.raw.clone()),
+                Expression::StringLiteral(lit) => Some(lit.value.to_compact_str()),
+                Expression::RegExpLiteral(lit) => Some(lit.regex.to_string().into()),
+                Expression::NumericLiteral(lit) => Some(lit.value.to_string().into()),
+                Expression::BigintLiteral(lit) => Some(lit.raw.to_compact_str()),
                 Expression::NullLiteral(_) => Some("null".into()),
-                Expression::TemplateLiteral(lit) => {
-                    lit.expressions.is_empty().then(|| lit.quasi()).flatten().cloned()
-                }
+                Expression::TemplateLiteral(lit) => lit
+                    .expressions
+                    .is_empty()
+                    .then(|| lit.quasi())
+                    .flatten()
+                    .map(Atom::to_compact_str),
                 _ => None,
             },
         }
@@ -496,16 +499,16 @@ impl<'a> PropertyKey<'a> {
         matches!(self, Self::PrivateIdentifier(_))
     }
 
-    pub fn private_name(&self) -> Option<Atom<'a>> {
+    pub fn private_name(&self) -> Option<&Atom<'a>> {
         match self {
-            Self::PrivateIdentifier(ident) => Some(ident.name.clone()),
+            Self::PrivateIdentifier(ident) => Some(&ident.name),
             _ => None,
         }
     }
 
-    pub fn name(&self) -> Option<Atom<'a>> {
+    pub fn name(&self) -> Option<CompactStr> {
         if self.is_private_identifier() {
-            self.private_name()
+            self.private_name().map(Atom::to_compact_str)
         } else {
             self.static_name()
         }
@@ -758,7 +761,7 @@ impl<'a> CallExpression<'a> {
         }
     }
 
-    pub fn is_symbol_or_symbol_for_call(&'a self) -> bool {
+    pub fn is_symbol_or_symbol_for_call(&self) -> bool {
         // TODO: is 'Symbol' reference to global object
         match &self.callee {
             Expression::Identifier(id) => id.name == "Symbol",
@@ -2017,7 +2020,7 @@ impl<'a> ClassElement<'a> {
         }
     }
 
-    pub fn static_name(&self) -> Option<Atom> {
+    pub fn static_name(&self) -> Option<CompactStr> {
         match self {
             Self::TSIndexSignature(_) | Self::StaticBlock(_) => None,
             Self::MethodDefinition(def) => def.key.static_name(),

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

@@ -130,7 +130,7 @@ impl GetMethod for ClassElement<'_> {
     fn get_method(&self) -> Option<Method> {
         match self {
             ClassElement::MethodDefinition(def) => def.key.static_name().map(|name| Method {
-                name: name.to_compact_str(),
+                name,
                 r#static: def.r#static,
                 call_signature: false,
                 kind: get_kind_from_key(&def.key),
@@ -145,7 +145,7 @@ impl GetMethod for TSSignature<'_> {
     fn get_method(&self) -> Option<Method> {
         match self {
             TSSignature::TSMethodSignature(sig) => sig.key.static_name().map(|name| Method {
-                name: name.to_compact_str(),
+                name,
                 r#static: false,
                 call_signature: false,
                 kind: get_kind_from_key(&sig.key),

crates/oxc_minifier/src/compressor/fold.rs

@@ -264,7 +264,7 @@ impl<'a> Compressor<'a> {
                 let right_string = get_string_value(right)?;
                 // let value = left_string.to_owned().
                 let value = left_string + right_string;
-                let string_literal = StringLiteral::new(span, Atom::from(value));
+                let string_literal = StringLiteral::new(span, self.ast.new_atom(&value));
                 Some(self.ast.literal_string_expression(string_literal))
             },
 

crates/oxc_semantic/src/binder.rs

@@ -1,12 +1,14 @@
 //! Declare symbol for `BindingIdentifier`s
 
+use std::borrow::Cow;
+
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::ast::*;
 use oxc_ast::{
     syntax_directed_operations::{BoundNames, IsSimpleParameterList},
     AstKind,
 };
-use oxc_span::{Atom, SourceType};
+use oxc_span::SourceType;
 
 use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder};
 
@@ -343,9 +345,9 @@ impl<'a> Binder for TSEnumMember<'a> {
             return;
         }
         let name = match &self.id {
-            TSEnumMemberName::Identifier(id) => id.name.clone(),
-            TSEnumMemberName::StringLiteral(s) => s.value.clone(),
-            TSEnumMemberName::NumericLiteral(n) => Atom::from(n.value.to_string()),
+            TSEnumMemberName::Identifier(id) => Cow::Borrowed(id.name.as_str()),
+            TSEnumMemberName::StringLiteral(s) => Cow::Borrowed(s.value.as_str()),
+            TSEnumMemberName::NumericLiteral(n) => Cow::Owned(n.value.to_string()),
             TSEnumMemberName::ComputedPropertyName(_) => panic!("TODO: implement"),
         };
         builder.declare_symbol(

crates/oxc_semantic/src/builder.rs

@@ -5,7 +5,7 @@ use std::{cell::RefCell, path::PathBuf, rc::Rc, sync::Arc};
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::{ast::*, AstKind, Trivias, TriviasMap, Visit};
 use oxc_diagnostics::Error;
-use oxc_span::{Atom, SourceType, Span};
+use oxc_span::{CompactStr, SourceType, Span};
 use oxc_syntax::{
     module_record::{ExportLocalName, ModuleRecord},
     operator::AssignmentOperator,
@@ -231,7 +231,7 @@ impl<'a> SemanticBuilder<'a> {
     pub fn declare_symbol_on_scope(
         &mut self,
         span: Span,
-        name: &Atom,
+        name: &str,
         scope_id: ScopeId,
         includes: SymbolFlags,
         excludes: SymbolFlags,
@@ -243,16 +243,16 @@ impl<'a> SemanticBuilder<'a> {
         }
 
         let includes = includes | self.current_symbol_flags;
-        let symbol_id = self.symbols.create_symbol(span, name.clone(), includes, scope_id);
+        let symbol_id = self.symbols.create_symbol(span, name, includes, scope_id);
         self.symbols.add_declaration(self.current_node_id);
-        self.scope.add_binding(scope_id, name.to_compact_str(), symbol_id);
+        self.scope.add_binding(scope_id, CompactStr::from(name), symbol_id);
         symbol_id
     }
 
     pub fn declare_symbol(
         &mut self,
         span: Span,
-        name: &Atom,
+        name: &str,
         includes: SymbolFlags,
         excludes: SymbolFlags,
     ) -> SymbolId {
@@ -263,14 +263,14 @@ impl<'a> SemanticBuilder<'a> {
         &mut self,
         scope_id: ScopeId,
         span: Span,
-        name: &Atom,
+        name: &str,
         excludes: SymbolFlags,
         report_error: bool,
     ) -> Option<SymbolId> {
         let symbol_id = self.scope.get_binding(scope_id, name)?;
         if report_error && self.symbols.get_flag(symbol_id).intersects(excludes) {
             let symbol_span = self.symbols.get_span(symbol_id);
-            self.error(Redeclaration(name.to_compact_str(), symbol_span, span));
+            self.error(Redeclaration(CompactStr::from(name), symbol_span, span));
         }
         Some(symbol_id)
     }
@@ -285,16 +285,15 @@ impl<'a> SemanticBuilder<'a> {
     /// Declares a `Symbol` for the node, shadowing previous declarations in the same scope.
     pub fn declare_shadow_symbol(
         &mut self,
-        name: &Atom,
+        name: &str,
         span: Span,
         scope_id: ScopeId,
         includes: SymbolFlags,
     ) -> SymbolId {
         let includes = includes | self.current_symbol_flags;
-        let symbol_id =
-            self.symbols.create_symbol(span, name.clone(), includes, self.current_scope_id);
+        let symbol_id = self.symbols.create_symbol(span, name, includes, self.current_scope_id);
         self.symbols.add_declaration(self.current_node_id);
-        self.scope.get_bindings_mut(scope_id).insert(name.to_compact_str(), symbol_id);
+        self.scope.get_bindings_mut(scope_id).insert(CompactStr::from(name), symbol_id);
         symbol_id
     }
 

crates/oxc_semantic/src/class/builder.rs

@@ -5,7 +5,7 @@ use oxc_ast::{
     },
     AstKind,
 };
-use oxc_span::GetSpan;
+use oxc_span::{Atom, GetSpan};
 use oxc_syntax::class::{ClassId, ElementKind};
 
 use crate::{AstNodeId, AstNodes};
@@ -57,15 +57,14 @@ impl ClassTableBuilder {
 
     pub fn declare_class_accessor(&mut self, property: &AccessorProperty) {
         let is_private = property.key.is_private_identifier();
-        let name =
-            if is_private { property.key.private_name() } else { property.key.static_name() };
+        let name = property.key.name();
 
         if let Some(name) = name {
             if let Some(class_id) = self.current_class_id {
                 self.classes.add_element(
                     class_id,
                     Element::new(
-                        name.to_compact_str(),
+                        name,
                         property.key.span(),
                         property.r#static,
                         is_private,
@@ -78,15 +77,14 @@ impl ClassTableBuilder {
 
     pub fn declare_class_property(&mut self, property: &PropertyDefinition) {
         let is_private = property.key.is_private_identifier();
-        let name =
-            if is_private { property.key.private_name() } else { property.key.static_name() };
+        let name = property.key.name();
 
         if let Some(name) = name {
             if let Some(class_id) = self.current_class_id {
                 self.classes.add_element(
                     class_id,
                     Element::new(
-                        name.to_compact_str(),
+                        name,
                         property.key.span(),
                         property.r#static,
                         is_private,
@@ -127,14 +125,18 @@ impl ClassTableBuilder {
             return;
         }
         let is_private = method.key.is_private_identifier();
-        let name = if is_private { method.key.private_name() } else { method.key.static_name() };
+        let name = if is_private {
+            method.key.private_name().map(Atom::to_compact_str)
+        } else {
+            method.key.static_name()
+        };
 
         if let Some(name) = name {
             if let Some(class_id) = self.current_class_id {
                 self.classes.add_element(
                     class_id,
                     Element::new(
-                        name.to_compact_str(),
+                        name,
                         method.key.span(),
                         method.r#static,
                         is_private,

crates/oxc_semantic/src/symbol.rs

@@ -117,12 +117,12 @@ impl SymbolTable {
     pub fn create_symbol(
         &mut self,
         span: Span,
-        name: Atom,
+        name: &str,
         flag: SymbolFlags,
         scope_id: ScopeId,
     ) -> SymbolId {
         _ = self.spans.push(span);
-        _ = self.names.push(name.into_compact_str());
+        _ = self.names.push(CompactStr::from(name));
         _ = self.flags.push(flag);
         _ = self.scope_ids.push(scope_id);
         _ = self.resolved_references.push(vec![]);

crates/oxc_semantic/tests/util/symbol_tester.rs

@@ -141,7 +141,7 @@ impl<'a> SymbolTester<'a> {
     pub fn is_exported(mut self) -> Self {
         self.test_result = match self.test_result {
             Ok(symbol_id) => {
-                let binding = Atom::from(self.target_symbol_name.clone());
+                let binding = self.target_symbol_name.clone();
                 if self.semantic.module_record().exported_bindings.contains_key(binding.as_str())
                     && self.semantic.scopes().get_root_binding(&binding) == Some(symbol_id)
                 {

crates/oxc_span/src/atom.rs

@@ -1,8 +1,4 @@
-use std::{
-    borrow::{Borrow, Cow},
-    fmt, hash,
-    ops::Deref,
-};
+use std::{borrow::Borrow, fmt, hash, ops::Deref};
 
 #[cfg(feature = "serde")]
 use serde::{Serialize, Serializer};
@@ -82,18 +78,6 @@ impl<'a> From<&'a str> for Atom<'a> {
     }
 }
 
-impl<'a> From<String> for Atom<'a> {
-    fn from(s: String) -> Self {
-        Self::Compact(CompactString::from(s))
-    }
-}
-
-impl<'a> From<Cow<'_, str>> for Atom<'a> {
-    fn from(s: Cow<'_, str>) -> Self {
-        Self::Compact(CompactString::from(s))
-    }
-}
-
 impl<'a> Deref for Atom<'a> {
     type Target = str;
 

crates/oxc_transformer/src/es2015/duplicate_keys.rs

@@ -3,7 +3,7 @@
 use std::{collections::HashSet, rc::Rc};
 
 use oxc_ast::{ast::*, AstBuilder};
-use oxc_span::{Atom, SPAN};
+use oxc_span::{CompactStr, SPAN};
 
 use crate::options::{TransformOptions, TransformTarget};
 
@@ -22,9 +22,9 @@ impl<'a> DuplicateKeys<'a> {
     }
 
     pub fn transform_object_expression<'b>(&mut self, obj_expr: &'b mut ObjectExpression<'a>) {
-        let mut visited_data: HashSet<Atom> = HashSet::new();
-        let mut visited_getters: HashSet<Atom> = HashSet::new();
-        let mut visited_setters: HashSet<Atom> = HashSet::new();
+        let mut visited_data: HashSet<CompactStr> = HashSet::new();
+        let mut visited_getters: HashSet<CompactStr> = HashSet::new();
+        let mut visited_setters: HashSet<CompactStr> = HashSet::new();
 
         for property in obj_expr.properties.iter_mut() {
             let ObjectPropertyKind::ObjectProperty(obj_property) = property else {
@@ -64,7 +64,7 @@ impl<'a> DuplicateKeys<'a> {
 
                 if is_duplicate {
                     obj_property.computed = true;
-                    let string_literal = StringLiteral::new(SPAN, name.clone());
+                    let string_literal = StringLiteral::new(SPAN, self.ast.new_atom(name));
                     let expr = self.ast.literal_string_expression(string_literal);
                     obj_property.key = PropertyKey::Expression(expr);
                 }