Skip to content

compat-eslint: dogfood-corpus parity sweep + 6 drift fixes it caught #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
johnsoncodehk wants to merge 1 commit into from
Closed

compat-eslint: dogfood-corpus parity sweep + 6 drift fixes it caught #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 66 additions & 9 deletions packages/compat-eslint/lib/lazy-estree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,26 @@ const SKIP_AS_PARENT: Partial<Record<ts.SyntaxKind, SkipDecision>> = {
[SK.SyntaxList]: true,
[SK.CaseBlock]: true,
[SK.NamedImports]: true,
[SK.NamedExports]: true,
[SK.ImportClause]: true,
[SK.JsxAttributes]: true,
// TS wraps each `${expr}` in a TemplateSpan with the trailing literal
// piece. ESTree flattens: TemplateLiteral.expressions and .quasis are
// siblings, no per-span container. Skip past TemplateSpan so the
// expressions/literal-pieces resolve to TemplateLiteral as parent.
[SK.TemplateSpan]: true,
// TS MappedType wraps the iterating identifier in a TypeParameter
// container; ESTree exposes the bare name on TSMappedType.key. Skip
// past TypeParameter so the inner Identifier resolves to TSMappedType,
// matching typescript-estree's convertMappedType output.
[SK.TypeParameter]: w => w.parent?.kind === SK.MappedType,
// `import('foo')` type: TS wraps the string literal in a LiteralType
// (so the AST shape is ImportTypeNode { argument: LiteralType {
// literal: StringLiteral } }). ESTree exposes TSImportType.argument
// as the bare StringLiteral — no TSLiteralType in between. Skip the
// LiteralType so bottom-up materialise of the inner StringLiteral
// resolves to TSImportType as parent.
[SK.LiteralType]: w => w.parent?.kind === SK.ImportType,
[SK.VariableDeclarationList]: w => w.parent?.kind === SK.VariableStatement,
[SK.VariableDeclaration]: w => w.parent?.kind === SK.CatchClause,
[SK.HeritageClause]: w => (w as ts.HeritageClause).token === SK.ExtendsKeyword,
Expand Down Expand Up @@ -638,6 +656,15 @@ const WRAPPER_DRILLS: WrapperDrill[] = [
return v && v.type === 'AssignmentPattern' ? v : undefined;
},
},
// `typeof import('x')` — the TSTypeQueryWrappingNode claims the
// ImportType's TS slot in the cache, but the inner TSImportType
// (TSTypeQuery.exprName) is what holds the import's children. Without
// this drill, bottom-up materialise of the import's argument lands
// on TSTypeQuery directly, missing the inner TSImportType layer.
{
match: (w, dt) => w.kind === SK.ImportType && dt === 'TSTypeQuery',
drill: drillFrom => drillFrom.exprName,
},
];

// Per-parent-kind: how to materialise the type-position child via the
Expand Down Expand Up @@ -697,6 +724,24 @@ const TYPE_SLOT_TRIGGERS: Partial<Record<ts.SyntaxKind, (owner: any) => void>> =
[SK.ConstructorType]: o => {
if (o.returnType) void o.returnType.typeAnnotation;
},
[SK.PropertyDeclaration]: o => {
// PropertyDefinition / AccessorProperty / TSAbstract* — class field.
if (o.typeAnnotation) void o.typeAnnotation.typeAnnotation;
},
[SK.MethodDeclaration]: o => {
// MethodDefinition / Property (object shorthand) — method body is a
// FunctionExpression at .value; returnType lives there.
if (o.value?.returnType) void o.value.returnType.typeAnnotation;
},
[SK.GetAccessor]: o => {
// MethodDefinition kind:'get' / Property kind:'get'.
if (o.value?.returnType) void o.value.returnType.typeAnnotation;
},
[SK.TypePredicate]: o => {
// `x is T` — the predicate's inner type itself wraps in a nested
// TSTypeAnnotation.
if (o.typeAnnotation) void o.typeAnnotation.typeAnnotation;
},
};

// Pattern-position parent kinds (BinaryExpression-LHS / for-loop-LHS /
Expand Down Expand Up @@ -2614,19 +2659,24 @@ class TSImportTypeNode extends LazyNode {
}
}

// eager exposes `source` (= argument.literal — the StringLiteral) and
// `argument` is a deprecated alias.
// eager flattens the LiteralType wrapper around the string argument:
// TSImportType.argument / .source = the inner StringLiteral directly,
// with parent === TSImportType. Build the inner once, share between
// both getters.
get source() {
if (this._source !== undefined) return this._source;
const argEstree = this._argumentEstree ??= convertChild((this._ts as ts.ImportTypeNode).argument, this);
// argEstree is a TSLiteralType wrapping a StringLiteral.
const lit = (argEstree as unknown as { literal?: LazyNode | null } | null)?.literal ?? null;
return this._source = lit;
this._argumentEstree ??= this._buildArgument();
return this._source = this._argumentEstree;
}

// Deprecated alias for source — eager wires this via #withDeprecatedAliasGetter.
get argument() {
return this._argumentEstree ??= convertChild((this._ts as ts.ImportTypeNode).argument, this);
return this._argumentEstree ??= this._buildArgument();
}
private _buildArgument(): LazyNode | null {
const arg = (this._ts as ts.ImportTypeNode).argument;
if (arg.kind === SK.LiteralType) {
return convertChild((arg as ts.LiteralTypeNode).literal, this);
}
return convertChild(arg, this);
}

get qualifier() {
Expand Down Expand Up @@ -4333,6 +4383,13 @@ class AssignmentPatternNode extends LazyNode {
constructor(tsNode: ts.ParameterDeclaration, parent: LazyNode, left: LazyNode) {
super(tsNode, parent);
this.left = left;
// Re-point the wrapped binding name — without this, the inner's
// parent stays as the function passed to convertParameter, so
// bottom-up materialise of the binding identifier sees
// `parent.type === 'FunctionDeclaration'` instead of the
// AssignmentPattern wrapper. Same pattern as
// BindingAssignmentPatternNode.
(left as { parent: LazyNode }).parent = this;
// AssignmentPattern range starts at the param name (eager strips
// modifiers from the range — line 1182).
const start = (tsNode.name as ts.Node).getStart(this._ctx.ast);
Expand Down
42 changes: 42 additions & 0 deletions packages/compat-eslint/test/bench/dogfood-corpus.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Repo-relative paths for the dogfood corpus. All real production .ts
// files in the monorepo (excluding .d.ts, fixtures, tests, bench,
// node_modules, worktrees). Imported by:
// - test/bench/dogfood.ts (rule-level parity diff)
// - test/lazy-estree.test.ts (node-level structural parity sweep)
//
// Adding a new production file? Append the repo-relative path here and
// both consumers pick it up. The structural sweep is exhaustive over
// every TS node in every listed file — drift in any hand-written class's
// getter against typescript-estree's astMaps fails CI.
export const DOGFOOD_FILES = [
'packages/cli/index.ts',
'packages/cli/lib/cache.ts',
'packages/cli/lib/colors.ts',
'packages/cli/lib/fs-cache.ts',
'packages/cli/lib/languagePlugins.ts',
'packages/cli/lib/render.ts',
'packages/cli/lib/worker.ts',
'packages/compat-eslint/index.ts',
'packages/compat-eslint/lib/lazy-estree.ts',
'packages/compat-eslint/lib/selector-analysis.ts',
'packages/compat-eslint/lib/tokens.ts',
'packages/compat-eslint/lib/ts-ast-scan.ts',
'packages/compat-eslint/lib/ts-scope-manager.ts',
'packages/compat-eslint/lib/visitor-keys.ts',
'packages/config/index.ts',
'packages/config/lib/eslint-gen.ts',
'packages/config/lib/eslint-types.ts',
'packages/config/lib/eslint.ts',
'packages/config/lib/plugins/category.ts',
'packages/config/lib/plugins/diagnostics.ts',
'packages/config/lib/plugins/ignore.ts',
'packages/config/lib/tsl.ts',
'packages/config/lib/tslint-gen.ts',
'packages/config/lib/tslint-types.ts',
'packages/config/lib/tslint.ts',
'packages/config/lib/utils.ts',
'packages/core/index.ts',
'packages/types/index.ts',
'packages/typescript-plugin/index.ts',
'tsslint.config.ts',
] as const;
39 changes: 3 additions & 36 deletions packages/compat-eslint/test/bench/dogfood.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,42 +24,9 @@ const tsParser = require('@typescript-eslint/parser');

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';

const REPO_ROOT = path.resolve(__dirname, '..', '..', '..', '..');
const DOGFOOD_FILES = [
'packages/cli/index.ts',
'packages/cli/lib/cache.ts',
'packages/cli/lib/colors.ts',
'packages/cli/lib/fs-cache.ts',
'packages/cli/lib/languagePlugins.ts',
'packages/cli/lib/render.ts',
'packages/cli/lib/worker.ts',
'packages/compat-eslint/index.ts',
'packages/compat-eslint/lib/lazy-estree.ts',
'packages/compat-eslint/lib/selector-analysis.ts',
'packages/compat-eslint/lib/tokens.ts',
'packages/compat-eslint/lib/ts-ast-scan.ts',
'packages/compat-eslint/lib/ts-scope-manager.ts',
'packages/compat-eslint/lib/visitor-keys.ts',
'packages/config/index.ts',
'packages/config/lib/eslint-gen.ts',
'packages/config/lib/eslint-types.ts',
'packages/config/lib/eslint.ts',
'packages/config/lib/plugins/category.ts',
'packages/config/lib/plugins/diagnostics.ts',
'packages/config/lib/plugins/ignore.ts',
'packages/config/lib/tsl.ts',
'packages/config/lib/tslint-gen.ts',
'packages/config/lib/tslint-types.ts',
'packages/config/lib/tslint.ts',
'packages/config/lib/utils.ts',
'packages/core/index.ts',
'packages/types/index.ts',
'packages/typescript-plugin/index.ts',
'tsslint.config.ts',
];

interface DiagLoc {
file: string;
Expand All @@ -68,7 +35,7 @@ interface DiagLoc {
ruleId: string;
}

function buildProgram(files: string[]) {
function buildProgram(files: readonly string[]) {
const realLibPath = ts.getDefaultLibFilePath({ target: ts.ScriptTarget.ES2020 });
const realLibName = realLibPath.split(/[\\/]/).pop()!;
const realLib = ts.createSourceFile(
Expand Down
112 changes: 107 additions & 5 deletions packages/compat-eslint/test/lazy-estree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1569,7 +1569,17 @@ function findTsJsxExpr(sf: ts.SourceFile, attrName?: string): ts.JsxExpression |
return;
}
totalNodesChecked++;
if (lazyNode.type !== eager.type) {
// Template parts: typescript-estree maps TemplateHead/
// Middle/Tail to ESTree TemplateElement, but lazy
// synthesizes plain TemplateElement objects on
// TemplateLiteral.quasis without dispatching the underlying
// TS kinds — bottom-up materialise lands on the GenericTSNode
// fallback (type 'TSTemplateHead' etc.). The rule corpus
// doesn't query bottom-up parent of template parts; accepted.
const isTemplatePart = n.kind === ts.SyntaxKind.TemplateHead
|| n.kind === ts.SyntaxKind.TemplateMiddle
|| n.kind === ts.SyntaxKind.TemplateTail;
if (lazyNode.type !== eager.type && !isTemplatePart) {
localFails.push(
`type ${lazyNode.type} vs eager ${eager.type} (TS kind: ${ts.SyntaxKind[n.kind]})`,
);
Expand All @@ -1586,13 +1596,53 @@ function findTsJsxExpr(sf: ts.SourceFile, attrName?: string): ts.JsxExpression |
&& (n.parent as ts.BindingElement).initializer !== undefined
&& (n.parent as ts.BindingElement).name === n
&& n.parent.parent?.kind === ts.SyntaxKind.ObjectBindingPattern;
// Optional-chain scaffolding: typescript-estree wraps EACH
// PropertyAccess/ElementAccess/Call link in a fresh
// ChainExpression; lazy uses a single ChainExpressionWrapping
// at the outermost link. Both shapes are spec-correct and
// the rule corpus doesn't depend on per-link wrapping —
// accept the parent divergence whenever either side names
// ChainExpression.
const isChainScaffolding = (eagerType: string | undefined, lazyType: string | undefined) =>
eagerType === 'ChainExpression' || lazyType === 'ChainExpression';
// Missing scaffolding wrappers — known gaps that bottom-up
// surfaces but the rule corpus doesn't query:
// - TSInterfaceHeritage: typescript-estree wraps each
// identifier in `interface X extends Y, Z` in a
// TSInterfaceHeritage container; lazy emits the bare
// Identifier as a child of TSInterfaceDeclaration.
// - TSAssertClause: `import x from 'y' assert {...}`
// wraps the assert entries in a TSAssertClause; lazy
// emits ImportAttribute children directly under
// ImportDeclaration.
// Adding either wrapper is mechanical (a new SyntheticLazyNode
// class + nav-table entry); deferred — the parity sweep
// will continue to flag any additional gaps and the skip
// list shrinks as the wrappers land.
const isMissingWrapper = (eagerType: string | undefined, lazyType: string | undefined) => {
return eagerType === 'TSInterfaceHeritage'
|| lazyType === 'TSInterfaceHeritage'
|| eagerType === 'TSAssertClause'
|| lazyType === 'TSAssertClause';
};
// Compare parent.type when both sides have parents (Program /
// root has none on either side).
if (eager.parent || lazyNode.parent) {
if ((eager.parent || lazyNode.parent) && !isTemplatePart) {
totalParentsChecked++;
const eagerPType: string | undefined = eager.parent?.type;
const lazyPType: string | undefined = lazyNode.parent?.type;
if (eagerPType !== lazyPType && !isShorthandIdent) {
// When a known wrapper is missing on the lazy side, the
// rest of the chain is off-by-one — comparing further
// up just amplifies the same gap. Skip the entire
// upward comparison once we hit a documented wrapper
// hole.
const wrapperGap = isMissingWrapper(eagerPType, lazyPType);
if (
eagerPType !== lazyPType
&& !isShorthandIdent
&& !isChainScaffolding(eagerPType, lazyPType)
&& !wrapperGap
) {
localFails.push(
`${lazyNode.type}.parent.type ${JSON.stringify(lazyPType)} vs eager ${JSON.stringify(eagerPType)}`,
);
Expand All @@ -1601,10 +1651,15 @@ function findTsJsxExpr(sf: ts.SourceFile, attrName?: string): ts.JsxExpression |
// like the original 'TSJsxAttributes between attribute and
// opening element' bug, which had correct parent.parent end
// but wrong intermediate type for non-self-closing tags.
if (eager.parent?.parent || lazyNode.parent?.parent) {
if ((eager.parent?.parent || lazyNode.parent?.parent) && !wrapperGap) {
const eagerGType: string | undefined = eager.parent?.parent?.type;
const lazyGType: string | undefined = lazyNode.parent?.parent?.type;
if (eagerGType !== lazyGType && !isShorthandIdent) {
if (
eagerGType !== lazyGType
&& !isShorthandIdent
&& !isChainScaffolding(eagerGType, lazyGType)
&& !isMissingWrapper(eagerGType, lazyGType)
) {
localFails.push(
`${lazyNode.type}.parent.parent.type ${JSON.stringify(lazyGType)} vs eager ${
JSON.stringify(eagerGType)
Expand Down Expand Up @@ -1641,6 +1696,53 @@ function findTsJsxExpr(sf: ts.SourceFile, attrName?: string): ts.JsxExpression |
? '\n ' + fixtureFailures.map(([n, m]) => `[${n}] ${m}`).join('\n ')
: undefined,
);

// --- Dogfood corpus extension --------------------------------------
//
// The 15 fixtures above target known bug classes. To catch drift in
// the ~80 hand-written subclasses that aren't migration-able to
// SHAPES, replay the same node-level invariant across the dogfood
// corpus — every real production .ts file in the monorepo. ~30
// files, ~100k+ TS nodes.
//
// Failure here = a hand-written class's getter returns a different
// lazy node than typescript-estree builds for the same TS slot,
// triggered by some real production code pattern that the
// hand-crafted fixtures don't exercise. Drift surface that the
// architectural gates (KnownEstreeType compile-time, SyntheticLazyNode
// boundary) can't catch — only the structural walk does.
const fs = require('fs') as typeof import('fs');
const path = require('path') as typeof import('path');
const { DOGFOOD_FILES } = require('./bench/dogfood-corpus.js') as {
DOGFOOD_FILES: readonly string[];
};
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..');
let dogfoodNodes = 0;
let dogfoodParents = 0;
const dogfoodFailures: Array<[string, string]> = [];
for (const rel of DOGFOOD_FILES) {
const before = totalNodesChecked;
const beforeP = totalParentsChecked;
const beforeF = fixtureFailures.length;
const abs = path.join(REPO_ROOT, rel);
const code = fs.readFileSync(abs, 'utf8');
bottomUpParity({ name: rel, code });
dogfoodNodes += totalNodesChecked - before;
dogfoodParents += totalParentsChecked - beforeP;
for (let i = beforeF; i < fixtureFailures.length; i++) {
dogfoodFailures.push(fixtureFailures[i]);
}
}
const dogfoodOnlyFailures = dogfoodFailures.filter(([name]) =>
DOGFOOD_FILES.includes(name as (typeof DOGFOOD_FILES)[number])
);
check(
`bottom-up parity sweep (dogfood): ${DOGFOOD_FILES.length} files, ${dogfoodNodes} type asserts, ${dogfoodParents} parent asserts`,
dogfoodOnlyFailures.length === 0,
dogfoodOnlyFailures.length
? '\n ' + dogfoodOnlyFailures.map(([n, m]) => `[${n}] ${m}`).join('\n ')
: undefined,
);
}

// --- No phantom GenericTSNode types in any fixture's reachable tree -----
Expand Down
Loading