From 1f85f1a5f7fa1c2c77d65f2fa6ba4ed757960915 Mon Sep 17 00:00:00 2001 From: rzvxa <3788964+rzvxa@users.noreply.github.com> Date: Tue, 25 Jun 2024 09:43:48 +0000 Subject: [PATCH] refactor(ast)!: revert adding `span` field to the `BindingPattern` type. (#3899) 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. --- crates/oxc_ast/src/ast/js.rs | 2 -- crates/oxc_ast/src/ast_builder.rs | 4 +--- crates/oxc_ast/src/ast_impl/js.rs | 4 ++-- crates/oxc_isolated_declarations/src/class.rs | 2 +- crates/oxc_isolated_declarations/src/declaration.rs | 1 - crates/oxc_isolated_declarations/src/function.rs | 1 - crates/oxc_isolated_declarations/src/module.rs | 3 +-- crates/oxc_parser/src/js/arrow.rs | 5 ++--- crates/oxc_parser/src/js/binding.rs | 7 +++---- crates/oxc_parser/src/js/declaration.rs | 12 ++---------- crates/oxc_transformer/src/es2015/arrow_functions.rs | 1 - crates/oxc_transformer/src/helpers/module_imports.rs | 2 +- crates/oxc_transformer/src/react/jsx_source.rs | 2 +- crates/oxc_transformer/src/typescript/enum.rs | 4 ++-- crates/oxc_transformer/src/typescript/module.rs | 2 +- crates/oxc_transformer/src/typescript/namespace.rs | 4 ++-- crates/oxc_traverse/src/ancestor.rs | 11 ----------- 17 files changed, 19 insertions(+), 48 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index c5f28d5a48129..74e34deca8e23 100644 --- 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 index b5466b10db666..c6f0412dc7c27 100644 --- 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>>, 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 index ac1400f2a8bb5..0b015d12d884d 100644 --- 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> { diff --git a/crates/oxc_isolated_declarations/src/class.rs b/crates/oxc_isolated_declarations/src/class.rs index 36168c2608e32..2a2727d8288bd 100644 --- 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, 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 index 6906262898f45..cfc2291254011 100644 --- 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 index 6d7b141262835..12c7278f6b992 100644 --- 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 index c0ffde42a16c9..04ca3cd9034f8 100644 --- 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 index cb8521e4d344d..2998766146452 100644 --- 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 index 98beef2ad8b0d..6e6ccd9f1a4cd 100644 --- a/crates/oxc_parser/src/js/binding.rs +++ b/crates/oxc_parser/src/js/binding.rs @@ -20,7 +20,6 @@ impl<'a> ParserImpl<'a> { &mut self, allow_question: bool, ) -> Result> { - 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 }; @@ -28,7 +27,7 @@ impl<'a> ParserImpl<'a> { 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> { @@ -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 index 81ab133b5c0a2..88f774d50a5f7 100644 --- 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 index 580bf369492ce..ee5abaf2e0b9c 100644 --- 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 index dd045bb5328c5..dad026b8e529e 100644 --- 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 index aa865ab51c712..185bf81d7a9b6 100644 --- 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 index 83307813394e6..7d053a0a43e71 100644 --- 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 index 6ee8d2b7fde07..ce55b4b9f991c 100644 --- 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 index 5937d9dd28c3c..592408299db88 100644 --- 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 index cfefec7ec9b8f..7eebf90dabb60 100644 --- 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>> { 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 {