Skip to content

Commit f8e8a78

Browse files
committed
fix: restore Reporter.withoutCache() — removing it was breaking
3.1.0 (e05416f) shipped Reporter.withoutCache() as the per-diagnostic opt-out for findings whose correctness depends on inputs the cache doesn't track. Dropping it in 47f5410 silently broke any rule that relied on it — the call site would hit a TypeError at runtime, or fail to type-check if written in TS. Restore the API with semantics that match the original: - Diagnostic is still returned for the current run. - cache-flow filters it out before serialising the rule entry to disk, so the next warm hit on this file won't replay it (rule has to re-run to surface it again). Marker is a Symbol.for('@tsslint/no-cache') stamped on the diagnostic in core. Symbol-keyed → invisible to JSON.stringify and to {...spread}, so it doesn't leak into the on-disk cache. Test 17 covers that explicitly. README's "Caching" section is rewritten to describe the new two-layer model and explain when withoutCache() still makes sense (external inputs / fs-driven side reads); for cross-file types, the recommended path is to read ctx.program once so layer 2 handles invalidation. Tests: - cache-flow.test #15: marked diagnostic returned but not persisted - cache-flow.test #16: warm replay drops the marked one - cache-flow.test #17: marker doesn't leak through serialisation
1 parent b815314 commit f8e8a78

5 files changed

Lines changed: 130 additions & 5 deletions

File tree

README.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,16 @@ defineConfig({
163163

164164
### Caching
165165

166-
Diagnostics are cached on disk under `os.tmpdir()/tsslint-cache/`, keyed by file mtime. The cache is shared across rules and survives between editor sessions.
166+
Diagnostics are cached on disk under `os.tmpdir()/tsslint-cache/` in two layers, picked per rule:
167167

168-
A diagnostic whose correctness depends on more than one file's mtime (e.g. anything that reads `ctx.program` for cross-file resolution and reports on the cached side) should opt out per-diagnostic via `.withoutCache()` on the reporter — the cached entry would otherwise go stale when an unrelated dependency file changes without invalidating its consumers' mtime.
168+
- **Layer 1** — invalidated by the linted file's mtime. Used for rules that don't read `ctx.program` (purely syntactic).
169+
- **Layer 2** — invalidated by TypeScript's `BuilderProgram` affected-file diff (transitive, includes ambient `.d.ts`). Used for rules that touch `ctx.program`. The first time a rule reads `ctx.program` it's classified type-aware and stays type-aware across sessions.
169170

170-
Pass `--force` to the CLI to ignore the cache.
171+
A diagnostic whose correctness depends on inputs neither layer tracks — external resources, env vars, sibling files the rule reads directly via `fs` — should opt out per-diagnostic via `.withoutCache()` on the reporter. The diagnostic still surfaces on the current run; it just isn't written to disk, so the next warm hit on this file won't replay it (the rule has to re-run to surface it again).
172+
173+
For diagnostics that depend on cross-file types, prefer reading `ctx.program` once instead — that re-classifies the rule type-aware and layer 2 handles invalidation properly.
174+
175+
Pass `--force` to the CLI to ignore the cache. `--list-rules` prints each rule's classification (type-aware vs syntactic) after the run.
171176

172177
### Debugging
173178

packages/cli/lib/cache-flow.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// type-aware cleanup.
1010

1111
import type * as ts from 'typescript';
12-
import type { Linter } from '@tsslint/core';
12+
import { NO_CACHE, type Linter } from '@tsslint/core';
1313
import type { FileCache, SerializedDiagnostic } from './cache.js';
1414

1515
export function lintWithCache(
@@ -97,9 +97,16 @@ export function lintWithCache(
9797
break;
9898
}
9999
}
100+
// Drop diagnostics the rule marked via `Reporter.withoutCache()` —
101+
// the rule is asserting they depend on inputs we don't track in
102+
// the cache key, so a warm replay would be unsound. They're still
103+
// included in the live `fresh` array returned from this function;
104+
// they just don't survive to disk.
100105
fileCache.rules[ruleId] = {
101106
hasFix,
102-
diagnostics: diags.map(serializeDiagnostic),
107+
diagnostics: diags
108+
.filter(d => !(d as any)[NO_CACHE])
109+
.map(serializeDiagnostic),
103110
};
104111
}
105112

packages/cli/test/cache-flow.test.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,98 @@ function emptyFileCache(mtime = 0): FileCache {
427427
);
428428
}
429429

430+
// ── Test 15: withoutCache() — diagnostic returned but not persisted ─────
431+
//
432+
// `Reporter.withoutCache()` is the rule's contract: "this finding's
433+
// correctness depends on inputs neither layer tracks; please don't
434+
// replay it from the cache." The current run still surfaces the
435+
// diagnostic; it just isn't written to disk, so the next warm hit on
436+
// the same file won't see it.
437+
{
438+
const ctx = makeContext({ '/a.ts': 'const x = 1;' });
439+
const config: Config = {
440+
rules: {
441+
env_dependent: ((rctx: RuleContext) => {
442+
rctx.report('depends on env', 0, 1).withoutCache();
443+
rctx.report('plain', 0, 1);
444+
}),
445+
},
446+
};
447+
const linter = core.createLinter(ctx, '/', config, () => []);
448+
const cache = emptyFileCache(1);
449+
const diags = cacheFlow.lintWithCache(
450+
linter,
451+
'/a.ts',
452+
cache,
453+
1,
454+
ctx.languageService.getProgram()!,
455+
);
456+
check('current run returns both diagnostics', diags.length === 2);
457+
check(
458+
'cache entry exists for the rule',
459+
!!cache.rules['env_dependent'],
460+
);
461+
check(
462+
'only the non-marked diagnostic is persisted',
463+
cache.rules['env_dependent']?.diagnostics.length === 1,
464+
`expected 1 cached diagnostic, got ${cache.rules['env_dependent']?.diagnostics.length}`,
465+
);
466+
check(
467+
'persisted diagnostic is the plain one',
468+
cache.rules['env_dependent']?.diagnostics[0]?.messageText === 'plain',
469+
);
470+
}
471+
472+
// ── Test 16: withoutCache() — second run cache-hits with the survivor ───
473+
{
474+
const ctx = makeContext({ '/a.ts': 'const x = 1;' });
475+
const config: Config = {
476+
rules: {
477+
env_dependent: ((rctx: RuleContext) => {
478+
rctx.report('depends on env', 0, 1).withoutCache();
479+
rctx.report('plain', 0, 1);
480+
}),
481+
},
482+
};
483+
const linter = core.createLinter(ctx, '/', config, () => []);
484+
const cache = emptyFileCache(1);
485+
cacheFlow.lintWithCache(linter, '/a.ts', cache, 1, ctx.languageService.getProgram()!);
486+
487+
// Second run with the same mtime — rule cache-hits, only the
488+
// persisted diagnostic comes back.
489+
const linter2 = core.createLinter(ctx, '/', config, () => []);
490+
const diags = cacheFlow.lintWithCache(
491+
linter2,
492+
'/a.ts',
493+
cache,
494+
1,
495+
ctx.languageService.getProgram()!,
496+
);
497+
check('warm cache hit returns 1 diagnostic', diags.length === 1);
498+
check('warm replay drops the marked one', diags[0]?.messageText === 'plain');
499+
}
500+
501+
// ── Test 17: NO_CACHE marker doesn't leak through serialisation ─────────
502+
//
503+
// Symbol-keyed property must stay invisible to JSON.stringify and to
504+
// `{...spread}` so the on-disk cache stays clean.
505+
{
506+
const ctx = makeContext({ '/a.ts': 'const x = 1;' });
507+
const config: Config = {
508+
rules: {
509+
r: ((rctx: RuleContext) => {
510+
rctx.report('marked', 0, 1).withoutCache();
511+
}),
512+
},
513+
};
514+
const linter = core.createLinter(ctx, '/', config, () => []);
515+
const cache = emptyFileCache(1);
516+
cacheFlow.lintWithCache(linter, '/a.ts', cache, 1, ctx.languageService.getProgram()!);
517+
// rule entry exists but with 0 diagnostics — no smuggled keys.
518+
const json = JSON.stringify(cache);
519+
check('NO_CACHE marker not visible in serialised cache', !json.includes('no-cache'));
520+
}
521+
430522
// ── Done ────────────────────────────────────────────────────────────────
431523
process.stdout.write('\n');
432524
if (failures.length) {

packages/core/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import minimatch = require('minimatch');
66

77
export type Linter = ReturnType<typeof createLinter>;
88

9+
// Marker stamped onto diagnostics by `Reporter.withoutCache()`. The CLI
10+
// cache-flow filters these out before serialising to disk so they're
11+
// never replayed from a warm cache hit. Symbol-keyed (via Symbol.for so
12+
// it's stable across module instances) — invisible to JSON.stringify and
13+
// to `{...spread}` so it doesn't leak into the serialised cache.
14+
export const NO_CACHE = Symbol.for('@tsslint/no-cache');
15+
916
export function createLinter(
1017
ctx: LinterContext,
1118
rootDir: string,
@@ -199,6 +206,10 @@ export function createLinter(
199206
});
200207
return this;
201208
},
209+
withoutCache() {
210+
(error as any)[NO_CACHE] = true;
211+
return this;
212+
},
202213
};
203214
}
204215
},

packages/types/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,14 @@ export interface Reporter {
5656
withUnnecessary(): Reporter;
5757
withFix(title: string, getChanges: () => FileTextChanges[]): Reporter;
5858
withRefactor(title: string, getChanges: () => FileTextChanges[]): Reporter;
59+
// Mark this diagnostic as ineligible for the CLI's per-file cache.
60+
// The diagnostic is still returned for the current run, but won't be
61+
// written to disk — so the next warm run (cache hit on this file)
62+
// won't replay it, and the rule must re-run to surface it again.
63+
// Use when a diagnostic's correctness depends on inputs the layer-1
64+
// mtime check doesn't track (external resources, env, sibling files
65+
// the rule reads directly via fs). For type-checker-derived findings
66+
// just read `ctx.program` once — that re-classifies the rule
67+
// type-aware, and layer 2 handles cross-file invalidation properly.
68+
withoutCache(): Reporter;
5969
}

0 commit comments

Comments
 (0)