Skip to content

Commit f330a98

Browse files
committed
ci(danger): PR-template section check
1 parent 18818c9 commit f330a98

5 files changed

Lines changed: 223 additions & 3 deletions

File tree

util/danger/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ npm --prefix util/danger run danger:ci # run Danger in CI mode (requires Git
3636
- Aggregate PR source churn triggers a warning above 5000 lines (much more generous than the per-commit limit, since well-sliced commits can still amount to a large overall change).
3737
- PR description length is checked against a log-scaled floor (`80 * log2(churn)` characters). Tiny diffs need ~80 chars; ~1k-line changes need ~800; ~30k-line changes need ~1200. HTML comments in the body (e.g. PR template scaffolding) are stripped before measurement.
3838
- Feature PRs (`feat:` commits, `feat` PR title, or `feature` label) without any `docs/` change get a light warning. Opt out with the `no-docs-needed` label or a `[skip danger docs]` marker in the PR body.
39-
- The PR template (`.github/pull_request_template.md`) mirrors these rules; following it tends to satisfy all of them automatically.
39+
- The PR template (`.github/pull_request_template.md`) mirrors these rules; following it tends to satisfy all of them automatically. Section headers from the template (any level) that are missing from the PR body are reported as an informational note rather than a warning.
4040

4141
## Updating rules
4242

util/danger/fixtures/sample-pr.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,6 @@
3232
],
3333
"prBody": "Sample rationale\\n\\nTesting: unit tests locally",
3434
"prTitle": "Sample Danger fixture",
35-
"labels": []
35+
"labels": [],
36+
"prTemplate": "_What this PR does and why._\n\n## Changes\n\n_Replace the lines that apply._\n\n## Testing\n\n_How this change stays tested._\n\n## Documentation\n\n_What was updated._\n"
3637
}

util/danger/logic.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,33 @@ import {
1414
evaluateDanger,
1515
expectedBodyLength,
1616
parseCommitSummary,
17+
parsePrTemplateSections,
18+
prTemplateInfos,
1719
basicChecks,
1820
summarizeScopes,
1921
validateCommits,
2022
type CommitInfo,
2123
type DangerInputs,
2224
} from "./logic";
2325

26+
const sampleTemplate = [
27+
"<!-- Fill this in yourself. -->",
28+
"",
29+
"_What this PR does and why._",
30+
"",
31+
"## Changes",
32+
"",
33+
"_Replace the lines that apply._",
34+
"",
35+
"## Testing",
36+
"",
37+
"_How this change stays tested._",
38+
"",
39+
"## Documentation",
40+
"",
41+
"_What was updated._",
42+
].join("\n");
43+
2444
describe("parseCommitSummary", () => {
2545
// Ensures we correctly extract type, scope, and subject when format is valid.
2646
it("parses valid commit summaries", () => {
@@ -139,6 +159,123 @@ describe("commitSizeInfos", () => {
139159
});
140160
});
141161

162+
describe("parsePrTemplateSections", () => {
163+
// Pulls H2 headers in document order; ignores intro paragraphs and comments.
164+
it("extracts H2 headers from the template", () => {
165+
expect(parsePrTemplateSections(sampleTemplate)).toEqual(["Changes", "Testing", "Documentation"]);
166+
});
167+
168+
// No headers means no sections to enforce.
169+
it("returns an empty array when there are no headers", () => {
170+
expect(parsePrTemplateSections("just a paragraph\n\nno headers here")).toEqual([]);
171+
});
172+
173+
// Handles any ATX heading level so future templates can mix H1/H2/H3.
174+
it("extracts headings of any ATX level", () => {
175+
const template = [
176+
"# Summary",
177+
"## Changes",
178+
"### Subsection",
179+
"#### Detail",
180+
"##### Deeper",
181+
"###### Deepest",
182+
].join("\n");
183+
expect(parsePrTemplateSections(template)).toEqual([
184+
"Summary",
185+
"Changes",
186+
"Subsection",
187+
"Detail",
188+
"Deeper",
189+
"Deepest",
190+
]);
191+
});
192+
193+
// Strips the optional ATX closing run of `#` characters.
194+
it("strips trailing closing hashes from ATX headers", () => {
195+
expect(parsePrTemplateSections("## Changes ##\n### Testing ###")).toEqual(["Changes", "Testing"]);
196+
});
197+
198+
// Dedupes by title (case-insensitive) so cross-level repeats don't double-flag.
199+
it("dedupes repeated titles across levels", () => {
200+
const template = ["## Notes", "### notes", "#### NOTES"].join("\n");
201+
expect(parsePrTemplateSections(template)).toEqual(["Notes"]);
202+
});
203+
});
204+
205+
describe("prTemplateInfos", () => {
206+
// A body that includes every template header produces no info.
207+
it("stays quiet when all template sections are present", () => {
208+
const body = [
209+
"Rationale for the change.",
210+
"",
211+
"## Changes",
212+
"- Source: tweak.",
213+
"",
214+
"## Testing",
215+
"Ran the suite.",
216+
"",
217+
"## Documentation",
218+
"No user-facing change.",
219+
].join("\n");
220+
expect(prTemplateInfos(body, ["Changes", "Testing", "Documentation"])).toEqual([]);
221+
});
222+
223+
// Missing sections are listed in the single info message.
224+
it("lists missing sections", () => {
225+
const body = ["Rationale.", "", "## Changes", "- Source: tweak."].join("\n");
226+
const infos = prTemplateInfos(body, ["Changes", "Testing", "Documentation"]);
227+
expect(infos).toHaveLength(1);
228+
expect(infos[0]).toContain("**Testing**");
229+
expect(infos[0]).toContain("**Documentation**");
230+
expect(infos[0]).not.toContain("**Changes**");
231+
});
232+
233+
// Header matching is case-insensitive so contributors don't trip over capitalization.
234+
it("matches headers case-insensitively", () => {
235+
const body = ["## changes", "## testing", "## documentation"].join("\n");
236+
expect(prTemplateInfos(body, ["Changes", "Testing", "Documentation"])).toEqual([]);
237+
});
238+
239+
// The body can use a different heading level than the template — title alone is what matters.
240+
it("matches sections regardless of heading level", () => {
241+
const body = ["# Changes", "### Testing", "###### Documentation"].join("\n");
242+
expect(prTemplateInfos(body, ["Changes", "Testing", "Documentation"])).toEqual([]);
243+
});
244+
245+
// An empty body is already covered by the empty-description warning — skip to avoid noise.
246+
it("skips when the PR body is empty", () => {
247+
expect(prTemplateInfos("", ["Changes", "Testing"])).toEqual([]);
248+
expect(prTemplateInfos(" \n\n ", ["Changes", "Testing"])).toEqual([]);
249+
});
250+
251+
// A template with no headers produces no info.
252+
it("skips when the template has no headers", () => {
253+
expect(prTemplateInfos("some body", [])).toEqual([]);
254+
});
255+
256+
// Singular wording when exactly one section is missing.
257+
it("uses singular wording for a single missing section", () => {
258+
const body = ["## Changes", "## Testing"].join("\n");
259+
const infos = prTemplateInfos(body, ["Changes", "Testing", "Documentation"]);
260+
expect(infos[0]).toContain("missing template section:");
261+
});
262+
});
263+
264+
describe("evaluateDanger pr-template info", () => {
265+
// The new check is wired into the infos channel so it renders as [!NOTE].
266+
it("surfaces missing template sections via infos", () => {
267+
const result = evaluateDanger({
268+
files: [{ filename: "src/lib/x.cpp", additions: 10, deletions: 1 }],
269+
commits: [{ sha: "ab", message: "fix: small change" }],
270+
prBody: "A short rationale that explains what is going on, with no template structure at all.",
271+
prTitle: "fix: small change",
272+
labels: [],
273+
prTemplate: sampleTemplate,
274+
});
275+
expect(result.infos.some((m) => m.includes("missing template sections"))).toBe(true);
276+
});
277+
});
278+
142279
describe("starterChecks", () => {
143280
// Checks that source changes without accompanying tests produce a warning.
144281
it("requests tests when source changes without coverage", () => {

util/danger/logic.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export interface DangerInputs {
9898
prBody: string;
9999
prTitle: string;
100100
labels: string[];
101+
prTemplate?: string;
101102
}
102103

103104
/**
@@ -505,6 +506,68 @@ export function commitSizeInfos(commits: CommitInfo[]): string[] {
505506
return messages;
506507
}
507508

509+
// Matches any ATX heading (#, ##, …, ######) with optional trailing # marks per the CommonMark spec.
510+
const atxHeadingPattern = /^[ \t]*#{1,6}[ \t]+(.+?)(?:[ \t]+#+)?[ \t]*$/;
511+
512+
/**
513+
* Extract section headers of any level (H1–H6) from a Markdown document.
514+
*
515+
* Used to discover the section structure of the project's PR template so that
516+
* missing sections can be reported back to contributors. Returns titles in
517+
* document order, deduped (case-insensitive) so a template that repeats a
518+
* heading at different levels does not double-flag.
519+
*/
520+
export function parsePrTemplateSections(template: string): string[] {
521+
const sections: string[] = [];
522+
const seen = new Set<string>();
523+
for (const raw of template.split("\n")) {
524+
const match = raw.match(atxHeadingPattern);
525+
if (!match) continue;
526+
const title = match[1].trim();
527+
const key = title.toLowerCase();
528+
if (!seen.has(key)) {
529+
seen.add(key);
530+
sections.push(title);
531+
}
532+
}
533+
return sections;
534+
}
535+
536+
function escapeRegExp(value: string): string {
537+
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
538+
}
539+
540+
/**
541+
* Report PR-template sections that the body is missing.
542+
*
543+
* Matches headings of any level (so a body can use `### Testing` even if the
544+
* template uses `## Testing`). The template is a convention, not a hard
545+
* requirement, so this is an informational note rather than a warning — the
546+
* empty-description warning already covers the case where the contributor
547+
* wrote nothing at all.
548+
*/
549+
export function prTemplateInfos(prBody: string, templateSections: string[]): string[] {
550+
if (!prBody.trim() || templateSections.length === 0) {
551+
return [];
552+
}
553+
const missing = templateSections.filter((section) => {
554+
const pattern = new RegExp(
555+
`^[ \\t]*#{1,6}[ \\t]+${escapeRegExp(section)}(?:[ \\t]+#+)?[ \\t]*$`,
556+
"im",
557+
);
558+
return !pattern.test(prBody);
559+
});
560+
if (missing.length === 0) {
561+
return [];
562+
}
563+
const noun = missing.length === 1 ? "section" : "sections";
564+
const list = missing.map((name) => `**${name}**`).join(", ");
565+
return [
566+
`PR description is missing template ${noun}: ${list}. ` +
567+
"Following the [pull request template](.github/pull_request_template.md) helps reviewers find rationale, testing notes, and docs status quickly.",
568+
];
569+
}
570+
508571
/**
509572
* Warn when the aggregate source-scope churn across the whole PR is large.
510573
*
@@ -687,7 +750,10 @@ export function evaluateDanger(input: DangerInputs): DangerResult {
687750
const summary = summarizeScopes(input.files);
688751
const commitValidation = validateCommits(input.commits);
689752

690-
const infos = commitSizeInfos(input.commits);
753+
const infos = [
754+
...commitSizeInfos(input.commits),
755+
...prTemplateInfos(input.prBody || "", parsePrTemplateSections(input.prTemplate || "")),
756+
];
691757

692758
const warnings = [
693759
...commitValidation.warnings,

util/danger/runner.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,25 @@
77
//
88
// Official repository: https://github.com/cppalliance/mrdocs
99
//
10+
import { readFileSync } from "node:fs";
11+
import { join } from "node:path";
1012
import type { DangerDSLType } from "danger";
1113
import { evaluateDanger, type CommitInfo, type FileChange } from "./logic";
1214
import { renderDangerReport } from "./format";
1315

16+
/**
17+
* Read the project's PR template so rules can check the body against it.
18+
* Missing or unreadable templates degrade gracefully — the template-section
19+
* check just stays quiet rather than failing the run.
20+
*/
21+
function loadPrTemplate(): string {
22+
try {
23+
return readFileSync(join(process.cwd(), ".github/pull_request_template.md"), "utf8");
24+
} catch {
25+
return "";
26+
}
27+
}
28+
1429
// Danger provides these as globals at runtime; we declare them for editors/typecheckers.
1530
declare const danger: DangerDSLType;
1631
declare function markdown(message: string, file?: string, line?: number): void;
@@ -113,6 +128,7 @@ export async function runDanger(): Promise<void> {
113128
prBody: pr.body || "",
114129
prTitle: pr.title || "",
115130
labels,
131+
prTemplate: loadPrTemplate(),
116132
});
117133

118134
const warnings = [...fetchWarnings, ...evaluation.warnings];

0 commit comments

Comments
 (0)