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
22 changes: 19 additions & 3 deletions scripts/check-checkers-wired.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 };
}

Expand Down
54 changes: 48 additions & 6 deletions test/unit/check-checkers-wired.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ describe("reachableNpmScripts", () => {
describe("resolveCheckerHome", () => {
const base = { scripts: {}, reachableFromTestCi: new Set<string>(), 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,
Expand All @@ -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", () => {
Expand Down