From dbe658e1a07ca18043459c3faeab6d6376974ac4 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Fri, 10 May 2024 23:53:56 -0400 Subject: [PATCH] Add links to check documentation in review --- .../clang_tidy_review/__init__.py | 33 ++++++++++++++--- tests/test_review.py | 35 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py index 69fc788..19c7bae 100644 --- a/post/clang_tidy_review/clang_tidy_review/__init__.py +++ b/post/clang_tidy_review/clang_tidy_review/__init__.py @@ -1085,6 +1085,29 @@ def set_output(key: str, val: str) -> bool: return True +def decorate_comment_body(comment: str) -> str: + """ + Split on first dash into two groups of string in [] at end of line + exception: if the first group starts with 'clang' such as 'clang-diagnostic-error' + exception to the exception: if the string starts with 'clang-analyzer', in which case, make it the first group + """ + version = "extra" + url = f"https://clang.llvm.org/{version}/clang-tidy/checks" + regex = r"(\[((?:clang-analyzer)|(?:(?!clang)[\w]+))-([\.\w-]+)\]$)" + subst = f"[\\g<1>({url}/\\g<2>/\\g<3>.html)]" + return re.sub(regex, subst, comment, 1, re.MULTILINE) + + +def decorate_comment(comment: PRReviewComment) -> PRReviewComment: + comment["body"] = decorate_comment_body(comment["body"]) + return comment + + +def decorate_comments(review: PRReview) -> PRReview: + review["comments"] = list(map(decorate_comment, review["comments"])) + return review + + def post_review( pull_request: PullRequest, review: Optional[PRReview], @@ -1104,12 +1127,12 @@ def post_review( total_comments = len(review["comments"]) - set_output("total_comments", total_comments) + set_output("total_comments", str(total_comments)) print("Removing already posted or extra comments", flush=True) trimmed_review = cull_comments(pull_request, review, max_comments) - if trimmed_review["comments"] == []: + if not trimmed_review["comments"]: print("Everything already posted!") return total_comments @@ -1117,8 +1140,10 @@ def post_review( pprint.pprint(review, width=130) return total_comments - print("Posting the review:\n", pprint.pformat(trimmed_review), flush=True) - pull_request.post_review(trimmed_review) + decorated_review = decorate_comments(trimmed_review) + + print("Posting the review:\n", pprint.pformat(decorated_review), flush=True) + pull_request.post_review(decorated_review) return total_comments diff --git a/tests/test_review.py b/tests/test_review.py index 7368429..dd67570 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -427,3 +427,38 @@ def test_config_file(monkeypatch, tmp_path): "not-clang-tidy", clang_tidy_checks="readability", config_file="" ) assert flag == "--checks=readability" + + +def test_decorate_comment_body(): + # No link to generic error so the message shouldn't be changed + error_message = ( + "warning: no member named 'ranges' in namespace 'std' [clang-diagnostic-error]" + ) + assert ctr.decorate_comment_body(error_message) == error_message + + todo_message = "warning: missing username/bug in TODO [google-readability-todo]" + todo_message_decorated = "warning: missing username/bug in TODO [[google-readability-todo](https://clang.llvm.org/extra/clang-tidy/checks/google/readability-todo.html)]" + assert ctr.decorate_comment_body(todo_message) == todo_message_decorated + + naming_message = "warning: invalid case style for constexpr variable 'foo' [readability-identifier-naming]" + 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)]" + assert ctr.decorate_comment_body(naming_message) == naming_message_decorated + + clang_analyzer_message = "warning: Array access (from variable 'foo') results in a null pointer dereference [clang-analyzer-core.NullDereference]" + 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)]" + assert ( + ctr.decorate_comment_body(clang_analyzer_message) + == clang_analyzer_message_decorated + ) + + # Not sure it's necessary to link to prior version documentation especially since we have to map versions such as + # "17" to "17.0.1" and "18" to "18.1.0" because no other urls exist + # version_message_pre_15_version = "14.0.0" + # version_message_pre_15 = "warning: missing username/bug in TODO [google-readability-todo]" + # 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)]" + # assert ctr.decorate_comment_body(version_message_pre_15, version_message_pre_15_version) == version_message_pre_15_decorated + # + # version_message_1500_version = "15.0.0" + # version_message_1500 = "warning: missing username/bug in TODO [google-readability-todo]" + # 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)]" + # assert ctr.decorate_comment_body(version_message_1500, version_message_1500_version) == version_message_1500_decorated