Skip to content

Commit 9cbdb11

Browse files
vbvictorgithub-actions[bot]
authored andcommitted
Automerge: [GitHub][CI] Add clang-tidy premerge workflow (#154829)
**KEY POINTS** - MVP workflow to automatically lint C++ files, located **only** in `clang-tools-extra/clang-tidy`. It's chosen this way as `clang-tools-extra/clang-tidy` is almost 100% `clang-tidy` complaint, thus we would automatically enforce a [high quality standard for clang-tidy source code](https://discourse.llvm.org/t/rfc-create-hardened-clang-tidy-config-for-clang-tidy-directory/87247). (llvm/llvm-project#147793) - Implementation is very similar to code-format job, but without the ability to run `code-lint-helper.py` locally. **FOUND ISSUES** + open questions - Speed: it takes ~ 1m40sec to download and unpack `clang-tidy` plus additional ~4 mins to configure and CodeGen targets. I see that `premerge.yaml` runs on special `llvm-premerge-linux-runners` runners which can use `sccache` for speed. Can we use the same runners for this job? Exact timings can be found [here](https://github.com/llvm/llvm-project/actions/runs/17135749067/job/48611150680?pr=154223). **TO DO** - Place common components from `code-lint-helper.py` and `code-format-helper.py` into a separate file and reuse it in both CI's. - Compute CodeGen targets based on `projects_to_build`, for now `clang-tablegen-targets` is hardcoded for `clang-tools-extra/`. - Automatically generate and upload `.yaml` for `clang-apply-replacements` - Create an RFC with a plan how to enable `clang-tidy` in other projects so that Maintainers of LLVM components could choose if they want `clang-tidy` or not. - Add more linters like `pylint`, `ruff` in the future.
2 parents bced4cd + 2ed7b9f commit 9cbdb11

File tree

3 files changed

+654
-0
lines changed

3 files changed

+654
-0
lines changed

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

Lines changed: 329 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,329 @@
1+
#!/usr/bin/env python3
2+
#
3+
# ====- clang-tidy-helper, runs clang-tidy from the ci --*- python -*-------==#
4+
#
5+
# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
6+
# See https://llvm.org/LICENSE.txt for license information.
7+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
8+
#
9+
# ==-------------------------------------------------------------------------==#
10+
"""A helper script to run clang-tidy linter in GitHub actions
11+
12+
This script is run by GitHub actions to ensure that the code in PR's conform to
13+
the coding style of LLVM. The canonical source of this script is in the LLVM
14+
source tree under llvm/utils/git.
15+
16+
You can learn more about the LLVM coding style on llvm.org:
17+
https://llvm.org/docs/CodingStandards.html
18+
"""
19+
20+
import argparse
21+
import os
22+
import subprocess
23+
import sys
24+
from typing import List, Optional
25+
26+
27+
class LintArgs:
28+
start_rev: str = None
29+
end_rev: str = None
30+
repo: str = None
31+
changed_files: List[str] = []
32+
token: str = None
33+
verbose: bool = True
34+
issue_number: int = 0
35+
build_path: str = "build"
36+
clang_tidy_binary: str = "clang-tidy"
37+
38+
def __init__(self, args: argparse.Namespace = None) -> None:
39+
if not args is None:
40+
self.start_rev = args.start_rev
41+
self.end_rev = args.end_rev
42+
self.repo = args.repo
43+
self.token = args.token
44+
self.changed_files = args.changed_files
45+
self.issue_number = args.issue_number
46+
self.verbose = args.verbose
47+
self.build_path = args.build_path
48+
self.clang_tidy_binary = args.clang_tidy_binary
49+
50+
51+
COMMENT_TAG = "<!--LLVM CODE LINT COMMENT: clang-tidy-->"
52+
53+
54+
def get_instructions(cpp_files: List[str]) -> str:
55+
files_str = " ".join(cpp_files)
56+
return f"""
57+
git diff -U0 origin/main...HEAD -- {files_str} |
58+
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py \\
59+
-path build -p1 -quiet"""
60+
61+
62+
def clean_clang_tidy_output(output: str) -> Optional[str]:
63+
"""
64+
- Remove 'Running clang-tidy in X threads...' line
65+
- Remove 'N warnings generated.' line
66+
- Strip leading workspace path from file paths
67+
"""
68+
if not output or output == "No relevant changes found.":
69+
return None
70+
71+
lines = output.split("\n")
72+
cleaned_lines = []
73+
74+
for line in lines:
75+
if line.startswith("Running clang-tidy in") or line.endswith("generated."):
76+
continue
77+
78+
# Remove everything up to rightmost "llvm-project/" for correct files names
79+
idx = line.rfind("llvm-project/")
80+
if idx != -1:
81+
line = line[idx + len("llvm-project/") :]
82+
83+
cleaned_lines.append(line)
84+
85+
if cleaned_lines:
86+
return "\n".join(cleaned_lines)
87+
return None
88+
89+
90+
# TODO: Add more rules when enabling other projects to use clang-tidy in CI.
91+
def should_lint_file(filepath: str) -> bool:
92+
return filepath.startswith("clang-tools-extra/clang-tidy/")
93+
94+
95+
def filter_changed_files(changed_files: List[str]) -> List[str]:
96+
filtered_files = []
97+
for filepath in changed_files:
98+
_, ext = os.path.splitext(filepath)
99+
if ext not in (".cpp", ".c", ".h", ".hpp", ".hxx", ".cxx"):
100+
continue
101+
if not should_lint_file(filepath):
102+
continue
103+
if os.path.exists(filepath):
104+
filtered_files.append(filepath)
105+
106+
return filtered_files
107+
108+
109+
def create_comment_text(warning: str, cpp_files: List[str]) -> str:
110+
instructions = get_instructions(cpp_files)
111+
return f"""
112+
:warning: C/C++ code linter clang-tidy found issues in your code. :warning:
113+
114+
<details>
115+
<summary>
116+
You can test this locally with the following command:
117+
</summary>
118+
119+
```bash
120+
{instructions}
121+
```
122+
123+
</details>
124+
125+
<details>
126+
<summary>
127+
View the output from clang-tidy here.
128+
</summary>
129+
130+
```
131+
{warning}
132+
```
133+
134+
</details>
135+
"""
136+
137+
138+
def find_comment(pr: any) -> any:
139+
for comment in pr.as_issue().get_comments():
140+
if COMMENT_TAG in comment.body:
141+
return comment
142+
return None
143+
144+
145+
def create_comment(
146+
comment_text: str, args: LintArgs, create_new: bool
147+
) -> Optional[dict]:
148+
import github
149+
150+
repo = github.Github(args.token).get_repo(args.repo)
151+
pr = repo.get_issue(args.issue_number).as_pull_request()
152+
153+
comment_text = COMMENT_TAG + "\n\n" + comment_text
154+
155+
existing_comment = find_comment(pr)
156+
157+
comment = None
158+
if create_new or existing_comment:
159+
comment = {"body": comment_text}
160+
if existing_comment:
161+
comment["id"] = existing_comment.id
162+
return comment
163+
164+
165+
def run_clang_tidy(changed_files: List[str], args: LintArgs) -> Optional[str]:
166+
if not changed_files:
167+
print("no c/c++ files found")
168+
return None
169+
170+
git_diff_cmd = [
171+
"git",
172+
"diff",
173+
"-U0",
174+
f"{args.start_rev}...{args.end_rev}",
175+
"--",
176+
] + changed_files
177+
178+
diff_proc = subprocess.run(
179+
git_diff_cmd,
180+
stdout=subprocess.PIPE,
181+
stderr=subprocess.PIPE,
182+
text=True,
183+
check=False,
184+
)
185+
186+
if diff_proc.returncode != 0:
187+
print(f"Git diff failed: {diff_proc.stderr}")
188+
return None
189+
190+
diff_content = diff_proc.stdout
191+
if not diff_content.strip():
192+
print("No diff content found")
193+
return None
194+
195+
tidy_diff_cmd = [
196+
"code-lint-tools/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py",
197+
"-path",
198+
args.build_path,
199+
"-p1",
200+
"-quiet",
201+
]
202+
203+
if args.verbose:
204+
print(f"Running clang-tidy-diff: {' '.join(tidy_diff_cmd)}")
205+
206+
proc = subprocess.run(
207+
tidy_diff_cmd,
208+
input=diff_content,
209+
stdout=subprocess.PIPE,
210+
stderr=subprocess.PIPE,
211+
text=True,
212+
check=False,
213+
)
214+
215+
return clean_clang_tidy_output(proc.stdout.strip())
216+
217+
218+
def run_linter(changed_files: List[str], args: LintArgs) -> tuple[bool, Optional[dict]]:
219+
changed_files = [arg for arg in changed_files if "third-party" not in arg]
220+
221+
cpp_files = filter_changed_files(changed_files)
222+
223+
tidy_result = run_clang_tidy(cpp_files, args)
224+
should_update_gh = args.token is not None and args.repo is not None
225+
226+
comment = None
227+
if tidy_result is None:
228+
if should_update_gh:
229+
comment_text = (
230+
":white_check_mark: With the latest revision "
231+
"this PR passed the C/C++ code linter."
232+
)
233+
comment = create_comment(comment_text, args, create_new=False)
234+
return True, comment
235+
elif len(tidy_result) > 0:
236+
if should_update_gh:
237+
comment_text = create_comment_text(tidy_result, cpp_files)
238+
comment = create_comment(comment_text, args, create_new=True)
239+
else:
240+
print(
241+
"Warning: C/C++ code linter, clang-tidy detected "
242+
"some issues with your code..."
243+
)
244+
return False, comment
245+
else:
246+
# The linter failed but didn't output a result (e.g. some sort of
247+
# infrastructure failure).
248+
comment_text = (
249+
":warning: The C/C++ code linter failed without printing "
250+
"an output. Check the logs for output. :warning:"
251+
)
252+
comment = create_comment(comment_text, args, create_new=False)
253+
return False, comment
254+
255+
256+
if __name__ == "__main__":
257+
parser = argparse.ArgumentParser()
258+
parser.add_argument(
259+
"--token", type=str, required=True, help="GitHub authentication token"
260+
)
261+
parser.add_argument("--issue-number", type=int, required=True)
262+
parser.add_argument(
263+
"--repo",
264+
type=str,
265+
default=os.getenv("GITHUB_REPOSITORY", "llvm/llvm-project"),
266+
help="The GitHub repository that we are working with in the form of <owner>/<repo> (e.g. llvm/llvm-project)",
267+
)
268+
parser.add_argument(
269+
"--start-rev",
270+
type=str,
271+
required=True,
272+
help="Compute changes from this revision.",
273+
)
274+
parser.add_argument(
275+
"--end-rev", type=str, required=True, help="Compute changes to this revision"
276+
)
277+
parser.add_argument(
278+
"--changed-files",
279+
type=str,
280+
help="Comma separated list of files that has been changed",
281+
)
282+
parser.add_argument(
283+
"--build-path",
284+
type=str,
285+
default="build",
286+
help="Path to build directory with compile_commands.json",
287+
)
288+
parser.add_argument(
289+
"--clang-tidy-binary",
290+
type=str,
291+
default="clang-tidy",
292+
help="Path to clang-tidy binary",
293+
)
294+
parser.add_argument(
295+
"--verbose", action="store_true", default=True, help="Verbose output"
296+
)
297+
298+
parsed_args = parser.parse_args()
299+
args = LintArgs(parsed_args)
300+
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+
308+
if args.verbose:
309+
print("running linter clang-tidy")
310+
311+
success, comment = run_linter(changed_files, args)
312+
313+
if not success:
314+
if args.verbose:
315+
print("linter clang-tidy failed")
316+
317+
# Write comments file if we have a comment
318+
if comment:
319+
if args.verbose:
320+
print(f"linter clang-tidy has comment: {comment}")
321+
322+
with open("comments", "w") as f:
323+
import json
324+
325+
json.dump([comment], f)
326+
327+
if not success:
328+
print("error: some linters failed: clang-tidy")
329+
sys.exit(1)

0 commit comments

Comments
 (0)