diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d108664ae9..6a0f41d52f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -121,6 +121,23 @@ jobs: - name: Check that diagrams resolve their colours from tokens run: node scripts/check-diagram-tokens.mjs + orphaned-images: + timeout-minutes: 5 + runs-on: ubuntu-24.04 + steps: + - name: Checkout 🛎️ + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup Node 🔧 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .node-version + + # No dependencies needed: the scanner is plain Node. Its own unit tests + # run in the `test` job. + - name: Check for orphaned images + run: node scripts/check-orphaned-images.mjs + build: timeout-minutes: 20 runs-on: ubuntu-24.04 diff --git a/.mergify.yml b/.mergify.yml index e5e0d77175..b426fd36f9 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -32,6 +32,7 @@ merge_protections: - check-success = test - check-success = diagram-tokens - check-success = test-broken-links + - check-success = orphaned-images - or: - check-success=Cloudflare Pages # fork can't have the preview build diff --git a/AGENTS.md b/AGENTS.md index 3fa440d0d6..e7420e94b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,6 +50,7 @@ pnpm test # Run Vitest unit tests # Safety pnpm check:internal-leaks # Scan docs for leaked internal information pnpm check:diagram-tokens # Scan diagrams for hardcoded colors +pnpm check:orphaned-images # Scan for images nothing references ``` ## Key Directories diff --git a/package.json b/package.json index bf446e2fcb..f61f9102cb 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "check:config-examples": "node scripts/validate-config-examples.mjs", "check:diagram-tokens": "node scripts/check-diagram-tokens.mjs", "check:internal-leaks": "node scripts/check-internal-leaks.mjs", + "check:orphaned-images": "node scripts/check-orphaned-images.mjs", "check:links": "linkinator / enterprise/ --server-root dist --config linkinator.config.mjs" }, "devDependencies": { diff --git a/scripts/check-orphaned-images.mjs b/scripts/check-orphaned-images.mjs new file mode 100644 index 0000000000..709eb66d61 --- /dev/null +++ b/scripts/check-orphaned-images.mjs @@ -0,0 +1,181 @@ +#!/usr/bin/env node +/** + * Scan `src/content/` for images nothing references. + * + * Prose moves — a page is rewritten, an example is cut, a page is deleted + * outright — and the image it imported is easy to leave behind, because + * removing a page never fails a build the way removing a component does. + * docs#12728 is the case that prompted this: it dropped a billing example's + * two `` uses and both `import` lines, and the PNGs stayed on disk + * with nothing pointing at them. + * + * A page imports an image by a relative path that always contains + * `/images/` — `../../images/billing/plan.png`, `./images/hero.jpg`, three + * levels up, one level up, it does not matter, that suffix is constant. So + * for every image file that sits under some `images/` directory anywhere in + * `src/content/`, this checks whether that exact suffix appears anywhere in + * the docs source. No suffix found anywhere means nothing imports the file, + * under any relative path. The leading `/` matters: it is what stops an + * unrelated string like `cdn-images/billing/plan.png` from reading as a + * reference to `images/billing/plan.png` just because one contains the + * other. + * + * This deliberately does NOT match by basename alone: two images in + * different directories can share a filename (`config-editor.png` exists + * under both `configuration/` and, until it was deleted as part of this same + * change, the removed `workflow/writing-your-first-rule/` page), and a + * basename-only sweep reads the orphan as referenced because a same-named + * file elsewhere genuinely is. That false negative is exactly how + * `workflow/writing-your-first-rule/config-editor.png` survived a first, + * basename-only pass. + * + * Nor does it assume images only live under `src/content/images/` — most do, + * but `src/content/docs/images/merge-queue-hero.jpg` is a real, referenced + * counterexample sitting right next to the page that imports it. Any + * `images/` directory under `src/content/` counts. + * + * What it cannot see: an image loaded through `import.meta.glob` or any + * other pattern that does not spell out the path literally. Nothing in the + * docs does that today; if it starts, add the path to ALLOWLIST below rather + * than trying to make the regex understand glob patterns. + * + * Two images under different `images/` roots can share the same suffix + * (same subpath after the nearest `images/` ancestor) and so need the same + * needle. `findOrphans` cannot tell which one an import actually resolves + * to, so it reports every image in a colliding group rather than guessing. + * + * Usage: + * node scripts/check-orphaned-images.mjs + * node scripts/check-orphaned-images.mjs --json + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const ROOT = path.resolve(fileURLToPath(import.meta.url), '..', '..'); +const CONTENT_DIR = 'src/content'; +const SEARCH_DIRS = ['src', 'integrations', 'plugins']; +const IMAGES_DIRNAME = 'images'; +const SEARCH_EXTENSIONS = ['.mdx', '.md', '.astro', '.ts', '.tsx', '.js', '.mjs']; +const IMAGE_EXTENSIONS = ['.png', '.jpg', '.jpeg', '.gif', '.svg', '.webp', '.avif']; + +/** + * Paths relative to the repo root that are known to be unreferenced on + * purpose, each with why. Empty today — a legitimate entry should be rare, + * since an image that nothing renders belongs in an asset host, not the + * docs source tree. + */ +export const ALLOWLIST = new Set([]); + +function* walk(dir) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) yield* walk(full); + else yield full; + } +} + +/** + * Image files under any `images/` directory within `src/content/`, as paths + * relative to the repo root (posix separators). A file with no `images/` + * ancestor is outside this checker's scope: nothing imports an image by a + * path containing `/images/` unless it sits under a directory named that. + */ +export function findImages(contentDir = path.join(ROOT, CONTENT_DIR)) { + const found = []; + for (const full of walk(contentDir)) { + if (!IMAGE_EXTENSIONS.includes(path.extname(full).toLowerCase())) continue; + const parts = path.relative(ROOT, full).split(path.sep); + if (parts.includes(IMAGES_DIRNAME)) found.push(parts.join('/')); + } + return found.sort(); +} + +/** + * The substring a relative import of `imagePath` (repo-relative, posix) must + * contain: `/images/` through the rest of the path, taken from the nearest + * `images` segment. Anchored on the leading slash so an unrelated + * `cdn-images/...` cannot satisfy it by accident. + */ +export function needleFor(imagePath) { + const parts = imagePath.split('/'); + const idx = parts.lastIndexOf(IMAGES_DIRNAME); + return `/${parts.slice(idx).join('/')}`; +} + +/** Concatenated text of every scanned source file under SEARCH_DIRS. */ +export function buildCorpus(root = ROOT, dirs = SEARCH_DIRS) { + let corpus = ''; + for (const dir of dirs) { + const abs = path.join(root, dir); + if (!fs.existsSync(abs)) continue; + for (const full of walk(abs)) { + if (SEARCH_EXTENSIONS.includes(path.extname(full).toLowerCase())) { + corpus += fs.readFileSync(full, 'utf8'); + corpus += '\n'; + } + } + } + return corpus; +} + +/** + * Images (repo-relative paths) that `corpus` never mentions, plus any image + * whose needle collides with another image's — two images living under + * different `images/` roots can end up with the identical suffix (e.g. + * `src/content/images/billing/hero.png` and + * `src/content/docs/images/billing/hero.png` both need `/images/billing/hero.png`), + * and a single import of either would otherwise clear both regardless of + * which one it actually references. Fail closed: a colliding pair is always + * reported, even when the needle is present, so a human resolves the + * ambiguity instead of the checker silently guessing. + */ +export function findOrphans(corpus, images = findImages(), allowlist = ALLOWLIST) { + const needleCounts = new Map(); + for (const img of images) { + const needle = needleFor(img); + needleCounts.set(needle, (needleCounts.get(needle) ?? 0) + 1); + } + return images.filter((img) => { + if (allowlist.has(img)) return false; + const needle = needleFor(img); + if (needleCounts.get(needle) > 1) return true; + return !corpus.includes(needle); + }); +} + +function main(argv) { + const jsonMode = argv.includes('--json'); + + const images = findImages(); + const corpus = buildCorpus(); + const orphans = findOrphans(corpus, images); + + if (jsonMode) { + process.stdout.write(`${JSON.stringify(orphans, null, 2)}\n`); + return orphans.length === 0 ? 0 : 1; + } + + console.log(`Scanned ${images.length} image(s) under ${CONTENT_DIR}/**/${IMAGES_DIRNAME}/.`); + if (orphans.length === 0) { + console.log('No orphaned images found.'); + return 0; + } + console.error(`\n${orphans.length} orphaned image(s) — nothing imports them:\n`); + for (const o of orphans) { + console.error(` ${o}`); + } + console.error( + '\nDelete the file, or if the page that used it still needs it, restore the\n' + + '`import` and `` that reference it. If it is genuinely unreferenced\n' + + 'on purpose, add it to ALLOWLIST in scripts/check-orphaned-images.mjs with why.' + ); + return 1; +} + +// Run as a CLI only when invoked directly, so tests can import the helpers. +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exit(main(process.argv.slice(2))); +} diff --git a/scripts/check-orphaned-images.test.mjs b/scripts/check-orphaned-images.test.mjs new file mode 100644 index 0000000000..a0c31a7b03 --- /dev/null +++ b/scripts/check-orphaned-images.test.mjs @@ -0,0 +1,120 @@ +import { describe, expect, it } from 'vitest'; +import { buildCorpus, findImages, findOrphans, needleFor } from './check-orphaned-images.mjs'; + +describe('needleFor', () => { + it('anchors on the nearest images/ ancestor, leading slash included', () => { + expect(needleFor('src/content/images/billing/plan.png')).toBe('/images/billing/plan.png'); + expect(needleFor('src/content/docs/images/hero.jpg')).toBe('/images/hero.jpg'); + }); +}); + +describe('findOrphans', () => { + it('reports an image no import mentions', () => { + const corpus = 'import hero from "../../images/other/hero.png"\n'; + expect( + findOrphans(corpus, [ + 'src/content/images/billing/plan.png', + 'src/content/images/other/hero.png', + ]) + ).toEqual(['src/content/images/billing/plan.png']); + }); + + it('matches any relative import depth, not just one style', () => { + const corpus = [ + 'import a from "../../images/billing/plan.png"', + 'import b from "../../../images/ci-insights/token.png"', + 'import c from "./images/merge-queue-hero.jpg"', + ].join('\n'); + expect( + findOrphans(corpus, [ + 'src/content/images/billing/plan.png', + 'src/content/images/ci-insights/token.png', + 'src/content/docs/images/merge-queue-hero.jpg', + ]) + ).toEqual([]); + }); + + it('finds images outside src/content/images/, under any images/ directory', () => { + // src/content/docs/images/merge-queue-hero.jpg is the real counterexample: + // an image colocated with the page that imports it rather than living + // under the shared src/content/images/ tree. + const corpus = 'import hero from "./images/merge-queue-hero.jpg"\n'; + expect(findOrphans(corpus, ['src/content/docs/images/merge-queue-hero.jpg'])).toEqual([]); + expect(findOrphans('', ['src/content/docs/images/merge-queue-hero.jpg'])).toEqual([ + 'src/content/docs/images/merge-queue-hero.jpg', + ]); + }); + + it('does not let a same-named file elsewhere hide a real orphan', () => { + // Two images share a basename in different directories; only one is + // imported. A basename-only check would call both referenced — this is + // the false negative that let workflow/writing-your-first-rule/ + // config-editor.png survive a first, basename-only sweep. + const corpus = 'import x from "../../images/configuration/config-editor.png"\n'; + const images = [ + 'src/content/images/configuration/config-editor.png', + 'src/content/images/workflow/first-rule/config-editor.png', + ]; + expect(findOrphans(corpus, images)).toEqual([ + 'src/content/images/workflow/first-rule/config-editor.png', + ]); + }); + + it('does not let a directory-prefix collision produce a false reference', () => { + // "/images/a/logo.png" must not read as referenced by an import of + // "images/prefix-a/logo.png" just because the shorter string is a + // substring of the longer one everywhere except at the images/ anchor. + const corpus = 'import x from "../../images/prefix-a/logo.png"\n'; + expect(findOrphans(corpus, ['src/content/images/a/logo.png'])).toEqual([ + 'src/content/images/a/logo.png', + ]); + }); + + it('does not let an unrelated cdn-images/ path satisfy the reference', () => { + // Before the leading-slash anchor, "cdn-images/billing/plan.png" (no + // slash before "images") would have contained "images/billing/plan.png" + // as a plain substring and wrongly cleared the file. + const corpus = 'See https://cdn-images/billing/plan.png for the source asset.\n'; + expect(findOrphans(corpus, ['src/content/images/billing/plan.png'])).toEqual([ + 'src/content/images/billing/plan.png', + ]); + }); + + it('honors the allowlist', () => { + const corpus = ''; + const images = ['src/content/images/billing/plan.png']; + expect(findOrphans(corpus, images, new Set(images))).toEqual([]); + }); + + it('reports an unreferenced .avif file', () => { + const corpus = 'import hero from "../../images/other/hero.png"\n'; + expect(findOrphans(corpus, ['src/content/images/billing/plan.avif'])).toEqual([ + 'src/content/images/billing/plan.avif', + ]); + }); + + it('fails closed when two images share a needle, even if it is present', () => { + // Same subpath after the nearest images/ ancestor, different roots — a + // single import of either satisfies both needles, so which one is truly + // referenced is ambiguous. Report both rather than clearing either. + const corpus = 'import hero from "../../images/billing/hero.png"\n'; + const images = [ + 'src/content/images/billing/hero.png', + 'src/content/docs/images/billing/hero.png', + ]; + expect(findOrphans(corpus, images)).toEqual(images); + }); +}); + +describe('published docs', () => { + it('contain no orphaned images', () => { + const images = findImages(); + const corpus = buildCorpus(); + const orphans = findOrphans(corpus, images); + if (orphans.length) { + throw new Error(`Orphaned image(s) — nothing imports them:\n${orphans.join('\n')}`); + } + // Sanity: the scan actually walked the images tree. + expect(images.length).toBeGreaterThan(50); + }); +}); diff --git a/src/content/images/billing/plan.png b/src/content/images/billing/plan.png deleted file mode 100644 index ae62be9e36..0000000000 Binary files a/src/content/images/billing/plan.png and /dev/null differ