From 872a5ec3c578bb3ba257fe66f9918da0d2480364 Mon Sep 17 00:00:00 2001 From: JacobDomagala Date: Wed, 29 Oct 2025 13:12:30 +0100 Subject: [PATCH 1/2] [#150]: Attempt on fixing CI errors --- .../test-static-analysis__sa-cmake-output.md | 9 + ...st-static-analysis__sa-non-cmake-output.md | 207 ++++++++++++++++++ src/patch_compile_commands.py | 11 +- test/test_utils.py | 39 +++- 4 files changed, 262 insertions(+), 4 deletions(-) diff --git a/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md b/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md index fa6b473..a14bb78 100644 --- a/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md +++ b/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md @@ -221,6 +221,15 @@ https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L33-L38 +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L35-L40 +```diff +!Line: 35 - error: pointee of variable 'ptr' of type 'int *' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+ + + https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L36-L41 ```diff !Line: 35 - error: pointee of variable 'ptr' of type 'int *' can be declared 'const' [misc-const-correctness,-warnings-as-errors] diff --git a/.github/testrepo_comment_fixtures/test-static-analysis__sa-non-cmake-output.md b/.github/testrepo_comment_fixtures/test-static-analysis__sa-non-cmake-output.md index 596dcf2..34d5f0d 100644 --- a/.github/testrepo_comment_fixtures/test-static-analysis__sa-non-cmake-output.md +++ b/.github/testrepo_comment_fixtures/test-static-analysis__sa-non-cmake-output.md @@ -57,3 +57,210 @@ https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L41-L44 *** +
:red_circle: clang-tidy found 23 issues! Click here to see details.
+ +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L2-L7 +```diff +!Line: 2 - error: included header string is not used directly [misc-include-cleaner,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L5-L10 +```diff +!Line: 5 - error: class 'Example' can be moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L5-L10 +```diff +!Line: 5 - error: class 'Example' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions,hicpp-special-member-functions,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L9-L14 +```diff +!Line: 9 - error: 42 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L11-L16 +```diff +!Line: 11 - error: parameter name 'p' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L21-L26 +```diff +!Line: 21 - error: function 'divide' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L21-L26 +```diff +!Line: 21 - error: parameter name 'a' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L21-L26 +```diff +!Line: 21 - error: parameter name 'b' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L23-L28 +```diff +!Line: 23 - error: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L30-L35 +```diff +!Line: 30 - error: variable 'ex' of type 'Example' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L30-L35 +```diff +!Line: 30 - error: variable name 'ex' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L31-L36 +```diff +!Line: 31 - error: variable 'x' of type 'int' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L31-L36 +```diff +!Line: 31 - error: variable name 'x' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L31-L36 +```diff +!Line: 31 - error: 10 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L32-L37 +```diff +!Line: 32 - error: variable 'y' of type 'int' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L32-L37 +```diff +!Line: 32 - error: variable name 'y' is too short, expected at least 3 characters [readability-identifier-length,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L33-L38 +```diff +!Line: 33 - error: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L35-L40 +```diff +!Line: 35 - error: pointee of variable 'ptr' of type 'int *' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L36-L41 +```diff +!Line: 36 - error: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L39-L44 +```diff +!Line: 39 - error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L39-L44 +```diff +!Line: 39 - error: 10 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L39-L44 +```diff +!Line: 39 - error: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl,-warnings-as-errors] + +``` +
+ + + +https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L41-L44 +```diff +!Line: 41 - error: variable 'unusedvec' of type 'std::vector' can be declared 'const' [misc-const-correctness,-warnings-as-errors] + +``` +
+

diff --git a/src/patch_compile_commands.py b/src/patch_compile_commands.py index e077c0f..619a561 100644 --- a/src/patch_compile_commands.py +++ b/src/patch_compile_commands.py @@ -5,7 +5,7 @@ from typing import Any, Dict, List, Sequence -NEW_PREFIX = "/github/workspace" +DEFAULT_PREFIX = "/github/workspace" def _collect_paths(entries: Sequence[Dict[str, Any]]) -> List[str]: @@ -17,17 +17,22 @@ def _collect_paths(entries: Sequence[Dict[str, Any]]) -> List[str]: ] +def _new_prefix() -> str: + return os.path.realpath(os.getenv("GITHUB_WORKSPACE", DEFAULT_PREFIX)) + + def patch_compile_commands(path: str) -> None: with open(path, "r", encoding="utf-8") as f: data: List[Dict[str, Any]] = json.load(f) old_prefix = os.path.commonpath(_collect_paths(data)) - print(f"[INFO] Patching compile_commands.json: '{old_prefix}' → '{NEW_PREFIX}'") + new_prefix = _new_prefix() + print(f"[INFO] Patching compile_commands.json: '{old_prefix}' → '{new_prefix}'") for entry in data: for key in ("file", "directory", "command"): val = entry.get(key) if isinstance(val, str) and old_prefix in val: - entry[key] = val.replace(old_prefix, NEW_PREFIX) + entry[key] = val.replace(old_prefix, new_prefix) with open(path, "w", encoding="utf-8") as f: json.dump(data, f, indent=2) diff --git a/test/test_utils.py b/test/test_utils.py index 45209c1..5cbb486 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -1,6 +1,8 @@ import unittest import os import sys +import json +import tempfile try: PROJECT_PATH = f"{os.sep}".join(os.path.abspath(__file__).split(os.sep)[:-2]) @@ -9,7 +11,7 @@ print(f"Can not add project path to system path! Exiting!\nERROR: {exception}") raise SystemExit(1) from exception -from src import sa_utils +from src import patch_compile_commands, sa_utils class TestUtils(unittest.TestCase): @@ -51,6 +53,41 @@ def test_is_excluded_dir_with_multiple_directories(self): os.environ["INPUT_EXCLUDE_DIR"] = previous_exclude_dir sa_utils.WORK_DIR = previous_work_dir + def test_patch_compile_commands_uses_current_workspace(self): + previous_workspace = os.environ.get("GITHUB_WORKSPACE") + + try: + os.environ["GITHUB_WORKSPACE"] = "/github/workspace/pr_tree" + with tempfile.NamedTemporaryFile("w+", encoding="utf-8") as compile_db: + json.dump( + [ + { + "directory": "/src/build", + "command": "clang++ -I/src/include -c /src/source.cpp", + "file": "/src/source.cpp", + } + ], + compile_db, + ) + compile_db.flush() + + patch_compile_commands.patch_compile_commands(compile_db.name) + compile_db.seek(0) + patched = json.load(compile_db) + + self.assertEqual(patched[0]["directory"], "/github/workspace/pr_tree/build") + self.assertEqual(patched[0]["file"], "/github/workspace/pr_tree/source.cpp") + self.assertEqual( + patched[0]["command"], + "clang++ -I/github/workspace/pr_tree/include " + "-c /github/workspace/pr_tree/source.cpp", + ) + finally: + if previous_workspace is None: + os.environ.pop("GITHUB_WORKSPACE", None) + else: + os.environ["GITHUB_WORKSPACE"] = previous_workspace + if __name__ == "__main__": unittest.main() From b50b361dd68e48fd5bbfd153b069c4c4711b7399 Mon Sep 17 00:00:00 2001 From: JacobDomagala Date: Wed, 29 Oct 2025 13:12:30 +0100 Subject: [PATCH 2/2] [#150]: Try fixing the patching of compile commands --- .../test-static-analysis__sa-cmake-output.md | 9 --- src/patch_compile_commands.py | 60 +++++++++++++++---- test/test_utils.py | 46 ++++++++------ 3 files changed, 78 insertions(+), 37 deletions(-) diff --git a/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md b/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md index a14bb78..94055d7 100644 --- a/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md +++ b/.github/testrepo_comment_fixtures/test-static-analysis__sa-cmake-output.md @@ -230,15 +230,6 @@ https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L35-L40 -https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L36-L41 -```diff -!Line: 35 - error: pointee of variable 'ptr' of type 'int *' can be declared 'const' [misc-const-correctness,-warnings-as-errors] - -``` -
- - - https://github.com/JacobDomagala/TestRepo/blob//source.cpp#L36-L41 ```diff !Line: 36 - error: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl,-warnings-as-errors] diff --git a/src/patch_compile_commands.py b/src/patch_compile_commands.py index 619a561..f44ca93 100644 --- a/src/patch_compile_commands.py +++ b/src/patch_compile_commands.py @@ -3,16 +3,19 @@ import os import sys -from typing import Any, Dict, List, Sequence +from typing import Any, Dict, List, Sequence, Tuple DEFAULT_PREFIX = "/github/workspace" -def _collect_paths(entries: Sequence[Dict[str, Any]]) -> List[str]: +def _collect_paths( + entries: Sequence[Dict[str, Any]], keys: Sequence[str] = ("directory", "file") +) -> List[str]: return [ - os.path.realpath(v) + os.path.normpath(v) for e in entries - for v in (e.get("directory"), e.get("file")) + for key in keys + for v in (e.get(key),) if isinstance(v, str) ] @@ -21,18 +24,55 @@ def _new_prefix() -> str: return os.path.realpath(os.getenv("GITHUB_WORKSPACE", DEFAULT_PREFIX)) +def _replace_prefixes(value: str, replacements: Sequence[Tuple[str, str]]) -> str: + result = value + for old_prefix, new_prefix in sorted( + replacements, key=lambda replacement: len(replacement[0]), reverse=True + ): + if old_prefix in result: + result = result.replace(old_prefix, new_prefix) + return result + + def patch_compile_commands(path: str) -> None: with open(path, "r", encoding="utf-8") as f: data: List[Dict[str, Any]] = json.load(f) - old_prefix = os.path.commonpath(_collect_paths(data)) + all_paths = _collect_paths(data) + directory_paths = _collect_paths(data, ("directory",)) + + old_prefix = os.path.commonpath(all_paths) + old_directory_prefix = ( + os.path.commonpath(directory_paths) if directory_paths else old_prefix + ) new_prefix = _new_prefix() - print(f"[INFO] Patching compile_commands.json: '{old_prefix}' → '{new_prefix}'") + new_directory_prefix = os.path.realpath(os.path.dirname(path)) + + print( + "[INFO] Patching compile_commands.json: " + f"source '{old_prefix}' -> '{new_prefix}', " + f"build '{old_directory_prefix}' -> '{new_directory_prefix}'" + ) + + command_replacements = ( + (old_directory_prefix, new_directory_prefix), + (old_prefix, new_prefix), + ) + for entry in data: - for key in ("file", "directory", "command"): - val = entry.get(key) - if isinstance(val, str) and old_prefix in val: - entry[key] = val.replace(old_prefix, new_prefix) + file_value = entry.get("file") + if isinstance(file_value, str): + entry["file"] = _replace_prefixes(file_value, ((old_prefix, new_prefix),)) + + directory_value = entry.get("directory") + if isinstance(directory_value, str): + entry["directory"] = _replace_prefixes( + directory_value, ((old_directory_prefix, new_directory_prefix),) + ) + + command_value = entry.get("command") + if isinstance(command_value, str): + entry["command"] = _replace_prefixes(command_value, command_replacements) with open(path, "w", encoding="utf-8") as f: json.dump(data, f, indent=2) diff --git a/test/test_utils.py b/test/test_utils.py index 5cbb486..153820b 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -58,28 +58,38 @@ def test_patch_compile_commands_uses_current_workspace(self): try: os.environ["GITHUB_WORKSPACE"] = "/github/workspace/pr_tree" - with tempfile.NamedTemporaryFile("w+", encoding="utf-8") as compile_db: - json.dump( - [ - { - "directory": "/src/build", - "command": "clang++ -I/src/include -c /src/source.cpp", - "file": "/src/source.cpp", - } - ], - compile_db, - ) - compile_db.flush() - - patch_compile_commands.patch_compile_commands(compile_db.name) - compile_db.seek(0) - patched = json.load(compile_db) - - self.assertEqual(patched[0]["directory"], "/github/workspace/pr_tree/build") + with tempfile.TemporaryDirectory() as temp_dir: + build_dir = os.path.join(temp_dir, "build") + os.mkdir(build_dir) + compile_db_path = os.path.join(build_dir, "compile_commands.json") + + with open(compile_db_path, "w", encoding="utf-8") as compile_db: + json.dump( + [ + { + "directory": "/src/build", + "command": ( + "clang++ -I/src/include -o " + "CMakeFiles/app.dir/source.cpp.o " + "-c /src/source.cpp" + ), + "file": "/src/source.cpp", + } + ], + compile_db, + ) + + patch_compile_commands.patch_compile_commands(compile_db_path) + + with open(compile_db_path, "r", encoding="utf-8") as compile_db: + patched = json.load(compile_db) + + self.assertEqual(patched[0]["directory"], os.path.realpath(build_dir)) self.assertEqual(patched[0]["file"], "/github/workspace/pr_tree/source.cpp") self.assertEqual( patched[0]["command"], "clang++ -I/github/workspace/pr_tree/include " + "-o CMakeFiles/app.dir/source.cpp.o " "-c /github/workspace/pr_tree/source.cpp", ) finally: