From d8b27afc35b51412e94967cf968754c4dcd8f3ce Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:09:05 +0000 Subject: [PATCH] refactor(ast)!: no unneccesary trailing underscores on `AstBuilder` method names (#8283) `AstBuilder` method names have an `_` added on end if method name is not a valid identifier (e.g. `super`). But no need for trailing underscore on `alloc_super`. Currently this only applies to `super`. But future-proof by checking against all Rust's reserved words. This is a breaking change, because `alloc_super` method was previously called `alloc_super_`. But probably no-one uses that method anyway - usually you'd use `expression_super` method to get an `Expression::Super`. --- crates/oxc_ast/src/generated/ast_builder.rs | 6 ++-- tasks/ast_tools/src/generators/ast_builder.rs | 34 +++++++++---------- tasks/ast_tools/src/util.rs | 6 +++- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/crates/oxc_ast/src/generated/ast_builder.rs b/crates/oxc_ast/src/generated/ast_builder.rs index 186c013af1c42..94d973544c94d 100644 --- a/crates/oxc_ast/src/generated/ast_builder.rs +++ b/crates/oxc_ast/src/generated/ast_builder.rs @@ -535,7 +535,7 @@ impl<'a> AstBuilder<'a> { /// - span: The [`Span`] covering this node #[inline] pub fn expression_super(self, span: Span) -> Expression<'a> { - Expression::Super(self.alloc_super_(span)) + Expression::Super(self.alloc_super(span)) } /// Build an [`Expression::ArrayExpression`] @@ -2974,7 +2974,7 @@ impl<'a> AstBuilder<'a> { /// Build a [`Super`]. /// - /// If you want the built node to be allocated in the memory arena, use [`AstBuilder::alloc_super_`] instead. + /// If you want the built node to be allocated in the memory arena, use [`AstBuilder::alloc_super`] instead. /// /// ## Parameters /// - span: The [`Span`] covering this node @@ -2990,7 +2990,7 @@ impl<'a> AstBuilder<'a> { /// ## Parameters /// - span: The [`Span`] covering this node #[inline] - pub fn alloc_super_(self, span: Span) -> Box<'a, Super> { + pub fn alloc_super(self, span: Span) -> Box<'a, Super> { Box::new_in(self.super_(span), self.allocator) } diff --git a/tasks/ast_tools/src/generators/ast_builder.rs b/tasks/ast_tools/src/generators/ast_builder.rs index cb94bf5e9b4e0..807a5f69467c2 100644 --- a/tasks/ast_tools/src/generators/ast_builder.rs +++ b/tasks/ast_tools/src/generators/ast_builder.rs @@ -11,7 +11,7 @@ use crate::{ schema::{ EnumDef, FieldDef, GetIdent, Schema, StructDef, ToType, TypeDef, TypeName, VariantDef, }, - util::{TypeAnalysis, TypeWrapper}, + util::{is_reserved_name, TypeAnalysis, TypeWrapper}, Generator, }; @@ -93,13 +93,14 @@ fn enum_builder_name(enum_name: String, var_name: String) -> Ident { format_ident!("{}_{}", fn_ident_name(enum_name), fn_ident_name(var_name)) } -fn struct_builder_name(struct_: &StructDef) -> Ident { - static RUST_KEYWORDS: [&str; 1] = ["super"]; - let mut ident = fn_ident_name(struct_.name.as_str()); - if RUST_KEYWORDS.contains(&ident.as_str()) { - ident.push('_'); +fn struct_builder_name(name: &str, does_alloc: bool) -> Ident { + if does_alloc { + format_ident!("alloc_{name}") + } else if is_reserved_name(name) { + format_ident!("{name}_") + } else { + format_ident!("{name}") } - format_ident!("{ident}") } fn generate_builder_fn(def: &TypeDef, schema: &Schema) -> TokenStream { @@ -131,20 +132,16 @@ fn generate_enum_variant_builder_fn( .or_else(|| var_type.transparent_type_id()) .and_then(|id| schema.get(id)) .expect("type not found!"); - let (params, mut inner_builder) = match ty { - TypeDef::Struct(it) => (get_struct_params(it, schema), struct_builder_name(it)), - TypeDef::Enum(_) => panic!("Unsupported!"), - }; - let does_alloc = matches!(var_type_name, TypeName::Box(_)); - if does_alloc { - inner_builder = format_ident!("alloc_{inner_builder}"); - } + let TypeDef::Struct(field_def) = ty else { panic!("Unsupported!") }; + let params = get_struct_params(field_def, schema); let params = params.into_iter().filter(Param::not_default).collect_vec(); let fields = params.iter().map(|it| it.ident.clone()); let (generic_params, where_clause) = get_generic_params(¶ms); + let does_alloc = matches!(var_type_name, TypeName::Box(_)); + let inner_builder = struct_builder_name(&fn_ident_name(&field_def.name), does_alloc); let inner = quote!(self.#inner_builder(#(#fields),*)); let article = article_for(enum_ident.to_string()); @@ -191,8 +188,9 @@ fn default_init_field(field: &FieldDef) -> bool { fn generate_struct_builder_fn(ty: &StructDef, schema: &Schema) -> TokenStream { let ident = ty.ident(); let as_type = ty.to_type(); - let fn_name = struct_builder_name(ty); - let alloc_fn_name = format_ident!("alloc_{fn_name}"); + let ty_name = fn_ident_name(&ty.name); + let fn_name = struct_builder_name(&ty_name, false); + let alloc_fn_name = struct_builder_name(&ty_name, true); let params_incl_defaults = get_struct_params(ty, schema); let (generic_params, where_clause) = get_generic_params(¶ms_incl_defaults); @@ -288,7 +286,7 @@ fn generate_struct_builder_fn(ty: &StructDef, schema: &Schema) -> TokenStream { }; if has_default_fields { - let fn_name = format_ident!("{fn_name}_with_{}", default_field_names.join("_and_")); + let fn_name = format_ident!("{ty_name}_with_{}", default_field_names.join("_and_")); let alloc_fn_name = format_ident!("alloc_{fn_name}"); let with = format!(" with `{}`", default_field_type_names.iter().join("` and `")); diff --git a/tasks/ast_tools/src/util.rs b/tasks/ast_tools/src/util.rs index 89a3b45d8bf44..c8aa31bbe164e 100644 --- a/tasks/ast_tools/src/util.rs +++ b/tasks/ast_tools/src/util.rs @@ -296,13 +296,17 @@ static RESERVED_NAMES: &[&str] = &[ "macro_rules", "union", // "dyn" also listed as a weak keyword, but is already on strict list ]; +pub fn is_reserved_name(name: &str) -> bool { + RESERVED_NAMES.contains(&name) +} + impl ToIdent for S where S: AsRef, { fn to_ident(&self) -> Ident { let name = self.as_ref(); - if RESERVED_NAMES.contains(&name) { + if is_reserved_name(name) { format_ident!("r#{name}") } else { format_ident!("{name}")