Skip to content

Commit

Permalink
refactor(ast)!: no unneccesary trailing underscores on AstBuilder m…
Browse files Browse the repository at this point in the history
…ethod 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`.
  • Loading branch information
overlookmotel committed Jan 6, 2025
1 parent 0db2a22 commit d8b27af
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 22 deletions.
6 changes: 3 additions & 3 deletions crates/oxc_ast/src/generated/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`]
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
34 changes: 16 additions & 18 deletions tasks/ast_tools/src/generators/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(&params);

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());
Expand Down Expand Up @@ -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(&params_incl_defaults);
Expand Down Expand Up @@ -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 `"));
Expand Down
6 changes: 5 additions & 1 deletion tasks/ast_tools/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S> ToIdent for S
where
S: AsRef<str>,
{
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}")
Expand Down

0 comments on commit d8b27af

Please sign in to comment.