From b16331ecfa7e4dab703ca7e26d9cc1129d0e59b2 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 7 May 2025 13:25:17 +0000 Subject: [PATCH] refactor(ast/estree): generalize concatenating fields with `Concat2` (#10848) Pure refactor. Replace `AppendTo` and `AppendToConcat` which are used in ESTree serializer to concatenate 2 fields with a more generalized `Concat2`, which performs both functions. --- crates/oxc_allocator/src/vec.rs | 11 ++++- crates/oxc_ast/src/generated/derive_estree.rs | 27 ++++-------- crates/oxc_ast/src/serialize.rs | 11 ++--- crates/oxc_estree/src/lib.rs | 2 - crates/oxc_estree/src/ser.rs | 41 ------------------- crates/oxc_estree/src/serialize/concat.rs | 28 +++++++++++++ crates/oxc_estree/src/serialize/mod.rs | 2 + .../oxc_span/src/generated/derive_estree.rs | 3 +- .../oxc_syntax/src/generated/derive_estree.rs | 3 +- napi/parser/src/generated/derive_estree.rs | 3 +- tasks/ast_tools/src/derives/estree.rs | 14 ++----- tasks/ast_tools/src/schema/defs/type.rs | 1 + 12 files changed, 58 insertions(+), 88 deletions(-) delete mode 100644 crates/oxc_estree/src/ser.rs create mode 100644 crates/oxc_estree/src/serialize/concat.rs diff --git a/crates/oxc_allocator/src/vec.rs b/crates/oxc_allocator/src/vec.rs index 2fff777512776..00d05057dd315 100644 --- a/crates/oxc_allocator/src/vec.rs +++ b/crates/oxc_allocator/src/vec.rs @@ -15,7 +15,7 @@ use std::{ use crate::vec2::Vec as InnerVec; #[cfg(any(feature = "serialize", test))] -use oxc_estree::{ESTree, Serializer as ESTreeSerializer}; +use oxc_estree::{ConcatElement, ESTree, SequenceSerializer, Serializer as ESTreeSerializer}; #[cfg(any(feature = "serialize", test))] use serde::{Serialize, Serializer as SerdeSerializer}; @@ -248,6 +248,15 @@ impl ESTree for Vec<'_, T> { } } +#[cfg(feature = "serialize")] +impl ConcatElement for Vec<'_, T> { + fn push_to_sequence(&self, seq: &mut S) { + for element in self { + seq.serialize_element(element); + } + } +} + impl Hash for Vec<'_, T> { #[inline(always)] fn hash(&self, state: &mut H) { diff --git a/crates/oxc_ast/src/generated/derive_estree.rs b/crates/oxc_ast/src/generated/derive_estree.rs index 57590d623a7b5..1f783c99a4c06 100644 --- a/crates/oxc_ast/src/generated/derive_estree.rs +++ b/crates/oxc_ast/src/generated/derive_estree.rs @@ -4,8 +4,7 @@ #![allow(unused_imports, clippy::match_same_arms, clippy::semicolon_if_nothing_returned)] use oxc_estree::{ - ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, - ser::{AppendTo, AppendToConcat}, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, }; use crate::ast::comment::*; @@ -635,7 +634,7 @@ impl ESTree for ArrayAssignmentTarget<'_> { state.serialize_field("type", &JsonSafeString("ArrayPattern")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field("elements", &AppendTo { array: &self.elements, after: &self.rest }); + state.serialize_field("elements", &Concat2(&self.elements, &self.rest)); state.serialize_ts_field("decorators", &crate::serialize::TsEmptyArray(self)); state.serialize_ts_field("optional", &crate::serialize::TsFalse(self)); state.serialize_ts_field("typeAnnotation", &crate::serialize::TsNull(self)); @@ -649,10 +648,7 @@ impl ESTree for ObjectAssignmentTarget<'_> { state.serialize_field("type", &JsonSafeString("ObjectPattern")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field( - "properties", - &AppendTo { array: &self.properties, after: &self.rest }, - ); + state.serialize_field("properties", &Concat2(&self.properties, &self.rest)); state.serialize_ts_field("decorators", &crate::serialize::TsEmptyArray(self)); state.serialize_ts_field("optional", &crate::serialize::TsFalse(self)); state.serialize_ts_field("typeAnnotation", &crate::serialize::TsNull(self)); @@ -1295,10 +1291,7 @@ impl ESTree for ObjectPattern<'_> { state.serialize_field("type", &JsonSafeString("ObjectPattern")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field( - "properties", - &AppendTo { array: &self.properties, after: &self.rest }, - ); + state.serialize_field("properties", &Concat2(&self.properties, &self.rest)); state.serialize_ts_field("decorators", &crate::serialize::TsEmptyArray(self)); state.serialize_ts_field("optional", &crate::serialize::TsFalse(self)); state.serialize_ts_field("typeAnnotation", &crate::serialize::TsNull(self)); @@ -1329,7 +1322,7 @@ impl ESTree for ArrayPattern<'_> { state.serialize_field("type", &JsonSafeString("ArrayPattern")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field("elements", &AppendTo { array: &self.elements, after: &self.rest }); + state.serialize_field("elements", &Concat2(&self.elements, &self.rest)); state.serialize_ts_field("decorators", &crate::serialize::TsEmptyArray(self)); state.serialize_ts_field("optional", &crate::serialize::TsFalse(self)); state.serialize_ts_field("typeAnnotation", &crate::serialize::TsNull(self)); @@ -1419,10 +1412,7 @@ impl ESTree for FunctionBody<'_> { state.serialize_field("type", &JsonSafeString("BlockStatement")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field( - "body", - &AppendToConcat { array: &self.directives, after: &self.statements }, - ); + state.serialize_field("body", &Concat2(&self.directives, &self.statements)); state.end(); } } @@ -3076,10 +3066,7 @@ impl ESTree for TSModuleBlock<'_> { state.serialize_field("type", &JsonSafeString("TSModuleBlock")); state.serialize_field("start", &self.span.start); state.serialize_field("end", &self.span.end); - state.serialize_field( - "body", - &AppendToConcat { array: &self.directives, after: &self.body }, - ); + state.serialize_field("body", &Concat2(&self.directives, &self.body)); state.end(); } } diff --git a/crates/oxc_ast/src/serialize.rs b/crates/oxc_ast/src/serialize.rs index 54e3f44fd0914..e0f36d0d6236e 100644 --- a/crates/oxc_ast/src/serialize.rs +++ b/crates/oxc_ast/src/serialize.rs @@ -6,9 +6,9 @@ use crate::ast::*; use oxc_ast_macros::ast_meta; use oxc_estree::{ CompactFixesJSSerializer, CompactFixesTSSerializer, CompactJSSerializer, CompactTSSerializer, - ESTree, FlatStructSerializer, JsonSafeString, LoneSurrogatesString, PrettyFixesJSSerializer, - PrettyFixesTSSerializer, PrettyJSSerializer, PrettyTSSerializer, SequenceSerializer, - Serializer, StructSerializer, ser::AppendToConcat, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, LoneSurrogatesString, + PrettyFixesJSSerializer, PrettyFixesTSSerializer, PrettyJSSerializer, PrettyTSSerializer, + SequenceSerializer, Serializer, StructSerializer, }; use oxc_span::GetSpan; @@ -171,10 +171,7 @@ impl ESTree for ProgramConverter<'_, '_> { state.serialize_field("type", &JsonSafeString("Program")); state.serialize_field("start", &span_start); state.serialize_field("end", &program.span.end); - state.serialize_field( - "body", - &AppendToConcat { array: &program.directives, after: &program.body }, - ); + state.serialize_field("body", &Concat2(&program.directives, &program.body)); state.serialize_field("sourceType", &program.source_type.module_kind()); state.serialize_field("hashbang", &program.hashbang); state.end(); diff --git a/crates/oxc_estree/src/lib.rs b/crates/oxc_estree/src/lib.rs index 998c9bc7430bd..ed6a304bc087a 100644 --- a/crates/oxc_estree/src/lib.rs +++ b/crates/oxc_estree/src/lib.rs @@ -1,6 +1,4 @@ #[cfg(feature = "serialize")] -pub mod ser; -#[cfg(feature = "serialize")] mod serialize; #[cfg(feature = "serialize")] pub use serialize::*; diff --git a/crates/oxc_estree/src/ser.rs b/crates/oxc_estree/src/ser.rs deleted file mode 100644 index 1992bd36ec0f0..0000000000000 --- a/crates/oxc_estree/src/ser.rs +++ /dev/null @@ -1,41 +0,0 @@ -use crate::{ESTree, SequenceSerializer, Serializer}; - -/// A helper struct for serializing a sequence followed by an optional element. -/// This is only used by generated ESTree serialization code. -pub struct AppendTo<'a, TVec, TAfter> { - pub array: &'a [TVec], - pub after: &'a Option, -} - -impl ESTree for AppendTo<'_, TVec, TAfter> { - fn serialize(&self, serializer: S) { - if let Some(after) = self.after { - let mut seq = serializer.serialize_sequence(); - for element in self.array { - seq.serialize_element(element); - } - seq.serialize_element(after); - seq.end(); - } else { - self.array.serialize(serializer); - } - } -} - -pub struct AppendToConcat<'a, TVec, TAfter> { - pub array: &'a [TVec], - pub after: &'a [TAfter], -} - -impl ESTree for AppendToConcat<'_, TVec, TAfter> { - fn serialize(&self, serializer: S) { - let mut seq = serializer.serialize_sequence(); - for element in self.array { - seq.serialize_element(element); - } - for element in self.after { - seq.serialize_element(element); - } - seq.end(); - } -} diff --git a/crates/oxc_estree/src/serialize/concat.rs b/crates/oxc_estree/src/serialize/concat.rs new file mode 100644 index 0000000000000..927ea93aba240 --- /dev/null +++ b/crates/oxc_estree/src/serialize/concat.rs @@ -0,0 +1,28 @@ +use crate::{ESTree, SequenceSerializer, Serializer}; + +/// Trait for types which can be concatenated. +/// +/// Implemented by `Option` and `Vec`. +pub trait ConcatElement { + fn push_to_sequence(&self, seq: &mut S); +} + +impl ConcatElement for Option { + fn push_to_sequence(&self, seq: &mut S) { + if let Some(value) = self { + seq.serialize_element(value); + } + } +} + +/// Helper struct for concatenating 2 elements into a sequence. +pub struct Concat2<'t, C1: ConcatElement, C2: ConcatElement>(pub &'t C1, pub &'t C2); + +impl ESTree for Concat2<'_, C1, C2> { + fn serialize(&self, serializer: S) { + let mut seq = serializer.serialize_sequence(); + self.0.push_to_sequence(&mut seq); + self.1.push_to_sequence(&mut seq); + seq.end(); + } +} diff --git a/crates/oxc_estree/src/serialize/mod.rs b/crates/oxc_estree/src/serialize/mod.rs index 1905e137165a4..275182fc8e9b4 100644 --- a/crates/oxc_estree/src/serialize/mod.rs +++ b/crates/oxc_estree/src/serialize/mod.rs @@ -8,6 +8,7 @@ use itoa::Buffer as ItoaBuffer; use oxc_data_structures::{code_buffer::CodeBuffer, stack::NonEmptyStack}; mod blanket; +mod concat; mod config; mod formatter; mod primitives; @@ -19,6 +20,7 @@ use formatter::{CompactFormatter, Formatter, PrettyFormatter}; use sequences::ESTreeSequenceSerializer; use structs::ESTreeStructSerializer; +pub use concat::{Concat2, ConcatElement}; pub use sequences::SequenceSerializer; pub use strings::{JsonSafeString, LoneSurrogatesString}; pub use structs::{FlatStructSerializer, StructSerializer}; diff --git a/crates/oxc_span/src/generated/derive_estree.rs b/crates/oxc_span/src/generated/derive_estree.rs index 292ecc3e09eab..b8e9e4c798be0 100644 --- a/crates/oxc_span/src/generated/derive_estree.rs +++ b/crates/oxc_span/src/generated/derive_estree.rs @@ -4,8 +4,7 @@ #![allow(unused_imports, clippy::match_same_arms, clippy::semicolon_if_nothing_returned)] use oxc_estree::{ - ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, - ser::{AppendTo, AppendToConcat}, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, }; use crate::source_type::*; diff --git a/crates/oxc_syntax/src/generated/derive_estree.rs b/crates/oxc_syntax/src/generated/derive_estree.rs index 7a6c3ccd494c2..c4ebb07acfbc9 100644 --- a/crates/oxc_syntax/src/generated/derive_estree.rs +++ b/crates/oxc_syntax/src/generated/derive_estree.rs @@ -4,8 +4,7 @@ #![allow(unused_imports, clippy::match_same_arms, clippy::semicolon_if_nothing_returned)] use oxc_estree::{ - ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, - ser::{AppendTo, AppendToConcat}, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, }; use crate::module_record::*; diff --git a/napi/parser/src/generated/derive_estree.rs b/napi/parser/src/generated/derive_estree.rs index 102299c097183..658e7977d1ac1 100644 --- a/napi/parser/src/generated/derive_estree.rs +++ b/napi/parser/src/generated/derive_estree.rs @@ -4,8 +4,7 @@ #![allow(unused_imports, clippy::match_same_arms, clippy::semicolon_if_nothing_returned)] use oxc_estree::{ - ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, - ser::{AppendTo, AppendToConcat}, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, }; use crate::raw_transfer_types::*; diff --git a/tasks/ast_tools/src/derives/estree.rs b/tasks/ast_tools/src/derives/estree.rs index a30c4e2f9778a..935f955548e5f 100644 --- a/tasks/ast_tools/src/derives/estree.rs +++ b/tasks/ast_tools/src/derives/estree.rs @@ -71,8 +71,7 @@ impl Derive for DeriveESTree { ///@@line_break use oxc_estree::{ - ser::{AppendTo, AppendToConcat}, - ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, + Concat2, ESTree, FlatStructSerializer, JsonSafeString, Serializer, StructSerializer, }; } } @@ -443,16 +442,9 @@ impl<'s> StructSerializerGenerator<'s> { let converter_path = get_converter_path(converter_name, self.krate, self.schema); quote!( #converter_path(#self_path) ) } else if let Some(append_field_index) = field.estree.append_field_index { - let append_field = &struct_def.fields[append_field_index]; - let append_from_ident = append_field.ident(); - let wrapper_name = if append_field.type_def(self.schema).is_option() { - "AppendTo" - } else { - "AppendToConcat" - }; - let wrapper_ident = create_safe_ident(wrapper_name); + let append_from_ident = struct_def.fields[append_field_index].ident(); quote! { - #wrapper_ident { array: &#self_path.#field_name_ident, after: &#self_path.#append_from_ident } + Concat2(&#self_path.#field_name_ident, &#self_path.#append_from_ident) } } else if field.estree.json_safe { // Wrap value in `JsonSafeString(...)` if field is tagged `#[estree(json_safe)]` diff --git a/tasks/ast_tools/src/schema/defs/type.rs b/tasks/ast_tools/src/schema/defs/type.rs index 6693a28516462..7ced196272086 100644 --- a/tasks/ast_tools/src/schema/defs/type.rs +++ b/tasks/ast_tools/src/schema/defs/type.rs @@ -161,6 +161,7 @@ impl TypeDef { } } + #[expect(dead_code)] pub fn is_option(&self) -> bool { matches!(self, Self::Option(_)) }