refactor(ast)!: revert adding span field to the BindingPattern type. (#3899)

rzvxa · rzvxa · commit 1f85f1a5f7fa · 2024-06-25T09:43:48.000Z
Since this is a temporary solution in the time that we are waiting for the `#[span]` hint, And there are already other workarounds used in our `ast_codegen` I propose removing it right away - sorry in my opinion adding it in the first place was a mistake - in favor of adding an edge case in the codegen. It is better to do the refactoring in the codegen instead of the production code which people may depend on.
diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs
@@ -1327,8 +1327,6 @@ pub struct DebuggerStatement {
 #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
 #[cfg_attr(feature = "serialize", serde(rename_all = "camelCase"))]
 pub struct BindingPattern<'a> {
-    #[cfg_attr(feature = "serialize", serde(skip))]
-    pub span: Span,
     // serde(flatten) the attributes because estree has no `BindingPattern`
     #[cfg_attr(feature = "serialize", serde(flatten))]
     #[cfg_attr(
diff --git a/crates/oxc_ast/src/ast_builder.rs b/crates/oxc_ast/src/ast_builder.rs
@@ -1191,12 +1191,11 @@ impl<'a> AstBuilder<'a> {
     #[inline]
     pub fn binding_pattern(
         self,
-        span: Span,
         kind: BindingPatternKind<'a>,
         type_annotation: Option<Box<'a, TSTypeAnnotation<'a>>>,
         optional: bool,
     ) -> BindingPattern<'a> {
-        BindingPattern { span, kind, type_annotation, optional }
+        BindingPattern { kind, type_annotation, optional }
     }
 
     #[inline]
@@ -1257,7 +1256,6 @@ impl<'a> AstBuilder<'a> {
     ) -> BindingPattern<'a> {
         let pattern = self.alloc(AssignmentPattern { span, left, right });
         BindingPattern {
-            span,
             kind: BindingPatternKind::AssignmentPattern(pattern),
             type_annotation: None,
             optional: false,
diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
@@ -872,8 +872,8 @@ impl<'a> Hash for CatchClause<'a> {
 }
 
 impl<'a> BindingPattern<'a> {
-    pub fn new_with_kind(span: Span, kind: BindingPatternKind<'a>) -> Self {
-        Self { span, kind, type_annotation: None, optional: false }
+    pub fn new_with_kind(kind: BindingPatternKind<'a>) -> Self {
+        Self { kind, type_annotation: None, optional: false }
     }
 
     pub fn get_identifier(&self) -> Option<Atom<'a>> {
diff --git a/crates/oxc_isolated_declarations/src/class.rs b/crates/oxc_isolated_declarations/src/class.rs
@@ -505,7 +505,7 @@ impl<'a> IsolatedDeclarations<'a> {
         kind: BindingPatternKind<'a>,
         type_annotation: Option<Box<'a, TSTypeAnnotation<'a>>>,
     ) -> Box<'a, FormalParameters<'a>> {
-        let pattern = BindingPattern { span: SPAN, kind, type_annotation, optional: false };
+        let pattern = BindingPattern { kind, type_annotation, optional: false };
         let parameter =
             self.ast.formal_parameter(SPAN, pattern, None, false, false, self.ast.new_vec());
         let items = self.ast.new_vec_single(parameter);
diff --git a/crates/oxc_isolated_declarations/src/declaration.rs b/crates/oxc_isolated_declarations/src/declaration.rs
@@ -95,7 +95,6 @@ impl<'a> IsolatedDeclarations<'a> {
             || self.ast.copy(&decl.id),
             |ts_type| {
                 self.ast.binding_pattern(
-                    SPAN,
                     self.ast.copy(&decl.id.kind),
                     Some(self.ast.ts_type_annotation(SPAN, ts_type)),
                     decl.id.optional,
diff --git a/crates/oxc_isolated_declarations/src/function.rs b/crates/oxc_isolated_declarations/src/function.rs
@@ -99,7 +99,6 @@ impl<'a> IsolatedDeclarations<'a> {
                 });
 
             pattern = self.ast.binding_pattern(
-                SPAN,
                 self.ast.copy(&pattern.kind),
                 type_annotation,
                 // if it's assignment pattern, it's optional
diff --git a/crates/oxc_isolated_declarations/src/module.rs b/crates/oxc_isolated_declarations/src/module.rs
@@ -65,8 +65,7 @@ impl<'a> IsolatedDeclarations<'a> {
                         self.error(default_export_inferred(expr.span()));
                     }
 
-                    let id =
-                        BindingPattern { span: SPAN, kind: id, type_annotation, optional: false };
+                    let id = BindingPattern { kind: id, type_annotation, optional: false };
                     let declarations = self
                         .ast
                         .new_vec_single(self.ast.variable_declarator(SPAN, kind, id, None, true));
diff --git a/crates/oxc_parser/src/js/arrow.rs b/crates/oxc_parser/src/js/arrow.rs
@@ -216,10 +216,9 @@ impl<'a> ParserImpl<'a> {
                 }
                 _ => unreachable!(),
             };
-            let span = ident.span;
-            let params_span = self.end_span(span);
+            let params_span = self.end_span(ident.span);
             let ident = self.ast.binding_pattern_identifier(ident);
-            let pattern = self.ast.binding_pattern(span, ident, None, false);
+            let pattern = self.ast.binding_pattern(ident, None, false);
             let formal_parameter = self.ast.plain_formal_parameter(params_span, pattern);
             self.ast.formal_parameters(
                 params_span,
diff --git a/crates/oxc_parser/src/js/binding.rs b/crates/oxc_parser/src/js/binding.rs
@@ -20,15 +20,14 @@ impl<'a> ParserImpl<'a> {
         &mut self,
         allow_question: bool,
     ) -> Result<BindingPattern<'a>> {
-        let span = self.start_span();
         let mut kind = self.parse_binding_pattern_kind()?;
         let optional =
             if allow_question && self.ts_enabled() { self.eat(Kind::Question) } else { false };
         let type_annotation = self.parse_ts_type_annotation()?;
         if let Some(type_annotation) = &type_annotation {
             Self::extend_binding_pattern_span_end(type_annotation.span, &mut kind);
         }
-        Ok(self.ast.binding_pattern(self.end_span(span), kind, type_annotation, optional))
+        Ok(self.ast.binding_pattern(kind, type_annotation, optional))
     }
 
     pub(crate) fn parse_binding_pattern_kind(&mut self) -> Result<BindingPatternKind<'a>> {
@@ -73,7 +72,7 @@ impl<'a> ParserImpl<'a> {
         }
         // The span is not extended to its type_annotation
         let type_annotation = self.parse_ts_type_annotation()?;
-        let pattern = self.ast.binding_pattern(self.end_span(span), kind, type_annotation, false);
+        let pattern = self.ast.binding_pattern(kind, type_annotation, false);
         // Rest element does not allow `= initializer`, .
         let argument = self
             .context(Context::In, Context::empty(), |p| p.parse_initializer(init_span, pattern))?;
@@ -110,7 +109,7 @@ impl<'a> ParserImpl<'a> {
                 shorthand = true;
                 let binding_identifier = BindingIdentifier::new(ident.span, ident.name.clone());
                 let identifier = self.ast.binding_pattern_identifier(binding_identifier);
-                let left = self.ast.binding_pattern(ident.span, identifier, None, false);
+                let left = self.ast.binding_pattern(identifier, None, false);
                 self.context(Context::In, Context::empty(), |p| p.parse_initializer(span, left))?
             } else {
                 return Err(self.unexpected());
diff --git a/crates/oxc_parser/src/js/declaration.rs b/crates/oxc_parser/src/js/declaration.rs
@@ -114,17 +114,9 @@ impl<'a> ParserImpl<'a> {
             if let Some(type_annotation) = &type_annotation {
                 Self::extend_binding_pattern_span_end(type_annotation.span, &mut binding_kind);
             }
-            (
-                self.ast.binding_pattern(
-                    self.end_span(span),
-                    binding_kind,
-                    type_annotation,
-                    optional,
-                ),
-                definite,
-            )
+            (self.ast.binding_pattern(binding_kind, type_annotation, optional), definite)
         } else {
-            (self.ast.binding_pattern(self.end_span(span), binding_kind, None, false), false)
+            (self.ast.binding_pattern(binding_kind, None, false), false)
         };
 
         let init =
diff --git a/crates/oxc_transformer/src/es2015/arrow_functions.rs b/crates/oxc_transformer/src/es2015/arrow_functions.rs
@@ -114,7 +114,6 @@ impl<'a> ArrowFunctions<'a> {
 
         if let Some(id) = &self.this_var {
             let binding_pattern = self.ctx.ast.binding_pattern(
-                SPAN,
                 self.ctx.ast.binding_pattern_identifier(id.create_binding_identifier()),
                 None,
                 false,
diff --git a/crates/oxc_transformer/src/helpers/module_imports.rs b/crates/oxc_transformer/src/helpers/module_imports.rs
@@ -134,7 +134,7 @@ impl<'a> ModuleImports<'a> {
                 name: name.imported,
                 symbol_id: Cell::new(Some(name.symbol_id)),
             };
-            self.ast.binding_pattern(SPAN, self.ast.binding_pattern_identifier(ident), None, false)
+            self.ast.binding_pattern(self.ast.binding_pattern_identifier(ident), None, false)
         };
         let decl = {
             let init = self.ast.call_expression(SPAN, callee, args, false, None);
diff --git a/crates/oxc_transformer/src/react/jsx_source.rs b/crates/oxc_transformer/src/react/jsx_source.rs
@@ -152,7 +152,7 @@ impl<'a> ReactJsxSource<'a> {
         let id = {
             let ident = filename_var.create_binding_identifier();
             let ident = self.ctx.ast.binding_pattern_identifier(ident);
-            self.ctx.ast.binding_pattern(SPAN, ident, None, false)
+            self.ctx.ast.binding_pattern(ident, None, false)
         };
         let decl = {
             let string = self.ctx.ast.string_literal(SPAN, &self.ctx.source_path.to_string_lossy());
diff --git a/crates/oxc_transformer/src/typescript/enum.rs b/crates/oxc_transformer/src/typescript/enum.rs
@@ -67,7 +67,7 @@ impl<'a> TypeScriptEnum<'a> {
         let span = decl.span;
         let ident = decl.id.clone();
         let kind = self.ctx.ast.binding_pattern_identifier(ident);
-        let id = self.ctx.ast.binding_pattern(SPAN, kind, None, false);
+        let id = self.ctx.ast.binding_pattern(kind, None, false);
 
         // ((Foo) => {
         let params =
@@ -127,7 +127,7 @@ impl<'a> TypeScriptEnum<'a> {
 
             let binding_identifier = BindingIdentifier::new(SPAN, enum_name.clone());
             let binding_pattern_kind = self.ctx.ast.binding_pattern_identifier(binding_identifier);
-            let binding = self.ctx.ast.binding_pattern(SPAN, binding_pattern_kind, None, false);
+            let binding = self.ctx.ast.binding_pattern(binding_pattern_kind, None, false);
             let decl =
                 self.ctx.ast.variable_declarator(SPAN, kind, binding, Some(call_expression), false);
 
diff --git a/crates/oxc_transformer/src/typescript/module.rs b/crates/oxc_transformer/src/typescript/module.rs
@@ -35,7 +35,7 @@ impl<'a> TypeScript<'a> {
         let decls = {
             let binding_identifier = BindingIdentifier::new(SPAN, decl.id.name.clone());
             let binding_pattern_kind = self.ctx.ast.binding_pattern_identifier(binding_identifier);
-            let binding = self.ctx.ast.binding_pattern(SPAN, binding_pattern_kind, None, false);
+            let binding = self.ctx.ast.binding_pattern(binding_pattern_kind, None, false);
             let decl_span = decl.span;
 
             let init = match &mut decl.module_reference {
diff --git a/crates/oxc_transformer/src/typescript/namespace.rs b/crates/oxc_transformer/src/typescript/namespace.rs
@@ -282,7 +282,7 @@ impl<'a> TypeScript<'a> {
         let declarations = {
             let ident = BindingIdentifier::new(SPAN, name);
             let pattern_kind = self.ctx.ast.binding_pattern_identifier(ident);
-            let binding = self.ctx.ast.binding_pattern(SPAN, pattern_kind, None, false);
+            let binding = self.ctx.ast.binding_pattern(pattern_kind, None, false);
             let decl = self.ctx.ast.variable_declarator(SPAN, kind, binding, None, false);
             self.ctx.ast.new_vec_single(decl)
         };
@@ -314,7 +314,7 @@ impl<'a> TypeScript<'a> {
             let params = {
                 let ident =
                     self.ctx.ast.binding_pattern_identifier(BindingIdentifier::new(SPAN, arg_name));
-                let pattern = self.ctx.ast.binding_pattern(SPAN, ident, None, false);
+                let pattern = self.ctx.ast.binding_pattern(ident, None, false);
                 let items =
                     self.ctx.ast.new_vec_single(self.ctx.ast.plain_formal_parameter(SPAN, pattern));
                 self.ctx.ast.formal_parameters(
diff --git a/crates/oxc_traverse/src/ancestor.rs b/crates/oxc_traverse/src/ancestor.rs
@@ -4922,7 +4922,6 @@ impl<'a> CatchParameterWithoutPattern<'a> {
     }
 }
 
-pub(crate) const OFFSET_BINDING_PATTERN_SPAN: usize = offset_of!(BindingPattern, span);
 pub(crate) const OFFSET_BINDING_PATTERN_KIND: usize = offset_of!(BindingPattern, kind);
 pub(crate) const OFFSET_BINDING_PATTERN_TYPE_ANNOTATION: usize =
     offset_of!(BindingPattern, type_annotation);
@@ -4933,11 +4932,6 @@ pub(crate) const OFFSET_BINDING_PATTERN_OPTIONAL: usize = offset_of!(BindingPatt
 pub struct BindingPatternWithoutKind<'a>(pub(crate) *const BindingPattern<'a>);
 
 impl<'a> BindingPatternWithoutKind<'a> {
-    #[inline]
-    pub fn span(&self) -> &Span {
-        unsafe { &*((self.0 as *const u8).add(OFFSET_BINDING_PATTERN_SPAN) as *const Span) }
-    }
-
     #[inline]
     pub fn type_annotation(&self) -> &Option<Box<'a, TSTypeAnnotation<'a>>> {
         unsafe {
@@ -4957,11 +4951,6 @@ impl<'a> BindingPatternWithoutKind<'a> {
 pub struct BindingPatternWithoutTypeAnnotation<'a>(pub(crate) *const BindingPattern<'a>);
 
 impl<'a> BindingPatternWithoutTypeAnnotation<'a> {
-    #[inline]
-    pub fn span(&self) -> &Span {
-        unsafe { &*((self.0 as *const u8).add(OFFSET_BINDING_PATTERN_SPAN) as *const Span) }
-    }
-
     #[inline]
     pub fn kind(&self) -> &BindingPatternKind<'a> {
         unsafe {

Original file line number	Diff line number	Diff line change
`@@ -872,8 +872,8 @@ impl<'a> Hash for CatchClause<'a> {`
`872`	`872`	`}`
`873`	`873`
`874`	`874`	`impl<'a> BindingPattern<'a> {`
`875`		`- pub fn new_with_kind(span: Span, kind: BindingPatternKind<'a>) -> Self {`
`876`		`- Self { span, kind, type_annotation: None, optional: false }`
	`875`	`+ pub fn new_with_kind(kind: BindingPatternKind<'a>) -> Self {`
	`876`	`+ Self { kind, type_annotation: None, optional: false }`
`877`	`877`	`}`
`878`	`878`
`879`	`879`	`pub fn get_identifier(&self) -> Option<Atom<'a>> {`
Original file line number	Diff line number	Diff line change
`@@ -65,8 +65,7 @@ impl<'a> IsolatedDeclarations<'a> {`
`65`	`65`	`self.error(default_export_inferred(expr.span()));`
`66`	`66`	`}`
`67`	`67`
`68`		`- let id =`
`69`		`- BindingPattern { span: SPAN, kind: id, type_annotation, optional: false };`
	`68`	`+ let id = BindingPattern { kind: id, type_annotation, optional: false };`
`70`	`69`	`let declarations = self`
`71`	`70`	`.ast`
`72`	`71`	`.new_vec_single(self.ast.variable_declarator(SPAN, kind, id, None, true));`