Skip to content

Commit 87ac156

Browse files
committed
refactor(ast_tools/formatter): compile list of no following nodes as TypeIds (#12930)
Follow-on after #12864. Pure refactor - does not alter generated code. There's a couple of things going on in this PR. Firstly, perf: * Prefer comparing `TypeId`s over string comparison, because it's cheaper. * Compile a single `Vec` of all types which have no following node at the start (including those listed in `AST_NODE_WITHOUT_FOLLOWING_NODE_LIST`), rather than checking 2 `Vec`s each time in `generate_struct_impls`. Secondly, the code previously was relying on the fact that almost all of `Statement`s variants have the same name as the types those variants contain e.g.: ```rs pub enum Statement<'a> { BlockStatement(Box<'a, BlockStatement<'a>>), // ... } ``` But that the names match is a bit of a co-incidence. This PR makes it so we don't rely on that co-incidence, and instead makes the "no following node" list from the actual *types* of the variants. This change reveals a side-effect of the previous behavior which may or may not be intentional. The only variants of `Statement` where the name of the variant and name of the *type* of the variant don't match are `Function` and `Class`. To maintain the same output as before, I've added an exclude list `AST_NODE_WITH_FOLLOWING_NODE_LIST` containing these 2 types. Presumably the reason why these 2 need an exclusion is because `Function` and `Class` can be either a `Statement` or an `Expression`. I don't know if this may be problematic and might need logic elsewhere to handle them differently, depending on the context? Note: `TypeDef::innermost_type` is to get the `TypeId` of the inner type of a variant (e.g. `BlockStatement`), rather than the type which is the actual enum variant (e.g. `Box<BlockStatement>`). @Dunqing I don't know the formatter at all, so these changes may be unhelpful. Feel free to close or modify this PR if so.
1 parent bdaf569 commit 87ac156

File tree

1 file changed

+34
-9
lines changed

1 file changed

+34
-9
lines changed

tasks/ast_tools/src/generators/formatter/ast_nodes.rs

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
Codegen, Generator,
1010
generators::define_generator,
1111
output::{Output, output_path},
12-
schema::{Def, EnumDef, FieldDef, Schema, StructDef, TypeDef, VariantDef},
12+
schema::{Def, EnumDef, FieldDef, Schema, StructDef, TypeDef, TypeId},
1313
};
1414

1515
pub fn get_node_type(ty: &TokenStream) -> TokenStream {
@@ -23,13 +23,15 @@ const FORMATTER_CRATE_PATH: &str = "crates/oxc_formatter";
2323
const AST_NODE_WITHOUT_FOLLOWING_NODE_LIST: &[&str] =
2424
&["AssignmentExpression", "FormalParameters", "StaticMemberExpression"];
2525

26+
const AST_NODE_WITH_FOLLOWING_NODE_LIST: &[&str] = &["Function", "Class"];
27+
2628
pub struct FormatterAstNodesGenerator;
2729

2830
define_generator!(FormatterAstNodesGenerator);
2931

3032
impl Generator for FormatterAstNodesGenerator {
3133
fn generate(&self, schema: &Schema, _codegen: &Codegen) -> Output {
32-
let all_statement_variants_names = get_all_statement_variants_names(schema);
34+
let no_following_node_type_ids = get_no_following_node_type_ids(schema);
3335

3436
let impls = schema
3537
.types
@@ -38,7 +40,7 @@ impl Generator for FormatterAstNodesGenerator {
3840
TypeDef::Struct(struct_def)
3941
if struct_def.visit.has_visitor() && !struct_def.builder.skip =>
4042
{
41-
Some(generate_struct_impls(struct_def, &all_statement_variants_names, schema))
43+
Some(generate_struct_impls(struct_def, &no_following_node_type_ids, schema))
4244
}
4345
TypeDef::Enum(enum_def) if enum_def.visit.has_visitor() => {
4446
Some(generate_enum_impls(enum_def, schema))
@@ -196,14 +198,38 @@ impl Generator for FormatterAstNodesGenerator {
196198
}
197199
}
198200

199-
fn get_all_statement_variants_names(schema: &Schema) -> Vec<&str> {
201+
/// Get [`TypeId`]s of types which do not have a following node.
202+
///
203+
/// These are:
204+
/// * All types which are variants of `Statement`.
205+
/// * PLUS types listed in `AST_NODE_WITHOUT_FOLLOWING_NODE_LIST`.
206+
/// * MINUS types listed in `AST_NODE_WITH_FOLLOWING_NODE_LIST`.
207+
fn get_no_following_node_type_ids(schema: &Schema) -> Vec<TypeId> {
208+
let exclude_type_ids = AST_NODE_WITH_FOLLOWING_NODE_LIST
209+
.iter()
210+
.map(|&name| schema.type_names[name])
211+
.collect::<Vec<_>>();
212+
213+
let mut type_ids = AST_NODE_WITHOUT_FOLLOWING_NODE_LIST
214+
.iter()
215+
.map(|&name| schema.type_names[name])
216+
.collect::<Vec<_>>();
217+
200218
let statement_enum = schema.type_by_name("Statement").as_enum().unwrap();
201-
statement_enum.all_variants(schema).map(VariantDef::name).collect()
219+
type_ids.extend(
220+
statement_enum
221+
.all_variants(schema)
222+
.filter_map(|variant| variant.field_type(schema))
223+
.map(|variant_type| variant_type.innermost_type(schema).id())
224+
.filter(|type_id| !exclude_type_ids.contains(type_id)),
225+
);
226+
227+
type_ids
202228
}
203229

204230
fn generate_struct_impls(
205231
struct_def: &StructDef,
206-
all_statement_variants_names: &[&str],
232+
no_following_node_type_ids: &[TypeId],
207233
schema: &Schema,
208234
) -> TokenStream {
209235
let type_ty = struct_def.ty(schema);
@@ -276,9 +302,8 @@ fn generate_struct_impls(
276302
quote! { &self.inner.#field_name }
277303
};
278304

279-
let should_not_have_following_node = all_statement_variants_names
280-
.contains(&struct_def.name.as_str())
281-
|| AST_NODE_WITHOUT_FOLLOWING_NODE_LIST.contains(&struct_def.name.as_str());
305+
let should_not_have_following_node =
306+
no_following_node_type_ids.contains(&struct_def.id);
282307
let mut following_node = if should_not_have_following_node {
283308
quote! {
284309
None

0 commit comments

Comments
 (0)