fix: exclude base-marked generated files from comment checks (#6091)

This commit is contained in:
Miguel Ángel
2026-10-22 21:18:17 +00:00
committed by GitHub
parent 807803a452
commit 36c4a9fd80
4 changed files with 100 additions and 9 deletions
+16 -1
View File
@@ -882,6 +882,19 @@ export function checkComments(root, files, scope = { kind: "all" }) {
return { failures: [...failures, ...blockFailures], warnings, examined };
}
export function generatedPaths(root, ref, paths) {
if (paths.length === 0) return new Set();
const attributes = execFileSync(
"git",
["check-attr", `--source=${ref}`, "-z", "--stdin", "linguist-generated"],
{ cwd: root, encoding: "utf8", input: `${paths.join("\0")}\0`, maxBuffer: 64e6 },
).split("\0");
const generated = new Set();
for (let i = 0; i + 2 < attributes.length; i += 3)
if (["set", "true"].includes(attributes[i + 2])) generated.add(attributes[i]);
return generated;
}
function mergeBase(root, ref) {
return execFileSync("git", ["merge-base", ref, "HEAD"], { cwd: root, encoding: "utf8" }).trim();
}
@@ -994,8 +1007,10 @@ if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
const base =
explicit.length > 0 ? null : mergeBase(root, process.env.COMMENT_CHECK_BASE ?? "origin/main");
// The probe is graded by the self-check only, or every PR that edits it would go red.
const files =
const candidates =
base === null ? explicit : changedFiles(root, base).filter((file) => file !== PROBE);
const generated = base === null ? new Set() : generatedPaths(root, base, candidates);
const files = candidates.filter((file) => !generated.has(file));
const scope = base === null ? { kind: "all" } : { kind: "lines", ...diffScope(root, base) };
const { failures, warnings, examined } = checkComments(root, files, scope);
+41
View File
@@ -653,6 +653,47 @@ function runCli(t, base, head, args = [], overrides = {}) {
}
const KIT = { "src/kit.ts": ["export const needsReview = 1;", ""].join("\n") };
const GENERATED = "src/_gen/output.ts";
const ATTRIBUTES = { ".gitattributes": "src/_gen/** linguist-generated=true\n" };
for (const exists of [false, true])
test(`the CLI skips base-marked generated citations, existing ${exists}`, (t) => {
const base = { ...KIT, ...ATTRIBUTES, ...(exists ? { [GENERATED]: KIT["src/kit.ts"] } : {}) };
const head = {
...base,
[GENERATED]: "// The `missing.ts` module owns this.\nexport const value = 1;\n",
};
const { status, output } = runCli(t, base, head);
assert.equal(status, 0, output);
});
test("the CLI does not accept a citation exemption added by the change", (t) => {
const { status, output } = runCli(t, KIT, {
...KIT,
...ATTRIBUTES,
[GENERATED]: "// The `missing.ts` module owns this.\nexport const value = 1;\n",
});
assert.equal(status, 1, output);
});
for (const attribute of ["linguist-generated", "linguist-generated=false", "-linguist-generated"])
test(`the CLI respects the base attribute ${attribute}`, (t) => {
const base = { ...KIT, ".gitattributes": `src/_gen/** ${attribute}\n` };
const { status, output } = runCli(t, base, {
...base,
[GENERATED]: "// The `missing.ts` module owns this.\nexport const value = 1;\n",
});
assert.equal(status, attribute === "linguist-generated" ? 0 : 1, output);
});
test("generated source remains a valid citation target", (t) => {
const base = { ...KIT, ...ATTRIBUTES, [GENERATED]: "export const value = 1;\n" };
const { status, output } = runCli(t, base, {
...base,
"src/subject.ts": "// The `src/_gen/output.ts` module owns this.\nexport const subject = 1;\n",
});
assert.equal(status, 0, output);
});
const explains = (symbol) =>
oneLiner(
`// The kit stops carrying \`${symbol}\`: the verdict map owns what a review row means.`,
+13 -8
View File
@@ -6,7 +6,7 @@
import { execFileSync } from "node:child_process";
import { readFileSync } from "node:fs";
import { extname } from "node:path";
import { commentBlocks } from "./check-comment-citations.mjs";
import { commentBlocks, generatedPaths } from "./check-comment-citations.mjs";
const MAX_BLOCK_LINES = 12;
@@ -100,10 +100,10 @@ function atBase(ref, file) {
// The bar for a new file: its package's share over files that existed at the base, so new files
// never set their own bar. A package that is new as a whole is held to every package's share.
function packageTotal(dir, isNew) {
function packageTotal(dir, isNew, generated) {
return git(["ls-files", dir])
.split("\n")
.filter((file) => isSource(file) && !isNew.has(file))
.filter((file) => isSource(file) && !isNew.has(file) && !generated.has(file))
.reduce(
(sum, file) => {
const entry = measure(readFileSync(file, "utf8"), extname(file));
@@ -113,16 +113,21 @@ function packageTotal(dir, isNew) {
);
}
function packageShare(pkg, isNew) {
const own = packageTotal(`${pkg}/src`, isNew);
return share(own.lines > 0 ? own : packageTotal("packages", isNew));
function packageShare(pkg, isNew, generated) {
const own = packageTotal(`${pkg}/src`, isNew, generated);
return share(own.lines > 0 ? own : packageTotal("packages", isNew, generated));
}
if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
const ref = git(["merge-base", process.env.COMMENT_CHECK_BASE ?? "origin/main", "HEAD"]).trim();
const generated = generatedPaths(
process.cwd(),
ref,
git(["ls-files", "packages"]).split("\n").filter(Boolean),
);
// Against the working tree, where contents are read from; in CI that is HEAD.
const changed = changedPaths(git(["diff", "--name-status", "-M", ref, "--", "packages"]))
.filter(({ status, to }) => status !== "D" && isSource(to))
.filter(({ status, to }) => status !== "D" && isSource(to) && !generated.has(to))
.sort((a, b) => a.to.localeCompare(b.to));
const graded = changed.map(({ from, to }) => ({ to, onBase: atBase(ref, from) }));
@@ -132,7 +137,7 @@ if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
for (const { to, onBase } of graded) {
const head = measure(readFileSync(to, "utf8"), extname(to));
const pkg = packageOf(to);
if (onBase === null && !shares.has(pkg)) shares.set(pkg, packageShare(pkg, isNew));
if (onBase === null && !shares.has(pkg)) shares.set(pkg, packageShare(pkg, isNew, generated));
const base = onBase === null ? null : measure(onBase, extname(to));
problems.push(...judge(to, head, base, shares.get(pkg) ?? 0));
}
+30
View File
@@ -209,6 +209,36 @@ function runCli(t, base, head) {
const FILE = "packages/web/src/a.ts";
const code = "export const a = 1;\nexport const b = 2;\nexport const c = 3;\n";
const GENERATED = "packages/web/src/_gen/output.ts";
const ATTRIBUTES = { ".gitattributes": "packages/web/src/_gen/** linguist-generated=true\n" };
for (const exists of [false, true])
test(`the CLI skips base-marked generated output, existing ${exists}`, (t) => {
const base = { ...ATTRIBUTES, [FILE]: code, ...(exists ? { [GENERATED]: code } : {}) };
const { status, output } = runCli(t, base, {
...base,
[GENERATED]: `// generated docs\n${code}`,
});
assert.equal(status, 0, output);
});
test("the CLI does not accept a generated exemption added by the change", (t) => {
const base = { [FILE]: code, [GENERATED]: code };
const { status, output } = runCli(t, base, {
...base,
...ATTRIBUTES,
[GENERATED]: `// authored docs\n${code}`,
});
assert.equal(status, 1, output);
});
test("generated comments do not raise the bar for new authored source", (t) => {
const base = { ...ATTRIBUTES, [FILE]: code, [GENERATED]: `${wall(30)}\n${code}` };
const fresh = "packages/web/src/new.ts";
const { status, output } = runCli(t, base, { ...base, [fresh]: `// authored docs\n${code}` });
assert.equal(status, 1, output);
assert.match(output, /NEW file over its package's comment share/);
});
test("the CLI fails a diff that adds comment lines to a package source file", (t) => {
const { status, output } = runCli(t, { [FILE]: code }, { [FILE]: `// why b is two\n${code}` });