Skip to content

fix(scripts): flip jsdoc-hygiene-ts to strict-zero gate (SD-673) #3517

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

Merged
caio-pizzol merged 1 commit into from
May 26, 2026
Merged

fix(scripts): flip jsdoc-hygiene-ts to strict-zero gate (SD-673) #3517

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
2 changes: 1 addition & 1 deletion packages/superdoc/scripts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ it stopped running.
| `verify-public-facade-emit.cjs` | postbuild | Per-facade expected symbol set + ESM/CJS parity + legacy command-signature compat. Derives the expected name set directly from the facade source file under `packages/superdoc/src/public/**`; rejects `export *` / `export * as X` in facade sources so the contract stays explicit. | Symbol set drift ships silently; CJS shims diverge from ESM; a wildcard re-export silently widens the public surface. |
| `report-declaration-reachability.cjs` | postbuild | Instrumentation (not a gate): per-bucket reachability ratio of emitted declarations. | Loses visibility into unreachable emit (the SD-2952 trim target). |
| `check-jsdoc.cjs` | wrapper stage 3 (`jsdoc-ratchet`) | Two gates: (a) per-file checkJs on the hand-curated `CHECKED_FILES` (currently 6 files; each must carry `// @ts-check` and stay clean against tsc); (b) ratchet over the public-reachable .js JSDoc surface — every file must be in `CHECKED_FILES`, carry `// @ts-check`, be on `jsdoc-allowlist.cjs` with a reason, or be in `jsdoc-debt-snapshot.json` as known pre-existing debt. New public JSDoc files that aren't accounted for fail with a clear "add @ts-check or allowlist" message. Stale snapshot entries (file gone, gained @ts-check, moved out of public surface) also fail. The allowlist contract is enforced too: every entry must carry a non-empty reason, point at an existing file, and still resolve to a public-reachable JSDoc file. Refresh the snapshot with `pnpm --filter superdoc run check:jsdoc -- --write`. Runs as stage 3 of `check:public:superdoc`. | New public-reachable JSDoc files could land without type coverage; existing ones could lose their `// @ts-check` directive without surfacing as a regression; the allowlist could grow silent / typo-shaped exemptions. |
| `check-jsdoc-hygiene-ts.cjs` | wrapper stage 5 (`jsdoc-hygiene-ts`) | Companion to `check-jsdoc.cjs` for the `.ts` side. Walks every `.ts` file under `packages/superdoc/src/` and `packages/super-editor/src/` (excluding `*.d.ts`, `*.test.ts`, `*.spec.ts`, `dev/`, `__mocks__/`, `__fixtures__/`) and flags type-bearing JSDoc tags: `@type`, `@typedef`, `@callback`, `@template`, `@implements`, `@extends`, `@augments`, `@enum` always; `@param`, `@returns`, `@return`, `@this` only when `tag.typeExpression` is set (prose-only forms pass). AST-based via `ts.getJSDocTags`; not regex. Baseline at `jsdoc-hygiene-ts-baseline.json` uses the stable key `file::enclosingSymbol::tagName::class::occurrenceIndex` so line shifts don't churn entries. Self-tested via `check-jsdoc-hygiene-ts-tests.cjs` (13 in-memory fixtures; name avoids the `*.test.*` glob so vitest doesn't pick it up as a unit-test suite). The self-test suite runs as wrapper stage 4 (`jsdoc-hygiene-ts-test`) immediately before this gate. Policy at `type-hygiene.md`. Refresh with `node packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs --write`. | Type-bearing JSDoc in `.ts` files is documentation-only (TS ignores it), so duplicate type information drifts silently — `@param {Element}` while signature said `HTMLElement` is the canonical example. Without this gate, that class of drift ships. |
| `check-jsdoc-hygiene-ts.cjs` | wrapper stage 5 (`jsdoc-hygiene-ts`) | Companion to `check-jsdoc.cjs` for the `.ts` side. Strict-zero gate (no grandfathered baseline, no `--write`): walks every `.ts` file under `packages/superdoc/src/` and `packages/super-editor/src/` (excluding `*.d.ts`, `*.test.ts`, `*.spec.ts`, `dev/`, `__mocks__/`, `__fixtures__/`) and fails on any type-bearing JSDoc tag — `@type`, `@typedef`, `@callback`, `@template`, `@implements`, `@extends`, `@augments`, `@enum` always; `@param`, `@returns`, `@return`, `@this` only when `tag.typeExpression` is set (prose-only forms pass). AST-based via `ts.getJSDocTags`; not regex. Self-tested via `check-jsdoc-hygiene-ts-tests.cjs` (13 in-memory fixtures; name avoids the `*.test.*` glob so vitest doesn't pick it up as a unit-test suite). The self-test suite runs as wrapper stage 4 (`jsdoc-hygiene-ts-test`) immediately before this gate. Policy at `type-hygiene.md`. Fix violations in place using the patterns there; `--write` was removed when the gate flipped to strict zero. | Type-bearing JSDoc in `.ts` files is documentation-only (TS ignores it), so duplicate type information drifts silently — `@param {Element}` while signature said `HTMLElement` is the canonical example. Without this gate, that class of drift ships. |

The repo also has a top-level public-contract tier gate. One script,
`scripts/report-public-contract.mjs`, with two modes:
Expand Down
164 changes: 42 additions & 122 deletions packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,9 @@
* - Every other JSDoc tag is ignored (allows `@deprecated`,
* `@example`, `@throws`, `@see`, `@typeParam`, etc.).
*
* Snapshot/ratchet pattern mirrors `check-jsdoc.cjs`:
*
* - Existing violations are recorded in
* `jsdoc-hygiene-ts-baseline.json`. New violations on top of the
* baseline fail.
* - Stale baseline entries (file removed, JSDoc cleaned up) also
* fail; rerun with --write to refresh.
*
* Refreshing the baseline:
*
* node packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs --write
* Strict-zero gate. Every type-bearing JSDoc tag in scope is a
* violation; the script exits non-zero on any violation. There is no
* baseline, grandfathering, or --write mode.
*
* Scope: see `type-hygiene.md` § Scope. Excludes test files and
* declaration files.
Expand All @@ -49,7 +41,6 @@ const ts = require('typescript');
const scriptDir = __dirname;
const repoRoot = path.resolve(scriptDir, '..', '..', '..');

const BASELINE_PATH = path.join(scriptDir, 'jsdoc-hygiene-ts-baseline.json');
const POLICY_RELATIVE = 'packages/superdoc/scripts/type-hygiene.md';

const SCAN_ROOTS = ['packages/superdoc/src', 'packages/super-editor/src'];
Expand Down Expand Up @@ -138,9 +129,9 @@ function renderName(nameNode) {

/**
* Walk up from a node to find the nearest named declaration ancestor.
* Used to anchor each violation to a stable enclosing-symbol name so
* the baseline key survives line shifts caused by edits elsewhere in
* the file.
* Used to render each violation's enclosing-symbol name in the
* failure message so a contributor can jump straight to the
* declaration that owns the bad JSDoc.
*
* Returns '<inline>' when no named ancestor is found (e.g. inline
* `/** @type *​/` cast inside an anonymous expression at module scope).
Expand Down Expand Up @@ -175,19 +166,13 @@ function enclosingSymbolName(node) {

/**
* Walk a source file's AST and emit one violation per type-bearing
* JSDoc tag.
*
* Each violation's identity is the stable tuple
* `{file, symbol, tag, occurrenceIndex}`. `line` is captured for
* human-readable display only; it is NOT part of the baseline key
* because line numbers shift under unrelated edits and would produce
* spurious stale-plus-new pairs on the ratchet.
* JSDoc tag. Returns findings with `{file, line, tag, class, symbol}`
* for display in the failure message.
*/
function findViolations(filePath, sourceText) {
const sf = ts.createSourceFile(filePath, sourceText, ts.ScriptTarget.Latest, /* setParentNodes */ true);
const findings = [];
const seenPositions = new Set();
const occurrenceCounter = new Map();

function visit(node) {
const tags = ts.getJSDocTags(node);
Expand All @@ -202,20 +187,13 @@ function findViolations(filePath, sourceText) {
const positionKey = `${tag.pos}:${tag.end}:${name}`;
if (seenPositions.has(positionKey)) continue;
seenPositions.add(positionKey);
// Occurrence index within (symbol, tag) so multiple `@param`s
// on the same method are distinguished without depending on
// line numbers.
const counterKey = `${symbol}::${name}`;
const occurrenceIndex = occurrenceCounter.get(counterKey) || 0;
occurrenceCounter.set(counterKey, occurrenceIndex + 1);
const { line } = sf.getLineAndCharacterOfPosition(tag.pos);
findings.push({
file: filePath,
line: line + 1,
tag: name,
class: violation,
symbol,
occurrenceIndex,
});
}
}
Expand All @@ -225,9 +203,9 @@ function findViolations(filePath, sourceText) {

findings.sort(
(a, b) =>
a.symbol.localeCompare(b.symbol) ||
a.tag.localeCompare(b.tag) ||
a.occurrenceIndex - b.occurrenceIndex,
a.file.localeCompare(b.file) ||
a.line - b.line ||
a.tag.localeCompare(b.tag),
);
return findings;
}
Expand All @@ -243,47 +221,22 @@ function classifyTag(name, tag) {
return null;
}

// ─── Baseline serialization ───────────────────────────────────────────

function loadBaseline() {
if (!fs.existsSync(BASELINE_PATH)) {
return { $comment: '', knownViolations: [] };
}
const raw = fs.readFileSync(BASELINE_PATH, 'utf8');
try {
return JSON.parse(raw);
} catch (e) {
throw new Error(`Failed to parse baseline at ${BASELINE_PATH}: ${e.message}`);
}
}

function violationKey(v) {
// Stable across line shifts and re-orderings of unrelated code.
// `line` is intentionally excluded: editing above a JSDoc block
// would otherwise produce one stale + one new entry for an
// unchanged violation, and the ratchet would be noisy.
return `${v.file}::${v.symbol}::${v.tag}::${v.class}::${v.occurrenceIndex}`;
}

function writeBaseline(violations) {
const data = {
$comment:
'Auto-managed by packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs. ' +
'Each entry is "file::enclosingSymbol::tagName::class::occurrenceIndex"; ' +
'line numbers are NOT part of the key, so the baseline survives unrelated ' +
'edits that shift line numbers. The gate grandfathers these and fails on ' +
'net-new violations. Refresh after intentional cleanup with --write. ' +
'Goal is to drain to zero, then flip to "zero allowed" in a separate PR. ' +
'See packages/superdoc/scripts/type-hygiene.md.',
knownViolations: violations.map(violationKey).sort(),
};
fs.writeFileSync(BASELINE_PATH, JSON.stringify(data, null, 2) + '\n');
}

// ─── Main ─────────────────────────────────────────────────────────────

function main() {
const writeMode = process.argv.includes('--write');
// --write was used during the ratchet phase to refresh the
// grandfathered baseline. Strict-zero mode rejects it loudly so
// contributors don't accidentally re-introduce grandfathering.
if (process.argv.includes('--write')) {
console.error(
'[jsdoc-hygiene-ts] --write is no longer supported. The gate is\n' +
'strict zero — every type-bearing JSDoc tag in scope must be\n' +
'fixed, not grandfathered. See ' +
POLICY_RELATIVE +
' for fix patterns.',
);
process.exit(2);
}

const files = [];
for (const root of SCAN_ROOTS) {
Expand All @@ -299,68 +252,35 @@ function main() {
allViolations.push(...violations);
}

const currentKeys = new Set(allViolations.map(violationKey));
const baseline = loadBaseline();
const baselineKeys = new Set(baseline.knownViolations);

const newViolations = allViolations.filter((v) => !baselineKeys.has(violationKey(v)));
const staleEntries = [...baselineKeys].filter((k) => !currentKeys.has(k)).sort();

if (writeMode) {
writeBaseline(allViolations);
console.log(`[jsdoc-hygiene-ts] wrote ${BASELINE_PATH} (${allViolations.length} entries).`);
return;
}

// Report
const total = allViolations.length;
const grandfathered = total - newViolations.length;
console.log(`[jsdoc-hygiene-ts] type-bearing JSDoc scanner`);
console.log(`[jsdoc-hygiene-ts] type-bearing JSDoc scanner (strict zero)`);
console.log('========================================================================');
console.log(`Scope: ${SCAN_ROOTS.join(', ')} (excludes test files / .d.ts)`);
console.log(`Files scanned: ${files.length}`);
console.log(`Total violations: ${total}`);
console.log(`Grandfathered: ${grandfathered}`);
console.log(`Baseline at: ${path.relative(repoRoot, BASELINE_PATH)}`);
console.log(`Total violations: ${allViolations.length}`);

if (newViolations.length > 0 || staleEntries.length > 0) {
if (allViolations.length > 0) {
console.log('');
if (newViolations.length > 0) {
console.log(`FAIL ${newViolations.length} new type-bearing JSDoc tag(s) in .ts source:`);
for (const v of newViolations.slice(0, 30)) {
console.log(` - ${v.file}:${v.line} @${v.tag} on \`${v.symbol}\` [${v.class}]`);
}
if (newViolations.length > 30) {
console.log(` ... and ${newViolations.length - 30} more.`);
}
console.log('');
console.log(
'Type-bearing JSDoc is not allowed in .ts source under packages/superdoc/src\n' +
'or packages/super-editor/src. Use TypeScript for shape (signatures, interfaces,\n' +
'`as Type` casts) and prose-only JSDoc for documentation. See ' +
POLICY_RELATIVE +
' for the rule and fix patterns.\n',
);
console.log(`FAIL ${allViolations.length} type-bearing JSDoc tag(s) in .ts source:`);
for (const v of allViolations.slice(0, 30)) {
console.log(` - ${v.file}:${v.line} @${v.tag} on \`${v.symbol}\` [${v.class}]`);
}
if (staleEntries.length > 0) {
console.log(`FAIL ${staleEntries.length} stale baseline entry/entries (no longer violating):`);
for (const k of staleEntries.slice(0, 30)) {
console.log(` - ${k}`);
}
if (staleEntries.length > 30) {
console.log(` ... and ${staleEntries.length - 30} more.`);
}
console.log('');
console.log(
'These entries were cleaned up but the baseline still records them.\n' +
'Run with --write to refresh the snapshot and lock in the win.\n',
);
if (allViolations.length > 30) {
console.log(` ... and ${allViolations.length - 30} more.`);
}
console.log('');
console.log(
'Type-bearing JSDoc is not allowed in .ts source under packages/superdoc/src\n' +
'or packages/super-editor/src. Use TypeScript for shape (signatures, interfaces,\n' +
'`as Type` casts) and prose-only JSDoc for documentation. Zero allowed — fix\n' +
'each violation in place. See ' +
POLICY_RELATIVE +
' for the rule and fix patterns.\n',
);
process.exit(1);
}

console.log('');
console.log(`OK ${total} violation(s) tracked as baseline; no net-new entries.`);
console.log(`OK zero type-bearing JSDoc in .ts source.`);
}

// Export the AST + classification helpers so the test runner can
Expand Down
4 changes: 0 additions & 4 deletions packages/superdoc/scripts/jsdoc-hygiene-ts-baseline.json
View file
Open in desktop

This file was deleted.

19 changes: 10 additions & 9 deletions packages/superdoc/scripts/type-hygiene.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,19 @@ type system via `// @ts-check`; that is enforced separately by
`check-jsdoc.cjs`. Both gates can coexist: TS files use TS syntax for
types, JS files use JSDoc for types, neither uses both.

## Refreshing the baseline
## Enforcement

The scanner snapshot lives at
`packages/superdoc/scripts/jsdoc-hygiene-ts-baseline.json`. It records
the existing violations the gate grandfathers. New violations on top
of the baseline fail CI.
Strict zero. Every type-bearing JSDoc tag in scope is a violation;
CI fails on any. There is no baseline, grandfathering, or `--write`
mode.

To refresh after intentional cleanup:
Run the gate locally with:

```sh
node packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs --write
node packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs
```

The goal is to drain the baseline to zero, then flip the gate to
"zero allowed" (separate PR).
If it fires on a tag you can't easily fix, fix the tag (see the patterns
above) rather than reaching for a grandfathering escape hatch. The
gate's failure message links back to this doc so the fix path is
always one click away.
15 changes: 8 additions & 7 deletions scripts/check-public-contract.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@
* Companion to jsdoc-ratchet on the
* .ts side: enforces TS syntax as
* the single source of truth for
* shape. Grandfathers an existing
* baseline; fails on net-new
* type-bearing JSDoc. See
* shape. Strict-zero gate (no
* grandfathered baseline); fails
* on any type-bearing JSDoc. See
* packages/superdoc/scripts/type-hygiene.md.
* 6. public-method-coverage - obligation-based ratchet over
* public SuperDoc methods +
Expand Down Expand Up @@ -173,11 +173,12 @@ const stages = [
args: ['packages/superdoc/scripts/check-jsdoc-hygiene-ts.cjs'],
blurb:
'Type-bearing JSDoc gate for .ts source under packages/superdoc/src and ' +
'packages/super-editor/src. Grandfathers an existing baseline of violations ' +
'and fails on net-new type-bearing JSDoc tags (@param {T}, @returns {T}, ' +
'packages/super-editor/src. Strict-zero gate (no grandfathered baseline, ' +
'no --write): fails on any type-bearing JSDoc tag (@param {T}, @returns {T}, ' +
'@type, @typedef, @template, etc.). See packages/superdoc/scripts/' +
'type-hygiene.md for the rule. Cheap; complements jsdoc-ratchet (which ' +
'covers .js files) by enforcing TS-as-single-source on the .ts side.',
'type-hygiene.md for the rule and fix patterns. Cheap; complements ' +
'jsdoc-ratchet (which covers .js files) by enforcing TS-as-single-source ' +
'on the .ts side.',
},
{
name: 'public-method-coverage',
Expand Down
Loading