Skip to content

Commit 43e4a73

Browse files
committed
refactor(compat-eslint): unify per-file caches into single state object
`sharedCache` (file → reports/errors) and `cachedEstree` (file → sourceCode/convertContext) were two separate module-level slots keyed by the same `ts.SourceFile`, populated together during the first tsslintRule call for a file, and invalidated together when the file changed. The split was historical, not semantic. Fold both into one `perFileState`: { file, sourceCode, convertContext, reports, errors }. As a follow-on: - `getEstree` (self-caching) becomes `buildEstree` (pure, returns { sourceCode, convertContext }) — caching is now the caller's responsibility, centralised on `perFileState`. - `runSharedTraversal` takes the state object directly instead of separate reports/errors maps. - `runTsScanInline` / `runCpaInline` take `convertContext` as a parameter instead of reading module-level state. Data flow becomes explicit per-call rather than implicit through globals. Net: one fewer module-level slot, one fewer self-caching helper, all four test suites still clean (bench / upstream / compat- pipeline / jsx-react-x).
1 parent 9cc7a05 commit 43e4a73

1 file changed

Lines changed: 121 additions & 119 deletions

File tree

packages/compat-eslint/index.ts

Lines changed: 121 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -127,27 +127,26 @@ interface DeferredReport {
127127
// Module-level state — populated by convertRule, queried at lint time.
128128
const ruleRegistry = new Map</* eslintRule */ ESLint.Rule.RuleModule, RuleEntry>();
129129

130-
// Per-file shared cache: stash all rules' deferred reports built during a single
131-
// traversal pass; each rule's tsslintRule call replays its own bucket. If a rule
132-
// listener throws, capture it in `errors` so the rule's call can rethrow at
133-
// replay time (preserving TSSLint core's per-rule type-aware retry semantics).
134-
let sharedCache: {
135-
file: ts.SourceFile;
136-
reports: Map</* eslintRule */ ESLint.Rule.RuleModule, DeferredReport[]>;
137-
errors: Map</* eslintRule */ ESLint.Rule.RuleModule, unknown>;
138-
} | undefined;
139-
140-
// sourceCode cache is per-file. The eventQueue itself is no longer
141-
// cached — every call to `runSharedTraversal` rebuilds it from the TS
142-
// AST scan, since the path now serves both CPA and non-CPA modes
143-
// uniformly (CPA's per-walk state can't be replayed from a stale queue
144-
// anyway). `convertContext` is kept so the TS-scan path can call
145-
// `materialize(tsNode, context)` for each hit without rebuilding the
146-
// converter state.
147-
let cachedEstree: {
130+
// Per-file lint state. Single object that bundles:
131+
// - `sourceCode` / `convertContext`: lazy ESTree + LazySourceCode for
132+
// this file. `convertContext` survives across rule replays so each
133+
// `materialize(tsNode, context)` hit hits the same identity-preserved
134+
// LazyNode cache.
135+
// - `reports`: deferred per-rule reports collected during a single
136+
// shared traversal; each tsslintRule call replays its own bucket.
137+
// - `errors`: per-rule listener throws, captured so a rule's tsslintRule
138+
// can rethrow at replay time (preserving TSSLint core's per-rule
139+
// type-aware retry semantics).
140+
//
141+
// Everything in here invalidates together when `file` changes — there's
142+
// no scenario where one part is reusable without the others, so a single
143+
// per-file slot replaces the two separate caches the earlier design had.
144+
let perFileState: {
148145
file: ts.SourceFile;
149146
sourceCode: ESLint.SourceCode;
150147
convertContext: unknown;
148+
reports: Map</* eslintRule */ ESLint.Rule.RuleModule, DeferredReport[]>;
149+
errors: Map</* eslintRule */ ESLint.Rule.RuleModule, unknown>;
151150
} | undefined;
152151

153152
export function convertRule(
@@ -175,17 +174,24 @@ export function convertRule(
175174
ruleRegistry.set(eslintRule, entry);
176175

177176
const tsslintRule: TSSLint.Rule = ({ file, report, program }) => {
178-
if (sharedCache?.file !== file) {
179-
sharedCache = { file, reports: new Map(), errors: new Map() };
180-
runSharedTraversal(file, program, sharedCache.reports, sharedCache.errors);
177+
if (perFileState?.file !== file) {
178+
const { sourceCode, convertContext } = buildEstree(file, program);
179+
perFileState = {
180+
file,
181+
sourceCode,
182+
convertContext,
183+
reports: new Map(),
184+
errors: new Map(),
185+
};
186+
runSharedTraversal(file, program, perFileState);
181187
}
182188

183-
const ruleError = sharedCache.errors.get(eslintRule);
189+
const ruleError = perFileState.errors.get(eslintRule);
184190
if (ruleError !== undefined) {
185191
throw ruleError;
186192
}
187193

188-
const myReports = sharedCache.reports.get(eslintRule);
194+
const myReports = perFileState.reports.get(eslintRule);
189195
if (!myReports || myReports.length === 0) {
190196
return;
191197
}
@@ -226,10 +232,9 @@ export function convertRule(
226232
function runSharedTraversal(
227233
file: ts.SourceFile,
228234
program: ts.Program,
229-
reports: Map<ESLint.Rule.RuleModule, DeferredReport[]>,
230-
errors: Map<ESLint.Rule.RuleModule, unknown>,
235+
state: NonNullable<typeof perFileState>,
231236
) {
232-
const { sourceCode } = getEstree(file, program);
237+
const { sourceCode, convertContext, reports, errors } = state;
233238
const cwd = program.getCurrentDirectory();
234239

235240
let currentNode: any;
@@ -402,10 +407,10 @@ function runSharedTraversal(
402407
// whose `emit` / `enterNode` / `leaveNode` hooks dispatch inline.
403408
// No event queue — CPA's order of calls IS the dispatch order.
404409
if (fast.codePath.size > 0) {
405-
runCpaInline(file, fast, errors, onTarget);
410+
runCpaInline(file, fast, errors, onTarget, convertContext);
406411
}
407412
else {
408-
runTsScanInline(file, fast, errors, onTarget);
413+
runTsScanInline(file, fast, errors, onTarget, convertContext);
409414
}
410415
}
411416

@@ -533,13 +538,11 @@ function runTsScanInline(
533538
fast: FastDispatch,
534539
errors: Map<ESLint.Rule.RuleModule, unknown>,
535540
onTarget: (target: unknown) => void,
541+
convertContext: unknown,
536542
): void {
537-
if (!cachedEstree) {
538-
throw new Error('runTsScanInline called without an active sourceCode cache');
539-
}
540543
const { tsScanTraverse } = require('./lib/ts-ast-scan') as typeof import('./lib/ts-ast-scan');
541544
const match = buildScanPredicate(fast);
542-
tsScanTraverse(file, match, cachedEstree.convertContext as any, {
545+
tsScanTraverse(file, match, convertContext as any, {
543546
enterNode(target) {
544547
dispatchTarget(target, true, fast, errors, onTarget);
545548
},
@@ -560,10 +563,8 @@ function runCpaInline(
560563
fast: FastDispatch,
561564
errors: Map<ESLint.Rule.RuleModule, unknown>,
562565
onTarget: (target: unknown) => void,
566+
convertContext: unknown,
563567
): void {
564-
if (!cachedEstree) {
565-
throw new Error('runCpaInline called without an active sourceCode cache');
566-
}
567568
const { tsScanTraverse } = require('./lib/ts-ast-scan') as typeof import('./lib/ts-ast-scan');
568569
const match = buildScanPredicate(fast);
569570
// CPA emits `onCodePathStart` / `onCodePathSegment*` / etc. Dispatch
@@ -597,7 +598,7 @@ function runCpaInline(
597598
};
598599
const { CodePathAnalyzer } = loadEslintInternals();
599600
const cpa = new CodePathAnalyzer(wrapped);
600-
tsScanTraverse(file, match, cachedEstree.convertContext as any, {
601+
tsScanTraverse(file, match, convertContext as any, {
601602
enterNode(target) {
602603
cpa.enterNode(target);
603604
},
@@ -799,89 +800,90 @@ function isIterable(obj: unknown): obj is Iterable<ESLint.Rule.Fix> {
799800
return obj != null && typeof (obj as { [Symbol.iterator]?: unknown })[Symbol.iterator] === 'function';
800801
}
801802

802-
function getEstree(file: ts.SourceFile, program: ts.Program) {
803-
if (cachedEstree?.file !== file) {
804-
// Skip @typescript-eslint/parser: parseForESLint dynamically loads the
805-
// whole parser package on first call (the heaviest single dep) and just
806-
// dispatches to typescript-estree's astConverter, which we already have
807-
// a ts.SourceFile for. Calling it directly avoids the parser require.
808-
const { visitorKeys } = require('./lib/visitor-keys') as typeof import('./lib/visitor-keys');
809-
const { TsScopeManager, applyEslintGlobals } = require(
810-
'./lib/ts-scope-manager',
811-
) as typeof import('./lib/ts-scope-manager');
812-
const { convertLazy } = require('./lib/lazy-estree') as typeof import('./lib/lazy-estree');
813-
const { LazySourceCode } = require('./lib/lazy-source-code') as typeof import('./lib/lazy-source-code');
814-
815-
// Lazy ESTree shim (lib/lazy-estree.ts). Byte-identical to
816-
// typescript-estree's eager Converter on every TS file under
817-
// packages/, but materialises children on first read. Rules see
818-
// real subtrees and can't null-deref into them.
819-
const { astMaps, estree, context: convertContext } = convertLazy(file) as {
820-
astMaps: any;
821-
estree: any;
822-
context: unknown;
823-
};
803+
// Build a fresh `LazySourceCode` + lazy-estree convert context for `file`.
804+
// Pure: no module-level caching here — callers cache via `perFileState`.
805+
//
806+
// Skips @typescript-eslint/parser: `parseForESLint` dynamically loads the
807+
// whole parser package on first call (the heaviest single dep) and just
808+
// dispatches to typescript-estree's astConverter, which we already have a
809+
// ts.SourceFile for. Calling our own converter directly avoids the require.
810+
function buildEstree(file: ts.SourceFile, program: ts.Program): {
811+
sourceCode: ESLint.SourceCode;
812+
convertContext: unknown;
813+
} {
814+
const { visitorKeys } = require('./lib/visitor-keys') as typeof import('./lib/visitor-keys');
815+
const { TsScopeManager, applyEslintGlobals } = require(
816+
'./lib/ts-scope-manager',
817+
) as typeof import('./lib/ts-scope-manager');
818+
const { convertLazy } = require('./lib/lazy-estree') as typeof import('./lib/lazy-estree');
819+
const { LazySourceCode } = require('./lib/lazy-source-code') as typeof import('./lib/lazy-source-code');
820+
821+
// Lazy ESTree shim (lib/lazy-estree.ts). Byte-identical to
822+
// typescript-estree's eager Converter on every TS file under
823+
// packages/, but materialises children on first read. Rules see
824+
// real subtrees and can't null-deref into them.
825+
const { astMaps, estree, context: convertContext } = convertLazy(file) as {
826+
astMaps: any;
827+
estree: any;
828+
context: unknown;
829+
};
824830

825-
// tokens / comments come from our own scanner-based converters
826-
// (lib/tokens.ts) — byte-identical to typescript-estree's
827-
// `convertTokens` / `convertComments` on every checked fixture.
828-
// Rules like no-unnecessary-type-assertion call
829-
// `sourceCode.getTokenAfter()` and need the tokens array — but
830-
// most rules never touch tokens/comments. Defer the scan via lazy
831-
// getters: cheap when no rule reads, ~80ms saved on large files.
832-
const { convertTokens, convertComments } = require('./lib/tokens') as typeof import('./lib/tokens');
833-
let _tokens: unknown[] | undefined;
834-
let _comments: unknown[] | undefined;
835-
Object.defineProperty(estree, 'tokens', {
836-
configurable: true,
837-
enumerable: true,
838-
get: () => _tokens ??= convertTokens(file),
839-
set: (v: unknown[]) => {
840-
_tokens = v;
841-
},
842-
});
843-
Object.defineProperty(estree, 'comments', {
844-
configurable: true,
845-
enumerable: true,
846-
get: () => _comments ??= convertComments(file),
847-
set: (v: unknown[]) => {
848-
_comments = v;
849-
},
850-
});
831+
// tokens / comments come from our own scanner-based converters
832+
// (lib/tokens.ts) — byte-identical to typescript-estree's
833+
// `convertTokens` / `convertComments` on every checked fixture.
834+
// Rules like no-unnecessary-type-assertion call
835+
// `sourceCode.getTokenAfter()` and need the tokens array — but
836+
// most rules never touch tokens/comments. Defer the scan via lazy
837+
// getters: cheap when no rule reads, ~80ms saved on large files.
838+
const { convertTokens, convertComments } = require('./lib/tokens') as typeof import('./lib/tokens');
839+
let _tokens: unknown[] | undefined;
840+
let _comments: unknown[] | undefined;
841+
Object.defineProperty(estree, 'tokens', {
842+
configurable: true,
843+
enumerable: true,
844+
get: () => _tokens ??= convertTokens(file),
845+
set: (v: unknown[]) => {
846+
_tokens = v;
847+
},
848+
});
849+
Object.defineProperty(estree, 'comments', {
850+
configurable: true,
851+
enumerable: true,
852+
get: () => _comments ??= convertComments(file),
853+
set: (v: unknown[]) => {
854+
_comments = v;
855+
},
856+
});
851857

852-
estree.sourceType = (file as { externalModuleIndicator?: unknown }).externalModuleIndicator
853-
? 'module'
854-
: 'script';
855-
const scopeManager = new TsScopeManager(file, program, estree, astMaps, estree.sourceType);
856-
// Inject ECMAScript built-ins + TS lib type globals so `no-undef`
857-
// doesn't fire on `undefined` / `Math` / `Record<K, V>` / etc.
858-
// `TsScopeManager` itself stays free of this lint-pipeline policy
859-
// (upstream eslint-scope parity tests rely on the un-injected
860-
// shape); the names + de-dupe logic live next to `addGlobals` in
861-
// `ts-scope-manager.ts`.
862-
applyEslintGlobals(scopeManager);
863-
const sourceCode = new LazySourceCode({
864-
text: file.text,
865-
ast: estree,
866-
tsFile: file,
867-
scopeManager,
868-
visitorKeys: visitorKeys as Record<string, string[]>,
869-
parserServices: {
870-
...astMaps,
871-
program,
872-
hasFullTypeInformation: true,
873-
emitDecoratorMetadata: undefined,
874-
experimentalDecorators: undefined,
875-
isolatedDeclarations: undefined,
876-
getSymbolAtLocation: (node: any) =>
877-
program.getTypeChecker().getSymbolAtLocation(astMaps.esTreeNodeToTSNodeMap.get(node)),
878-
getTypeAtLocation: (node: any) =>
879-
program.getTypeChecker().getTypeAtLocation(astMaps.esTreeNodeToTSNodeMap.get(node)),
880-
},
881-
}) as unknown as ESLint.SourceCode;
882-
cachedEstree = { file, sourceCode, convertContext };
883-
}
884-
return {
885-
sourceCode: cachedEstree.sourceCode,
886-
};
858+
estree.sourceType = (file as { externalModuleIndicator?: unknown }).externalModuleIndicator
859+
? 'module'
860+
: 'script';
861+
const scopeManager = new TsScopeManager(file, program, estree, astMaps, estree.sourceType);
862+
// Inject ECMAScript built-ins + TS lib type globals so `no-undef`
863+
// doesn't fire on `undefined` / `Math` / `Record<K, V>` / etc.
864+
// `TsScopeManager` itself stays free of this lint-pipeline policy
865+
// (upstream eslint-scope parity tests rely on the un-injected
866+
// shape); the names + de-dupe logic live next to `addGlobals` in
867+
// `ts-scope-manager.ts`.
868+
applyEslintGlobals(scopeManager);
869+
const sourceCode = new LazySourceCode({
870+
text: file.text,
871+
ast: estree,
872+
tsFile: file,
873+
scopeManager,
874+
visitorKeys: visitorKeys as Record<string, string[]>,
875+
parserServices: {
876+
...astMaps,
877+
program,
878+
hasFullTypeInformation: true,
879+
emitDecoratorMetadata: undefined,
880+
experimentalDecorators: undefined,
881+
isolatedDeclarations: undefined,
882+
getSymbolAtLocation: (node: any) =>
883+
program.getTypeChecker().getSymbolAtLocation(astMaps.esTreeNodeToTSNodeMap.get(node)),
884+
getTypeAtLocation: (node: any) =>
885+
program.getTypeChecker().getTypeAtLocation(astMaps.esTreeNodeToTSNodeMap.get(node)),
886+
},
887+
}) as unknown as ESLint.SourceCode;
888+
return { sourceCode, convertContext };
887889
}

0 commit comments

Comments
 (0)