Skip to content

Commit 2dec2df

Browse files
committed
fix(lint): report an unparseable source instead of scoring it clean (#10653)
Four validators in packages/lint parsed authored source with `ts.createSourceFile` and never read `parseDiagnostics`. The call cannot throw, so a source with syntax errors came back as a recovery tree, was walked like any other, and produced no findings. Two of the sites wrapped the parse in a `try/catch` that never ran; deleting only those would have closed nothing. Adds a lint-local checked parse (`checked-parse.ts`) that reads the diagnostics and RETURNS them, and wires four advisory rule ids onto it. Additive: every finding these rules produce today they still produce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
1 parent 55809a0 commit 2dec2df

12 files changed

Lines changed: 894 additions & 25 deletions
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
Report an unparseable source instead of scoring it CLEAN (#10653).
6+
7+
Four validators parsed authored source with `ts.createSourceFile` and never read
8+
`parseDiagnostics`. That call **cannot throw**, so a source with syntax errors
9+
came back as a tree built by error recovery, got walked like any other, and
10+
produced no findings — a source the validator could not read, reported as a
11+
source with nothing to report. Two of the sites carried a `try/catch` around the
12+
parse that never once ran.
13+
14+
Each now reports what it could not read, as a finding the author receives rather
15+
than as an exit — a publish-time validator is handed metadata by someone else,
16+
so ending the process on their input is not its call. Four new advisory
17+
(`warning`) rule ids, all additive: every finding these rules produce today they
18+
still produce, including from a partially recovered tree.
19+
20+
- `react-page-source-unparseable` — `kind:'react'` page source
21+
(`validateReactPageProps`)
22+
- `startup-source-unparseable` — plugin source (`findStartupRegistryVerdicts`)
23+
- `hook-body-source-unparseable` — L2 hook body (`validateHookBodyWrites`)
24+
- `action-body-source-unparseable` — L2 action body (`validateActionBodyWrites`)
25+
26+
New exports: the four rule-id constants, plus `describeParseFailure`,
27+
`PARSE_FAILURE_HINT` and the `SourceParseFailure` / `CheckedParse` /
28+
`CheckedParseOptions` types. `ExtractedHookBodyWriteSet` gains an optional
29+
`parseFailure`, so a consumer of the extractor can tell "wrote nothing" from
30+
"could not be read" — the distinction that was missing.
31+
32+
Nothing is removed or renamed, and no source that parses gains a finding. A
33+
stack whose authored sources all parse lints exactly as before; one carrying a
34+
source with a syntax error gains a warning that names the file, line and column
35+
instead of silently skipping the checks.
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #10653 — the premise, pinned. Every claim in `checked-parse.ts`'s header is a
4+
// measurement, and a measurement that lives only in a comment is one nobody
5+
// re-runs. The two that matter:
6+
//
7+
// 1. `ts.createSourceFile` does not throw on a wreck. That is what makes the
8+
// `try/catch` this module replaced dead code, and what makes the unread
9+
// `parseDiagnostics` the LIVE path. If a future TypeScript ever starts
10+
// throwing, `cannot throw` below fails and the callers' catch-free parses
11+
// need revisiting — loudly, rather than by a caller crashing in the field.
12+
// 2. The diagnostics are REACHED. `parseDiagnostics` is internal to the
13+
// compiler and typed optional here, so a rename would make every checked
14+
// parse silently return "no failure" — the exact green-line-that-lies this
15+
// module exists to remove, wearing its own badge. `the property is really
16+
// there` is the non-vacuity proof (#4690: a check that has only ever been
17+
// green must be tellable apart from a dead one).
18+
//
19+
// The test imports `typescript` directly, which the RULES may not do — they take
20+
// the compiler as a parameter and load it lazily (`lazy-deps.test.ts` guards
21+
// that). A test file is not on the kernel boot path.
22+
import ts from 'typescript';
23+
import { describe, it, expect } from 'vitest';
24+
25+
import { createSourceFileChecked, describeParseFailure } from './checked-parse.js';
26+
27+
/** Wrecks, and the ScriptKind each is measured under. */
28+
const WRECKS: ReadonlyArray<{ label: string; source: string; kind: ts.ScriptKind }> = [
29+
{
30+
label: 'merge-conflict markers',
31+
source: '<<<<<<< HEAD\nconst a = 1;\n=======\nconst a = 2;\n>>>>>>> other\n',
32+
kind: ts.ScriptKind.TS,
33+
},
34+
{ label: 'truncated function body', source: 'export function f() {\n const x = 1;\n', kind: ts.ScriptKind.TS },
35+
{ label: 'unterminated string', source: "const s = 'oops;\n", kind: ts.ScriptKind.TS },
36+
{ label: 'unterminated block comment', source: 'function f() {\n /* TODO\n}\n', kind: ts.ScriptKind.TS },
37+
{ label: 'unterminated template literal', source: 'const s = `oops;\n', kind: ts.ScriptKind.TS },
38+
{ label: 'a JSX element read as TS', source: 'const a = <div>hi</div>;\n', kind: ts.ScriptKind.TS },
39+
];
40+
41+
const parse = (source: string, kind: ts.ScriptKind, synthesizedLinesBefore?: number) =>
42+
createSourceFileChecked(ts, 'probe.tsx', source, {
43+
target: ts.ScriptTarget.Latest,
44+
setParentNodes: true,
45+
scriptKind: kind,
46+
...(synthesizedLinesBefore === undefined ? {} : { synthesizedLinesBefore }),
47+
});
48+
49+
describe('createSourceFileChecked — the premise (#10653)', () => {
50+
it.each(WRECKS)('cannot throw: $label', ({ source, kind }) => {
51+
// The claim is about `createSourceFile` itself, so it is called RAW here —
52+
// going through the helper would prove only that the helper does not throw.
53+
expect(() => ts.createSourceFile('probe.tsx', source, ts.ScriptTarget.Latest, true, kind)).not.toThrow();
54+
});
55+
56+
it.each(WRECKS)('reports a failure instead of a clean tree: $label', ({ source, kind }) => {
57+
const { failure } = parse(source, kind);
58+
expect(failure, 'a source that does not parse must come back with a failure').toBeDefined();
59+
expect(failure!.count).toBeGreaterThan(0);
60+
expect(failure!.message.length).toBeGreaterThan(0);
61+
expect(failure!.line).toBeGreaterThanOrEqual(1);
62+
expect(failure!.column).toBeGreaterThanOrEqual(1);
63+
});
64+
65+
it('the property is really there — non-vacuity of the whole module (#4690)', () => {
66+
// If `parseDiagnostics` is ever renamed or dropped, the optional read in
67+
// checked-parse.ts yields undefined and EVERY checked parse in this package
68+
// goes quietly back to scoring wreckage clean. Nothing else would fail. So
69+
// the property is asserted directly, on the compiler, once.
70+
const sf = ts.createSourceFile('probe.ts', 'const a = ;\n', ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
71+
const diagnostics = (sf as unknown as { parseDiagnostics?: readonly ts.Diagnostic[] }).parseDiagnostics;
72+
expect(Array.isArray(diagnostics), '`SourceFile.parseDiagnostics` is gone — checked-parse.ts is now inert').toBe(
73+
true,
74+
);
75+
expect(diagnostics!.length).toBeGreaterThan(0);
76+
});
77+
});
78+
79+
describe('createSourceFileChecked — a source that DOES parse', () => {
80+
it.each([
81+
['plain TS', 'export const a: number = 1;\n', ts.ScriptKind.TS],
82+
['TSX with JSX', 'export default function P() {\n return <ObjectForm objectName="a" />;\n}\n', ts.ScriptKind.TSX],
83+
['empty', '', ts.ScriptKind.TS],
84+
] as const)('reports no failure: %s', (_label, source, kind) => {
85+
expect(parse(source, kind).failure).toBeUndefined();
86+
});
87+
88+
it('the SAME source under the wrong ScriptKind is a failure, not an opinion', () => {
89+
// The ScriptKind hat: `<div>…</div>` is a JSX element in TSX and a wreck in
90+
// TS. Both verdicts are correct for their kind, and neither is silent.
91+
const jsx = 'const a = <div>hi</div>;\n';
92+
expect(parse(jsx, ts.ScriptKind.TSX).failure).toBeUndefined();
93+
expect(parse(jsx, ts.ScriptKind.TS).failure).toBeDefined();
94+
});
95+
});
96+
97+
describe('createSourceFileChecked — position reporting', () => {
98+
it('reports the line the error is on', () => {
99+
const { failure } = parse('const a = 1;\nconst b = 2;\nconst c = ;\n', ts.ScriptKind.TS);
100+
expect(failure!.line).toBe(3);
101+
});
102+
103+
it('subtracts the caller-synthesised lines, so a wrapper never shifts the blame', () => {
104+
// What validate-hook-body-writes.ts does: the author wrote `const c = ;` on
105+
// line 2 of their BODY, and the wrapper puts it on line 3 of what is parsed.
106+
const body = 'const a = 1;\nconst c = ;\n';
107+
const wrapped = `async function __body(ctx) {\n${body}\n}`;
108+
expect(parse(wrapped, ts.ScriptKind.TS).failure!.line, 'raw: the wrapper line counts').toBe(3);
109+
expect(parse(wrapped, ts.ScriptKind.TS, 1).failure!.line, "remapped: the author's own line").toBe(2);
110+
});
111+
112+
it('clamps to line 1 — a diagnostic on a synthesised line never points above the source', () => {
113+
// A diagnostic that lands ON the wrapper's own first line would remap to 0,
114+
// which is not a line anybody wrote.
115+
const failure = parse('const a = ;\n', ts.ScriptKind.TS, 5).failure!;
116+
expect(failure.line).toBe(1);
117+
});
118+
119+
it('counts every diagnostic, and names the first', () => {
120+
const failure = parse('<<<<<<< HEAD\nconst a = 1;\n=======\nconst a = 2;\n>>>>>>> other\n', ts.ScriptKind.TS)
121+
.failure!;
122+
expect(failure.count).toBeGreaterThan(1);
123+
expect(describeParseFailure(failure)).toContain(`${failure.count} syntax errors in total`);
124+
expect(describeParseFailure(failure)).toContain(failure.message);
125+
});
126+
127+
it('a single diagnostic is described without a count', () => {
128+
const failure = parse('const a = ;\n', ts.ScriptKind.TS).failure!;
129+
expect(failure.count).toBe(1);
130+
expect(describeParseFailure(failure)).not.toContain('in total');
131+
});
132+
});

‎packages/lint/src/checked-parse.ts‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// `@objectstack/lint`'s ONE answer to "did this source actually parse?" (#10653).
4+
//
5+
// ## The defect this closes — and it is NOT the `try/catch` it replaced
6+
//
7+
// **`ts.createSourceFile` never throws.** Hand it merge-conflict markers, an
8+
// unterminated comment or a truncated body and it returns a `SourceFile` built
9+
// by error recovery, indistinguishable from any other: the errors are parked on
10+
// `parseDiagnostics`, which nothing in this package read. A validator then walks
11+
// that wreckage, finds none of the shapes it is looking for, and returns an
12+
// empty finding list — **a source the validator could not read, reported as a
13+
// source with nothing to report.**
14+
//
15+
// The three call sites this module was extracted from each carried a
16+
// `try/catch` around the parse, and two of them read as if the catch were the
17+
// guard. It never ran. Measured here on 2026-08-21 against TypeScript 6.0.3 —
18+
// every one of these returns a tree, throws nothing, and is walked as if clean:
19+
//
20+
// source ScriptKind.TS ScriptKind.TSX threw?
21+
// --------------------------- ------------- -------------- ------
22+
// merge-conflict markers 3 diagnostics 3 diagnostics no
23+
// truncated function body 1 diagnostic 1 diagnostic no
24+
// unterminated string 1 diagnostic 1 diagnostic no
25+
// unterminated block comment 1 diagnostic 1 diagnostic no
26+
// a JSX element 1 diagnostic 0 no
27+
//
28+
// The last row is the same defect wearing the `ScriptKind` hat, and it is why
29+
// the answer lives in one module rather than three: the question "did this
30+
// parse?" has one right answer and three places that were getting it wrong in
31+
// slightly different ways. `checked-parse.test.ts` pins the measurement so the
32+
// premise above cannot rot into a comment nobody re-ran — including the row
33+
// that matters most, that the diagnostics are REACHED (see the non-vacuity
34+
// case there, and `parseDiagnostics` below).
35+
//
36+
// ## Why a returned FINDING rather than a refusal
37+
//
38+
// `scripts/ts-parse.mjs` answers the same question for repo tooling and answers
39+
// it by REFUSING — correct there, because a `scripts/**` gate audits a tree its
40+
// own author controls. A publish-time validator is in the opposite position: it
41+
// is handed metadata by someone else, and ending the process on their input is
42+
// not its call. So this module reports, and every caller turns the report into
43+
// a finding the author receives at authoring time. Nothing here throws, and
44+
// nothing here decides severity — that belongs to the rule.
45+
//
46+
// ## Why lint-local, and not the two neighbours it could have been
47+
//
48+
// • NOT `scripts/ts-parse.mjs`: `@objectstack/lint` is PUBLISHED and packs
49+
// `dist, README.md, CHANGELOG.md`. A published package that asks repo
50+
// tooling whether something parsed trades this bug for a worse one — the
51+
// objection `invoked-as.mjs` already argued for its own `packages/cli`
52+
// sibling, and the reason #10606 existed at all.
53+
// • NOT a new shared `@objectstack/…` package: a published dependency built
54+
// for four call sites, argued down in the same place.
55+
//
56+
// The TypeScript module is a PARAMETER, never an import: `@objectstack/lint`
57+
// sits on the kernel boot path and `typescript` is ~9 MB of CJS, so every
58+
// caller here loads it lazily and only when a source actually needs parsing.
59+
// This module must therefore stay type-only in its imports (`lazy-deps.test.ts`
60+
// is the guard).
61+
import type ts from 'typescript';
62+
63+
/**
64+
* `parseDiagnostics` is where the parser parks syntax errors, and it is not on
65+
* the public `SourceFile` type — the property is internal to the compiler and
66+
* reached here by a narrowing cast rather than by `as any`.
67+
*
68+
* Declaring it OPTIONAL is deliberate: if a future TypeScript renames or drops
69+
* it, this reads `undefined` and every checked parse silently returns to the
70+
* behaviour this module exists to remove — a green line about a source nobody
71+
* could read. That failure mode is invisible by construction, so it is pinned
72+
* in `checked-parse.test.ts` instead of being trusted here.
73+
*/
74+
interface SourceFileWithParseDiagnostics {
75+
readonly parseDiagnostics?: readonly ts.Diagnostic[];
76+
}
77+
78+
/** What a caller needs to tell an author WHICH source went unread, and where. */
79+
export interface SourceParseFailure {
80+
/** The first parse diagnostic, in the compiler's own wording, flattened to one line. */
81+
message: string;
82+
/** 1-based line, in the AUTHORED source's coordinates (see `synthesizedLinesBefore`). */
83+
line: number;
84+
/** 1-based column. */
85+
column: number;
86+
/** How many parse diagnostics in total — `message` is the first of `count`. */
87+
count: number;
88+
}
89+
90+
/** A parse plus the verdict on whether it succeeded. `failure` absent ⇒ it parsed. */
91+
export interface CheckedParse {
92+
/**
93+
* The tree, ALWAYS returned — including when `failure` is set. Error recovery
94+
* produces a partial tree, and a caller that already reports findings from it
95+
* keeps doing so: the fix here is the missing SIGNAL, not the removal of
96+
* whatever the recovered tree could still be read for.
97+
*/
98+
sourceFile: ts.SourceFile;
99+
/** Set when the parser reported at least one syntax diagnostic. */
100+
failure?: SourceParseFailure;
101+
}
102+
103+
export interface CheckedParseOptions {
104+
target: ts.ScriptTarget;
105+
setParentNodes: boolean;
106+
scriptKind: ts.ScriptKind;
107+
/**
108+
* Lines the CALLER synthesised ahead of the authored source, subtracted from
109+
* the reported position so it lands in the author's coordinates.
110+
*
111+
* `validate-hook-body-writes.ts` parses an L2 hook body wrapped in
112+
* `async function __body(ctx) {\n…\n}` — the shape the runtime compiles it
113+
* into — so its diagnostics are one line low. The reported line is clamped to
114+
* at least 1, so a diagnostic that lands on a synthesised line is attributed
115+
* to the nearest AUTHORED line and never to a line the author did not write.
116+
*/
117+
synthesizedLinesBefore?: number;
118+
}
119+
120+
/**
121+
* `ts.createSourceFile`, with the diagnostics READ.
122+
*
123+
* Never throws and never exits: the verdict comes back as data.
124+
*/
125+
export function createSourceFileChecked(
126+
tsc: typeof ts,
127+
fileName: string,
128+
source: string,
129+
options: CheckedParseOptions,
130+
): CheckedParse {
131+
const sourceFile = tsc.createSourceFile(
132+
fileName,
133+
source,
134+
options.target,
135+
options.setParentNodes,
136+
options.scriptKind,
137+
);
138+
const diagnostics = (sourceFile as unknown as SourceFileWithParseDiagnostics).parseDiagnostics;
139+
if (!diagnostics || diagnostics.length === 0) return { sourceFile };
140+
141+
const first = diagnostics[0]!;
142+
const { line, character } = sourceFile.getLineAndCharacterOfPosition(first.start ?? 0);
143+
const offset = options.synthesizedLinesBefore ?? 0;
144+
return {
145+
sourceFile,
146+
failure: {
147+
message: tsc.flattenDiagnosticMessageText(first.messageText, ' '),
148+
line: Math.max(1, line + 1 - offset),
149+
column: character + 1,
150+
count: diagnostics.length,
151+
},
152+
};
153+
}
154+
155+
/**
156+
* The one wording every caller's message embeds, so three findings about the
157+
* same defect do not describe it three ways.
158+
*/
159+
export function describeParseFailure(failure: SourceParseFailure): string {
160+
const more = failure.count > 1 ? `; ${failure.count} syntax errors in total` : '';
161+
return `line ${failure.line}, column ${failure.column}: ${failure.message}${more}`;
162+
}
163+
164+
/**
165+
* The hint every caller's finding carries. It says what the finding IS — a
166+
* statement about what the checker could read, not a second syntax verdict —
167+
* because a source that does not parse is not scored, and an author who reads
168+
* "no problems found" about it would be reading a green line that lied.
169+
*/
170+
export const PARSE_FAILURE_HINT =
171+
'The source is parsed (never executed) so it can be checked. A source with syntax errors is only ' +
172+
'partially recovered, so the checks that follow may have skipped real problems — fix the syntax ' +
173+
'error and re-run to get a verdict that covers the whole source.';

‎packages/lint/src/index.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export {
5858
STARTUP_VERDICT_HINT,
5959
STARTUP_OPEN_VOCABULARY_VERDICT,
6060
STARTUP_VERDICT_ASSERTIVE_WORDING,
61+
STARTUP_SOURCE_UNPARSEABLE,
6162
} from './lint-startup-registry-verdict.js';
6263
export type {
6364
StartupRegistryVerdictFinding,
@@ -147,8 +148,16 @@ export {
147148
REACT_CHART_AXIS_UNKNOWN,
148149
REACT_CHART_DRILLDOWN_INVALID,
149150
REACT_BLOCK_NEEDS_RECORD_CONTEXT,
151+
REACT_PAGE_SOURCE_UNPARSEABLE,
150152
} from './validate-react-page-props.js';
151153
export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js';
154+
155+
// [#10653] The package-local checked parse behind the three `*-source-unparseable`
156+
// rules above and below. Exported because a consumer that receives one of those
157+
// findings may want the failure's shape (line/column/count) rather than
158+
// re-parsing the message string — the same reason the rule ids are exported.
159+
export { describeParseFailure, PARSE_FAILURE_HINT } from './checked-parse.js';
160+
export type { SourceParseFailure, CheckedParse, CheckedParseOptions } from './checked-parse.js';
152161
export { validatePageSourceStyling, PAGE_SOURCE_CLASSNAME } from './validate-page-source-styling.js';
153162
export type { SourceStyleFinding, SourceStyleSeverity } from './validate-page-source-styling.js';
154163

@@ -506,6 +515,7 @@ export {
506515
HOOK_BODY_WRITE_EXCLUSIONS,
507516
HOOK_BODY_WRITE_UNKNOWN_FIELD,
508517
HOOK_BODY_WRITE_UNPROVISIONED_ANCHOR,
518+
HOOK_BODY_SOURCE_UNPARSEABLE,
509519
} from './validate-hook-body-writes.js';
510520
export type {
511521
HookBodyWriteFinding,
@@ -530,6 +540,7 @@ export {
530540
ACTION_BODY_WRITE_UNKNOWN_FIELD,
531541
ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR,
532542
ACTION_RECORD_WRITE_DISCARDED,
543+
ACTION_BODY_SOURCE_UNPARSEABLE,
533544
} from './validate-action-body-writes.js';
534545
export type {
535546
ActionBodyWriteFinding,

0 commit comments

Comments
 (0)