Skip to content

Commit b6270c2

Browse files
author
Markus Paulsen
committed
Convert the template checker from Python to Java
The checker was the repository's only Python file. Nothing linted, formatted or tested it, and it needed its own `.gitattributes` rule, because the repo default of `* text=auto eol=crlf` would otherwise have checked it out with CRLF line endings on the Linux runners and broken its shebang. Rewrite it as a single-file Java program run through the source-code launcher (`java .github/scripts/CheckPullRequestTemplate.java`, Java 11 and later). No build step, no artefact, and no language added to a Java repository. The `.gitattributes` rule is reverted, since `.java` needs no exception: the file is passed to `java` rather than executed directly, so CRLF is harmless, exactly as it already is for the 671 sources under `src/`. The conversion exposed a defect in the Python version. Its empty-table-row rule was dead code: the guard meant to skip the Markdown separator row, `^\s*\|[\s:-]*\|`, also matches a row whose cells are blank, so the rule could never fire. A body pasting the unfilled coverage table passed. The guard was redundant to begin with, since a separator row's cells are not whitespace, so it is dropped rather than repaired. Line endings are now normalised on input instead of being absorbed by whitespace trimming. The repository checks Markdown out as CRLF and GitHub delivers bodies with CRLF, so this removes a class of latent pattern bugs. Verified against all ten open pull request bodies (no false positives) and four rejection cases: an empty body, foreign headings, the template left unfilled, and the unfilled coverage table that the Python version wrongly accepted.
1 parent 83873f9 commit b6270c2

5 files changed

Lines changed: 228 additions & 152 deletions

File tree

.gitattributes

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ pom.xml text eol=lf
3232
# otherwise apply and leave it with CRLF line endings, which makes ./gradlew fail
3333
# with "bad interpreter" on Linux and macOS.
3434
gradlew text eol=lf
35-
# Python scripts carry a shebang and run on the Linux CI runners. The eol=crlf default
36-
# above would give them CRLF line endings even on Linux, which breaks direct execution
37-
# with "bad interpreter" exactly as it would for gradlew.
38-
*.py text eol=lf
3935
# These are explicitly windows files and should use crlf
4036
*.bat text eol=crlf
4137
*.cmd text eol=crlf
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
import java.io.IOException;
2+
import java.nio.charset.StandardCharsets;
3+
import java.nio.file.Files;
4+
import java.nio.file.Path;
5+
import java.nio.file.StandardOpenOption;
6+
import java.util.ArrayList;
7+
import java.util.LinkedHashMap;
8+
import java.util.List;
9+
import java.util.Locale;
10+
import java.util.Map;
11+
import java.util.regex.Matcher;
12+
import java.util.regex.Pattern;
13+
14+
/**
15+
* Checks a pull request body against {@code .github/PULL_REQUEST_TEMPLATE.md}.
16+
*
17+
* <p>Run with {@code java .github/scripts/CheckPullRequestTemplate.java} (single-file source-code
18+
* mode, Java 11 and later). The body arrives through the {@code PR_BODY} environment variable. It
19+
* is untrusted input on fork pull requests and must never be interpolated into a shell command.
20+
*
21+
* <p>The required section headings are read from the template itself rather than duplicated here,
22+
* so editing the template cannot leave this check behind. The check validates shape, not substance:
23+
* that every section exists, that none was left empty, and that no unfilled stub survived. It
24+
* deliberately does not require checkboxes to be ticked, which would only train contributors to
25+
* tick them.
26+
*/
27+
public class CheckPullRequestTemplate {
28+
29+
private static final Path TEMPLATE = Path.of(".github/PULL_REQUEST_TEMPLATE.md");
30+
31+
private static final Pattern HEADING = Pattern.compile("^## .+$", Pattern.MULTILINE);
32+
33+
private static final Pattern HTML_COMMENT = Pattern.compile("<!--.*?-->", Pattern.DOTALL);
34+
35+
private static final Pattern FENCED_BLOCK = Pattern.compile("^```.*?^```", Pattern.DOTALL | Pattern.MULTILINE);
36+
37+
private static final Pattern BARE_LIST_MARKER = Pattern.compile("\\s*\\d+\\.\\s*");
38+
39+
/**
40+
* A table row whose every cell is blank. The template's separator row ({@code | --- | ---: |})
41+
* never matches this, because its cells are not whitespace, so no separate exclusion is needed.
42+
*/
43+
private static final Pattern EMPTY_TABLE_ROW = Pattern.compile("\\s*\\|(\\s*\\|)+\\s*");
44+
45+
/**
46+
* Escape hatches the template itself documents. A section carrying one of these is complete by
47+
* definition and is exempt from the leftover-stub scan.
48+
*/
49+
private static final List<String> ESCAPE_HATCHES = List.of(
50+
"no production java code changed",
51+
"not reproducible from an exercise",
52+
"no mode-specific behaviour changed",
53+
"no improvement");
54+
55+
public static void main(String[] args) throws IOException {
56+
System.exit(run());
57+
}
58+
59+
private static int run() throws IOException {
60+
String template;
61+
try {
62+
template = normalise(Files.readString(TEMPLATE, StandardCharsets.UTF_8));
63+
} catch (IOException error) {
64+
System.out.println("::error::cannot read " + TEMPLATE + ": " + error.getMessage());
65+
return 1;
66+
}
67+
68+
List<String> required = headings(template);
69+
if (required.isEmpty()) {
70+
System.out.println("::error::" + TEMPLATE + " declares no '## ' headings; nothing to enforce");
71+
return 1;
72+
}
73+
74+
String body = normalise(System.getenv().getOrDefault("PR_BODY", ""));
75+
List<String> problems = new ArrayList<>();
76+
77+
if (visible(body).isEmpty()) {
78+
problems.add("The pull request body is empty. Start from " + TEMPLATE + " and fill in every section.");
79+
report(problems, required);
80+
return 1;
81+
}
82+
83+
List<String> present = new ArrayList<>();
84+
for (String heading : required) {
85+
if (body.contains(heading)) {
86+
present.add(heading);
87+
} else {
88+
problems.add("Missing section heading: '" + heading + "'");
89+
}
90+
}
91+
92+
for (Map.Entry<String, String> section : sections(body, present).entrySet()) {
93+
problems.addAll(inspect(section.getKey(), section.getValue()));
94+
}
95+
96+
report(problems, required);
97+
return problems.isEmpty() ? 0 : 1;
98+
}
99+
100+
/** Collects the problems in a single section, or an empty list when it is well formed. */
101+
private static List<String> inspect(String heading, String content) {
102+
List<String> problems = new ArrayList<>();
103+
String prose = visible(content);
104+
105+
if (prose.isEmpty()) {
106+
problems.add("Section '" + heading
107+
+ "' is empty. The template states what to write when it does not apply.");
108+
return problems;
109+
}
110+
111+
String lowered = prose.toLowerCase(Locale.ROOT);
112+
for (String hatch : ESCAPE_HATCHES) {
113+
if (lowered.contains(hatch)) {
114+
return problems;
115+
}
116+
}
117+
118+
String scannable = FENCED_BLOCK.matcher(HTML_COMMENT.matcher(content).replaceAll("")).replaceAll("");
119+
for (String line : scannable.split("\n", -1)) {
120+
if (BARE_LIST_MARKER.matcher(line).matches()) {
121+
problems.add("Section '" + heading + "' still contains an unfilled list stub ('"
122+
+ line.trim() + "' with nothing after it).");
123+
break;
124+
}
125+
}
126+
for (String line : scannable.split("\n", -1)) {
127+
if (EMPTY_TABLE_ROW.matcher(line).matches()) {
128+
problems.add("Section '" + heading + "' still contains the empty template table row. "
129+
+ "Fill it in, or use the documented escape hatch.");
130+
break;
131+
}
132+
}
133+
return problems;
134+
}
135+
136+
/** The '## ' headings of a document, in order, trimmed. */
137+
private static List<String> headings(String text) {
138+
List<String> found = new ArrayList<>();
139+
Matcher matcher = HEADING.matcher(text);
140+
while (matcher.find()) {
141+
found.add(matcher.group().trim());
142+
}
143+
return found;
144+
}
145+
146+
/** Splits text into heading to body, for the given headings only, in document order. */
147+
private static Map<String, String> sections(String text, List<String> wanted) {
148+
List<int[]> bounds = new ArrayList<>();
149+
List<String> names = new ArrayList<>();
150+
Matcher matcher = HEADING.matcher(text);
151+
while (matcher.find()) {
152+
String heading = matcher.group().trim();
153+
if (wanted.contains(heading)) {
154+
bounds.add(new int[] { matcher.start(), matcher.end() });
155+
names.add(heading);
156+
}
157+
}
158+
159+
Map<String, String> result = new LinkedHashMap<>();
160+
for (int index = 0; index < names.size(); index++) {
161+
int end = index + 1 < bounds.size() ? bounds.get(index + 1)[0] : text.length();
162+
result.put(names.get(index), text.substring(bounds.get(index)[1], end));
163+
}
164+
return result;
165+
}
166+
167+
/** The prose a reader actually sees: HTML comments removed, surrounding whitespace trimmed. */
168+
private static String visible(String text) {
169+
return HTML_COMMENT.matcher(text).replaceAll("").trim();
170+
}
171+
172+
/**
173+
* Normalises line endings. The repository checks Markdown out with CRLF (see .gitattributes)
174+
* and GitHub delivers pull request bodies with CRLF, so every pattern here would otherwise have
175+
* to tolerate a stray carriage return.
176+
*/
177+
private static String normalise(String text) {
178+
return text.replace("\r\n", "\n").replace("\r", "\n");
179+
}
180+
181+
private static void report(List<String> problems, List<String> required) throws IOException {
182+
List<String> lines = new ArrayList<>();
183+
if (problems.isEmpty()) {
184+
lines.add("## Pull request template check passed");
185+
lines.add("");
186+
lines.add("All " + required.size() + " required sections are present and filled in.");
187+
System.out.println("All " + required.size() + " required sections are present and filled in.");
188+
} else {
189+
lines.add("## Pull request template check failed");
190+
lines.add("");
191+
lines.add("The body does not follow [" + TEMPLATE + "](" + TEMPLATE + "). It declares "
192+
+ required.size() + " required sections.");
193+
lines.add("");
194+
for (String problem : problems) {
195+
lines.add("- " + problem);
196+
System.out.println("::error::" + problem);
197+
}
198+
lines.add("");
199+
lines.add("Copy the template into the body, fill in every section, and the check re-runs "
200+
+ "automatically when the description is edited.");
201+
}
202+
203+
String summaryPath = System.getenv("GITHUB_STEP_SUMMARY");
204+
if (summaryPath != null && !summaryPath.isBlank()) {
205+
Files.writeString(Path.of(summaryPath), String.join("\n", lines) + "\n", StandardCharsets.UTF_8,
206+
StandardOpenOption.CREATE, StandardOpenOption.APPEND);
207+
}
208+
}
209+
}

.github/scripts/check_pull_request_template.py

Lines changed: 0 additions & 146 deletions
This file was deleted.

.github/workflows/pullrequest-template.yml

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,19 @@ jobs:
2626
with:
2727
persist-credentials: false
2828

29+
- name: Set up JDK 21
30+
uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0
31+
with:
32+
java-version: '21'
33+
distribution: 'temurin'
34+
2935
# The pull request body is untrusted input: on a fork pull request an outside
3036
# contributor controls it verbatim. It is therefore passed through the environment
31-
# and read with os.environ, never interpolated into the shell with ${{ }}, which
37+
# and read with System.getenv, never interpolated into the shell with ${{ }}, which
3238
# would be a script injection sink.
39+
#
40+
# Run in single-file source-code mode, so there is no build step and no artefact.
3341
- name: Check the pull request body against the template
3442
env:
3543
PR_BODY: ${{ github.event.pull_request.body }}
36-
run: python3 .github/scripts/check_pull_request_template.py
44+
run: java .github/scripts/CheckPullRequestTemplate.java

0 commit comments

Comments
 (0)