Publish GitHub issues to files touched by the patch

bot/code_review_bot/__init__.py

@@ -182,6 +182,10 @@ def is_publishable(self):
     def in_patch(self):
         return self.revision.contains(self)
 
+    @property
+    def in_touched_files(self):
+        return self.revision.in_touched_files(self)
+
     @cached_property
     def hash(self):
         """

bot/code_review_bot/report/github.py

@@ -65,25 +65,31 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
             ]
 
             # Issues that are not in patch cannot be published directly through a Github review
-            inside_patch_issues = [
-                issue for issue in publishable_issues if issue.in_patch
+            issues_in_patch_files = [
+                issue for issue in publishable_issues if issue.in_touched_files
             ]
-            outside_patch_issues = [
-                issue for issue in publishable_issues if not issue.in_patch
+            issues_outside_patch_files = [
+                issue for issue in publishable_issues if not issue.in_touched_files
             ]
-            if outside_patch_issues:
+            if issues_outside_patch_files:
                 # Mention issues outside of the patch in the review main comment, after a new line
                 messages.append("")
                 messages.append(
-                    f"{len(outside_patch_issues)} issue{'s' if len(outside_patch_issues) > 1 else ''} "
-                    f"{'are' if len(outside_patch_issues) > 1 else 'is'} located outside of the patch:"
+                    f"{len(issues_outside_patch_files)} issue{'s' if len(issues_outside_patch_files) > 1 else ''} "
+                    f"{'are' if len(issues_outside_patch_files) > 1 else 'is'} located outside of the files "
+                    "touched by the patch:"
                 )
-                for issue in outside_patch_issues:
-                    messages.append(f"* `{issue.path}:{issue.line}` {issue.as_text()}")
+                for issue in issues_outside_patch_files:
+                    if issue.line:
+                        messages.append(
+                            f"* `{issue.path}:{issue.line}` {issue.as_text()}"
+                        )
+                    else:
+                        messages.append(f"* `{issue.path}` {issue.as_text()}")
 
             # Publish a review summarizing detected, unresolved and closed issues
             self.github_client.publish_review(
-                issues=inside_patch_issues,
+                issues=issues_in_patch_files,
                 revision=revision,
                 message="\n".join(messages),
                 event=ReviewEvent.RequestChanges,

bot/code_review_bot/revisions/base.py

@@ -166,6 +166,13 @@ def contains(self, issue):
         lines = set(range(issue.line, issue.line + issue.nb_lines))
         return not lines.isdisjoint(modified_lines)
 
+    def in_touched_files(self, issue):
+        """
+        Check if the issue (path+lines) is in this patch
+        """
+        assert isinstance(issue, Issue)
+        return issue.path in self.files
+
     def get_file_content(
         self, file_path: str, local_cache_repository: Path | None = None
     ):

bot/tests/test_reporter_github.py

@@ -10,6 +10,7 @@
 import responses
 from conftest import FIXTURES_DIR
 
+from code_review_bot import Level
 from code_review_bot.report.github import GithubReporter
 from code_review_bot.revisions import GithubRevision, Revision
 from code_review_bot.tasks.clang_tidy import ClangTidyIssue, ClangTidyTask
@@ -33,8 +34,8 @@ def test_github_review(
     assert isinstance(revision, GithubRevision)
     revision.lines = {
         # Add dummy lines diff
-        "test.txt": [0],
-        "path/to/test.cpp": [0],
+        "test.txt": [42],
+        "path/to/test.cpp": [42],
         "another_test.cpp": [41, 42, 43],
     }
     revision.files = ["test.txt", "test.cpp", "another_test.cpp"]
@@ -59,8 +60,23 @@ def test_github_review(
         "dummy message",
     )
     assert issue_clang_tidy.in_patch is True
+    assert issue_clang_tidy.in_touched_files is True
     assert issue_clang_tidy.is_publishable()
 
+    issue_on_touched_file = ClangTidyIssue(
+        analyzer=mock_task(ClangTidyTask, "source-test-clang-tidy"),
+        revision=revision,
+        path="test.txt",
+        line=10,
+        column=0,
+        level=Level("error"),
+        check="Some check",
+        message="Some error",
+    )
+    assert issue_on_touched_file.in_patch is False
+    assert issue_on_touched_file.in_touched_files is True
+    assert issue_on_touched_file.is_publishable()
+
     issue_coverage = CoverageIssue(
         mock_task(ZeroCoverageTask, "coverage"),
         "path/to/test.cpp",
@@ -92,7 +108,9 @@ def test_github_review(
         json=[],
     )
 
-    reporter.publish([issue_clang_tidy, issue_coverage], revision, [], [], [])
+    reporter.publish(
+        [issue_clang_tidy, issue_on_touched_file, issue_coverage], revision, [], [], []
+    )
     assert [(call.request.method, call.request.url) for call in responses.calls] == [
         ("GET", "https://github.com/owner/repo-name/pull/1.diff"),
         ("GET", "https://api.github.com:443/app/installations"),
@@ -124,9 +142,9 @@ def test_github_review(
     review_creation = responses.calls[-1]
     assert json.loads(review_creation.request.body) == {
         "body": dedent("""
-            2 issues have been found in this revision.
+            3 issues have been found in this revision.
 
-            1 issue is located outside of the patch:
+            1 issue is located outside of the files touched by the patch:
             * `path/to/test.cpp:1` This file is uncovered
         """).strip(),
         "comments": [
@@ -135,6 +153,11 @@ def test_github_review(
                 "path": "another_test.cpp",
                 "line": 42,
             },
+            {
+                "body": "Some error",
+                "path": "test.txt",
+                "line": 10,
+            },
         ],
         "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
         "event": "REQUEST_CHANGES",