fix(scripts): type js-comment-mask.mjs's parameters so consumer call sites are checked

.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.

scripts/__tests__/js-comment-mask-param-types-9324.test.ts

@@ -0,0 +1,134 @@
+import { describe, expect, it } from 'vitest';
+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');
+
+/** 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([]);
+    });
+  }
+});

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);