diff --git a/scripts/check-checkers-wired.ts b/scripts/check-checkers-wired.ts index aab13f240..bd60ddad5 100644 --- a/scripts/check-checkers-wired.ts +++ b/scripts/check-checkers-wired.ts @@ -109,6 +109,22 @@ export type CheckerHome = | { kind: "allowed"; via: string } | { kind: "none" }; +/** Drop line (`//`) and block comments so a prose mention of a checker is not treated as an import (#10048). */ +function stripScriptComments(source: string): string { + return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, ""); +} + +/** + * True when `source` has a real import, re-export, or dynamic-import of `./base` (optional .ts or .js). + * Specifier must sit in statement position — a bare substring or comment mention does not count (#10048). + */ +function sourceImportsChecker(source: string, base: string): boolean { + const code = stripScriptComments(source); + const escaped = base.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const specifier = `\\./${escaped}(?:\\.(?:ts|js))?`; + return new RegExp(`(?:(?:import|export)\\s[^;]*?\\sfrom\\s*|import\\s*\\(\\s*)['"]${specifier}['"]`).test(code); +} + /** Where a checker actually runs, or `none`. Pure, so every branch is testable without a filesystem. */ export function resolveCheckerHome(input: { file: string; @@ -126,10 +142,10 @@ export function resolveCheckerHome(input: { const workflowRef = invokers.find((name) => input.workflowText.includes(name)) ?? (input.workflowText.includes(input.file) ? input.file : undefined); if (workflowRef !== undefined) return { kind: "workflow", via: workflowRef }; - // Imported by a sibling script => a shared module, not an entry point. Matched on the extensionless - // specifier because a TS import may or may not carry `.ts`/`.js`. + // Imported by a sibling script => a shared module, not an entry point. Require an actual import / + // re-export / dynamic-import statement after stripping comments — a prose mention must not count (#10048). const base = input.file.replace(/\.ts$/, ""); - if (input.otherScriptSources.some((source) => source.includes(`${base}.ts`) || source.includes(`${base}.js`) || source.includes(`./${base}"`) || source.includes(`./${base}'`))) { + if (input.otherScriptSources.some((source) => sourceImportsChecker(source, base))) { return { kind: "imported", via: base }; } diff --git a/test/unit/check-checkers-wired.test.ts b/test/unit/check-checkers-wired.test.ts index 862b99e5a..b664fbbd2 100644 --- a/test/unit/check-checkers-wired.test.ts +++ b/test/unit/check-checkers-wired.test.ts @@ -50,6 +50,13 @@ describe("reachableNpmScripts", () => { describe("resolveCheckerHome", () => { const base = { scripts: {}, reachableFromTestCi: new Set(), workflowText: "", otherScriptSources: [], allowed: {} }; + // Assembled at runtime: `check-import-specifiers.ts` greps this repo's own sources for module + // specifiers and cannot tell a fixture string from a real import, so a literal here fails that + // sibling checker. The value under test is identical either way. + const fromSpecifier = (name: string, suffix = "") => `import { helper } from "./${name}${suffix}";\n`; + const exportFromSpecifier = (name: string, suffix = "") => `export { helper } from "./${name}${suffix}";\n`; + const dynamicImportSpecifier = (name: string, suffix = "") => `await import("./${name}${suffix}");\n`; + it("finds a checker wired into the local gate", () => { const home = resolveCheckerHome({ ...base, @@ -69,12 +76,47 @@ describe("resolveCheckerHome", () => { }); it("treats a checker imported by a sibling script as a shared module, not an entry point", () => { - // Assembled at runtime rather than written as a literal: `check-import-specifiers.ts` greps this repo's - // own sources for module specifiers and cannot tell a fixture string from a real import, so a literal - // here fails that sibling checker. The value under test is identical either way. - const importLine = `import { x } from "./${"check-foo-core"}.ts";`; - const home = resolveCheckerHome({ ...base, file: "check-foo-core.ts", otherScriptSources: [importLine] }); - expect(home.kind).toBe("imported"); + const home = resolveCheckerHome({ ...base, file: "check-foo-core.ts", otherScriptSources: [fromSpecifier("check-foo-core")] }); + expect(home).toEqual({ kind: "imported", via: "check-foo-core" }); + }); + + it("matches import specifiers with .js and .ts suffixes, re-exports, and dynamic import (#10048)", () => { + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: [fromSpecifier("check-foo", ".js")] })).toEqual({ + kind: "imported", + via: "check-foo", + }); + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: [fromSpecifier("check-foo", ".ts")] })).toEqual({ + kind: "imported", + via: "check-foo", + }); + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: [exportFromSpecifier("check-foo")] })).toEqual({ + kind: "imported", + via: "check-foo", + }); + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: [dynamicImportSpecifier("check-foo")] })).toEqual({ + kind: "imported", + via: "check-foo", + }); + }); + + it("does not treat a // comment mention as an imported home (#10048)", () => { + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: ["// see check-foo.ts for the pattern\n"] })).toEqual({ kind: "none" }); + }); + + it("does not treat a /* */ block comment mention as an imported home (#10048)", () => { + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: ["/* check-foo.ts is the sibling */\n"] })).toEqual({ kind: "none" }); + }); + + it("does not treat a near-miss specifier as an imported home (#10048)", () => { + // `./check-foobar` must not mark `check-foo.ts` as imported — the old substring scan would. + expect(resolveCheckerHome({ ...base, file: "check-foo.ts", otherScriptSources: [fromSpecifier("check-foobar")] })).toEqual({ kind: "none" }); + }); + + it("does not treat the check-fixture-clock-races prose mention as an imported home (#10048)", () => { + // Live sentence from scripts/check-fixture-clock-races.ts:95 — the bug that motivated the tighter matcher. + const prose = + " // STRING LITERALS, exactly as check-turbo-typecheck-inputs.ts's own fixtures do. Those are not real\n"; + expect(resolveCheckerHome({ ...base, file: "check-turbo-typecheck-inputs.ts", otherScriptSources: [prose] })).toEqual({ kind: "none" }); }); it("accepts an explicit allowlist entry, carrying its reason", () => {