From 092de67c47de2018ab81f37b271e57533709e8f7 Mon Sep 17 00:00:00 2001 From: ottomated <31470743+ottomated@users.noreply.github.com> Date: Sat, 9 Nov 2024 05:23:53 +0000 Subject: [PATCH] fix(types)!: append `rest` field into `elements` for objects and arrays to align with estree (#7212) Fixes https://github.com/oxc-project/oxc/pull/7149#discussion_r1831677446 and removes the need for some manual typescript. --- crates/oxc_ast/src/ast/js.rs | 4 -- crates/oxc_ast/src/generated/derive_estree.rs | 20 +++++++-- crates/oxc_estree/src/lib.rs | 21 +-------- crates/oxc_estree/src/ser.rs | 23 ++++++++++ npm/oxc-types/types.d.ts | 12 ++---- tasks/ast_tools/src/derives/estree.rs | 13 +++--- tasks/ast_tools/src/generators/typescript.rs | 43 +++++++++++++++++-- 7 files changed, 91 insertions(+), 45 deletions(-) create mode 100644 crates/oxc_estree/src/ser.rs diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index d260cdba0..2d8733090 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -807,7 +807,6 @@ pub use match_assignment_target_pattern; #[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)] pub struct ArrayAssignmentTarget<'a> { pub span: Span, - #[estree(type = "Array")] pub elements: Vec<'a, Option>>, #[estree(append_to = "elements")] pub rest: Option>, @@ -823,7 +822,6 @@ pub struct ArrayAssignmentTarget<'a> { #[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)] pub struct ObjectAssignmentTarget<'a> { pub span: Span, - #[estree(type = "Array")] pub properties: Vec<'a, AssignmentTargetProperty<'a>>, #[estree(append_to = "properties")] pub rest: Option>, @@ -1482,7 +1480,6 @@ pub struct AssignmentPattern<'a> { #[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)] pub struct ObjectPattern<'a> { pub span: Span, - #[estree(type = "Array")] pub properties: Vec<'a, BindingProperty<'a>>, #[estree(append_to = "properties")] pub rest: Option>>, @@ -1505,7 +1502,6 @@ pub struct BindingProperty<'a> { #[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)] pub struct ArrayPattern<'a> { pub span: Span, - #[estree(type = "Array")] pub elements: Vec<'a, Option>>, #[estree(append_to = "elements")] pub rest: Option>>, diff --git a/crates/oxc_ast/src/generated/derive_estree.rs b/crates/oxc_ast/src/generated/derive_estree.rs index 17cd751ba..6c43b3f73 100644 --- a/crates/oxc_ast/src/generated/derive_estree.rs +++ b/crates/oxc_ast/src/generated/derive_estree.rs @@ -684,7 +684,10 @@ impl<'a> Serialize for ArrayAssignmentTarget<'a> { let mut map = serializer.serialize_map(None)?; map.serialize_entry("type", "ArrayAssignmentTarget")?; self.span.serialize(serde::__private::ser::FlatMapSerializer(&mut map))?; - map.serialize_entry("elements", &oxc_estree::AppendTo(&self.elements, &self.rest))?; + map.serialize_entry( + "elements", + &oxc_estree::ser::AppendTo { array: &self.elements, after: &self.rest }, + )?; map.end() } } @@ -694,7 +697,10 @@ impl<'a> Serialize for ObjectAssignmentTarget<'a> { let mut map = serializer.serialize_map(None)?; map.serialize_entry("type", "ObjectAssignmentTarget")?; self.span.serialize(serde::__private::ser::FlatMapSerializer(&mut map))?; - map.serialize_entry("properties", &oxc_estree::AppendTo(&self.properties, &self.rest))?; + map.serialize_entry( + "properties", + &oxc_estree::ser::AppendTo { array: &self.properties, after: &self.rest }, + )?; map.end() } } @@ -1308,7 +1314,10 @@ impl<'a> Serialize for ObjectPattern<'a> { let mut map = serializer.serialize_map(None)?; map.serialize_entry("type", "ObjectPattern")?; self.span.serialize(serde::__private::ser::FlatMapSerializer(&mut map))?; - map.serialize_entry("properties", &oxc_estree::AppendTo(&self.properties, &self.rest))?; + map.serialize_entry( + "properties", + &oxc_estree::ser::AppendTo { array: &self.properties, after: &self.rest }, + )?; map.end() } } @@ -1331,7 +1340,10 @@ impl<'a> Serialize for ArrayPattern<'a> { let mut map = serializer.serialize_map(None)?; map.serialize_entry("type", "ArrayPattern")?; self.span.serialize(serde::__private::ser::FlatMapSerializer(&mut map))?; - map.serialize_entry("elements", &oxc_estree::AppendTo(&self.elements, &self.rest))?; + map.serialize_entry( + "elements", + &oxc_estree::ser::AppendTo { array: &self.elements, after: &self.rest }, + )?; map.end() } } diff --git a/crates/oxc_estree/src/lib.rs b/crates/oxc_estree/src/lib.rs index c08502367..aa2673754 100644 --- a/crates/oxc_estree/src/lib.rs +++ b/crates/oxc_estree/src/lib.rs @@ -1,25 +1,6 @@ #[cfg(feature = "serialize")] -use serde::ser::{Serialize, SerializeSeq, Serializer}; +pub mod ser; /// Empty trait that will be used later for custom serialization and TypeScript /// generation for AST nodes. pub trait ESTree {} - -#[cfg(feature = "serialize")] -pub struct AppendTo<'a, TVec, TChild>(pub &'a [TVec], pub &'a Option); - -#[cfg(feature = "serialize")] -impl<'b, TVec: Serialize, TChild: Serialize> Serialize for AppendTo<'b, TVec, TChild> { - fn serialize(&self, serializer: S) -> Result { - if let Some(child) = self.1 { - let mut seq = serializer.serialize_seq(Some(self.0.len() + 1))?; - for element in self.0 { - seq.serialize_element(element)?; - } - seq.serialize_element(child)?; - seq.end() - } else { - self.0.serialize(serializer) - } - } -} diff --git a/crates/oxc_estree/src/ser.rs b/crates/oxc_estree/src/ser.rs new file mode 100644 index 000000000..77410fc18 --- /dev/null +++ b/crates/oxc_estree/src/ser.rs @@ -0,0 +1,23 @@ +use serde::ser::{Serialize, SerializeSeq, 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<'b, TVec: Serialize, TAfter: Serialize> Serialize for AppendTo<'b, TVec, TAfter> { + fn serialize(&self, serializer: S) -> Result { + if let Some(after) = self.after { + let mut seq = serializer.serialize_seq(Some(self.array.len() + 1))?; + for element in self.array { + seq.serialize_element(element)?; + } + seq.serialize_element(after)?; + seq.end() + } else { + self.array.serialize(serializer) + } + } +} diff --git a/npm/oxc-types/types.d.ts b/npm/oxc-types/types.d.ts index 1caa9915c..b861c67d3 100644 --- a/npm/oxc-types/types.d.ts +++ b/npm/oxc-types/types.d.ts @@ -386,14 +386,12 @@ export type AssignmentTargetPattern = ArrayAssignmentTarget | ObjectAssignmentTa export interface ArrayAssignmentTarget extends Span { type: 'ArrayAssignmentTarget'; - elements: Array; - rest: AssignmentTargetRest | null; + elements: Array; } export interface ObjectAssignmentTarget extends Span { type: 'ObjectAssignmentTarget'; - properties: Array; - rest: AssignmentTargetRest | null; + properties: Array; } export interface AssignmentTargetRest extends Span { @@ -730,8 +728,7 @@ export interface AssignmentPattern extends Span { export interface ObjectPattern extends Span { type: 'ObjectPattern'; - properties: Array; - rest: BindingRestElement | null; + properties: Array; } export interface BindingProperty extends Span { @@ -744,8 +741,7 @@ export interface BindingProperty extends Span { export interface ArrayPattern extends Span { type: 'ArrayPattern'; - elements: Array; - rest: BindingRestElement | null; + elements: Array; } export interface BindingRestElement extends Span { diff --git a/tasks/ast_tools/src/derives/estree.rs b/tasks/ast_tools/src/derives/estree.rs index a92b724bb..eaf9584cc 100644 --- a/tasks/ast_tools/src/derives/estree.rs +++ b/tasks/ast_tools/src/derives/estree.rs @@ -118,11 +118,11 @@ fn serialize_struct(def: &StructDef, schema: &Schema) -> TokenStream { None => false, }; - let append_child = append_to.get(&ident.to_string()); + let append_after = append_to.get(&ident.to_string()); if always_flatten || field.markers.derive_attributes.estree.flatten { assert!( - append_child.is_none(), + append_after.is_none(), "Cannot flatten and append to the same field (on {ident})" ); fields.push(quote! { @@ -130,12 +130,15 @@ fn serialize_struct(def: &StructDef, schema: &Schema) -> TokenStream { serde::__private::ser::FlatMapSerializer(&mut map) )?; }); - } else if let Some(append_child) = append_child { - let child_ident = append_child.ident().unwrap(); + } else if let Some(append_after) = append_after { + let after_ident = append_after.ident().unwrap(); fields.push(quote! { map.serialize_entry( #name, - &oxc_estree::AppendTo(&self.#ident, &self.#child_ident) + &oxc_estree::ser::AppendTo { + array: &self.#ident, + after: &self.#after_ident + } )?; }); } else { diff --git a/tasks/ast_tools/src/generators/typescript.rs b/tasks/ast_tools/src/generators/typescript.rs index 53ad654d8..8baae4f4c 100644 --- a/tasks/ast_tools/src/generators/typescript.rs +++ b/tasks/ast_tools/src/generators/typescript.rs @@ -1,12 +1,12 @@ use convert_case::{Case, Casing}; use itertools::Itertools; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ output::Output, schema::{ serialize::{enum_variant_name, get_always_flatten_structs, get_type_tag}, - EnumDef, GetIdent, Schema, StructDef, TypeDef, TypeName, + EnumDef, FieldDef, GetIdent, Schema, StructDef, TypeDef, TypeName, }, Generator, TypeId, }; @@ -72,11 +72,26 @@ fn typescript_struct(def: &StructDef, always_flatten_structs: &FxHashSet fields.push_str(&format!("\n\ttype: '{type_tag}';")); } + let mut append_to: FxHashMap = FxHashMap::default(); + + // Scan through to find all append_to fields for field in &def.fields { - if field.markers.derive_attributes.estree.skip { + let Some(parent) = field.markers.derive_attributes.estree.append_to.as_ref() else { + continue; + }; + assert!( + append_to.insert(parent.clone(), field).is_none(), + "Duplicate append_to target (on {ident})" + ); + } + + for field in &def.fields { + if field.markers.derive_attributes.estree.skip + || field.markers.derive_attributes.estree.append_to.is_some() + { continue; } - let ty = match &field.markers.derive_attributes.estree.typescript_type { + let mut ty = match &field.markers.derive_attributes.estree.typescript_type { Some(ty) => ty.clone(), None => type_to_string(field.typ.name()), }; @@ -91,6 +106,26 @@ fn typescript_struct(def: &StructDef, always_flatten_structs: &FxHashSet continue; } + let ident = field.ident().unwrap(); + if let Some(append_after) = append_to.get(&ident.to_string()) { + let after_type = match &append_after.markers.derive_attributes.estree.typescript_type { + Some(ty) => ty.clone(), + None => { + let typ = append_after.typ.name(); + if let TypeName::Opt(inner) = typ { + type_to_string(inner) + } else { + panic!("expected field labeled with append_to to be Option<...>, but found {typ}"); + } + } + }; + if let Some(inner) = ty.strip_prefix("Array<") { + ty = format!("Array<{after_type} | {inner}"); + } else { + panic!("expected append_to target to be a Vec, but found {ty}"); + } + } + let name = match &field.markers.derive_attributes.estree.rename { Some(rename) => rename.to_string(), None => field.name.clone().unwrap().to_case(Case::Camel),