Skip to content

Commit 9552857

Browse files
committed
[Github][CI] Add doc8 for clang-tidy documentation formatting
1 parent b39a9db commit 9552857

File tree

3 files changed

+203
-27
lines changed

3 files changed

+203
-27
lines changed

.github/workflows/containers/github-action-ci-tooling/Dockerfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/cla
9494
COPY clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py ${LLVM_SYSROOT}/bin/clang-tidy-diff.py
9595

9696
# Install dependencies for 'pr-code-lint.yml' job
97+
RUN apt-get update && \
98+
DEBIAN_FRONTEND=noninteractive apt-get install -y python3-doc8 && \
99+
apt-get clean && \
100+
rm -rf /var/lib/apt/lists/*
97101
COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
98102
RUN pip install -r requirements_linting.txt --break-system-packages && \
99103
rm requirements_linting.txt

.github/workflows/pr-code-lint.yml

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ jobs:
3030
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
3131
with:
3232
fetch-depth: 2
33-
33+
3434
- name: Get changed files
3535
id: changed-files
3636
uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0
@@ -39,14 +39,14 @@ jobs:
3939
skip_initial_fetch: true
4040
base_sha: 'HEAD~1'
4141
sha: 'HEAD'
42-
42+
4343
- name: Listed files
4444
env:
4545
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
4646
run: |
4747
echo "Changed files:"
4848
echo "$CHANGED_FILES"
49-
49+
5050
# TODO: create special mapping for 'codegen' targets, for now build predefined set
5151
# TODO: add entrypoint in 'compute_projects.py' that only adds a project and its direct dependencies
5252
- name: Configure and CodeGen
@@ -71,25 +71,38 @@ jobs:
7171
-DLLVM_INCLUDE_TESTS=OFF \
7272
-DCLANG_INCLUDE_TESTS=OFF \
7373
-DCMAKE_BUILD_TYPE=Release
74-
74+
7575
ninja -C build \
7676
clang-tablegen-targets \
7777
genconfusable # for "ConfusableIdentifierCheck.h"
7878
79-
- name: Run code linter
79+
- name: Run clang-tidy linter
8080
env:
8181
GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
8282
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
8383
run: |
8484
echo "[]" > comments &&
8585
python3 llvm/utils/git/code-lint-helper.py \
86+
--linter clang-tidy \
8687
--token ${{ secrets.GITHUB_TOKEN }} \
8788
--issue-number $GITHUB_PR_NUMBER \
8889
--start-rev HEAD~1 \
8990
--end-rev HEAD \
9091
--verbose \
9192
--changed-files "$CHANGED_FILES"
92-
93+
94+
- name: Run doc8 linter
95+
env:
96+
GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
97+
run: |
98+
python3 llvm/utils/git/code-lint-helper.py \
99+
--linter doc8 \
100+
--token ${{ secrets.GITHUB_TOKEN }} \
101+
--issue-number $GITHUB_PR_NUMBER \
102+
--start-rev HEAD~1 \
103+
--end-rev HEAD \
104+
--verbose
105+
93106
- name: Upload results
94107
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
95108
if: always()

llvm/utils/git/code-lint-helper.py

Lines changed: 180 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ class LintArgs:
3434
issue_number: int = 0
3535
build_path: str = "build"
3636
clang_tidy_binary: str = "clang-tidy"
37+
doc8_binary: str = "doc8"
38+
linter: str = None
3739

3840
def __init__(self, args: argparse.Namespace = None) -> None:
3941
if not args is None:
@@ -46,9 +48,12 @@ def __init__(self, args: argparse.Namespace = None) -> None:
4648
self.verbose = args.verbose
4749
self.build_path = args.build_path
4850
self.clang_tidy_binary = args.clang_tidy_binary
51+
self.doc8_binary = args.doc8_binary
52+
self.linter = args.linter
4953

5054

51-
COMMENT_TAG = "<!--LLVM CODE LINT COMMENT: clang-tidy-->"
55+
COMMENT_TAG_CLANG_TIDY = "<!--LLVM CODE LINT COMMENT: clang-tidy-->"
56+
COMMENT_TAG_DOC8 = "<!--LLVM CODE LINT COMMENT: doc8-->"
5257

5358

5459
def get_instructions(cpp_files: List[str]) -> str:
@@ -135,13 +140,22 @@ def create_comment_text(warning: str, cpp_files: List[str]) -> str:
135140
"""
136141

137142

138-
def find_comment(pr: any) -> any:
143+
def find_comment(pr: any, args: LintArgs) -> any:
144+
comment_tag = get_comment_tag(args.linter)
139145
for comment in pr.as_issue().get_comments():
140-
if COMMENT_TAG in comment.body:
146+
if comment_tag in comment.body:
141147
return comment
142148
return None
143149

144150

151+
def get_comment_tag(linter: str) -> str:
152+
if linter == "clang-tidy":
153+
return COMMENT_TAG_CLANG_TIDY
154+
elif linter == "doc8":
155+
return COMMENT_TAG_DOC8
156+
raise ValueError(f"Unknown linter: {linter}")
157+
158+
145159
def create_comment(
146160
comment_text: str, args: LintArgs, create_new: bool
147161
) -> Optional[dict]:
@@ -150,9 +164,10 @@ def create_comment(
150164
repo = github.Github(args.token).get_repo(args.repo)
151165
pr = repo.get_issue(args.issue_number).as_pull_request()
152166

153-
comment_text = COMMENT_TAG + "\n\n" + comment_text
167+
comment_tag = get_comment_tag(args.linter)
168+
comment_text = comment_tag + "\n\n" + comment_text
154169

155-
existing_comment = find_comment(pr)
170+
existing_comment = find_comment(pr, args)
156171

157172
comment = None
158173
if create_new or existing_comment:
@@ -215,7 +230,126 @@ def run_clang_tidy(changed_files: List[str], args: LintArgs) -> Optional[str]:
215230
return clean_clang_tidy_output(proc.stdout.strip())
216231

217232

218-
def run_linter(changed_files: List[str], args: LintArgs) -> tuple[bool, Optional[dict]]:
233+
234+
def clean_doc8_output(output: str) -> Optional[str]:
235+
if not output:
236+
return None
237+
238+
lines = output.split("\n")
239+
cleaned_lines = []
240+
in_summary = False
241+
242+
for line in lines:
243+
if line.startswith("Scanning...") or line.startswith("Validating..."):
244+
continue
245+
if line.startswith("========"):
246+
in_summary = True
247+
continue
248+
if in_summary:
249+
continue
250+
if line.strip():
251+
cleaned_lines.append(line)
252+
253+
if cleaned_lines:
254+
return "\n".join(cleaned_lines)
255+
return None
256+
257+
258+
def get_doc8_instructions() -> str:
259+
# TODO: use git diff
260+
return "doc8 ./clang-tools-extra/docs/clang-tidy/checks/"
261+
262+
263+
def create_doc8_comment_text(doc8_output: str) -> str:
264+
instructions = get_doc8_instructions()
265+
return f"""
266+
:warning: Documentation linter doc8 found issues in your code. :warning:
267+
268+
<details>
269+
<summary>
270+
You can test this locally with the following command:
271+
</summary>
272+
273+
```bash
274+
{instructions}
275+
```
276+
277+
</details>
278+
279+
<details>
280+
<summary>
281+
View the output from doc8 here.
282+
</summary>
283+
284+
```
285+
{doc8_output}
286+
```
287+
288+
</details>
289+
"""
290+
291+
292+
def run_doc8(args: LintArgs) -> tuple[int, Optional[str]]:
293+
doc8_cmd = [args.doc8_binary, "./clang-tools-extra/docs/clang-tidy/checks/"]
294+
295+
if args.verbose:
296+
print(f"Running doc8: {' '.join(doc8_cmd)}")
297+
298+
proc = subprocess.run(
299+
doc8_cmd,
300+
stdout=subprocess.PIPE,
301+
stderr=subprocess.PIPE,
302+
text=True,
303+
check=False,
304+
)
305+
306+
cleaned_output = clean_doc8_output(proc.stdout.strip())
307+
if proc.returncode != 0 and cleaned_output is None:
308+
# Infrastructure failure
309+
return proc.returncode, proc.stderr.strip()
310+
311+
return proc.returncode, cleaned_output
312+
313+
314+
def run_doc8_linter(args: LintArgs) -> tuple[bool, Optional[dict]]:
315+
returncode, result = run_doc8(args)
316+
should_update_gh = args.token is not None and args.repo is not None
317+
comment = None
318+
319+
if returncode == 0:
320+
if should_update_gh:
321+
comment_text = (
322+
":white_check_mark: With the latest revision "
323+
"this PR passed the documentation linter."
324+
)
325+
comment = create_comment(comment_text, args, create_new=False)
326+
return True, comment
327+
else:
328+
if should_update_gh:
329+
if result:
330+
comment_text = create_doc8_comment_text(result)
331+
comment = create_comment(comment_text, args, create_new=True)
332+
else:
333+
comment_text = (
334+
":warning: The documentation linter failed without printing "
335+
"an output. Check the logs for output. :warning:"
336+
)
337+
comment = create_comment(comment_text, args, create_new=False)
338+
else:
339+
if result:
340+
print(
341+
"Warning: Documentation linter, doc8 detected "
342+
"some issues with your code..."
343+
)
344+
print(result)
345+
else:
346+
print("Warning: Documentation linter, doc8 failed to run.")
347+
return False, comment
348+
349+
350+
def run_clang_tidy_linter(
351+
changed_files: List[str], args: LintArgs
352+
) -> tuple[bool, Optional[dict]]:
219353
changed_files = [arg for arg in changed_files if "third-party" not in arg]
220354

221355
cpp_files = filter_changed_files(changed_files)
@@ -255,6 +389,13 @@ def run_linter(changed_files: List[str], args: LintArgs) -> tuple[bool, Optional
255389

256390
if __name__ == "__main__":
257391
parser = argparse.ArgumentParser()
392+
parser.add_argument(
393+
"--linter",
394+
type=str,
395+
choices=["clang-tidy", "doc8"],
396+
required=True,
397+
help="The linter to run.",
398+
)
258399
parser.add_argument(
259400
"--token", type=str, required=True, help="GitHub authentication token"
260401
)
@@ -291,39 +432,57 @@ def run_linter(changed_files: List[str], args: LintArgs) -> tuple[bool, Optional
291432
default="clang-tidy",
292433
help="Path to clang-tidy binary",
293434
)
435+
parser.add_argument(
436+
"--doc8-binary",
437+
type=str,
438+
default="doc8",
439+
help="Path to doc8 binary",
440+
)
294441
parser.add_argument(
295442
"--verbose", action="store_true", default=True, help="Verbose output"
296443
)
297444

298445
parsed_args = parser.parse_args()
299446
args = LintArgs(parsed_args)
300447

301-
changed_files = []
302-
if args.changed_files:
303-
changed_files = args.changed_files.split(",")
304-
305-
if args.verbose:
306-
print(f"got changed files: {changed_files}")
307-
308448
if args.verbose:
309-
print("running linter clang-tidy")
449+
print(f"running linter {args.linter}")
310450

311-
success, comment = run_linter(changed_files, args)
451+
success, comment = False, None
452+
if args.linter == "clang-tidy":
453+
changed_files = []
454+
if args.changed_files:
455+
changed_files = args.changed_files.split(",")
456+
if args.verbose:
457+
print(f"got changed files: {changed_files}")
458+
success, comment = run_clang_tidy_linter(changed_files, args)
459+
elif args.linter == "doc8":
460+
success, comment = run_doc8_linter(args)
312461

313462
if not success:
314463
if args.verbose:
315-
print("linter clang-tidy failed")
464+
print(f"linter {args.linter} failed")
316465

317466
# Write comments file if we have a comment
318467
if comment:
468+
import json
319469
if args.verbose:
320-
print(f"linter clang-tidy has comment: {comment}")
470+
print(f"linter {args.linter} has comment: {comment}")
321471

322-
with open("comments", "w") as f:
323-
import json
472+
existing_comments = []
473+
if os.path.exists("comments"):
474+
with open("comments", "r") as f:
475+
try:
476+
existing_comments = json.load(f)
477+
except json.JSONDecodeError:
478+
# File might be empty or invalid, start fresh
479+
pass
324480

325-
json.dump([comment], f)
481+
existing_comments.append(comment)
482+
483+
with open("comments", "w") as f:
484+
json.dump(existing_comments, f)
326485

327486
if not success:
328-
print("error: some linters failed: clang-tidy")
487+
print(f"error: linter {args.linter} failed")
329488
sys.exit(1)

0 commit comments

Comments
 (0)