Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 35 additions & 0 deletions .changeset/lint-checked-parse-findings.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/lint": minor
---

Report an unparseable source instead of scoring it CLEAN (#10653).

Four validators parsed authored source with `ts.createSourceFile` and never read
`parseDiagnostics`. That call **cannot throw**, so a source with syntax errors
came back as a tree built by error recovery, got walked like any other, and
produced no findings — a source the validator could not read, reported as a
source with nothing to report. Two of the sites carried a `try/catch` around the
parse that never once ran.

Each now reports what it could not read, as a finding the author receives rather
than as an exit — a publish-time validator is handed metadata by someone else,
so ending the process on their input is not its call. Four new advisory
(`warning`) rule ids, all additive: every finding these rules produce today they
still produce, including from a partially recovered tree.

- `react-page-source-unparseable` — `kind:'react'` page source
(`validateReactPageProps`)
- `startup-source-unparseable` — plugin source (`findStartupRegistryVerdicts`)
- `hook-body-source-unparseable` — L2 hook body (`validateHookBodyWrites`)
- `action-body-source-unparseable` — L2 action body (`validateActionBodyWrites`)

New exports: the four rule-id constants, plus `describeParseFailure`,
`PARSE_FAILURE_HINT` and the `SourceParseFailure` / `CheckedParse` /
`CheckedParseOptions` types. `ExtractedHookBodyWriteSet` gains an optional
`parseFailure`, so a consumer of the extractor can tell "wrote nothing" from
"could not be read" — the distinction that was missing.

Nothing is removed or renamed, and no source that parses gains a finding. A
stack whose authored sources all parse lints exactly as before; one carrying a
source with a syntax error gains a warning that names the file, line and column
instead of silently skipping the checks.
132 changes: 132 additions & 0 deletions packages/lint/src/checked-parse.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// #10653 — the premise, pinned. Every claim in `checked-parse.ts`'s header is a
// measurement, and a measurement that lives only in a comment is one nobody
// re-runs. The two that matter:
//
// 1. `ts.createSourceFile` does not throw on a wreck. That is what makes the
// `try/catch` this module replaced dead code, and what makes the unread
// `parseDiagnostics` the LIVE path. If a future TypeScript ever starts
// throwing, `cannot throw` below fails and the callers' catch-free parses
// need revisiting — loudly, rather than by a caller crashing in the field.
// 2. The diagnostics are REACHED. `parseDiagnostics` is internal to the
// compiler and typed optional here, so a rename would make every checked
// parse silently return "no failure" — the exact green-line-that-lies this
// module exists to remove, wearing its own badge. `the property is really
// there` is the non-vacuity proof (#4690: a check that has only ever been
// green must be tellable apart from a dead one).
//
// The test imports `typescript` directly, which the RULES may not do — they take
// the compiler as a parameter and load it lazily (`lazy-deps.test.ts` guards
// that). A test file is not on the kernel boot path.
import ts from 'typescript';
import { describe, it, expect } from 'vitest';

import { createSourceFileChecked, describeParseFailure } from './checked-parse.js';

/** Wrecks, and the ScriptKind each is measured under. */
const WRECKS: ReadonlyArray<{ label: string; source: string; kind: ts.ScriptKind }> = [
{
label: 'merge-conflict markers',
source: '<<<<<<< HEAD\nconst a = 1;\n=======\nconst a = 2;\n>>>>>>> other\n',
kind: ts.ScriptKind.TS,
},
{ label: 'truncated function body', source: 'export function f() {\n const x = 1;\n', kind: ts.ScriptKind.TS },
{ label: 'unterminated string', source: "const s = 'oops;\n", kind: ts.ScriptKind.TS },
{ label: 'unterminated block comment', source: 'function f() {\n /* TODO\n}\n', kind: ts.ScriptKind.TS },
{ label: 'unterminated template literal', source: 'const s = `oops;\n', kind: ts.ScriptKind.TS },
{ label: 'a JSX element read as TS', source: 'const a = <div>hi</div>;\n', kind: ts.ScriptKind.TS },
];

const parse = (source: string, kind: ts.ScriptKind, synthesizedLinesBefore?: number) =>
createSourceFileChecked(ts, 'probe.tsx', source, {
target: ts.ScriptTarget.Latest,
setParentNodes: true,
scriptKind: kind,
...(synthesizedLinesBefore === undefined ? {} : { synthesizedLinesBefore }),
});

describe('createSourceFileChecked — the premise (#10653)', () => {
it.each(WRECKS)('cannot throw: $label', ({ source, kind }) => {
// The claim is about `createSourceFile` itself, so it is called RAW here —
// going through the helper would prove only that the helper does not throw.
expect(() => ts.createSourceFile('probe.tsx', source, ts.ScriptTarget.Latest, true, kind)).not.toThrow();
});

it.each(WRECKS)('reports a failure instead of a clean tree: $label', ({ source, kind }) => {
const { failure } = parse(source, kind);
expect(failure, 'a source that does not parse must come back with a failure').toBeDefined();
expect(failure!.count).toBeGreaterThan(0);
expect(failure!.message.length).toBeGreaterThan(0);
expect(failure!.line).toBeGreaterThanOrEqual(1);
expect(failure!.column).toBeGreaterThanOrEqual(1);
});

it('the property is really there — non-vacuity of the whole module (#4690)', () => {
// If `parseDiagnostics` is ever renamed or dropped, the optional read in
// checked-parse.ts yields undefined and EVERY checked parse in this package
// goes quietly back to scoring wreckage clean. Nothing else would fail. So
// the property is asserted directly, on the compiler, once.
const sf = ts.createSourceFile('probe.ts', 'const a = ;\n', ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
const diagnostics = (sf as unknown as { parseDiagnostics?: readonly ts.Diagnostic[] }).parseDiagnostics;
expect(Array.isArray(diagnostics), '`SourceFile.parseDiagnostics` is gone — checked-parse.ts is now inert').toBe(
true,
);
expect(diagnostics!.length).toBeGreaterThan(0);
});
});

describe('createSourceFileChecked — a source that DOES parse', () => {
it.each([
['plain TS', 'export const a: number = 1;\n', ts.ScriptKind.TS],
['TSX with JSX', 'export default function P() {\n return <ObjectForm objectName="a" />;\n}\n', ts.ScriptKind.TSX],
['empty', '', ts.ScriptKind.TS],
] as const)('reports no failure: %s', (_label, source, kind) => {
expect(parse(source, kind).failure).toBeUndefined();
});

it('the SAME source under the wrong ScriptKind is a failure, not an opinion', () => {
// The ScriptKind hat: `<div>…</div>` is a JSX element in TSX and a wreck in
// TS. Both verdicts are correct for their kind, and neither is silent.
const jsx = 'const a = <div>hi</div>;\n';
expect(parse(jsx, ts.ScriptKind.TSX).failure).toBeUndefined();
expect(parse(jsx, ts.ScriptKind.TS).failure).toBeDefined();
});
});

describe('createSourceFileChecked — position reporting', () => {
it('reports the line the error is on', () => {
const { failure } = parse('const a = 1;\nconst b = 2;\nconst c = ;\n', ts.ScriptKind.TS);
expect(failure!.line).toBe(3);
});

it('subtracts the caller-synthesised lines, so a wrapper never shifts the blame', () => {
// What validate-hook-body-writes.ts does: the author wrote `const c = ;` on
// line 2 of their BODY, and the wrapper puts it on line 3 of what is parsed.
const body = 'const a = 1;\nconst c = ;\n';
const wrapped = `async function __body(ctx) {\n${body}\n}`;
expect(parse(wrapped, ts.ScriptKind.TS).failure!.line, 'raw: the wrapper line counts').toBe(3);
expect(parse(wrapped, ts.ScriptKind.TS, 1).failure!.line, "remapped: the author's own line").toBe(2);
});

it('clamps to line 1 — a diagnostic on a synthesised line never points above the source', () => {
// A diagnostic that lands ON the wrapper's own first line would remap to 0,
// which is not a line anybody wrote.
const failure = parse('const a = ;\n', ts.ScriptKind.TS, 5).failure!;
expect(failure.line).toBe(1);
});

it('counts every diagnostic, and names the first', () => {
const failure = parse('<<<<<<< HEAD\nconst a = 1;\n=======\nconst a = 2;\n>>>>>>> other\n', ts.ScriptKind.TS)
.failure!;
expect(failure.count).toBeGreaterThan(1);
expect(describeParseFailure(failure)).toContain(`${failure.count} syntax errors in total`);
expect(describeParseFailure(failure)).toContain(failure.message);
});

it('a single diagnostic is described without a count', () => {
const failure = parse('const a = ;\n', ts.ScriptKind.TS).failure!;
expect(failure.count).toBe(1);
expect(describeParseFailure(failure)).not.toContain('in total');
});
});
173 changes: 173 additions & 0 deletions packages/lint/src/checked-parse.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// `@objectstack/lint`'s ONE answer to "did this source actually parse?" (#10653).
//
// ## The defect this closes — and it is NOT the `try/catch` it replaced
//
// **`ts.createSourceFile` never throws.** Hand it merge-conflict markers, an
// unterminated comment or a truncated body and it returns a `SourceFile` built
// by error recovery, indistinguishable from any other: the errors are parked on
// `parseDiagnostics`, which nothing in this package read. A validator then walks
// that wreckage, finds none of the shapes it is looking for, and returns an
// empty finding list — **a source the validator could not read, reported as a
// source with nothing to report.**
//
// The three call sites this module was extracted from each carried a
// `try/catch` around the parse, and two of them read as if the catch were the
// guard. It never ran. Measured here on 2026-08-21 against TypeScript 6.0.3 —
// every one of these returns a tree, throws nothing, and is walked as if clean:
//
// source ScriptKind.TS ScriptKind.TSX threw?
// --------------------------- ------------- -------------- ------
// merge-conflict markers 3 diagnostics 3 diagnostics no
// truncated function body 1 diagnostic 1 diagnostic no
// unterminated string 1 diagnostic 1 diagnostic no
// unterminated block comment 1 diagnostic 1 diagnostic no
// a JSX element 1 diagnostic 0 no
//
// The last row is the same defect wearing the `ScriptKind` hat, and it is why
// the answer lives in one module rather than three: the question "did this
// parse?" has one right answer and three places that were getting it wrong in
// slightly different ways. `checked-parse.test.ts` pins the measurement so the
// premise above cannot rot into a comment nobody re-ran — including the row
// that matters most, that the diagnostics are REACHED (see the non-vacuity
// case there, and `parseDiagnostics` below).
//
// ## Why a returned FINDING rather than a refusal
//
// `scripts/ts-parse.mjs` answers the same question for repo tooling and answers
// it by REFUSING — correct there, because a `scripts/**` gate audits a tree its
// own author controls. A publish-time validator is in the opposite position: it
// is handed metadata by someone else, and ending the process on their input is
// not its call. So this module reports, and every caller turns the report into
// a finding the author receives at authoring time. Nothing here throws, and
// nothing here decides severity — that belongs to the rule.
//
// ## Why lint-local, and not the two neighbours it could have been
//
// • NOT `scripts/ts-parse.mjs`: `@objectstack/lint` is PUBLISHED and packs
// `dist, README.md, CHANGELOG.md`. A published package that asks repo
// tooling whether something parsed trades this bug for a worse one — the
// objection `invoked-as.mjs` already argued for its own `packages/cli`
// sibling, and the reason #10606 existed at all.
// • NOT a new shared `@objectstack/…` package: a published dependency built
// for four call sites, argued down in the same place.
//
// The TypeScript module is a PARAMETER, never an import: `@objectstack/lint`
// sits on the kernel boot path and `typescript` is ~9 MB of CJS, so every
// caller here loads it lazily and only when a source actually needs parsing.
// This module must therefore stay type-only in its imports (`lazy-deps.test.ts`
// is the guard).
import type ts from 'typescript';

/**
* `parseDiagnostics` is where the parser parks syntax errors, and it is not on
* the public `SourceFile` type — the property is internal to the compiler and
* reached here by a narrowing cast rather than by `as any`.
*
* Declaring it OPTIONAL is deliberate: if a future TypeScript renames or drops
* it, this reads `undefined` and every checked parse silently returns to the
* behaviour this module exists to remove — a green line about a source nobody
* could read. That failure mode is invisible by construction, so it is pinned
* in `checked-parse.test.ts` instead of being trusted here.
*/
interface SourceFileWithParseDiagnostics {
readonly parseDiagnostics?: readonly ts.Diagnostic[];
}

/** What a caller needs to tell an author WHICH source went unread, and where. */
export interface SourceParseFailure {
/** The first parse diagnostic, in the compiler's own wording, flattened to one line. */
message: string;
/** 1-based line, in the AUTHORED source's coordinates (see `synthesizedLinesBefore`). */
line: number;
/** 1-based column. */
column: number;
/** How many parse diagnostics in total — `message` is the first of `count`. */
count: number;
}

/** A parse plus the verdict on whether it succeeded. `failure` absent ⇒ it parsed. */
export interface CheckedParse {
/**
* The tree, ALWAYS returned — including when `failure` is set. Error recovery
* produces a partial tree, and a caller that already reports findings from it
* keeps doing so: the fix here is the missing SIGNAL, not the removal of
* whatever the recovered tree could still be read for.
*/
sourceFile: ts.SourceFile;
/** Set when the parser reported at least one syntax diagnostic. */
failure?: SourceParseFailure;
}

export interface CheckedParseOptions {
target: ts.ScriptTarget;
setParentNodes: boolean;
scriptKind: ts.ScriptKind;
/**
* Lines the CALLER synthesised ahead of the authored source, subtracted from
* the reported position so it lands in the author's coordinates.
*
* `validate-hook-body-writes.ts` parses an L2 hook body wrapped in
* `async function __body(ctx) {\n…\n}` — the shape the runtime compiles it
* into — so its diagnostics are one line low. The reported line is clamped to
* at least 1, so a diagnostic that lands on a synthesised line is attributed
* to the nearest AUTHORED line and never to a line the author did not write.
*/
synthesizedLinesBefore?: number;
}

/**
* `ts.createSourceFile`, with the diagnostics READ.
*
* Never throws and never exits: the verdict comes back as data.
*/
export function createSourceFileChecked(
tsc: typeof ts,
fileName: string,
source: string,
options: CheckedParseOptions,
): CheckedParse {
const sourceFile = tsc.createSourceFile(
fileName,
source,
options.target,
options.setParentNodes,
options.scriptKind,
);
const diagnostics = (sourceFile as unknown as SourceFileWithParseDiagnostics).parseDiagnostics;
if (!diagnostics || diagnostics.length === 0) return { sourceFile };

const first = diagnostics[0]!;
const { line, character } = sourceFile.getLineAndCharacterOfPosition(first.start ?? 0);
const offset = options.synthesizedLinesBefore ?? 0;
return {
sourceFile,
failure: {
message: tsc.flattenDiagnosticMessageText(first.messageText, ' '),
line: Math.max(1, line + 1 - offset),
column: character + 1,
count: diagnostics.length,
},
};
}

/**
* The one wording every caller's message embeds, so three findings about the
* same defect do not describe it three ways.
*/
export function describeParseFailure(failure: SourceParseFailure): string {
const more = failure.count > 1 ? `; ${failure.count} syntax errors in total` : '';
return `line ${failure.line}, column ${failure.column}: ${failure.message}${more}`;
}

/**
* The hint every caller's finding carries. It says what the finding IS — a
* statement about what the checker could read, not a second syntax verdict —
* because a source that does not parse is not scored, and an author who reads
* "no problems found" about it would be reading a green line that lied.
*/
export const PARSE_FAILURE_HINT =
'The source is parsed (never executed) so it can be checked. A source with syntax errors is only ' +
'partially recovered, so the checks that follow may have skipped real problems — fix the syntax ' +
'error and re-run to get a verdict that covers the whole source.';
11 changes: 11 additions & 0 deletions packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,6 +58,7 @@ export {
STARTUP_VERDICT_HINT,
STARTUP_OPEN_VOCABULARY_VERDICT,
STARTUP_VERDICT_ASSERTIVE_WORDING,
STARTUP_SOURCE_UNPARSEABLE,
} from './lint-startup-registry-verdict.js';
export type {
StartupRegistryVerdictFinding,
Expand DownExpand Up@@ -147,8 +148,16 @@ export {
REACT_CHART_AXIS_UNKNOWN,
REACT_CHART_DRILLDOWN_INVALID,
REACT_BLOCK_NEEDS_RECORD_CONTEXT,
REACT_PAGE_SOURCE_UNPARSEABLE,
} from './validate-react-page-props.js';
export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js';

// [#10653] The package-local checked parse behind the three `*-source-unparseable`
// rules above and below. Exported because a consumer that receives one of those
// findings may want the failure's shape (line/column/count) rather than
// re-parsing the message string — the same reason the rule ids are exported.
export { describeParseFailure, PARSE_FAILURE_HINT } from './checked-parse.js';
export type { SourceParseFailure, CheckedParse, CheckedParseOptions } from './checked-parse.js';
export { validatePageSourceStyling, PAGE_SOURCE_CLASSNAME } from './validate-page-source-styling.js';
export type { SourceStyleFinding, SourceStyleSeverity } from './validate-page-source-styling.js';

Expand DownExpand Up@@ -506,6 +515,7 @@ export {
HOOK_BODY_WRITE_EXCLUSIONS,
HOOK_BODY_WRITE_UNKNOWN_FIELD,
HOOK_BODY_WRITE_UNPROVISIONED_ANCHOR,
HOOK_BODY_SOURCE_UNPARSEABLE,
} from './validate-hook-body-writes.js';
export type {
HookBodyWriteFinding,
Expand All@@ -530,6 +540,7 @@ export {
ACTION_BODY_WRITE_UNKNOWN_FIELD,
ACTION_BODY_WRITE_UNPROVISIONED_ANCHOR,
ACTION_RECORD_WRITE_DISCARDED,
ACTION_BODY_SOURCE_UNPARSEABLE,
} from './validate-action-body-writes.js';
export type {
ActionBodyWriteFinding,
Expand Down
Loading
Loading