Skip to content

Commit

Permalink
refactor(ast)!: revert adding span field to the BindingPattern ty…
Browse files Browse the repository at this point in the history
…pe. (#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.
  • Loading branch information
rzvxa committed Jun 25, 2024
1 parent 4bf405d commit 1f85f1a
Show file tree
Hide file tree
Showing 17 changed files with 19 additions and 48 deletions.
2 changes: 0 additions & 2 deletions crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_ast/src/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_isolated_declarations/src/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_isolated_declarations/src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_isolated_declarations/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
5 changes: 2 additions & 3 deletions crates/oxc_parser/src/js/arrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions crates/oxc_parser/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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))?;
Expand Down Expand Up @@ -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());
Expand Down
12 changes: 2 additions & 10 deletions crates/oxc_parser/src/js/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_transformer/src/es2015/arrow_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_transformer/src/helpers/module_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_transformer/src/react/jsx_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_transformer/src/typescript/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_transformer/src/typescript/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_transformer/src/typescript/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
Expand Down Expand Up @@ -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(
Expand Down
11 changes: 0 additions & 11 deletions crates/oxc_traverse/src/ancestor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 1f85f1a

Please sign in to comment.