-
-
Notifications
You must be signed in to change notification settings - Fork 720
Description
To support printing comments in formatter and codegen, we are discussing adding a CommentNodeId field to every AST type (#11213).
However, what we really want is standard NodeId on every AST node.
Why?
At present, when you have an AstNode, via AstKind you can get the actual AST type (e.g. BinaryExpression). However you can't go in the opposite direction e.g. start with a BinaryExpression, obtain its NodeId, and use Semantic to e.g. get its parent node.
If we store NodeId in all AST structs, we'll be able to go in both directions, which allows moving around and querying the AST much more easily.
What does AstKind have to do with this?
The main blocker to achieving this is that AstKind is rather inconsistent:
- Most AST types which are structs have a corresponding
AstKind, but some don't. - Most AST types which are enums do not have a corresponding
AstKind, but a few do!
Only types which have an AstKind get a NodeId. So, to align NodeId in Semantic with NodeIds stored in AST structs, we need to standardize what types have AstKinds.
What we need to do
We need to:
- Add
AstKinds for all AST structs (e.g.StaticMemberExpression). - Remove
AstKinds for all AST enums (e.g.MemberExpression).
How to do it
oxc_ast_tools contains 2 lists of the exceptions to the above rules:
oxc/tasks/ast_tools/src/generators/ast_kind.rs
Lines 28 to 83 in 7e88451
| const STRUCTS_BLACK_LIST: &[&str] = &[ | |
| "TemplateElement", | |
| "ComputedMemberExpression", | |
| "StaticMemberExpression", | |
| "PrivateFieldExpression", | |
| "AssignmentTargetRest", | |
| "AssignmentTargetPropertyIdentifier", | |
| "AssignmentTargetPropertyProperty", | |
| "BindingPattern", | |
| "BindingProperty", | |
| "AccessorProperty", | |
| "WithClause", | |
| "ImportAttribute", | |
| "JSXOpeningFragment", | |
| "JSXClosingFragment", | |
| "JSXEmptyExpression", | |
| "JSXAttribute", | |
| "JSXSpreadChild", | |
| "TSTypeOperator", | |
| "TSArrayType", | |
| "TSTupleType", | |
| "TSOptionalType", | |
| "TSRestType", | |
| "TSInterfaceBody", | |
| "TSIndexSignature", | |
| "TSCallSignatureDeclaration", | |
| "TSIndexSignatureName", | |
| "TSTypePredicate", | |
| "TSFunctionType", | |
| "TSConstructorType", | |
| "TSNamespaceExportDeclaration", | |
| "JSDocNullableType", | |
| "JSDocNonNullableType", | |
| "JSDocUnknownType", | |
| "Span", | |
| ]; | |
| /// Enums to create an `AstKind` for. | |
| /// | |
| /// Apart from this list, enums don't have `AstKind`s. | |
| const ENUMS_WHITE_LIST: &[&str] = &[ | |
| "ArrayExpressionElement", | |
| "PropertyKey", | |
| "MemberExpression", | |
| "Argument", | |
| "AssignmentTarget", | |
| "SimpleAssignmentTarget", | |
| "AssignmentTargetPattern", | |
| "ForStatementInit", | |
| "ModuleDeclaration", | |
| "JSXElementName", | |
| "JSXMemberExpressionObject", | |
| "JSXAttributeItem", | |
| "TSTypeName", | |
| "TSModuleReference", | |
| ]; |
We need to reduce both these lists to zero.
We can approach this goal in small steps.
e.g. for MemberExpression:
- Add
AstKinds forComputedMemberExpression,StaticMemberExpressionandPrivateFieldExpression(by removing them fromSTRUCTS_BLACK_LIST). - Remove
AstKindforMemberExpression(by removing it fromENUMS_WHITE_LIST). - Run
cargo run -p oxc_ast_toolsto regenerate the definition ofAstKindandVisitetc. - Update all code which used the old
AstKind::MemberExpressionto use the newAstKind::StaticMemberExpressionetc instead.
The changes will be mostly in the linter, which is the heaviest user of AstKind.
e.g. in eslint/no-caller rule:
oxc/crates/oxc_linter/src/rules/eslint/no_caller.rs
Lines 77 to 85 in 7e88451
| if let AstKind::MemberExpression(MemberExpression::StaticMemberExpression(expr)) = | |
| node.kind() | |
| { | |
| if (expr.property.name == "callee" || expr.property.name == "caller") | |
| && expr.object.is_specific_id("arguments") | |
| { | |
| ctx.diagnostic(no_caller_diagnostic(expr.property.span, &expr.property.name)); | |
| } | |
| } |
- if let AstKind::MemberExpression(MemberExpression::StaticMemberExpression(expr)) = node.kind() {
+ if let AstKind::StaticMemberExpression(expr) = node.kind() {