-
Notifications
You must be signed in to change notification settings - Fork 168
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This adds a clang-tidy static analysis job to our CI. On GitHub Actions, I tested this to run for about 3h, which is clearly too long. I leverage the mechanism that I originally intended for the GPU CI to run this job on CERN GitLab. Here it only takes about 35min. This is behind additional permission checks, where we maintain an allow-list of people who the CI acts for. Otherwise this check should remain neutral, allowing merging without overriding. The CMake configuration should allow us to relatively flexibly define which checks to include in the report, and if we want we can even fail the build for others. I would propose we go about like this: 1. This PR enables a number of checks, not all of which we intend to completely fix. 2. We see how this behaves in practice. This shouldn't block anything from merging for now, other than due to compilation failures (i.e. clang-tidy warnings are reported but ignored) 3. If this seems to make sense, I'll add a restricted set of checks that will fail the build, and add a PR to fix these checks. Thoughts?
- Loading branch information
1 parent
bcd9e01
commit 8948fa3
Showing
13 changed files
with
471 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
--- | ||
Checks: '-*,readability-*,-readability-redundant-member-init,misc-*,-misc-unused-parameters,bugprone-*,performance-*,modernize-*,-modernize-use-auto,clang-analyzer-deadcode.*,clang-analyzer-*,-clang-analyzer-osx.*,-clang-analyzer-unix.*,cppcoreguidelines-*,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-constant-array' | ||
Checks: '-*,readability-*,-readability-redundant-member-init,misc-*,-misc-unused-parameters,bugprone-*,performance-*,modernize-*,-modernize-use-auto,-modernize-use-trailing-return-type,clang-analyzer-deadcode.*,clang-analyzer-*,-clang-analyzer-osx.*,-clang-analyzer-unix.*,cppcoreguidelines-*,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-constant-array' | ||
HeaderFilterRegex: '.*(?<!nlohmann\/json)\.(hpp|cpp|ipp)$' | ||
AnalyzeTemporaryDtors: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
#!/usr/bin/env python3 | ||
import argparse | ||
import sys | ||
from pathlib import Path | ||
import json | ||
import itertools | ||
import fnmatch | ||
import re | ||
|
||
import yaml | ||
from rich.console import Console, Group | ||
from rich.text import Text | ||
from rich.panel import Panel | ||
from rich.rule import Rule | ||
from rich.emoji import Emoji | ||
from rich.table import Table | ||
|
||
|
||
from item import Item, ItemCollection | ||
|
||
|
||
def main(): | ||
p = argparse.ArgumentParser() | ||
p.add_argument("--report", type=Path, required=True) | ||
p.add_argument("--config", type=Path, required=True) | ||
p.add_argument("--strip-prefix-path", type=Path) | ||
|
||
args = p.parse_args() | ||
|
||
console = Console() | ||
|
||
with args.config.open() as fh: | ||
config = yaml.safe_load(fh) | ||
|
||
data = [] | ||
with args.report.open() as fh: | ||
data = ItemCollection(__root__=json.load(fh)).__root__ | ||
for item in data: | ||
if args.strip_prefix_path and not item.path.is_absolute: | ||
item.path = item.path.relative_to(args.strip_prefix_path) | ||
|
||
counts = config["limits"].copy() | ||
|
||
kf = lambda i: i.path | ||
for file, items in itertools.groupby(sorted(data, key=kf), key=kf): | ||
|
||
output = [] | ||
for item in items: | ||
if item.code in config["ignore"]: | ||
continue | ||
|
||
emoji = Emoji( | ||
{"warning": "yellow_circle", "error": "red_circle"}[item.severity] | ||
) | ||
|
||
style = "bold " | ||
if item.severity == "warning": | ||
style += "yellow" | ||
elif item.severity == "error": | ||
style += "red" | ||
|
||
s = Text() | ||
s.append(f"{emoji}") | ||
s.append(f" {item.path}:{item.line}:{item.col}", style="bold") | ||
s.append(f" {item.severity.upper()} ", style=style) | ||
s.append("[") | ||
s.append(item.code, style="bold") | ||
s.append(f"]") | ||
|
||
output.append(s) | ||
|
||
def subpath(m): | ||
return f"[bold]{m.group(1)}[/bold]:" | ||
|
||
message = re.sub(r"([\w/.\-+]+:\d+:\d+):", subpath, item.message) | ||
output.append(Panel(message)) | ||
output.append(Rule()) | ||
|
||
for pattern in counts.keys(): | ||
|
||
if not fnmatch.fnmatch(item.code, pattern): | ||
continue | ||
counts[pattern] += 1 | ||
|
||
output = output[:-1] | ||
console.print(Panel(Group(*output), title=str(file))) | ||
|
||
table = Table() | ||
table.add_column("", width=2) | ||
table.add_column("code / pattern") | ||
table.add_column("count", justify="right") | ||
table.add_column("limit", justify="right") | ||
exit = 0 | ||
for pattern, count in counts.items(): | ||
limit = config["limits"][pattern] | ||
emoji = Emoji("green_circle") | ||
style = "green" | ||
if count > limit: | ||
exit = 1 | ||
emoji = Emoji("red_circle") | ||
style = "red bold" | ||
table.add_row(emoji, pattern, str(count), str(limit), style=style) | ||
|
||
console.rule() | ||
console.print(Panel.fit(table, title="Results"), justify="center") | ||
|
||
if exit != 0: | ||
console.print( | ||
Panel( | ||
Text(f"{Emoji('red_circle')} FAILURE", justify="center"), | ||
style="red bold", | ||
) | ||
) | ||
else: | ||
console.print( | ||
Panel( | ||
Text(f"{Emoji('green_circle')} SUCCESS", justify="center"), | ||
style="green bold", | ||
) | ||
) | ||
|
||
sys.exit(exit) | ||
|
||
|
||
if "__main__" == __name__: | ||
main() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
from dataclasses import dataclass | ||
from pathlib import Path | ||
from typing import List | ||
|
||
import pydantic | ||
|
||
|
||
class Item(pydantic.BaseModel): | ||
path: Path | ||
line: int | ||
col: int | ||
message: str | ||
code: str | ||
severity: str | ||
|
||
def __hash__(self): | ||
return hash((self.path, self.line, self.col, self.code)) | ||
|
||
def __eq__(self, other): | ||
return (self.path, self.line, self.col, self.code) == ( | ||
other.path, | ||
other.line, | ||
other.col, | ||
other.code, | ||
) | ||
|
||
|
||
class ItemCollection(pydantic.BaseModel): | ||
__root__: List[Item] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
limits: {} | ||
# "readability-inconsistent-declaration-parameter-name": 0 | ||
# "readability-named-parameter": 0 | ||
# "readability-container-size-empty": 0 | ||
# "modernize-use-using": 0 | ||
#"readability-braces-around-statements": 0 | ||
# "modernize-use-override": 0 | ||
# "modernize-use-equals-default" : 0 | ||
# "readability-implicit-bool-cast": 0 | ||
# "modernize-use-default-member-init": 0 | ||
# "performance-unnecessary-value-param": 0 | ||
# "modernize-use-equals-default": 0 | ||
# "modernize-use-nullptr": 0 | ||
|
||
ignore: | ||
- cppcoreguidelines-pro-type-vararg |
Oops, something went wrong.