cpp-linter · 2bndy5 · Oct 18, 2024 · Oct 18, 2024 · Oct 18, 2024
diff --git a/cpp_linter/loggers.py b/cpp_linter/loggers.py
@@ -12,7 +12,7 @@
 
     logging.basicConfig(
         format="%(name)s: %(message)s",
-        handlers=[RichHandler(show_time=False)],
+        handlers=[RichHandler(show_time=False, show_path=False)],
     )
 
 except ImportError:  # pragma: no cover

diff --git a/cpp_linter/rest_api/__init__.py b/cpp_linter/rest_api/__init__.py
@@ -40,6 +40,9 @@ class RestApiClient(ABC):
     def __init__(self, rate_limit_headers: RateLimitHeaders) -> None:
         self.session = requests.Session()
 
+        #: The brand name of the git server that provides the REST API.
+        self._name: str = "Generic"
+
         # The remain API requests allowed under the given token (if any).
         self._rate_limit_remaining = -1  # -1 means unknown
         # a counter for avoiding secondary rate limits
@@ -53,7 +56,8 @@ def _rate_limit_exceeded(self):
         logger.error("RATE LIMIT EXCEEDED!")
         if self._rate_limit_reset is not None:
             logger.error(
-                "Gitlab REST API rate limit resets on %s",
+                "%s REST API rate limit resets on %s",
+                self._name,
                 time.strftime("%d %B %Y %H:%M +0000", self._rate_limit_reset),
             )
         sys.exit(1)

diff --git a/cpp_linter/rest_api/github_api.py b/cpp_linter/rest_api/github_api.py
@@ -46,6 +46,8 @@ def __init__(self) -> None:
         # create default headers to be used for all HTTP requests
         self.session.headers.update(self.make_headers())
 
+        self._name = "GitHub"
+
         #: The base domain for the REST API
         self.api_url = environ.get("GITHUB_API_URL", "https://api.github.com")
         #: The ``owner``/``repository`` name.
@@ -134,18 +136,37 @@ def _get_changed_files_paginated(
             else:
                 file_list = response.json()["files"]
             for file in file_list:
-                assert "filename" in file
-                file_name = file["filename"]
+                try:
+                    file_name = file["filename"]
+                except KeyError as exc:  # pragma: no cover
+                    logger.error(
+                        f"Missing 'filename' key in file:\n{json.dumps(file, indent=2)}"
+                    )
+                    raise exc
                 if not file_filter.is_source_or_ignored(file_name):
                     continue
+                if lines_changed_only > 0 and cast(int, file.get("changes", 0)) == 0:
+                    continue  # also prevents KeyError below when patch is not provided
                 old_name = file_name
                 if "previous_filename" in file:
                     old_name = file["previous_filename"]
-                assert "patch" in file
+                if "patch" not in file:
+                    if lines_changed_only > 0:
+                        # diff info is needed for further operations
+                        raise KeyError(  # pragma: no cover
+                            f"{file_name} has no patch info:\n{json.dumps(file, indent=2)}"
+                        )
+                    elif (
+                        cast(int, file.get("changes", 0)) == 0
+                    ):  # in case files-changed-only is true
+                        # file was likely renamed without source changes
+                        files.append(FileObj(file_name))  # scan entire file instead
+                        continue
                 file_diff = (
                     f"diff --git a/{old_name} b/{file_name}\n"
                     + f"--- a/{old_name}\n+++ b/{file_name}\n"
                     + file["patch"]
+                    + "\n"
                 )
                 files.extend(parse_diff(file_diff, file_filter, lines_changed_only))
         return files

diff --git a/tests/list_changes/pull_request_files_pg2.json b/tests/list_changes/pull_request_files_pg2.json
@@ -10,5 +10,14 @@
         "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
         "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.hpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
         "patch": "@@ -5,12 +5,10 @@\n class Dummy {\n     char* useless;\n     int numb;\n+    Dummy() :numb(0), useless(\"\\0\"){}\n \n     public:\n-    void *not_usefull(char *str){\n-        useless = str;\n-        return 0;\n-    }\n+    void *not_useful(char *str){useless = str;}\n };\n \n \n@@ -28,14 +26,11 @@ class Dummy {\n \n \n \n-\n-\n-\n-\n \n \n struct LongDiff\n {\n+\n     long diff;\n \n };"
+    },
+    {
+      "sha": "17694f6803e9efd8cdceda06ea12c266793abacb",
+      "filename": "include/test/tree.hpp",
+      "status": "renamed",
+      "additions": 0,
+      "deletions": 0,
+      "changes": 0,
+      "previous_filename": "include/test-tree.hpp"
     }
 ]
diff --git a/tests/list_changes/push_files_pg2.json b/tests/list_changes/push_files_pg2.json
@@ -11,6 +11,15 @@
       "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
       "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.hpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
       "patch": "@@ -5,12 +5,10 @@\n class Dummy {\n     char* useless;\n     int numb;\n+    Dummy() :numb(0), useless(\"\\0\"){}\n \n     public:\n-    void *not_usefull(char *str){\n-        useless = str;\n-        return 0;\n-    }\n+    void *not_useful(char *str){useless = str;}\n };\n \n \n@@ -28,14 +26,11 @@ class Dummy {\n \n \n \n-\n-\n-\n-\n \n \n struct LongDiff\n {\n+\n     long diff;\n \n };"
+    },
+    {
+      "sha": "17694f6803e9efd8cdceda06ea12c266793abacb",
+      "filename": "include/test/tree.hpp",
+      "status": "renamed",
+      "additions": 0,
+      "deletions": 0,
+      "changes": 0,
+      "previous_filename": "include/test-tree.hpp"
     }
   ]
 }
diff --git a/tests/list_changes/test_get_file_changes.py b/tests/list_changes/test_get_file_changes.py
@@ -16,40 +16,60 @@
 
 
 @pytest.mark.parametrize(
-    "event_name,paginated,fake_runner",
+    "event_name,paginated,fake_runner,lines_changed_only",
     [
         # push event (full diff)
         (
             "unknown",  # let coverage include logged warning about unknown event
             False,
             True,
+            1,
         ),
         # pull request event (full diff)
         (
             "pull_request",
             False,
             True,
+            1,
         ),
         # push event (paginated diff)
         (
             "push",  # let coverage include logged warning about unknown event
             True,
             True,
+            1,
         ),
         # pull request event (paginated diff)
         (
             "pull_request",
             True,
             True,
+            1,
+        ),
+        # push event (paginated diff with all lines)
+        (
+            "push",  # let coverage include logged warning about unknown event
+            True,
+            True,
+            0,
+        ),
+        # pull request event (paginated diff with all lines)
+        (
+            "pull_request",
+            True,
+            True,
+            0,
         ),
         # local dev env
-        ("", False, False),
+        ("", False, False, 1),
     ],
     ids=[
         "push",
         "pull_request",
         "push(paginated)",
         "pull_request(paginated)",
+        "push(all-lines,paginated)",
+        "pull_request(all-lines,paginated)",
         "local_dev",
     ],
 )
@@ -60,6 +80,7 @@ def test_get_changed_files(
     event_name: str,
     paginated: bool,
     fake_runner: bool,
+    lines_changed_only: int,
 ):
     """test getting a list of changed files for an event."""
     caplog.set_level(logging.DEBUG, logger=logger.name)
@@ -121,8 +142,11 @@ def test_get_changed_files(
                 )
 
         files = gh_client.get_list_of_changed_files(
-            FileFilter(extensions=["cpp", "hpp"]), lines_changed_only=1
+            FileFilter(extensions=["cpp", "hpp"]), lines_changed_only=lines_changed_only
         )
         assert files
         for file in files:
-            assert file.name in ("src/demo.cpp", "src/demo.hpp")
+            expected = ["src/demo.cpp", "src/demo.hpp"]
+            if lines_changed_only == 0:
+                expected.append("include/test/tree.hpp")
+            assert file.name in expected