Skip to content

Commit 3ff13b4

Browse files
authored
Merge pull request #123 from bwrsandman/links-in-comments
Add links to check documentation in review
2 parents 9008958 + dbe658e commit 3ff13b4

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

post/clang_tidy_review/clang_tidy_review/__init__.py

+29-4
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,29 @@ def set_output(key: str, val: str) -> bool:
10851085
return True
10861086

10871087

1088+
def decorate_comment_body(comment: str) -> str:
1089+
"""
1090+
Split on first dash into two groups of string in [] at end of line
1091+
exception: if the first group starts with 'clang' such as 'clang-diagnostic-error'
1092+
exception to the exception: if the string starts with 'clang-analyzer', in which case, make it the first group
1093+
"""
1094+
version = "extra"
1095+
url = f"https://clang.llvm.org/{version}/clang-tidy/checks"
1096+
regex = r"(\[((?:clang-analyzer)|(?:(?!clang)[\w]+))-([\.\w-]+)\]$)"
1097+
subst = f"[\\g<1>({url}/\\g<2>/\\g<3>.html)]"
1098+
return re.sub(regex, subst, comment, 1, re.MULTILINE)
1099+
1100+
1101+
def decorate_comment(comment: PRReviewComment) -> PRReviewComment:
1102+
comment["body"] = decorate_comment_body(comment["body"])
1103+
return comment
1104+
1105+
1106+
def decorate_comments(review: PRReview) -> PRReview:
1107+
review["comments"] = list(map(decorate_comment, review["comments"]))
1108+
return review
1109+
1110+
10881111
def post_review(
10891112
pull_request: PullRequest,
10901113
review: Optional[PRReview],
@@ -1104,21 +1127,23 @@ def post_review(
11041127

11051128
total_comments = len(review["comments"])
11061129

1107-
set_output("total_comments", total_comments)
1130+
set_output("total_comments", str(total_comments))
11081131

11091132
print("Removing already posted or extra comments", flush=True)
11101133
trimmed_review = cull_comments(pull_request, review, max_comments)
11111134

1112-
if trimmed_review["comments"] == []:
1135+
if not trimmed_review["comments"]:
11131136
print("Everything already posted!")
11141137
return total_comments
11151138

11161139
if dry_run:
11171140
pprint.pprint(review, width=130)
11181141
return total_comments
11191142

1120-
print("Posting the review:\n", pprint.pformat(trimmed_review), flush=True)
1121-
pull_request.post_review(trimmed_review)
1143+
decorated_review = decorate_comments(trimmed_review)
1144+
1145+
print("Posting the review:\n", pprint.pformat(decorated_review), flush=True)
1146+
pull_request.post_review(decorated_review)
11221147

11231148
return total_comments
11241149

tests/test_review.py

+35
Original file line numberDiff line numberDiff line change
@@ -427,3 +427,38 @@ def test_config_file(monkeypatch, tmp_path):
427427
"not-clang-tidy", clang_tidy_checks="readability", config_file=""
428428
)
429429
assert flag == "--checks=readability"
430+
431+
432+
def test_decorate_comment_body():
433+
# No link to generic error so the message shouldn't be changed
434+
error_message = (
435+
"warning: no member named 'ranges' in namespace 'std' [clang-diagnostic-error]"
436+
)
437+
assert ctr.decorate_comment_body(error_message) == error_message
438+
439+
todo_message = "warning: missing username/bug in TODO [google-readability-todo]"
440+
todo_message_decorated = "warning: missing username/bug in TODO [[google-readability-todo](https://clang.llvm.org/extra/clang-tidy/checks/google/readability-todo.html)]"
441+
assert ctr.decorate_comment_body(todo_message) == todo_message_decorated
442+
443+
naming_message = "warning: invalid case style for constexpr variable 'foo' [readability-identifier-naming]"
444+
naming_message_decorated = "warning: invalid case style for constexpr variable 'foo' [[readability-identifier-naming](https://clang.llvm.org/extra/clang-tidy/checks/readability/identifier-naming.html)]"
445+
assert ctr.decorate_comment_body(naming_message) == naming_message_decorated
446+
447+
clang_analyzer_message = "warning: Array access (from variable 'foo') results in a null pointer dereference [clang-analyzer-core.NullDereference]"
448+
clang_analyzer_message_decorated = "warning: Array access (from variable 'foo') results in a null pointer dereference [[clang-analyzer-core.NullDereference](https://clang.llvm.org/extra/clang-tidy/checks/clang-analyzer/core.NullDereference.html)]"
449+
assert (
450+
ctr.decorate_comment_body(clang_analyzer_message)
451+
== clang_analyzer_message_decorated
452+
)
453+
454+
# Not sure it's necessary to link to prior version documentation especially since we have to map versions such as
455+
# "17" to "17.0.1" and "18" to "18.1.0" because no other urls exist
456+
# version_message_pre_15_version = "14.0.0"
457+
# version_message_pre_15 = "warning: missing username/bug in TODO [google-readability-todo]"
458+
# version_message_pre_15_decorated = "warning: missing username/bug in TODO [[google-readability-todo](https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/google-readability-todo.html)]"
459+
# assert ctr.decorate_comment_body(version_message_pre_15, version_message_pre_15_version) == version_message_pre_15_decorated
460+
#
461+
# version_message_1500_version = "15.0.0"
462+
# version_message_1500 = "warning: missing username/bug in TODO [google-readability-todo]"
463+
# version_message_1500_decorated = "warning: missing username/bug in TODO [[google-readability-todo](https://releases.llvm.org/15.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/google/readability-todo.html)]"
464+
# assert ctr.decorate_comment_body(version_message_1500, version_message_1500_version) == version_message_1500_decorated

0 commit comments

Comments
 (0)