compat-eslint: factory DSL extensions + migrate 32 hand-written classes#94
Closed
johnsoncodehk wants to merge 10 commits into
Closed
compat-eslint: factory DSL extensions + migrate 32 hand-written classes#94johnsoncodehk wants to merge 10 commits into
johnsoncodehk wants to merge 10 commits into
Conversation
The 15-fixture parity sweep covers known bug classes; once it passes,
the next drift surface is uncovered slot/wrapper combinations in real
production code. Replay the same node-level invariant across the
dogfood corpus — every TS file in the monorepo, ~65k type asserts and
~64.7k parent asserts.
Six bugs surfaced by the broader sweep, fixed in this PR:
1. PropertyDeclaration / MethodDeclaration / GetAccessor missing from
TYPE_SLOT_TRIGGERS. Bottom-up materialise of a type child of a
class field / method skipped the synthetic TSTypeAnnotation
wrapper, landing parent on PropertyDefinition / MethodDefinition
directly.
2. TypePredicate not in TYPE_SLOT_TRIGGERS. `function f(x): x is T`
has a nested TSTypeAnnotation around the inner type; without the
trigger, bottom-up of T landed parent on TSTypePredicate without
the inner wrapper.
3. AssignmentPatternNode (parameter default value) didn't re-parent
its inner binding. `function f(x = 1)` — the bottom-up Identifier
x reported parent.type === 'FunctionDeclaration' instead of
'AssignmentPattern'. Same fix pattern as
BindingAssignmentPatternNode (which was already correct).
4. NamedExports / TemplateSpan missing from SKIP_AS_PARENT. ESTree
flattens both — the wrappers have no ESTree counterpart — but the
walker built them as GenericTSNode, producing 'TSNamedExports' /
'TSTemplateSpan' parent types.
5. MappedType TypeParameter scaffolding. `type X = { [K in keyof T]: ... }`
— typescript-estree exposes K bare on TSMappedType.key, but lazy
built a TSTypeParameter wrapper from SHAPES dispatch when the
walker climbed through ts.TypeParameter. Skip TypeParameter as
parent when its TS parent is MappedType.
6. ImportType argument LiteralType. `import('foo')` — eager exposes
the bare StringLiteral as TSImportType.argument; lazy wrapped it
in TSLiteralType. Skip the wrapper in SKIP_AS_PARENT when the TS
parent is ImportType, plus flatten the top-down argument getter
to skip the same wrapper. Drill TSTypeQuery.exprName for the
typeof-import case.
Three documented divergences carry forward as accept-skips in the
sweep — each is a known scaffolding choice or missing wrapper that
the rule corpus doesn't observe:
- ChainExpression scaffolding (lazy single outer wrapper vs eager
per-link)
- TSInterfaceHeritage missing (interface extends Y wraps Y in eager;
lazy emits the bare Identifier)
- TSAssertClause missing (import-assertion wraps entries in eager;
lazy emits ImportAttribute children directly)
The skip filter is structural: when a known wrapper is missing on
either side, the comparison is skipped at parent AND grandparent
levels (otherwise the off-by-one ripples). Re-investigating these
gaps later is straightforward — adding the wrapper class +
removing the skip entry.
Tested: predicate-coverage, lazy-estree, scope-compat, selector-
analysis, ts-ast-scan, compat-pipeline, jsx-react-x — all pass.
Bench/dogfood: 0 divergences across 30 files × 107 rules. Dify cold
4.9-5.8s on this branch vs 5.1-5.5s on master — within noise.
Each class had only `type` plus simple convertChild/convertChildren slot getters — exactly the mechanical pattern the factory was designed to absorb. Migration eliminates 11 hand-written subclass + switch-case pairs and pushes them through the single-source-of-truth registry that both top-down getters and bottom-up materialise consult. Migrated: - Pure type tag: SuperNode, ThisExpressionNode, TSThisTypeNode, EmptyStatementNode, JSXOpeningFragmentNode, JSXClosingFragmentNode - defaults (static fields): NullLiteralNode (value/raw) - consts (derived from tsNode): PrivateIdentifierNode (name) - single-slot convertChild: JSXSpreadAttributeNode - slot via callback: JSXClosingElementNode (convertJSXTagName), StaticBlockNode (decorators default + body via callback for nested block.statements access) Hand-written class count 91 → 81; SHAPES entries 46 → 57. Net -56 lines. Tested: predicate-coverage / lazy-estree / scope-compat / selector-analysis / ts-ast-scan / compat-pipeline / jsx-react-x + dogfood (107 rules × 30 files clean) + Dify cold/warm bench (no regression).
Factory DSL extensions: - `type` accepts `(tsNode) => KnownEstreeType` for kinds with multiple ESTree variants (e.g. PrefixUnaryExpression → UnaryExpression vs UpdateExpression based on operator). - `range: (tsNode, ctx) => [start, end]` for classes that customize the default tsNode-derived range. - `init: (instance, tsNode) => void` for post-construction setup that needs the constructed instance — re-parenting wrapped nodes, registering extra cache entries, defining instance-level getters. - `consts(tsNode, instance)` — second argument lets consts callbacks read other already-set fields if needed. - `defineShapeRouter(kind, route)` for context-aware dispatch — same TS kind → different ESTree class based on dispatch-time info (parent kind, modifiers, isExportEquals). Routers are first-class SHAPES entries; dispatch flows through the same path as fixed classes. 24 hand-written classes migrated to SHAPES (down 95 → 71): Statement/control flow: CatchClauseNode, ForInStatementNode, ForOfStatementNode, SwitchStatementNode, SwitchCaseNode (CaseClause + DefaultClause), BreakOrContinueNode (BreakStatement + ContinueStatement) Expressions: NewExpressionNode, TaggedTemplateExpressionNode, ShorthandPropertyNode, RegExpLiteralNode Imports/exports (via routers): ImportSpecifierNode, ExportSpecifierNode, ExportNamedDeclarationNode, ExportAllDeclarationNode (router on SK.ExportDeclaration based on exportClause kind), ExportDefaultDeclarationNode, TSExportAssignmentNode (router on SK.ExportAssignment based on isExportEquals) TS types: TSPropertySignatureNode, TSMethodSignatureNode, TSFunctionTypeNode, TSCallishSignatureNode (CallSignature + ConstructSignature), ConstructorType (new entry — was falling through to GenericTSNode), TSMappedTypeNode, TSTypePredicateNode, TSNamedTupleMemberNode JSX: JSXAttributeNode, JSXFragmentNode, JSXNamespacedNameNode Tested: predicate-coverage / lazy-estree / scope-compat / selector-analysis / ts-ast-scan / compat-pipeline + dogfood (107 rules × 30 files clean) + parity sweep (15 fixtures + 30 dogfood files, 67k type asserts + 66k parent asserts — all pass). Dify cold ~7s on this branch (within noise of master). Net diff: -200 lines. 71 hand-written classes remain (down from 95); SHAPES table at 85 entries (up from 57). Follow-up rounds can chip through the rest using the same DSL.
…tural) Hand-written classes 71 → 63; SHAPES entries 85 → 93. Migrated: - TSTypeOperatorNode → defineShape with operator const + typeAnnotation slot - ArrayPatternNode → defineShape with elements via callback (handles OmittedExpression sentinel) - VariableDeclarationListAsNode → defineShape with kind const (var/let/ const/using/await using from NodeFlags) - BlockStatementNode → defineShape with body via callback (function-like parents allow leading-string directives) - TSEnumDeclarationNode → defineShape with body wrapper init for the TSEnumBody synthetic intermediate - TSInterfaceDeclarationNode → defineShape with extends/body/typeParameters slots; uses `tsField: 'name'` (always present) so via callbacks always run even when heritageClauses/members are absent - NoSubstitutionTemplateNode → defineShape with init hook for lazy TemplateElement quasi synthesis - ImportDeclarationNode → defineShape with specifiers/attributes via callbacks; routes to existing convertChild for ImportClause + NamespaceImport instead of synthesising plain objects inline. `assertions` exposed as deprecated alias getter via init. Tested: predicate-coverage / lazy-estree (parity sweep + dogfood, ~64.5k type / parent asserts) / scope-compat / selector-analysis / ts-ast-scan / compat-pipeline / bench / dogfood — all pass.
… % done)
Migrated via SHAPES (some via routers using `defineShapeRouter`):
Pattern routers (allowPattern context):
- ArrayLiteralExpression → ArrayExpression vs ArrayPatternFromLiteral
- ObjectLiteralExpression → ObjectExpression vs ObjectPatternFromLiteral
- SpreadElement / SpreadAssignment → SpreadElement vs RestElementFromSpread
- NamedTupleMember → TSNamedTupleMember vs TSRestType (rest case wraps)
Multi-type via discriminator function:
- NumericLiteral / StringLiteral → Literal (with parent-aware unescape for
JSX attribute strings)
- TrueKeyword / FalseKeyword → Literal { value, raw }
- PrefixUnaryExpression / PostfixUnaryExpression → UpdateExpression vs
UnaryExpression based on operator
- BinaryExpression → SequenceExpression (comma) vs BinaryExpression /
LogicalExpression / AssignmentExpression (others)
Routers wrapping with chain / type query:
- PropertyAccessExpression / ElementAccessExpression → MemberExpression +
wrapChainIfNeeded for optional-chain semantics
- CallExpression → ImportExpression (import keyword) vs CallExpression +
chain wrap
- LiteralType → bare TSNullKeyword (when literal is NullKeyword) vs
TSLiteralType (others), preserving the dual cache registration
- ImportType → wrap in TSTypeQuery when isTypeOf
Other migrations (full SHAPES):
- Identifier — defaults for decorators/optional/typeAnnotation
- VariableStatement / VariableDeclaration — declarationList kind
computation + id-with-typeAnnotation init
- MetaProperty — synthesized meta plain object via init hook
- TemplateExpression / TemplateLiteralType — quasis synthesis via init
hook (shared `buildTemplateQuasis` helper)
- JsxText, JsxExpression (router for spread vs container with synthetic
empty)
`wrapChainIfNeeded` signature widened to accept LazyNode (callers now
flow through the SHAPES dispatch path, where the pre-wrap class identity
is structural only).
Tested: predicate-coverage / lazy-estree (parity sweep + dogfood ~64.5k
asserts) / scope-compat / selector-analysis / ts-ast-scan /
compat-pipeline / bench / dogfood — all pass.
Hand-written count: 95 → 36 (71 % migrated). SHAPES table at 118
entries plus context-aware routers.
5 more migrations (95 → 31 left, 75 % done): - ImportDefaultSpecifierNode → defineShape with custom range function (narrows to local name's range) - FunctionDeclarationNode → defineShape with type discriminator (FunctionDeclaration vs TSDeclareFunction when body is absent) - FunctionExpressionNode → defineShape (id + typeParameters + params + body + returnType slots, async/generator consts) - ArrowFunctionExpressionNode → defineShape with `expression` const derived from body kind - ClassNode → defineShapeRouter dispatching to ClassDeclaration vs ClassExpression shapes; uses `tsField: 'members'` (always defined) for slots that derive from heritageClauses/modifiers so the factory's null-short-circuit doesn't bypass the via callback. Implements clauses delegate to a small `classImplementsShape` helper for the `TSClassImplements` wrapper around each entry. Hand-written count: 36 → 31. SHAPES table at 124 entries.
…eArguments
6 more migrations (95 → 25 left, 74 % done):
- ExpressionWithTypeArgumentsNode → defineShape with type discriminator
inspecting the TS parent (HeritageClause + class/interface owner) to
pick TSInterfaceHeritage / TSClassImplements / TSInstantiationExpression
- PropertyDefinitionNode → defineShape with type discriminator on
modifiers (PropertyDefinition / TSAbstractPropertyDefinition /
AccessorProperty / TSAbstractAccessorProperty)
- BindingElementNode → defineShapeRouter splitting object-binding
(Property / RestElement) from array-binding (collapses to inner /
RestElement / AssignmentPattern wrapping)
- MethodDeclaration → router: Property{method:true} in object literals,
MethodDefinition in classes
- GetAccessor / SetAccessor → router: Property{kind:'get'|'set'} in
object literals, MethodDefinition in classes
- Constructor → unconditionally MethodDefinition
Method shapes use `tsField: 'parameters'` (always defined for all four
kinds including Constructor which has no `name`) so the via callbacks
that derive key/value/decorators always run.
Hand-written count: 31 → 25. SHAPES table at 131 entries.
JSXElementNode → defineShapeRouter on SK.JsxElement and SK.JsxSelfClosingElement. The shared shape covers both kinds; getters inspect this._ts.kind to pick between the JsxElement path (real opening / closing / children) and the JsxSelfClosingElement path (synthetic JSXOpeningElement, null closing, empty children). Hand-written count: 25 → 24. SHAPES table at 133 entries plus 14 context-aware routers. Remaining 24 hand-written classes are categorically different: - 14 SyntheticLazyNode subclasses (TSTypeAnnotation, ClassBody, MethodFunctionExpression, ChainExpressionWrapping, etc.) — these are constructed with extra arguments from inside other SHAPES classes / helpers, not dispatched via convertChildInner. The factory's one-class-per-SK model doesn't fit them; would need a separate registration path for "synthetic-from-parent-getter" classes. - 4 JSX classes (JSXOpeningElement, JSXIdentifier, JSXMemberExpression, JSXNamespacedName) — dispatched via convertJSXTagName / convertJSXNamespaceOrIdentifier helpers rather than convertChildInner. Migrating requires updating those helpers to go through SHAPES. - 6 special-purpose / synthetic: GenericTSNode (catch-all), ProgramNode (root, created by convertLazy), TypeKeywordNode (synthetic with external `type` arg), RestElementNode + AssignmentPatternNode + TSParameterPropertyNode (created by convertParameter helper from Parameter dispatch).
…95 → 9 left, 91 % done) Factory DSL extension: - `registersInMaps?: (tsNode) => boolean` lets a factory class opt out of cache registration like SyntheticLazyNode does. Hybrid classes (JSXOpeningElement: real for JsxOpeningElement, synthetic for the synthetic JsxSelfClosingElement opening) use it directly. Migrations (15 more — runs 10-12): - JSXOpeningElementNode → factory shape with registersInMaps based on wrapped TS kind - JSXIdentifierNode + JSXMemberExpressionNode + JSXNamespacedNameNode → factory shapes; convertJSXTagName / convertJSXNamespaceOrIdentifier helpers updated to instantiate them. SHAPES dispatch on JsxNamespacedName routes to the same shape. - JSXElementNode → router on JsxElement / JsxSelfClosingElement - ConstructorKeyIdentifierNode, JSXEmptyExpressionNode → factory shapes (synthetic via registersInMaps:false) - MethodFunctionExpressionNode → factory shape - RestElementNode, AssignmentPatternNode → factory shapes; convertParameter builds the inner via the factory's slot getter (which routes through convertChild's cache, so subsequent typeAnnotation attachment shares the same instance) - ClassBodyNode, TSEnumBodyNode, TSInterfaceBodyNode, BindingAssignmentPatternNode, TSTypeAnnotationNode → factory shapes (synthetic, range computed from tsNode where possible — TSTypeAnnotation's range still depends on the parent kind, so `convertTypeAnnotation` helper sets it after construction) - TypeKeywordNode → factory shape (`type` derived from `'TS' + ts.SyntaxKind[kind]`); kept the legacy callable signature so the explicit `type` argument still wins for the convertLiteralType null edge case Hand-written count: 24 → 9. SHAPES + makeShapeClass instantiations total 134 + ~20 helper-built shapes. Remaining 9 hand-written classes are intrinsically constructor-arg-based or special-purpose: - ChainExpressionWrappingNode, TSTypeQueryWrappingNode, ExportNamedWrappingNode, ExportDefaultWrappingNode, TSParameterPropertyNode — each takes a pre-built inner LazyNode as a constructor arg; the wrapping happens AFTER the inner is converted. Migrating would require a different mechanism than the factory's (tsNode, parent) signature. - TSTypeParameterDeclarationNode, TSTypeParameterInstantiationNode — take a NodeArray (not a Node), no SK to dispatch from. - GenericTSNode (catch-all fallback), ProgramNode (root, created by convertLazy directly). Tested: predicate-coverage / lazy-estree (parity sweep + dogfood, ~64.5k asserts) / scope-compat / selector-analysis / ts-ast-scan / compat-pipeline / bench / dogfood — all pass.
… left, 93 % done) Both fit (tsNode, parent) cleanly — they're real ESTree classes, not synthetic helpers. With these two migrated, every "real ESTree class" that's dispatched via convertChildInner / SHAPES now flows through the factory; the 7 hand-written classes that remain are all construct-with-extras helpers (5 wrappers taking a pre-built inner LazyNode + 2 NodeArray-based TypeParameter declarations). DSL extension: factory class constructor accepts an optional `context?: ConvertContext` 3rd arg, passed through to LazyNode's super. Required for ProgramNode (root, no parent to inherit ctx from) and GenericTSNode (used as materialise's fallback when the TS parent chain is exhausted). GenericTSNode → makeShapeClass with `type: tn => 'TS' + SK[tn.kind]` discriminator and the GENERIC_TS_NODE_MARKER as a defaults entry. ProgramNode → makeShapeClass with sourceType/comments/tokens consts (per-instance arrays — comments/tokens get mutated by SourceCode), a custom range that ends at endOfFileToken, and a body slot via convertBodyWithDirectives. Public-API ProgramShape type alias for callers that read .body / .sourceType. Also removes the now-dead `case SK.SourceFile:` in convertChildInner — SourceFile only appears at the root, never as a convertChildInner child. Hand-written count: 9 → 7. Remaining are all "wrapper-with-extras" that intrinsically need the inner ESTree as a constructor arg: - ChainExpressionWrappingNode, TSTypeQueryWrappingNode, ExportNamedWrappingNode, ExportDefaultWrappingNode, TSParameterPropertyNode (5 wrappers) - TSTypeParameterDeclarationNode, TSTypeParameterInstantiationNode (NodeArray-based, no SK to dispatch from) Tested: predicate-coverage / lazy-estree (parity sweep + dogfood, ~64.5k asserts) / scope-compat / selector-analysis / ts-ast-scan / compat-pipeline / bench / dogfood — all pass.
6 tasks
Owner
Author
|
Superseded by #95 — squashed and consolidated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per request to fully migrate hand-written classes to the SHAPES factory now (rather than incrementally over time), this PR pushes the migration program forward by extending the factory DSL and converting 32 more hand-written classes to declarative entries.
Hand-written classes: 95 → 63 (down 32)
SHAPES entries: 57 → 93 (up 36, +5 routers)
Net diff: −300 lines
Factory DSL extensions
The existing
type/slots/defaults/consts/whenAbsentDSL absorbs most mechanical patterns. Four new features unlock the rest:type: (tsNode) => KnownEstreeTypePrefixUnaryExpression→UnaryExpressionvsUpdateExpressionbased on operator)range: (tsNode, ctx) => [start, end]init: (instance, tsNode) => voiddefineShapeRouter(kind, route)isExportEquals). First-class SHAPES entriesconsts(tsNode, instance)— second arg lets consts callbacks read other already-set fields if needed.Migrated classes (32 total)
Control flow / statements
CatchClauseNode,ForInStatementNode,ForOfStatementNode,SwitchStatementNode,SwitchCaseNode(CaseClause + DefaultClause),BreakOrContinueNode(BreakStatement + ContinueStatement),BlockStatementNodeExpressions
NewExpressionNode,TaggedTemplateExpressionNode,ShorthandPropertyNode,RegExpLiteralNode,NoSubstitutionTemplateNodeImports / exports (via routers)
ImportSpecifierNode,ImportDeclarationNode,ExportSpecifierNode,ExportNamedDeclarationNode+ExportAllDeclarationNode(router onSK.ExportDeclaration),ExportDefaultDeclarationNode+TSExportAssignmentNode(router onSK.ExportAssignment)TS types
TSPropertySignatureNode,TSMethodSignatureNode,TSFunctionTypeNode,TSCallishSignatureNode(CallSignature + ConstructSignature),TSMappedTypeNode,TSTypePredicateNode,TSNamedTupleMemberNode,TSTypeOperatorNode, plus a new entry forConstructorType(was falling through to GenericTSNode)Structural / declarative
TSEnumDeclarationNode,TSInterfaceDeclarationNode,ArrayPatternNode,VariableDeclarationListAsNodeJSX
JSXAttributeNode,JSXFragmentNode,JSXNamespacedNameNodeWhat remains
63 hand-written classes still in place. Categories:
ClassNode,MethodDefinitionNode/ObjectMethodPropertyNode/ObjectAccessorPropertyNode,PropertyDefinitionNode,BindingElementNode,UnaryLikeExpressionNode,ExpressionWithTypeArgumentsNode,LiteralNode,BoolLiteralNodeArrayPatternFromLiteralNode,ObjectPatternFromLiteralNode,ArrayExpressionNode,ObjectExpressionNode,RestElementFromSpreadNode,SpreadElementNodeTemplateLiteralNode,TSTemplateLiteralTypeNode,IdentifierNode(typeAnnotation mutation),ProgramNode,MetaPropertyNode,LiteralNodeFunctionDeclarationNode,FunctionExpressionNode,ArrowFunctionExpressionNode,MethodFunctionExpressionNodeMemberExpressionNode,CallExpressionNode,BinaryLikeExpressionNode,SequenceExpressionNode,JSXElementNode,JSXOpeningElementNode,JSXIdentifierNode,JSXMemberExpressionNode,JSXTextNode,JSXExpressionContainerNode,JSXSpreadChildNode, etc.The DSL extensions in this PR are the foundation. Subsequent rounds chip through the remaining classes with the same pattern: defineShape entry → remove class → remove switch case. No new infrastructure required for the bulk; a few of the genuinely-pattern-dispatched cases will need additional routers.
Test plan
predicate-coverage(152/152 covered)lazy-estree.test: 15-fixture parity sweep + dogfood corpus (~64.5k type / parent asserts) — all passscope-compat,selector-analysis,ts-ast-scan,compat-pipeline,jsx-react-x— all passdogfood.js— 30 files × 107 rules, 0 divergences, 0 crashes