Skip to content

Commit 7c8743e

Browse files
committed
refactor(ast_tools): improve correctness and performance of idents
1 parent cef5ef1 commit 7c8743e

File tree

9 files changed

+114
-81
lines changed

9 files changed

+114
-81
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tasks/ast_tools/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ indexmap = { workspace = true }
2222
itertools = { workspace = true }
2323
lazy_static = { workspace = true }
2424
oxc_index = { workspace = true }
25+
phf = { workspace = true, features = ["macros"] }
2526
prettyplease = { workspace = true }
2627
proc-macro2 = { workspace = true }
2728
quote = { workspace = true }

tasks/ast_tools/src/derives/clone_in.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
//! Derive for `CloneIn` trait.
22
33
use proc_macro2::TokenStream;
4-
use quote::{format_ident, quote};
4+
use quote::quote;
55
use syn::Ident;
66

77
use crate::{
88
schema::{Def, EnumDef, FieldDef, Schema, StructDef, TypeDef},
9+
utils::create_safe_ident,
910
Result,
1011
};
1112

@@ -151,7 +152,7 @@ fn generate_impl(
151152
has_lifetime: bool,
152153
uses_allocator: bool,
153154
) -> TokenStream {
154-
let alloc_ident = format_ident!("{}", if uses_allocator { "allocator" } else { "_" });
155+
let alloc_ident = create_safe_ident(if uses_allocator { "allocator" } else { "_" });
155156

156157
if has_lifetime {
157158
quote! {

tasks/ast_tools/src/derives/content_eq.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
//! Derive for `ContentEq` trait.
22
33
use proc_macro2::TokenStream;
4-
use quote::{format_ident, quote};
4+
use quote::quote;
55

66
use crate::{
77
schema::{Def, EnumDef, Schema, StructDef, TypeDef},
8+
utils::create_safe_ident,
89
Result,
910
};
1011

@@ -69,11 +70,11 @@ impl Derive for DeriveContentEq {
6970
}
7071

7172
fn derive_struct(struct_def: &StructDef, schema: &Schema) -> TokenStream {
72-
let mut other_name = "other";
73+
let mut uses_other = true;
7374

7475
let body = if struct_def.content_eq.skip {
7576
// Struct has `#[content_eq(skip)]` attr. So `content_eq` always returns true.
76-
other_name = "_";
77+
uses_other = false;
7778
quote!(true)
7879
} else {
7980
let fields = struct_def
@@ -96,20 +97,20 @@ fn derive_struct(struct_def: &StructDef, schema: &Schema) -> TokenStream {
9697
let mut body = quote!( #(#fields)&&* );
9798
if body.is_empty() {
9899
body = quote!(true);
99-
other_name = "_";
100+
uses_other = false;
100101
};
101102
body
102103
};
103104

104-
generate_impl(&struct_def.ty_anon(schema), other_name, &body)
105+
generate_impl(&struct_def.ty_anon(schema), &body, uses_other)
105106
}
106107

107108
fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream {
108-
let mut other_name = "other";
109+
let mut uses_other = true;
109110

110111
let body = if enum_def.content_eq.skip {
111112
// Enum has `#[content_eq(skip)]` attr. So `content_eq` always returns true.
112-
other_name = "_";
113+
uses_other = false;
113114
quote!(true)
114115
} else if enum_def.is_fieldless() {
115116
// We assume fieldless enums implement `PartialEq`
@@ -132,11 +133,11 @@ fn derive_enum(enum_def: &EnumDef, schema: &Schema) -> TokenStream {
132133
}
133134
};
134135

135-
generate_impl(&enum_def.ty_anon(schema), other_name, &body)
136+
generate_impl(&enum_def.ty_anon(schema), &body, uses_other)
136137
}
137138

138-
fn generate_impl(ty: &TokenStream, other_name: &str, body: &TokenStream) -> TokenStream {
139-
let other_ident = format_ident!("{other_name}");
139+
fn generate_impl(ty: &TokenStream, body: &TokenStream, uses_other: bool) -> TokenStream {
140+
let other_ident = create_safe_ident(if uses_other { "other" } else { "_" });
140141
quote! {
141142
impl ContentEq for #ty {
142143
fn content_eq(&self, #other_ident: &Self) -> bool {

tasks/ast_tools/src/derives/get_span.rs

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
//! Derive for `GetSpan` trait.
22
33
use proc_macro2::TokenStream;
4-
use quote::{format_ident, quote};
4+
use quote::quote;
55
use syn::Ident;
66

77
use crate::{
88
schema::{Def, EnumDef, Schema, StructDef},
9+
utils::create_safe_ident,
910
Result,
1011
};
1112

@@ -54,15 +55,17 @@ impl Derive for DeriveGetSpan {
5455
}
5556

5657
fn derive(&self, type_def: StructOrEnum, schema: &Schema) -> TokenStream {
58+
let trait_ident = create_safe_ident("GetSpan");
59+
let method_ident = create_safe_ident("span");
5760
let self_ty = quote!(&self);
5861
let result_ty = quote!(Span);
5962
let result_expr = quote!(self.span);
6063
let reference = quote!( & );
6164

6265
derive_type(
6366
type_def,
64-
"GetSpan",
65-
"span",
67+
&trait_ident,
68+
&method_ident,
6669
&self_ty,
6770
&result_ty,
6871
&result_expr,
@@ -96,15 +99,17 @@ impl Derive for DeriveGetSpanMut {
9699
}
97100

98101
fn derive(&self, type_def: StructOrEnum, schema: &Schema) -> TokenStream {
102+
let trait_ident = create_safe_ident("GetSpanMut");
103+
let method_ident = create_safe_ident("span_mut");
99104
let self_ty = quote!(&mut self);
100105
let result_ty = quote!(&mut Span);
101106
let result_expr = quote!(&mut self.span);
102107
let reference = quote!( &mut );
103108

104109
derive_type(
105110
type_def,
106-
"GetSpanMut",
107-
"span_mut",
111+
&trait_ident,
112+
&method_ident,
108113
&self_ty,
109114
&result_ty,
110115
&result_expr,
@@ -118,36 +123,28 @@ impl Derive for DeriveGetSpanMut {
118123
#[expect(clippy::too_many_arguments)]
119124
fn derive_type(
120125
type_def: StructOrEnum,
121-
trait_name: &str,
122-
method_name: &str,
126+
trait_ident: &Ident,
127+
method_ident: &Ident,
123128
self_ty: &TokenStream,
124129
result_ty: &TokenStream,
125130
result_expr: &TokenStream,
126131
reference: &TokenStream,
127132
schema: &Schema,
128133
) -> TokenStream {
129-
let trait_ident = format_ident!("{trait_name}");
130-
let method_ident = format_ident!("{method_name}");
131134
match type_def {
132135
StructOrEnum::Struct(struct_def) => derive_struct(
133136
struct_def,
134-
&trait_ident,
135-
&method_ident,
137+
trait_ident,
138+
method_ident,
136139
self_ty,
137140
result_ty,
138141
result_expr,
139142
reference,
140143
schema,
141144
),
142-
StructOrEnum::Enum(enum_def) => derive_enum(
143-
enum_def,
144-
&trait_ident,
145-
&method_ident,
146-
self_ty,
147-
result_ty,
148-
reference,
149-
schema,
150-
),
145+
StructOrEnum::Enum(enum_def) => {
146+
derive_enum(enum_def, trait_ident, method_ident, self_ty, result_ty, reference, schema)
147+
}
151148
}
152149
}
153150

tasks/ast_tools/src/generators/ast_builder.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use syn::Ident;
88
use crate::{
99
output::{output_path, Output},
1010
schema::{Def, EnumDef, FieldDef, Schema, StructDef, TypeDef, VariantDef},
11-
utils::is_reserved_name,
11+
utils::{create_safe_ident, is_reserved_name},
1212
Codegen, Generator, AST_CRATE_PATH,
1313
};
1414

@@ -276,11 +276,11 @@ fn get_struct_params<'s>(
276276
TypeDef::Primitive(primitive_def) => match primitive_def.name() {
277277
"Atom" if !has_atom_generic => {
278278
has_atom_generic = true;
279-
Some(format_ident!("A"))
279+
Some(create_safe_ident("A"))
280280
}
281281
"&str" if !has_str_generic => {
282282
has_str_generic = true;
283-
Some(format_ident!("S"))
283+
Some(create_safe_ident("S"))
284284
}
285285
_ => None,
286286
},
@@ -450,7 +450,8 @@ fn struct_builder_name(snake_name: &str, does_alloc: bool) -> Ident {
450450
} else if is_reserved_name(snake_name) {
451451
format_ident!("{snake_name}_")
452452
} else {
453-
format_ident!("{snake_name}")
453+
// We just checked name is not a reserved word
454+
create_safe_ident(snake_name)
454455
}
455456
}
456457

0 commit comments

Comments
 (0)