compat-eslint: factory DSL full migration + dogfood-corpus parity sweep#95
Merged
Conversation
This was referenced May 3, 2026
7d7a636 to
bdf721c
Compare
There was a problem hiding this comment.
Pull request overview
This PR expands compat-eslint’s AST parity testing to run over a shared “dogfood” production corpus, and completes a large migration of lazy-estree node classes to the SHAPES factory/DSL to reduce drift between top-down and bottom-up conversion paths.
Changes:
- Extend the bottom-up parity sweep test to run across the monorepo dogfood corpus and add targeted accept-skips for known structural divergences (template parts, chain scaffolding, known missing wrappers).
- Extract
DOGFOOD_FILESinto a shared module and reuse it from both the rule-level dogfood benchmark and the node-level parity sweep. - Migrate the majority of
lazy-estree.tssubclasses to the SHAPES factory/routers and expand the DSL to support dynamictype,range,init, computed slots, and per-instanceregistersInMaps.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/compat-eslint/test/lazy-estree.test.ts | Adds dogfood-corpus parity sweep and documents/accepts a few known structural divergences during bottom-up parent/type comparison. |
| packages/compat-eslint/test/bench/dogfood.ts | Imports shared DOGFOOD_FILES list and tightens the buildProgram parameter type. |
| packages/compat-eslint/test/bench/dogfood-corpus.ts | New shared module exporting the repo-relative list of production .ts files used as the dogfood corpus. |
| packages/compat-eslint/lib/lazy-estree.ts | Large refactor: introduces/extends SHAPES DSL, migrates most node classes to factory-built shapes/routers, and applies multiple parity fixes surfaced by the broader sweep. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3692
to
+3694
| // Method-as-FunctionExpression — eager (line 826) builds the FunctionExpression | ||
| // with `id: null`, `range: [parameters.pos - 1, end]`, and per-context kind. | ||
| // Used as `value` for both class MethodDefinition and object Property. |
Comment on lines
+3741
to
+3746
| // the binding NAME (not the BindingElement's outer start, which would | ||
| // include the property key in the object case) through the initializer. | ||
| // `[a = 1] = …` and `{ b: c = 2 } = …` — wraps an inner pattern with a | ||
| // default value. Range covers from the binding NAME (eager strips the | ||
| // outer BindingElement start, which would include the property key in | ||
| // the object case) through the initializer. |
Comment on lines
+25
to
+28
| const { RULES } = require('./rules.config.js') as { RULES: Array<[string, unknown[]?]> }; | ||
|
|
||
| // Repo-root-relative paths for the dogfood corpus. All real | ||
| // production .ts files in the monorepo (excluding .d.ts, fixtures, | ||
| // tests, bench, node_modules, worktrees). | ||
| import { DOGFOOD_FILES } from './dogfood-corpus'; | ||
|
|
Two related pieces:
## Dogfood-corpus parity sweep (test infrastructure)
Extends the existing 15-fixture bottom-up parity sweep to the dogfood
corpus — every production .ts file in the monorepo, ~67k type asserts +
~67k parent asserts. The broader walk surfaced 10 real drift bugs
(fixed below) plus 3 known scaffolding ambiguities documented as
accept-skips (ChainExpression per-link, TSInterfaceHeritage missing
wrapper, TSAssertClause missing wrapper). DOGFOOD_FILES extracted into
a shared module so the sweep and the rule-level dogfood diff stay
in lock-step.
## Drift fixes the sweep caught
1. PropertyDeclaration / MethodDeclaration / GetAccessor / TypePredicate
missing from TYPE_SLOT_TRIGGERS — class field / method type
annotations skipped the synthetic TSTypeAnnotation wrapper.
2. AssignmentPatternNode constructor didn't re-parent its inner — the
binding identifier in `function f(x = 1)` reported parent.type ===
'FunctionDeclaration' instead of 'AssignmentPattern'.
3. NamedExports / TemplateSpan missing from SKIP_AS_PARENT — the
walker built GenericTSNode wrappers (`TSNamedExports` /
`TSTemplateSpan`) where eager flattens.
4. MappedType TypeParameter scaffolding — typescript-estree exposes
the bare iterating identifier on TSMappedType.key; lazy was
building a TSTypeParameter wrapper from SHAPES dispatch.
5. ImportType argument LiteralType wrapper — eager flattens the
`LiteralType { literal: StringLiteral }` to expose the bare
StringLiteral on TSImportType.argument.
6. `typeof import('x')` — TSTypeQuery wraps an inner TSImportType but
claims the cache slot; bottom-up materialise of the import's
children needed a WRAPPER_DRILLS entry to drill TSTypeQuery.exprName.
## Factory DSL extension + class migration
Extends the SHAPES factory so 88 of 95 hand-written subclasses can be
defined declaratively. New DSL features:
- `type: KnownEstreeType | ((tsNode) => KnownEstreeType)` — kinds with
multiple ESTree variants (e.g. PrefixUnaryExpression →
UpdateExpression vs UnaryExpression based on operator).
- `range: (tsNode, ctx) => [start, end]` — classes that customise the
default tsNode-derived range (method ranges, parameter ranges, …).
- `init: (instance, tsNode) => void` — post-construction setup needing
the constructed instance: re-parenting wrapped nodes, registering
extra cache entries, defining instance-level getters.
- `consts(tsNode, instance)` — second arg lets consts callbacks read
other already-set fields if needed.
- `registersInMaps: (tsNode) => boolean` — the LazyNode-level predicate
on a per-instance basis. Hybrid classes like JSXOpeningElement (real
for JsxOpeningElement, synthetic for JsxSelfClosingElement) need this.
- `defineShapeRouter(kind, route)` — context-aware dispatch. Same TS
kind → different ESTree class based on dispatch-time info (parent
kind, modifiers, isExportEquals, allowPattern).
- 3rd ctor arg `context?: ConvertContext` — needed for ProgramNode
(root, no parent) and GenericTSNode (materialise's bottom-up
fallback).
Migrated classes (88 total):
- Statement / control-flow: CatchClause, ForIn, ForOf, Switch (+ Case +
Default), Break / Continue, Block, EmptyStatement, IfStatement,
Return, While, DoWhile, For, Throw, Try, Labeled, Debugger.
- Expressions: Identifier, NewExpression, Member (with chain wrap),
Call (with chain wrap + import keyword router), Tagged-template,
Conditional, Sequence (router), Binary-like (router on operator),
Unary-like (multi-type), Spread (pattern router), Shorthand,
ObjectExpression / ArrayExpression (pattern routers), Yield, Await,
Template, NoSubstitutionTemplate, RegExp, Literal (Numeric / String
with JSX-attribute unescape), BoolLiteral (True / False), Null.
- Imports / exports (routers): Import, ImportSpecifier,
ImportDefaultSpecifier, NamespaceImport, Export* (named / all /
default / TS-equals via routers).
- Classes: Class (router on Decl / Expr), ClassBody, MethodDefinition,
ObjectMethod / ObjectAccessor (routers), PropertyDefinition with
abstract / accessor variants.
- TS types: TS-keyword (Any / Unknown / Number / …), Union /
Intersection, ArrayType, TupleType (+ NamedTupleMember + RestType
router), TypeOperator, FunctionType / ConstructorType, ConditionalType,
InferType, IndexedAccessType, MappedType, LiteralType (router for
null special case), Predicate, ImportType (with TSTypeQuery wrap
router), QualifiedName, TypeReference, TypeAssertion / Satisfies /
NonNull, Decorator, Template-literal-type, IndexSignature,
TypeParameter (in regular dispatch — MappedType skips), Enum (+ body),
Interface (+ body), TypeAlias, ParameterProperty (kept hand-written),
Module*, Property / Method signatures.
- JSX: JSXElement (with JsxSelfClosingElement variant), JSXOpeningElement
(hybrid synthetic via registersInMaps), JSXClosingElement,
JSXOpeningFragment / ClosingFragment, JSXFragment, JSXAttribute,
JSXSpreadAttribute, JSXSpreadChild, JSXExpressionContainer (with
empty-expression init), JSXText, JSXIdentifier, JSXMemberExpression,
JSXNamespacedName.
- Bindings: VariableStatement, VariableDeclaration, BindingElement
(router for array / object destructure), AssignmentPattern,
RestElement, ArrayBindingPattern, BindingAssignmentPattern.
- Other: Program (root), TypeKeyword (synthetic helper), GenericTSNode
(catch-all fallback).
7 classes remain hand-written by design — all are wrapper-with-extras
helpers whose constructor signature documents their API:
- ChainExpressionWrapping / TSTypeQueryWrapping — wrap a pre-built
inner expression; the outer claims the TS node's cache slot.
- ExportNamedWrapping / ExportDefaultWrapping — wrap an inner
declaration with the cache re-pointed.
- TSParameterProperty — wraps a parameter with class-property modifiers.
- TSTypeParameterDeclaration / TSTypeParameterInstantiation —
NodeArray-based, no SK to dispatch from.
Net diff: -672 lines. Hand-written subclass count 95 → 7 (93 % migrated).
SHAPES table at 134 entries plus 16 context-aware routers.
Tested: predicate-coverage / lazy-estree (15-fixture parity sweep + new
dogfood-corpus sweep with ~67k asserts) / scope-compat /
selector-analysis / ts-ast-scan / compat-pipeline / jsx-react-x / bench
/ dogfood (107 rules × 30 files clean) — all pass. Dify cold lint
within noise of master.
bdf721c to
9935b9d
Compare
johnsoncodehk
added a commit
that referenced
this pull request
May 3, 2026
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
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.
Two related pieces — the test infrastructure caught real bugs that the migration could then verify against, so they ship together.
1. Dogfood-corpus parity sweep
Extends the bottom-up parity sweep from 15 hand-crafted fixtures to the dogfood corpus — every production .ts file in the monorepo. Result: ~67k type asserts + ~67k parent asserts on real production code.
DOGFOOD_FILESextracted into a shared module so the sweep and the rule-level dogfood diff stay in lock-step.The broader walk surfaced 10 real drift bugs that fixture-sized testing didn't reach:
class X { y: T[] }— type child of class member missedTSTypeAnnotationwrapperTYPE_SLOT_TRIGGERSfunction f(x = 1)— parameter binding insideAssignmentPatternreportedparent.type === 'FunctionDeclaration'export { … }/\${x}`— bottom-up walker builtTSNamedExports/TSTemplateSpan` GenericTSNode wrappersSKIP_AS_PARENTtype X = { [K in keyof T]: … }— bottom-up wrapped K inTSTypeParameterfrom SHAPES dispatch; eager exposes K bare onTSMappedType.keyimport('foo')/typeof import('foo')— TSLiteralType wrapper between StringLiteral and TSImportType / TSTypeQueryargumentgetter + drillTSTypeQuery.exprNamevia WRAPPER_DRILLSThree known scaffolding ambiguities documented as accept-skips in the sweep (not bugs — choice or missing-but-unused wrapper):
2. Factory DSL full migration
Extends the SHAPES factory enough that 88 of 95 hand-written subclasses become declarative entries.
DSL extensions
type: KnownEstreeType | ((tsNode) => KnownEstreeType)range: (tsNode, ctx) => [start, end]init: (instance, tsNode) => voidconsts(tsNode, instance)constsread other already-set fieldsregistersInMaps: (tsNode) => booleandefineShapeRouter(kind, route)isExportEquals,allowPattern)context?What's migrated (88)
Statement / control flow, expressions (with chain-aware routers for Member/Call), imports / exports (routers for named / default / equals), classes (with abstract / accessor variants), TS types (Union / Intersection / Array / Tuple / Mapped / Predicate / ImportType / …), JSX (Element / OpeningElement hybrid / Identifier / MemberExpression / NamespacedName / leaves), bindings (BindingElement router for array vs object destructure, AssignmentPattern, RestElement), Program (root), TypeKeyword (synthetic helper), GenericTSNode (catch-all fallback).
What stays hand-written (7)
All "wrapper-with-extras" helpers whose constructor signature literally documents their API:
ChainExpressionWrappingNode,TSTypeQueryWrappingNode,ExportNamedWrappingNode,ExportDefaultWrappingNode,TSParameterPropertyNode— wrap a pre-built inner LazyNode + extra argsTSTypeParameterDeclarationNode,TSTypeParameterInstantiationNode— NodeArray-based, no SK to dispatch fromThese could be migrated by adding variadic constructor args to the factory, but the resulting
makeShapeClass<T, [Inner, ExtraArg]>signature obscures their "this is an active builder, not a passive shape" nature. Hand-written stays clearer.Final numbers
lazy-estree.ts, the parity sweep test, and the dogfood module splitBug count caught by this work
17 pre-existing bugs:
AccessorPropertyfromKnownEstreeType(compile-time gate caught it)7 migration-induced bugs caught by tests during the work:
[]vs null (factory short-circuit on null tsValue)convertChild)[]vs null (same short-circuit)[](Constructor has noname, factory short-circuit)no-shadowfalse positiveTotal: 24 bugs (17 pre-existing + 7 migration-self-caught).
Test plan
predicate-coverage(152/152 covered, 16 ignored)lazy-estree.test: 15-fixture parity sweep + dogfood corpus (~67k type / parent asserts) + factory tests + phantom-types invariantscope-compat(24/24 fixtures clean),selector-analysis,ts-ast-scan,compat-pipeline,jsx-react-x— all passbench— 107 rules × 33 files = clean paritydogfood.js— 30 files × 107 rules, 0 divergences, 0 crashesCloses