diff --git a/.changelog/unreleased/fixed-586-workspaces-bun-parity.md b/.changelog/unreleased/fixed-586-workspaces-bun-parity.md new file mode 100644 index 00000000..b9dc8cc8 --- /dev/null +++ b/.changelog/unreleased/fixed-586-workspaces-bun-parity.md @@ -0,0 +1,8 @@ +- **The release-age exclusion audit reads workspace patterns in both shapes Bun accepts.** + + The root `package.json` `workspaces` field is an array of strings or an object + with a `packages` array; when the bunfig has release-age exclusions, any other + shape is refused with a named error, and so is a pattern whose class cannot + compile (`packages/[z-a]`). Workspace patterns also support character classes (`packages/[ab]`) and brace alternatives + (`packages/{a,b}`), and a directory named by a literal pattern stays applied when + a later `!` pattern names it. diff --git a/scripts/check-dep-ages.mjs b/scripts/check-dep-ages.mjs index 38635c6c..92345cc2 100755 --- a/scripts/check-dep-ages.mjs +++ b/scripts/check-dep-ages.mjs @@ -24,7 +24,8 @@ * or one below 7 days, invalid or unused exceptions, an excluded name without an * unexpired exception or an exact declaration Bun applies, an excluded name pinned * exactly only in a manifest Bun does not apply, an excluded name in a nested - * package.json's overrides or as a resolutions key, no external resolutions, refused + * package.json's overrides or as a resolutions key, an unreadable `workspaces` + * shape or pattern in the root package.json, no external resolutions, refused * CI overrides, unexpected arguments, or registry fetch failures * */ @@ -255,7 +256,16 @@ if (excludeNames.length > 0) { ); console.error(""); for (const problem of problems) { - if (problem.kind === "uncovered") { + if (problem.kind === "workspaces") { + console.error( + ` the root package.json \`workspaces\` could not be read: ${problem.message}.`, + ); + console.error( + problem.pattern !== undefined + ? ` Remedy: fix or remove the \`workspaces\` pattern \`${problem.pattern}\`.` + : " Remedy: declare `workspaces` as an array of strings, or as an object with a `packages` array of strings, which is what Bun reads.", + ); + } else if (problem.kind === "uncovered") { const why = problem.error ? `its dated entry is invalid: ${problem.error.message}` : "no dated entry under `## Exceptions` names it"; diff --git a/scripts/lib/check-dep-ages-collect.mjs b/scripts/lib/check-dep-ages-collect.mjs index 6ddc1a76..608a94dc 100644 --- a/scripts/lib/check-dep-ages-collect.mjs +++ b/scripts/lib/check-dep-ages-collect.mjs @@ -249,23 +249,142 @@ function pathSegments(path) { .filter((segment) => segment !== "" && segment !== "."); } -/** Whether one glob segment (`*` matches any run within a segment, `?` one unit) matches a path segment. */ -function matchGlobSegment(value, pattern) { +/** + * Bun's `detect_glob_syntax`: a pattern is a glob when it is `!`-prefixed or + * carries a `*`, `{`, `[` or `?`. + */ +function isGlobPattern(pattern) { + if (pattern.startsWith("!")) return true; + return /[*{[?]/.test(pattern); +} + +/** Strip every leading `!`; an odd count is a negation. */ +function stripNegation(pattern) { + let i = 0; + while (i < pattern.length && pattern[i] === "!") i++; + return { inner: pattern.slice(i), negated: i % 2 === 1 }; +} + +/** Split a brace body on its top-level commas (a comma inside `{...}` or `[...]` is a member). */ +function splitBraceAlternatives(body) { + const alternatives = []; + let depth = 0; + let inClass = false; + let start = 0; + for (let i = 0; i < body.length; i++) { + const ch = body[i]; + if (ch === "[") inClass = true; + else if (ch === "]") inClass = false; + else if (!inClass && ch === "{") depth++; + else if (!inClass && ch === "}") depth--; + else if (!inClass && depth === 0 && ch === ",") { + alternatives.push(body.slice(start, i)); + start = i + 1; + } + } + alternatives.push(body.slice(start)); + return alternatives; +} + +/** + * Expand `{a,b}` into its alternatives (Bun matches one comma-separated branch; + * branches nest). A branch that spans a separator is not a form Bun's directory + * walk expands, so the whole pattern matches nothing — `null` reports that. + */ +function expandBraces(pattern) { + const expanded = []; + const recurse = (current) => { + const open = current.indexOf("{"); + if (open === -1) { + expanded.push(current); + return true; + } + let depth = 0; + let close = -1; + for (let i = open; i < current.length; i++) { + if (current[i] === "{") depth++; + else if (current[i] === "}") { + depth--; + if (depth === 0) { + close = i; + break; + } + } + } + if (close === -1) return false; + const body = current.slice(open + 1, close); + if (body.includes("/")) return false; + for (const alternative of splitBraceAlternatives(body)) { + if (!recurse(current.slice(0, open) + alternative + current.slice(close + 1))) return false; + } + return true; + }; + return recurse(pattern) ? expanded : null; +} + +const REGEX_SPECIAL = /[.+^${}()|[\]\\]/; + +/** + * Whether one glob segment matches one path segment: `*` and `?` stay inside the + * segment, `[...]` is a character class (`[!...]`/`[^...]` negate it, `a-z` is a + * range) and `\` escapes the next character. + */ +function segmentRegex(pattern) { let source = "^"; - for (const ch of pattern) { - if (ch === "*") source += "[^/]*"; - else if (ch === "?") source += "[^/]"; - else source += ch.replace(/[.+^${}()|[\]\\]/g, "\\$&"); + let i = 0; + while (i < pattern.length) { + const ch = pattern[i]; + if (ch === "*") { + source += "[^/]*"; + i++; + } else if (ch === "?") { + source += "[^/]"; + i++; + } else if (ch === "[") { + let end = i + 1; + if (pattern[end] === "!" || pattern[end] === "^") end++; + if (pattern[end] === "]") end++; + while (end < pattern.length && pattern[end] !== "]") end++; + if (end >= pattern.length) { + source += "\\["; + i++; + continue; + } + const body = pattern.slice(i + 1, end); + const negated = body.startsWith("!") || body.startsWith("^"); + const members = negated ? body.slice(1) : body; + source += `[${negated ? "^" : ""}${members.replace(/\\/g, "\\\\")}]`; + i = end + 1; + } else if (ch === "\\" && i + 1 < pattern.length) { + const next = pattern[i + 1]; + source += REGEX_SPECIAL.test(next) ? `\\${next}` : next; + i += 2; + } else { + source += REGEX_SPECIAL.test(ch) ? `\\${ch}` : ch; + i++; + } } - return new RegExp(`${source}$`).test(value); + return new RegExp(`${source}$`); } -/** Whether a workspace glob matches a manifest's directory; `**` spans segments. */ -function matchGlobDir(dir, pattern) { - const value = pathSegments(dir); - const glob = pathSegments(pattern); - // Bun's glob walk skips dot-directories; only a pattern without `*` or `?` reaches one. - if (/[*?]/.test(pattern) && value.some((segment) => segment.startsWith("."))) return false; +function matchGlobSegment(value, pattern) { + return segmentRegex(pattern).test(value); +} + +/** The message of the error a glob's segments raise when compiled, or `null` when they all compile. */ +function globError(pattern) { + const expanded = expandBraces(stripNegation(pattern).inner); + if (expanded === null) return null; + try { + for (const candidate of expanded) for (const segment of pathSegments(candidate)) segmentRegex(segment); + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + return null; +} + +/** Whether the path segments match the (brace-expanded) glob segments; `**` spans segments. */ +function matchSegments(value, glob) { function match(i, j) { if (j === glob.length) return i === value.length; if (glob[j] === "**") { @@ -277,30 +396,102 @@ function matchGlobDir(dir, pattern) { return match(0, 0); } +/** Whether a workspace glob matches a manifest's directory. */ +function matchGlobDir(dir, pattern) { + const expanded = expandBraces(pattern); + if (expanded === null) return false; + const value = pathSegments(dir); + // Bun's glob walk skips dot-directories; only a pattern without glob syntax reaches one. + if (isGlobPattern(pattern) && value.some((segment) => segment.startsWith("."))) return false; + for (const candidate of expanded) { + if (matchSegments(value, pathSegments(candidate))) return true; + } + return false; +} + +/** + * The workspace patterns a root manifest declares, in Bun's two accepted shapes: + * an array of strings, or an object whose `packages` key is an array of strings. + * Any other shape is refused with a named error, because a shape the audit + * misreads would hide the manifests Bun applies. + * + * @returns {{ patterns: string[], error: string | null }} + */ +export function workspacePatterns(workspaces) { + if (workspaces === undefined) return { patterns: [], error: null }; + if (Array.isArray(workspaces)) { + if (workspaces.some((pattern) => typeof pattern !== "string")) { + return { patterns: [], error: "`workspaces` is an array, but not every entry is a string" }; + } + return { patterns: workspaces, error: null }; + } + if (workspaces !== null && typeof workspaces === "object") { + const packages = workspaces.packages; + if (!Array.isArray(packages) || packages.some((pattern) => typeof pattern !== "string")) { + return { + patterns: [], + error: "`workspaces` is an object, but `workspaces.packages` is not an array of strings", + }; + } + return { patterns: packages, error: null }; + } + return { + patterns: [], + error: "`workspaces` is neither an array of strings nor an object with a `packages` array", + }; +} + /** * The manifests Bun applies when it installs: the root manifest, and the * manifests of the directories the root manifest's `workspaces` patterns name. - * The globs use the `*`, `?` and `**` segments Bun resolves; a `!`-prefixed - * pattern removes an earlier match (the last matching pattern wins). + * A pattern without glob syntax is a literal directory; a glob one is walked with + * `*`, `?`, `**`, `[...]` and `{a,b}`. A `!`-negated pattern removes an earlier + * glob match it matches; a literal directory is never removed. + * + * @returns {{ applied: Set, error: string | null, pattern?: string }} + * `error` names a `workspaces` shape or pattern the audit cannot read; `applied` + * is then incomplete and must not be used. `pattern` is set only for a pattern. */ export function appliedManifestPaths(manifests) { const applied = new Set(["package.json"]); const root = manifests.find((pj) => pj.path === "package.json"); - const workspaces = root?.json?.workspaces; - if (!Array.isArray(workspaces)) return applied; - const patterns = workspaces.filter((pattern) => typeof pattern === "string"); + const { patterns, error } = workspacePatterns(root?.json?.workspaces); + if (error !== null) return { applied, error }; + const literals = []; + const globs = []; + for (const raw of patterns) { + if (raw === "" || raw === "." || raw === "./" || raw === ".\\") continue; + if (isGlobPattern(raw)) globs.push(raw); + else literals.push(raw); + } + const byDir = new Map(); for (const pj of manifests) { if (pj.path === "package.json") continue; - const dir = pathSegments(pj.path).slice(0, -1).join("/"); - let included = false; - for (const raw of patterns) { - const negated = raw.startsWith("!"); - const pattern = (negated ? raw.slice(1) : raw).replace(/\/+$/, ""); - if (pattern !== "" && matchGlobDir(dir, pattern)) included = !negated; + byDir.set(pathSegments(pj.path).slice(0, -1).join("/"), pj.path); + } + for (const literal of literals) { + const path = byDir.get(pathSegments(literal).join("/")); + if (path !== undefined) applied.add(path); + } + for (const glob of globs) { + const reason = globError(glob); + if (reason !== null) { + return { applied, error: `workspaces pattern \`${glob}\` cannot be read: ${reason}`, pattern: glob }; + } + } + for (let i = 0; i < globs.length; i++) { + const { inner, negated } = stripNegation(globs[i]); + if (negated) continue; // a negated pattern removes an earlier glob match, and adds none of its own + for (const [dir, path] of byDir) { + if (!matchGlobDir(dir, inner)) continue; + const excluded = globs.slice(i + 1).some((later) => { + const removed = stripNegation(later); + return removed.negated && matchGlobDir(dir, removed.inner); + }); + if (!excluded) applied.add(path); } - if (included) applied.add(pj.path); } - return applied; + return { applied, error: null }; } /** @@ -324,12 +515,14 @@ export function appliedManifestPaths(manifests) { * | {kind: "nested-override", name: string, path: string} * | {kind: "resolution", name: string, path: string, key: string} * | {kind: "unused-manifest", name: string, path: string} - * | {kind: "unpinned", name: string}> + * | {kind: "unpinned", name: string} + * | {kind: "workspaces", message: string, pattern?: string}> */ export function auditExcludes({ excludes, exceptionEntries, exceptionErrors, packageJsons }) { const problems = []; const manifests = packageJsons ?? []; - const applied = appliedManifestPaths(manifests); + const { applied, error: workspacesError, pattern } = appliedManifestPaths(manifests); + if (workspacesError !== null) return [{ kind: "workspaces", message: workspacesError, pattern }]; for (const name of excludes ?? []) { let covered = false; for (const key of (exceptionEntries ?? new Map()).keys()) { diff --git a/test/check-dep-ages-blockers.test.ts b/test/check-dep-ages-blockers.test.ts index 2577af39..63f23687 100644 --- a/test/check-dep-ages-blockers.test.ts +++ b/test/check-dep-ages-blockers.test.ts @@ -188,6 +188,10 @@ describe("the exclusion audit counts only the declarations Bun applies", () => { ["a ? pattern does not match two characters", ["packages/?"], "packages/ab", false], ["a ./ prefix and trailing slash", ["./packages/*/"], "packages/a", true], ["a * pattern does not reach a nested directory", ["packages/*"], "packages/a/b", false], + ["an object-form workspace", { packages: ["packages/*"] }, "packages/w", true], + ["a character-class workspace", ["packages/[wx]"], "packages/w", true], + ["a brace workspace", ["packages/{w,z}"], "packages/w", true], + ["a literal named once and negated", ["packages/w", "!packages/w"], "packages/w", true], ])("%s", (_label, workspaces, dir, expectApplied) => { appliedFixture((root) => { writeManifest(root, "package.json", { name: "fixture", workspaces }); @@ -204,6 +208,38 @@ describe("the exclusion audit counts only the declarations Bun applies", () => { }); }); + it("refuses an unrecognised workspaces shape with a named error", () => { + appliedFixture((root) => { + writeManifest(root, "package.json", { name: "fixture", workspaces: "packages/*" }); + writeManifest(root, join("packages", "w", "package.json"), { + name: "w", + dependencies: { "dep-a": "1.0.0" }, + }); + const output = runGate(root); + expect(output).toContain( + "`workspaces` is neither an array of strings nor an object with a `packages` array", + ); + expect(output).toContain("Remedy: declare `workspaces` as an array of strings"); + expect(output).not.toContain("Checking"); + }); + }); + + it("refuses a workspaces pattern with a reversed class range, naming it, without a stack trace", () => { + appliedFixture((root) => { + writeManifest(root, "package.json", { name: "fixture", workspaces: ["packages/[z-a]"] }); + writeManifest(root, join("packages", "a", "package.json"), { + name: "a", + dependencies: { "dep-a": "1.0.0" }, + }); + const output = runGate(root); + expect(output).toContain("workspaces pattern `packages/[z-a]` cannot be read:"); + expect(output).toContain("Remedy: fix or remove the `workspaces` pattern `packages/[z-a]`."); + expect(output).not.toContain("declare `workspaces` as an array of strings"); + expect(output).not.toContain("Checking"); + expect(output).not.toMatch(/\n\s+at /); + }); + }); + it("refuses an excluded name pinned exactly only in a dot-directory under a * workspace pattern", () => { appliedFixture((root) => { writeManifest(root, "package.json", { name: "fixture", workspaces: ["packages/*"] }); diff --git a/test/check-dep-ages-install-age-bun.test.ts b/test/check-dep-ages-install-age-bun.test.ts index c092a529..e06beaf7 100644 --- a/test/check-dep-ages-install-age-bun.test.ts +++ b/test/check-dep-ages-install-age-bun.test.ts @@ -358,19 +358,70 @@ it.each([ ["packages/x", "packages/y"], ["packages/x", "packages/y"], ], + [ + "the object form of workspaces", + { packages: ["packages/*"] }, + ["packages/a", "tools/t"], + ["packages/a"], + ], + [ + "a character class", + ["packages/[ab]"], + ["packages/a", "packages/b", "packages/c"], + ["packages/a", "packages/b"], + ], + [ + "a negated character class", + ["packages/[!a]"], + ["packages/a", "packages/b", "packages/c"], + ["packages/b", "packages/c"], + ], + ["a ^-negated character class", ["packages/[^a]"], ["packages/a", "packages/b"], ["packages/b"]], + [ + "a dot-directory under a character class", + ["packages/[.a]*"], + ["packages/.h", "packages/a", "packages/ab"], + ["packages/a", "packages/ab"], + ], + [ + "a dot-directory under brace alternatives", + ["packages/{.h,a}"], + ["packages/.h", "packages/a", "packages/b"], + ["packages/a"], + ], + [ + "a brace branch spanning /", + ["packages/{a/b,c}"], + ["packages/a/b", "packages/c", "packages/a"], + [], + ], + [ + "brace alternatives", + ["packages/{a,b}"], + ["packages/a", "packages/b", "packages/c"], + ["packages/a", "packages/b"], + ], + [ + "a negation of a literal pattern", + ["packages/x", "!packages/x"], + ["packages/x", "packages/y"], + ["packages/x"], + ], ] as const)( "the gate applies the same workspaces real Bun installs for %s", async (_label, workspaces, dirs, expected) => { const root = mkdtempSync(join(tmpdir(), "age-workspaces-")); try { const manifests: Record = { - "package.json": { name: "fixture", workspaces: [...workspaces] }, + "package.json": { name: "fixture", workspaces }, }; for (const dir of dirs) manifests[`${dir}/package.json`] = { name: `w-${dir.replaceAll("/", "-")}` }; const project = writeProject(root, manifests, "http://127.0.0.1:1"); const install = await bunInstall(project, ["--no-cache"]); expect({ exit: install.exit, output: install.output }).toMatchObject({ exit: 0 }); - const lock = parseBunLock(readFileSync(join(project, "bun.lock"), "utf8")); + const lockPath = join(project, "bun.lock"); + // Bun writes no lockfile when no workspace and no dependency is installed. + const lock = existsSync(lockPath) ? parseBunLock(readFileSync(lockPath, "utf8")) : { workspaces: {} }; const installed = Object.keys(lock.workspaces) .filter((key) => key !== "") .map((key) => `${key}/package.json`) @@ -378,7 +429,9 @@ it.each([ expect(installed).toEqual(expected.map((dir) => `${dir}/package.json`)); const packageJsons = Object.entries(manifests).map(([path, json]) => ({ path, json })); - const gate = [...appliedManifestPaths(packageJsons)].filter((path) => path !== "package.json").sort(); + const gate = [...appliedManifestPaths(packageJsons).applied] + .filter((path) => path !== "package.json") + .sort(); expect(gate).toEqual(installed); } finally { rmSync(root, { recursive: true, force: true });