diff --git a/package.json b/package.json index dd96100dbc8e..f3193061db30 100644 --- a/package.json +++ b/package.json @@ -1522,7 +1522,6 @@ "check:static-import-sccs": "pnpm check:madge-import-cycles", "check:temp-path-guardrails": "node --import tsx scripts/check-temp-path-guardrails.ts", "check:wrapper-shadowing": "node --import tsx scripts/check-wrapper-shadowing.mts", - "check:wrapper-shadowing:gen": "node --import tsx scripts/check-wrapper-shadowing.mts --update-debt-baseline", "check:test-types": "pnpm tsgo:test", "check:timed": "node --import tsx scripts/check-timed.mts", "check:timed:all-types": "node --import tsx scripts/check-timed.mts --include-test-types", @@ -1649,7 +1648,6 @@ "lint:tmp:channel-agnostic-boundaries": "node --import tsx scripts/check-channel-agnostic-boundaries.mts", "lint:tmp:dynamic-import-warts": "node --import tsx scripts/check-dynamic-import-warts.mts", "lint:tmp:export-name-collisions": "node --import tsx scripts/check-export-name-collisions.mts", - "lint:tmp:export-name-collisions:gen": "node --import tsx scripts/check-export-name-collisions.mts --update-debt-baseline", "lint:tmp:no-random-messaging": "node --import tsx scripts/check-no-random-messaging-tmp.mts", "lint:tmp:no-raw-channel-fetch": "node --import tsx scripts/check-no-raw-channel-fetch.mts", "lint:tmp:no-raw-http2-imports": "node --import tsx scripts/check-no-raw-http2-imports.mts", diff --git a/scripts/check-changed.mts b/scripts/check-changed.mts index a6891f585726..5389197e9448 100644 --- a/scripts/check-changed.mts +++ b/scripts/check-changed.mts @@ -107,7 +107,7 @@ const PLUGIN_SDK_SURFACE_PATH_RE = const DEPRECATION_HYGIENE_PATH_RE = /^(?:package\.json$|src\/|extensions\/|packages\/|scripts\/(?:check-deprecated-api-usage\.mts$|plugin-boundary-report\.ts$|lib\/plugin-sdk))/u; const WRAPPER_SHADOWING_PATH_RE = - /^(?:package\.json$|src\/|scripts\/(?:check-(?:export-name-collisions|wrapper-shadowing)\.mts$|lib\/(?:export-name-collision-baseline\.json$|ts-guard-utils\.mts$|wrapper-shadowing-baseline\.json$)))/u; + /^(?:package\.json$|src\/|scripts\/(?:check-(?:export-name-collisions|wrapper-shadowing)\.mts$|lib\/ts-guard-utils\.mts$))/u; const CANVAS_A2UI_NATIVE_RESOURCE_PATH_RE = /^(?:pnpm-lock\.yaml$|apps\/(?:android\/app\/build\.gradle\.kts$|ios\/project\.yml$|linux\/src-tauri\/(?:build\.rs$|src\/canvas\.rs$)|shared\/OpenClawKit\/Sources\/OpenClawKit\/Resources\/CanvasA2UI\/)|extensions\/canvas\/(?:package\.json$|scripts\/bundle-a2ui\.mjs$|src\/host\/a2ui(?:\/(?:index\.html|a2ui\.bundle\.js|\.bundle\.hash)$|-app\/))|scripts\/(?:bundle-a2ui|sync-native-a2ui)\.mts$)/u; const CONTROL_UI_I18N_VERIFY_PATH_RE = diff --git a/scripts/check-export-name-collisions.mts b/scripts/check-export-name-collisions.mts index 32c1a2e2d75b..a43a0231ba75 100644 --- a/scripts/check-export-name-collisions.mts +++ b/scripts/check-export-name-collisions.mts @@ -3,7 +3,6 @@ import fs from "node:fs/promises"; import path from "node:path"; import ts from "typescript"; -import { z } from "zod"; import { resolveRepoRoot } from "./lib/repo-root.mjs"; import { collectTypeScriptFilesFromRoots, @@ -57,17 +56,6 @@ export type ModuleExports = { valueDefinitions: Map; }; -const exportNameCollisionSchema = z - .object({ - name: z.string(), - files: z.array(z.string()), - sdk: z.literal(true).optional(), - }) - .strict(); -const exportNameCollisionBaselineSchema = z.array(exportNameCollisionSchema); - -const baselineRelativePath = "scripts/lib/export-name-collision-baseline.json"; -const baselineRegenCommand = "pnpm lint:tmp:export-name-collisions:gen"; const failurePrefix = "check-export-name-collisions"; const extraExcludedFileSuffixes = [".test-support.ts", ".test-helpers.ts", ".d.ts"]; @@ -695,51 +683,6 @@ export function findExportNameCollisions(modules: SourceModule[]): ExportNameCol return analyzeExportNames(modules).collisions; } -type CollisionChange = { - baseline?: ExportNameCollision; - current?: ExportNameCollision; -}; - -/** Compares every collision cluster so additions fail and removals ratchet debt down. */ -export function compareExportNameCollisionDebt( - current: ExportNameCollision[], - baseline: ExportNameCollision[], -) { - const currentByName = new Map(current.map((collision) => [collision.name, collision])); - const baselineByName = new Map(baseline.map((collision) => [collision.name, collision])); - const regressions: CollisionChange[] = []; - const improvements: CollisionChange[] = []; - const names = [...new Set([...currentByName.keys(), ...baselineByName.keys()])].toSorted(); - - for (const name of names) { - const currentCollision = currentByName.get(name); - const baselineCollision = baselineByName.get(name); - if (!baselineCollision) { - regressions.push({ current: currentCollision }); - continue; - } - if (!currentCollision) { - improvements.push({ baseline: baselineCollision }); - continue; - } - const baselineFiles = new Set(baselineCollision.files); - const currentFiles = new Set(currentCollision.files); - const hasAddedFile = currentCollision.files.some((file) => !baselineFiles.has(file)); - const hasRemovedFile = baselineCollision.files.some((file) => !currentFiles.has(file)); - if (hasAddedFile || (currentCollision.sdk === true && baselineCollision.sdk !== true)) { - regressions.push({ baseline: baselineCollision, current: currentCollision }); - } - if (hasRemovedFile || (baselineCollision.sdk === true && currentCollision.sdk !== true)) { - improvements.push({ baseline: baselineCollision, current: currentCollision }); - } - } - return { regressions, improvements }; -} - -function resolveBaselinePath(repoRoot: string) { - return path.join(repoRoot, ...baselineRelativePath.split("/")); -} - async function collectRepositoryModules(repoRoot: string) { const sourceCollectOptions = { fileExtensions: [".ts", ".mts", ".js", ".mjs"], @@ -784,28 +727,6 @@ export async function collectRepositoryCollisions(repoRoot: string) { return (await collectRepositoryExportAnalysis(repoRoot)).collisions; } -async function readBaseline(repoRoot: string) { - try { - return exportNameCollisionBaselineSchema.parse( - JSON.parse(await fs.readFile(resolveBaselinePath(repoRoot), "utf8")), - ); - } catch (error) { - if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { - return null; - } - throw error; - } -} - -async function writeBaseline(repoRoot: string, collisions: ExportNameCollision[]) { - await fs.writeFile(resolveBaselinePath(repoRoot), `${JSON.stringify(collisions, null, 2)}\n`); - return collisions.length; -} - -function formatCollision(collision: ExportNameCollision | undefined) { - return JSON.stringify(collision); -} - function printAliasingReExports(reExports: AliasingReExport[]) { if (reExports.length === 0) { return; @@ -818,51 +739,27 @@ function printAliasingReExports(reExports: AliasingReExport[]) { } } -export async function main() { - const repoRoot = resolveRepoRoot(import.meta.url); - if (process.argv.includes("--update-debt-baseline")) { - const analysis = await collectRepositoryExportAnalysis(repoRoot); - const count = await writeBaseline(repoRoot, analysis.collisions); - console.log(`Wrote ${baselineRelativePath} (${count} entries)`); - printAliasingReExports(analysis.aliasingReExports); - return 0; +export async function main( + repoRoot = resolveRepoRoot(import.meta.url), + argv = process.argv.slice(2), +) { + if (argv.length > 0) { + console.error(`Unknown argument(s): ${argv.join(", ")}`); + return 2; } - const baseline = await readBaseline(repoRoot); - if (!baseline) { - console.error( - `Missing ${baselineRelativePath}; run \`${baselineRegenCommand}\` and commit it.`, - ); - return 1; - } const analysis = await collectRepositoryExportAnalysis(repoRoot); - const debt = compareExportNameCollisionDebt(analysis.collisions, baseline); printAliasingReExports(analysis.aliasingReExports); - if (debt.regressions.length === 0 && debt.improvements.length === 0) { + if (analysis.collisions.length === 0) { console.log("export name collision guard passed."); return 0; } - if (debt.regressions.length > 0) { - console.error( - `Found new exported function/const name collisions beyond ${baselineRelativePath}:`, - ); - for (const regression of debt.regressions) { - console.error(`- ${formatCollision(regression.current)}`); - } - console.error( - `Give each behavior one exported spelling. If the debt increase is intentional, run \`${baselineRegenCommand}\` and commit the generated baseline.`, - ); - } - if (debt.improvements.length > 0) { - console.error(`Export name collision debt dropped below ${baselineRelativePath}:`); - for (const improvement of debt.improvements) { - console.error( - `- ${improvement.baseline?.name}: ${formatCollision(improvement.baseline)} -> ${formatCollision(improvement.current)}`, - ); - } - console.error(`Run \`${baselineRegenCommand}\` to ratchet the baseline down and commit it.`); + console.error("Found exported function/const name collisions:"); + for (const collision of analysis.collisions) { + console.error(`- ${JSON.stringify(collision)}`); } + console.error("Give each behavior one exported spelling."); return 1; } diff --git a/scripts/check-wrapper-shadowing.mts b/scripts/check-wrapper-shadowing.mts index c252dc37a3ff..7fe32e58644f 100644 --- a/scripts/check-wrapper-shadowing.mts +++ b/scripts/check-wrapper-shadowing.mts @@ -2,7 +2,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { z } from "zod"; import { collectModuleExportNames, isExcludedExportCollisionSource, @@ -24,18 +23,6 @@ export type WrapperShadowingViolation = { via?: string; }; -const violationSchema = z - .object({ - name: z.string(), - wrapped: z.string(), - wrapper: z.string(), - via: z.string().optional(), - }) - .strict(); -const baselineSchema = z.array(violationSchema); - -const baselineRelativePath = "scripts/lib/wrapper-shadowing-baseline.json"; -const baselineRegenCommand = "pnpm check:wrapper-shadowing:gen"; const failurePrefix = "check-wrapper-shadowing"; function normalizeRelativePath(filePath: string) { @@ -178,86 +165,23 @@ export async function collectRepositoryWrapperShadowing(repoRoot: string) { return findWrapperShadowingViolations(modules); } -function resolveBaselinePath(repoRoot: string) { - return path.join(repoRoot, ...baselineRelativePath.split("/")); -} - -async function readBaseline(repoRoot: string) { - try { - return baselineSchema.parse( - JSON.parse(await fs.readFile(resolveBaselinePath(repoRoot), "utf8")), - ); - } catch (error) { - if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { - return null; - } - throw error; - } -} - -export function findNewWrapperShadowingViolations( - current: WrapperShadowingViolation[], - baseline: WrapperShadowingViolation[], -) { - const baselineKeys = new Set(baseline.map(violationKey)); - return current.filter((violation) => !baselineKeys.has(violationKey(violation))); -} - -export async function evaluateWrapperShadowing(repoRoot: string) { - const baseline = await readBaseline(repoRoot); - if (!baseline) { - return { - baseline: null, - current: await collectRepositoryWrapperShadowing(repoRoot), - regressions: [] as WrapperShadowingViolation[], - }; - } - const current = await collectRepositoryWrapperShadowing(repoRoot); - return { - baseline, - current, - regressions: findNewWrapperShadowingViolations(current, baseline), - }; -} - -async function writeBaseline(repoRoot: string) { - const violations = await collectRepositoryWrapperShadowing(repoRoot); - await fs.writeFile(resolveBaselinePath(repoRoot), `${JSON.stringify(violations, null, 2)}\n`); - return violations.length; -} - export async function main( repoRoot = resolveRepoRoot(import.meta.url), argv = process.argv.slice(2), ) { - const updateBaseline = argv.includes("--update-debt-baseline"); - const unknownArgs = argv.filter((arg) => arg !== "--update-debt-baseline"); - if (unknownArgs.length > 0) { - console.error(`Unknown argument(s): ${unknownArgs.join(", ")}`); + if (argv.length > 0) { + console.error(`Unknown argument(s): ${argv.join(", ")}`); return 2; } - if (updateBaseline) { - const count = await writeBaseline(repoRoot); - console.log(`Wrote ${baselineRelativePath} (${count} entries)`); + + const violations = await collectRepositoryWrapperShadowing(repoRoot); + if (violations.length === 0) { + console.log("wrapper shadowing guard passed."); return 0; } - const result = await evaluateWrapperShadowing(repoRoot); - if (!result.baseline) { - console.error( - `Missing ${baselineRelativePath}; run \`${baselineRegenCommand}\` and commit it.`, - ); - return 1; - } - if (result.regressions.length === 0) { - console.log( - `wrapper shadowing guard passed (${result.current.length} current, ${result.baseline.length} baselined).`, - ); - return 0; - } - - console.error(`Found new same-name wrapper shadowing beyond ${baselineRelativePath}:`); - for (const violation of result.regressions) { + console.error("Found same-name wrapper shadowing:"); + for (const violation of violations) { console.error(`- ${JSON.stringify(violation)}`); } console.error( diff --git a/scripts/lib/export-name-collision-baseline.json b/scripts/lib/export-name-collision-baseline.json deleted file mode 100644 index fe51488c7066..000000000000 --- a/scripts/lib/export-name-collision-baseline.json +++ /dev/null @@ -1 +0,0 @@ -[] diff --git a/scripts/lib/wrapper-shadowing-baseline.json b/scripts/lib/wrapper-shadowing-baseline.json deleted file mode 100644 index fe51488c7066..000000000000 --- a/scripts/lib/wrapper-shadowing-baseline.json +++ /dev/null @@ -1 +0,0 @@ -[] diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 9547414a6442..e391a9c5c510 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -1881,12 +1881,17 @@ describe("scripts/changed-lanes", () => { "src/channels/turn/run-channel-turn.ts", "scripts/check-wrapper-shadowing.mts", "scripts/check-export-name-collisions.mts", - "scripts/lib/wrapper-shadowing-baseline.json", "scripts/lib/ts-guard-utils.mts", "package.json", ]), ).toBe(true); - expect(shouldRunWrapperShadowingCheck(["docs/concepts/message-lifecycle.md"])).toBe(false); + expect( + shouldRunWrapperShadowingCheck([ + "docs/concepts/message-lifecycle.md", + "scripts/lib/wrapper-shadowing-baseline.json", + "scripts/lib/export-name-collision-baseline.json", + ]), + ).toBe(false); const plan = createChangedCheckPlan( detectChangedLanes(["scripts/check-wrapper-shadowing.mts"]), diff --git a/test/scripts/check-export-name-collisions.test.ts b/test/scripts/check-export-name-collisions.test.ts index 5a250beff175..e273ccbda95b 100644 --- a/test/scripts/check-export-name-collisions.test.ts +++ b/test/scripts/check-export-name-collisions.test.ts @@ -1,16 +1,21 @@ +import { spawnSync } from "node:child_process"; import fs from "node:fs/promises"; import path from "node:path"; +import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; import { collectModuleExportNames, collectRepositoryCollisions, - compareExportNameCollisionDebt, findAliasingReExports, findExportNameCollisions, isExcludedExportCollisionSource, } from "../../scripts/check-export-name-collisions.mts"; import { withTempDir } from "../../src/test-utils/temp-dir.js"; +const guardScriptPath = fileURLToPath( + new URL("../../scripts/check-export-name-collisions.mts", import.meta.url), +); + describe("export name collision guard", () => { it.each([ ["src/example.test.ts", true], @@ -244,34 +249,17 @@ describe("export name collision guard", () => { }, ]); }); -}); -describe("export name collision debt baseline", () => { - it("separates new debt from baseline improvements", () => { - expect( - compareExportNameCollisionDebt( - [ - { name: "added", files: ["src/a.ts", "src/b.ts"] }, - { name: "expanded", files: ["src/a.ts", "src/b.ts", "src/c.ts"], sdk: true }, - ], - [ - { name: "expanded", files: ["src/a.ts", "src/b.ts"] }, - { name: "removed", files: ["src/c.ts", "src/d.ts"] }, - ], - ), - ).toEqual({ - regressions: [ - { current: { name: "added", files: ["src/a.ts", "src/b.ts"] } }, - { - baseline: { name: "expanded", files: ["src/a.ts", "src/b.ts"] }, - current: { - name: "expanded", - files: ["src/a.ts", "src/b.ts", "src/c.ts"], - sdk: true, - }, - }, - ], - improvements: [{ baseline: { name: "removed", files: ["src/c.ts", "src/d.ts"] } }], - }); + it("rejects debt-baseline updates with the collision trailer", () => { + const result = spawnSync( + process.execPath, + ["--import", "tsx", guardScriptPath, "--update-debt-baseline"], + { encoding: "utf8" }, + ); + + expect(result.status).toBe(2); + expect(result.stderr.trimEnd().split("\n").at(-1)).toBe( + "[check-export-name-collisions] FAILED (exit 2)", + ); }); }); diff --git a/test/scripts/check-wrapper-shadowing.test.ts b/test/scripts/check-wrapper-shadowing.test.ts index af82146c25e3..956e4e2a36ea 100644 --- a/test/scripts/check-wrapper-shadowing.test.ts +++ b/test/scripts/check-wrapper-shadowing.test.ts @@ -3,38 +3,29 @@ import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; -import { - evaluateWrapperShadowing, - type WrapperShadowingViolation, -} from "../../scripts/check-wrapper-shadowing.mts"; +import { collectRepositoryWrapperShadowing } from "../../scripts/check-wrapper-shadowing.mts"; import { withTempDir } from "../../src/test-utils/temp-dir.js"; const guardScriptPath = fileURLToPath( new URL("../../scripts/check-wrapper-shadowing.mts", import.meta.url), ); -type GuardFixture = { - baseline?: WrapperShadowingViolation[]; - files: Record; -}; +type GuardFixture = Record; -async function runFixture(fixture: GuardFixture) { +async function runFixture(files: GuardFixture) { return await withTempDir("openclaw-wrapper-shadowing-", async (repoRoot) => { await Promise.all( - Object.entries(fixture.files).map(async ([repoPath, content]) => { + Object.entries(files).map(async ([repoPath, content]) => { const filePath = path.join(repoRoot, repoPath); await fs.mkdir(path.dirname(filePath), { recursive: true }); await fs.writeFile(filePath, content); }), ); - const baselinePath = path.join(repoRoot, "scripts/lib/wrapper-shadowing-baseline.json"); - await fs.mkdir(path.dirname(baselinePath), { recursive: true }); - await fs.writeFile(baselinePath, `${JSON.stringify(fixture.baseline ?? [], null, 2)}\n`); - return await evaluateWrapperShadowing(repoRoot); + return await collectRepositoryWrapperShadowing(repoRoot); }); } -const directViolation: GuardFixture["files"] = { +const directViolation: GuardFixture = { "src/inner.ts": "export function runTask() { return 'inner'; }\n", "src/outer.ts": [ 'import { runTask as runTaskInner } from "./inner.js";', @@ -47,71 +38,26 @@ const directViolation: GuardFixture["files"] = { describe("wrapper shadowing guard", () => { it("fails for a same-name wrapper around an imported implementation", async () => { - const result = await runFixture({ files: directViolation }); + const result = await runFixture(directViolation); - expect(result.regressions).toEqual([ - { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }, - ]); + expect(result).toEqual([{ name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }]); }); it("passes for a pure re-export", async () => { const result = await runFixture({ - files: { - "src/inner.ts": "export function runTask() { return 'inner'; }\n", - "src/outer.ts": 'export { runTask } from "./inner.js";\n', - }, + "src/inner.ts": "export function runTask() { return 'inner'; }\n", + "src/outer.ts": 'export { runTask } from "./inner.js";\n', }); - expect(result.current).toEqual([]); - expect(result.regressions).toEqual([]); + expect(result).toEqual([]); }); - it("passes for a baselined violation", async () => { - const violation = { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }; - const result = await runFixture({ - baseline: [ - violation, - { name: "removedTask", wrapped: "src/old-inner.ts", wrapper: "src/old-outer.ts" }, - ], - files: directViolation, - }); - - expect(result.current).toEqual([violation]); - expect(result.regressions).toEqual([]); - }); - - it("fails for a new violation on top of the baseline", async () => { - const baseline = { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }; - const result = await runFixture({ - baseline: [baseline], - files: { - ...directViolation, - "src/barrel.ts": 'export { sendTask } from "./sender.js";\n', - "src/sender.ts": "export const sendTask = () => 'sent';\n", - "src/send-wrapper.ts": [ - 'import { sendTask as sendTaskInner } from "./barrel.js";', - "export const sendTask = (...args: unknown[]) => {", - " recordSend();", - " return sendTaskInner(...args);", - "};", - ].join("\n"), - }, - }); - - expect(result.regressions).toEqual([ - { - name: "sendTask", - wrapped: "src/sender.ts", - wrapper: "src/send-wrapper.ts", - via: "src/barrel.ts", - }, - ]); - }); - - it("ends failures with the wrapper trailer", () => { - const result = spawnSync(process.execPath, ["--import", "tsx", guardScriptPath, "--invalid"], { - encoding: "utf8", - }); + it("rejects debt-baseline updates with the wrapper trailer", () => { + const result = spawnSync( + process.execPath, + ["--import", "tsx", guardScriptPath, "--update-debt-baseline"], + { encoding: "utf8" }, + ); expect(result.status).toBe(2); expect(result.stderr.trimEnd().split("\n").at(-1)).toBe(