From 4a3873fe139055e7fcb7e54fcd90181a256aa970 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 18:34:07 +0000 Subject: [PATCH 1/2] fix(scripts): type js-comment-mask.mjs's parameters so consumer call sites are checked `tsconfig.scripts.json` compiles `scripts/**` with `allowJs: true` and `checkJs: false` on purpose (objectui#3494), so a `.ts` consumer's types for these helpers come from inference over the `.mjs` source, steered by JSDoc and by nothing else. Four exports take parameters; only `scanSource` documented them. `blank` had a one-line prose docblock, `stripComments` and `maskComments` had prose-only multi-line ones -- so their parameters inferred `any` and every consumer call site was accepted unchecked. Measured on this tree before the change: `maskComments(12345)` -- a number passed to a string parameter -- type-checked clean at exit 0. The error was not merely unreported, it was unreachable, which is why objectui#9322 had to carry a hand-written `const mask: (source: string) => string = maskComments;` to keep its one call site checked. `scanSource` is the control that proved the gap was the missing tag rather than `allowJs` failing to type anything: it is imported alongside `maskComments` in the same files, under the same config, and it was checked. It is left exactly as it was. No masking behaviour changes. All four exports were run over a nine-file corpus of real repo sources (299,880 characters, 141,075 of them flagged as comment) before and after, and every output digest is identical -- which matters because several gates read this module as the single authority on "is this span a comment, or code?". The new test is what keeps the tags from being decorative. It compiles real calls against the real module with the real project options and asserts the DIAGNOSTIC rather than the tag's spelling: a wrong-typed argument must produce TS2345 and a correctly-typed one must produce nothing. Deleting a `@param` again makes the wrong-typed half stop erroring, which is the direction a grep for the tag cannot see. Part of objectui#9324 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr --- .changeset/9324-comment-mask-param-tags.md | 33 +++++ .../js-comment-mask-param-types-9324.test.ts | 136 ++++++++++++++++++ scripts/js-comment-mask.mjs | 14 +- 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 .changeset/9324-comment-mask-param-tags.md create mode 100644 scripts/__tests__/js-comment-mask-param-types-9324.test.ts diff --git a/.changeset/9324-comment-mask-param-tags.md b/.changeset/9324-comment-mask-param-tags.md new file mode 100644 index 0000000000..1422fb460f --- /dev/null +++ b/.changeset/9324-comment-mask-param-tags.md @@ -0,0 +1,33 @@ +--- +--- + +Tooling only, no runtime change and no behaviour change: the three parameter-taking +exports of `scripts/js-comment-mask.mjs` that carried no `@param` tag now carry one +(objectui#9324). + +`tsconfig.scripts.json` compiles `scripts/**` with `allowJs: true` and `checkJs: false` +deliberately (objectui#3494), so a `.ts` consumer's types for these helpers come from +inference over the `.mjs` source, steered by JSDoc and by nothing else. Four exports take +parameters; only `scanSource` documented them. `blank` had a one-line prose docblock, +`stripComments` and `maskComments` had prose-only multi-line ones — so their parameters +inferred `any` and every consumer call site was accepted unchecked. Measured on the same +tree: `maskComments(12345)` — a number passed to a string parameter — type-checked clean +at exit 0. + +`scanSource` was the control that proved this was the missing tag rather than `allowJs` +failing to type anything: it is imported alongside `maskComments` in the same files, +under the same config, and it was checked. It is left exactly as it was. + +The masker's OUTPUT is untouched, which matters because several gates in this repo read +it as the single authority on "is this span a comment, or code?" and objectui#9183 routed +29 test files onto it for that reason. All four exports were run over a nine-file corpus +of real repo sources (299,880 characters, 141,075 of them flagged as comment) before and +after the change, and every digest is identical. + +`js-comment-mask-param-types-9324.test.ts` is what keeps this from being decorative. It +compiles real calls against the real module with the real project options and asserts the +DIAGNOSTIC rather than the tag's spelling: a wrong-typed argument must produce TS2345 and +a correctly-typed one must produce nothing. Deleting a `@param` again makes the +wrong-typed half stop erroring — the direction a grep for the tag cannot see. The +correctly-typed half is there so that a signature narrowed until every real call fails +cannot pass either. diff --git a/scripts/__tests__/js-comment-mask-param-types-9324.test.ts b/scripts/__tests__/js-comment-mask-param-types-9324.test.ts new file mode 100644 index 0000000000..3eb81c2cf8 --- /dev/null +++ b/scripts/__tests__/js-comment-mask-param-types-9324.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +/** + * objectui#9324 — `scripts/js-comment-mask.mjs` is the repo's single answer to + * "is this span a comment, or code?", and `tsconfig.scripts.json` compiles + * `scripts/**` with `allowJs: true` and `checkJs: false` on purpose + * (objectui#3494): a `.ts` consumer's types for these helpers come from + * inference over the `.mjs` source, steered by JSDoc and by nothing else. + * + * Three of the four parameter-taking exports carried no `@param` — `blank` had + * a one-line prose docblock, `stripComments` and `maskComments` had prose-only + * multi-line ones — so their parameters inferred `any` and EVERY consumer call + * site went unchecked. `scanSource` carried `@param {string} source` all along + * and was checked from the same module under the same config, which is what + * proved the gap was the missing tag rather than `allowJs` failing to type + * anything. + * + * ## Why this test builds a program instead of grepping for `@param` + * + * Asserting the TAG is present asserts the spelling, and a docblock that says + * `@param` is worth nothing unless TypeScript acts on it. So each case compiles + * a real call against the real `.mjs`, with the real `tsconfig.scripts.json` + * options, and asserts the DIAGNOSTIC — a wrong-typed argument must produce + * TS2345, and the correctly-typed one must produce nothing. A regression that + * deletes a `@param` makes the wrong-typed case stop erroring, which is the + * direction a spelling check cannot see. + * + * The correctly-typed half is not ceremony: without it, "the wrong call errors" + * is also satisfied by a signature so narrow that every real call errors too. + */ +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const configPath = path.join(repoRoot, 'tsconfig.scripts.json'); +const maskerPath = path.join(repoRoot, 'scripts/js-comment-mask.mjs'); + +/** The real options `pnpm type-check:scripts` runs with, parsed by tsc itself. */ +function scriptsCompilerOptions(): ts.CompilerOptions { + const read = ts.readConfigFile(configPath, ts.sys.readFile); + expect( + read.error && ts.flattenDiagnosticMessageText(read.error.messageText, ' '), + 'tsconfig.scripts.json must parse', + ).toBeFalsy(); + const parsed = ts.parseJsonConfigFileContent(read.config, ts.sys, repoRoot, undefined, configPath); + expect(parsed.options.allowJs, '`allowJs` is what makes JSDoc load-bearing here').toBe(true); + return { ...parsed.options, noEmit: true }; +} + +/** + * Type-check one snippet as a `scripts/**` file and return its semantic + * diagnostics. The snippet is served from memory at a real path inside + * `scripts/__tests__/`, so its relative import of the masker resolves to the + * genuine `.mjs` on disk rather than to a stub. + */ +function diagnosticsFor(snippet: string): ts.Diagnostic[] { + const virtualPath = path.join(repoRoot, 'scripts/__tests__/__js-comment-mask-9324-case.ts'); + const options = scriptsCompilerOptions(); + const host = ts.createCompilerHost(options, true); + const readFile = host.readFile.bind(host); + const fileExists = host.fileExists.bind(host); + const getSourceFile = host.getSourceFile.bind(host); + + host.fileExists = (name) => (name === virtualPath ? true : fileExists(name)); + host.readFile = (name) => (name === virtualPath ? snippet : readFile(name)); + host.getSourceFile = (name, languageVersion, onError, shouldCreate) => + name === virtualPath + ? ts.createSourceFile(name, snippet, languageVersion, true, ts.ScriptKind.TS) + : getSourceFile(name, languageVersion, onError, shouldCreate); + + const program = ts.createProgram([virtualPath], options, host); + const source = program.getSourceFile(virtualPath); + expect(source, 'the case file must be in the program, or a green here measures nothing').toBeDefined(); + return [...program.getSemanticDiagnostics(source)]; +} + +/** Render diagnostics compactly so a failure names the codes it actually saw. */ +function codesOf(diagnostics: ts.Diagnostic[]): string[] { + return diagnostics.map( + (d) => `TS${d.code}: ${ts.flattenDiagnosticMessageText(d.messageText, ' ').slice(0, 120)}`, + ); +} + +const IMPORT = "import { scanSource, blank, stripComments, maskComments } from '../js-comment-mask.mjs';"; + +/** + * One export's pair of cases. `wrong` must produce TS2345 and `correct` must + * produce nothing — the two halves together are what distinguishes "now + * checked" from both "still `any`" and "typed too narrowly to use". + */ +const CASES: ReadonlyArray<{ name: string; wrong: string; correct: string }> = [ + { + name: 'maskComments', + wrong: 'export const x = maskComments(12345);', + correct: "export const x: string = maskComments('const a = 1; // c');", + }, + { + name: 'stripComments', + wrong: 'export const x = stripComments(12345);', + correct: "export const x: string = stripComments('const a = 1; // c');", + }, + { + name: 'blank (first parameter)', + wrong: "export const x = blank(12345, new Uint8Array([1]));", + correct: "export const x: string = blank('abc', new Uint8Array([1, 0, 0]));", + }, + { + name: 'blank (second parameter)', + wrong: "export const x = blank('abc', 'not-a-Uint8Array');", + correct: "export const x: string = blank('abc', new Uint8Array([1, 0, 0]));", + }, + { + // The control that was already correct before objectui#9324 and is left + // untouched by it. It fires for the same reason the other three now do, from + // the same module under the same config — so a run where the three below go + // quiet while this one still fires points at those three `@param` tags, and + // a run where ALL FOUR go quiet points at `allowJs` or at this harness. + name: 'scanSource (control — carried `@param` all along)', + wrong: 'export const x = scanSource(12345);', + correct: "export const x = scanSource('const a = 1; // c').comment;", + }, +]; + +describe('js-comment-mask.mjs — its `@param` tags are load-bearing, not decorative (objectui#9324)', () => { + for (const c of CASES) { + it(`rejects a wrong-typed argument to ${c.name}`, () => { + const found = codesOf(diagnosticsFor(`${IMPORT}\n${c.wrong}\n`)); + expect(found.join('\n')).toMatch(/TS2345/); + }); + + it(`still accepts a correctly-typed argument to ${c.name}`, () => { + expect(codesOf(diagnosticsFor(`${IMPORT}\n${c.correct}\n`))).toEqual([]); + }); + } +}); diff --git a/scripts/js-comment-mask.mjs b/scripts/js-comment-mask.mjs index e9e8f43a63..561d285741 100644 --- a/scripts/js-comment-mask.mjs +++ b/scripts/js-comment-mask.mjs @@ -436,7 +436,13 @@ export function scanSource(source) { return { comment, literal, interpolation }; } -/** Replace every flagged character with a space, keeping newlines and offsets. */ +/** + * Replace every flagged character with a space, keeping newlines and offsets. + * + * @param {string} source + * @param {Uint8Array} flags One byte per character of `source`; non-zero blanks it. + * @returns {string} + */ export function blank(source, flags) { const out = source.split(''); for (let k = 0; k < out.length; k++) if (flags[k] && out[k] !== '\n') out[k] = ' '; @@ -465,6 +471,9 @@ export function blank(source, flags) { * Pick by what the caller does with the result: reports a LINE or an offset * into the original text -> `maskComments`; feeds a scanner and reports neither * -> `stripComments`. + * + * @param {string} source + * @returns {string} */ export function stripComments(source) { const { comment } = scanSource(source); @@ -481,6 +490,9 @@ export function stripComments(source) { * Strings, templates and regex literals are left INTACT: a gate's signal is * usually itself a string literal, so "drop everything quoted" would erase the * thing being looked for. Only prose goes. + * + * @param {string} source + * @returns {string} */ export function maskComments(source) { return blank(source, scanSource(source).comment); From b82347722da03ed658b7551d967a7767b7e11cb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 18:46:05 +0000 Subject: [PATCH 2/2] fix(scripts): drop two dead declarations from the js-comment-mask param-type test `Lint` was red on the repo-wide root run, and the single error in it was this file's line 2: an unused `import fs`. `object-ui/no-unused-imports` is error-level and `@typescript-eslint/no-unused-vars` is warning-level, so the same finding was reported twice and only the first one failed the gate. Both declarations are leftovers from an earlier draft that read the masker from disk, before the harness switched to resolving it through the case file's own relative import. Neither is scaffolding: counted over this file with comments stripped, `fs` and `maskerPath` each occurred exactly once -- their own declaration, with zero uses -- while `path`, `ts`, `repoRoot` and `configPath` all hit as controls on the same input. So they are removed rather than renamed; prefixing an underscore would have silenced the report without retiring the dead code, and `maskerPath`'s report was only a warning and was failing nothing. `scripts/js-comment-mask.mjs` is untouched, verified by blob hash. Nothing else in the run is touched: the 32 warnings that remain are pre-existing, on unrelated files. `pnpm lint:root` exit 1 -> 0 (35 problems / 1 error -> 32 problems / 0 errors). `pnpm type-check:scripts` exit 0. The param-type test and its neighbours still pass: 4 files, 75 tests. Part of objectui#9324 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr --- scripts/__tests__/js-comment-mask-param-types-9324.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/__tests__/js-comment-mask-param-types-9324.test.ts b/scripts/__tests__/js-comment-mask-param-types-9324.test.ts index 3eb81c2cf8..2ad3117881 100644 --- a/scripts/__tests__/js-comment-mask-param-types-9324.test.ts +++ b/scripts/__tests__/js-comment-mask-param-types-9324.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from 'vitest'; -import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import ts from 'typescript'; @@ -34,7 +33,6 @@ import ts from 'typescript'; */ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const configPath = path.join(repoRoot, 'tsconfig.scripts.json'); -const maskerPath = path.join(repoRoot, 'scripts/js-comment-mask.mjs'); /** The real options `pnpm type-check:scripts` runs with, parsed by tsc itself. */ function scriptsCompilerOptions(): ts.CompilerOptions {