Skip to content

compat-eslint: factory DSL full migration + dogfood-corpus parity sweep#95

Merged
johnsoncodehk merged 1 commit into
masterfrom
refactor/factory-dsl-full
May 3, 2026
Merged

compat-eslint: factory DSL full migration + dogfood-corpus parity sweep#95
johnsoncodehk merged 1 commit into
masterfrom
refactor/factory-dsl-full

Conversation

@johnsoncodehk
Copy link
Copy Markdown
Owner

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_FILES extracted 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:

# Pattern Fix
1 class X { y: T[] } — type child of class member missed TSTypeAnnotation wrapper Add 4 entries (PropertyDeclaration / MethodDeclaration / GetAccessor / TypePredicate) to TYPE_SLOT_TRIGGERS
2 function f(x = 1) — parameter binding inside AssignmentPattern reported parent.type === 'FunctionDeclaration' Re-parent in the wrapper constructor
3 export { … } / \${x}`— bottom-up walker builtTSNamedExports/TSTemplateSpan` GenericTSNode wrappers Add to SKIP_AS_PARENT
4 type X = { [K in keyof T]: … } — bottom-up wrapped K in TSTypeParameter from SHAPES dispatch; eager exposes K bare on TSMappedType.key Skip TypeParameter as parent when TS parent is MappedType
5 import('foo') / typeof import('foo') — TSLiteralType wrapper between StringLiteral and TSImportType / TSTypeQuery Skip LiteralType in ImportType + flatten argument getter + drill TSTypeQuery.exprName via WRAPPER_DRILLS

Three known scaffolding ambiguities documented as accept-skips in the sweep (not bugs — choice or missing-but-unused wrapper):

  • ChainExpression scaffolding (lazy single outer wrapper vs eager per-link)
  • TSInterfaceHeritage missing wrapper
  • TSAssertClause missing wrapper

2. Factory DSL full migration

Extends the SHAPES factory enough that 88 of 95 hand-written subclasses become declarative entries.

DSL extensions

Feature Use
type: KnownEstreeType | ((tsNode) => KnownEstreeType) Multi-variant kinds (PrefixUnary → Update vs Unary by operator)
range: (tsNode, ctx) => [start, end] Custom range computation
init: (instance, tsNode) => void Post-construction setup needing the constructed instance
consts(tsNode, instance) Second arg lets consts read other already-set fields
registersInMaps: (tsNode) => boolean Per-instance LazyNode-level predicate (hybrid synthetic/real classes)
defineShapeRouter(kind, route) Context-aware dispatch (parent kind, modifiers, isExportEquals, allowPattern)
3rd ctor arg context? Root Program + GenericTSNode fallback

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 args
  • TSTypeParameterDeclarationNode, TSTypeParameterInstantiationNode — NodeArray-based, no SK to dispatch from

These 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

  • Hand-written subclass count 95 → 7 (93% migrated)
  • SHAPES table 57 → 134 entries + 16 context-aware routers
  • Net −672 lines across lazy-estree.ts, the parity sweep test, and the dogfood module split

Bug count caught by this work

17 pre-existing bugs:

  • 6 in the JSX attribute parent regression + parity gaps from earlier work
  • 1 missing AccessorProperty from KnownEstreeType (compile-time gate caught it)
  • 10 from the new dogfood-corpus parity sweep

7 migration-induced bugs caught by tests during the work:

  • ImportSpecifier setter conflict (factory limitation discovered)
  • InterfaceDeclaration extends [] vs null (factory short-circuit on null tsValue)
  • ImportDeclaration specifiers range off-by-one (synthesized plain object instead of convertChild)
  • Class implements [] vs null (same short-circuit)
  • BindingElement context-aware dispatch (array vs object binding)
  • MethodDefinition decorators null vs [] (Constructor has no name, factory short-circuit)
  • TypeKeywordNode self-named function expression triggering no-shadow false positive

Total: 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 invariant
  • scope-compat (24/24 fixtures clean), selector-analysis, ts-ast-scan, compat-pipeline, jsx-react-x — all pass
  • bench — 107 rules × 33 files = clean parity
  • dogfood.js — 30 files × 107 rules, 0 divergences, 0 crashes
  • Dify web cold lint within noise of master

Closes

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_FILES into 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.ts subclasses to the SHAPES factory/routers and expand the DSL to support dynamic type, range, init, computed slots, and per-instance registersInMaps.

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.
@johnsoncodehk johnsoncodehk force-pushed the refactor/factory-dsl-full branch from bdf721c to 9935b9d Compare May 3, 2026 15:31
@johnsoncodehk johnsoncodehk merged commit 52949a3 into master May 3, 2026
1 check passed
@johnsoncodehk johnsoncodehk deleted the refactor/factory-dsl-full branch May 27, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants