From b843193e218e546c9c9274ebeb8aaa4b67276558 Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 15:47:13 +0100
Subject: [PATCH 01/11] CI: Add project config for black/ruff

---
 post/clang_tidy_review/pyproject.toml | 42 +++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/post/clang_tidy_review/pyproject.toml b/post/clang_tidy_review/pyproject.toml
index c7ab2a9..34cf300 100644
--- a/post/clang_tidy_review/pyproject.toml
+++ b/post/clang_tidy_review/pyproject.toml
@@ -33,6 +33,10 @@ post = "clang_tidy_review.post:main"
 tests = [
     "pytest >= 3.3.0",
 ]
+lint = [
+  "black",
+  "ruff",
+]
 
 [tool.setuptools]
 packages = ["clang_tidy_review"]
@@ -40,3 +44,41 @@ packages = ["clang_tidy_review"]
 [tool.setuptools_scm]
 root = "../.."
 fallback_version = "0.0.0-dev"
+
+[tool.black]
+extend_exclude = "_version.py"
+
+[tool.ruff.lint]
+extend-select = [
+  "B",           # flake8-bugbear
+  "I",           # isort
+  "ARG",         # flake8-unused-arguments
+  "C4",          # flake8-comprehensions
+  "ICN",         # flake8-import-conventions
+  "G",           # flake8-logging-format
+  "PGH",         # pygrep-hooks
+  "PIE",         # flake8-pie
+  "PL",          # pylint
+  "PT",          # flake8-pytest-style
+  "PTH",         # flake8-use-pathlib
+  "RET",         # flake8-return
+  "RUF",         # Ruff-specific
+  "SIM",         # flake8-simplify
+  "UP",          # pyupgrade
+  "YTT",         # flake8-2020
+  "EXE",         # flake8-executable
+  "NPY",         # NumPy specific rules
+  "PD",          # pandas-vet
+  "FURB",        # refurb
+]
+ignore = [
+  "PLR2004",     # magic-comparison
+  "B9",          # flake8-bugbear opinionated warnings
+  "PLC0414",     # useless-import-alias
+  "PLR0913",     # too-many-arguments
+  "PLR0917",     # too-many-positional
+  "PLR0914",     # too-many-locals
+  "PLR0915",     # too-many-statements
+  "PLR0912",     # too-many-branches
+  "PTH123",      # builtin-open
+]

From 16267bc1f53e127e78bbbbe39757242974e2ae99 Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 15:48:24 +0100
Subject: [PATCH 02/11] Apply ruff fixes

---
 .../clang_tidy_review/__init__.py             | 56 +++++++++----------
 .../clang_tidy_review/post.py                 | 14 ++---
 .../clang_tidy_review/review.py               | 11 ++--
 tests/test_review.py                          |  2 +-
 4 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index fca4ded..c002d9d 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -5,31 +5,31 @@
 
 import argparse
 import base64
+import contextlib
+import datetime
 import fnmatch
 import glob
+import io
 import itertools
 import json
 import os
-from operator import itemgetter
-import pprint
 import pathlib
+import pprint
+import re
 import subprocess
 import textwrap
+import zipfile
+from operator import itemgetter
+from typing import Any, Dict, List, Optional, TypedDict
+
 import unidiff
 import urllib3
 import yaml
-import contextlib
-import datetime
-import re
-import io
-import textwrap
-import zipfile
-from github import Github, Auth
+from github import Auth, Github
 from github.GithubException import GithubException
-from github.Requester import Requester
 from github.PaginatedList import PaginatedList
+from github.Requester import Requester
 from github.WorkflowRun import WorkflowRun
-from typing import Any, List, Optional, TypedDict, Dict
 
 DIFF_HEADER_LINE_LENGTH = 5
 FIXES_FILE = "clang_tidy_review.yaml"
@@ -253,7 +253,7 @@ def config_file_or_checks(
 def load_clang_tidy_warnings():
     """Read clang-tidy warnings from FIXES_FILE. Can be produced by build_clang_tidy_warnings"""
     try:
-        with open(FIXES_FILE, "r") as fixes_file:
+        with open(FIXES_FILE) as fixes_file:
             return yaml.safe_load(fixes_file)
     except FileNotFoundError:
         return {}
@@ -405,7 +405,7 @@ def make_file_offset_lookup(filenames):
     lookup = {}
 
     for filename in filenames:
-        with open(filename, "r") as file:
+        with open(filename) as file:
             lines = file.readlines()
         # Length of each line
         line_lengths = map(len, lines)
@@ -431,16 +431,14 @@ def get_diagnostic_file_path(clang_tidy_diagnostic, build_dir):
             return ""
         elif os.path.isabs(file_path):
             return os.path.normpath(os.path.abspath(file_path))
-        else:
-            # Make the relative path absolute with the build dir
-            if "BuildDirectory" in clang_tidy_diagnostic:
-                return os.path.normpath(
-                    os.path.abspath(
-                        os.path.join(clang_tidy_diagnostic["BuildDirectory"], file_path)
-                    )
+        elif "BuildDirectory" in clang_tidy_diagnostic:
+            return os.path.normpath(
+                os.path.abspath(
+                    os.path.join(clang_tidy_diagnostic["BuildDirectory"], file_path)
                 )
-            else:
-                return os.path.normpath(os.path.abspath(file_path))
+            )
+        else:
+            return os.path.normpath(os.path.abspath(file_path))
 
     # Pre-clang-tidy-9 format
     elif "FilePath" in clang_tidy_diagnostic:
@@ -476,7 +474,7 @@ def find_line_number_from_offset(offset_lookup, filename, offset):
 def read_one_line(filename, line_offset):
     """Read a single line from a source file"""
     # Could cache the files instead of opening them each time?
-    with open(filename, "r") as file:
+    with open(filename) as file:
         file.seek(line_offset)
         return file.readline().rstrip("\n")
 
@@ -660,7 +658,7 @@ def fix_absolute_paths(build_compile_commands, base_dir):
     """
 
     basedir = pathlib.Path(base_dir).resolve()
-    newbasedir = pathlib.Path(".").resolve()
+    newbasedir = pathlib.Path().resolve()
 
     if basedir == newbasedir:
         return
@@ -668,7 +666,7 @@ def fix_absolute_paths(build_compile_commands, base_dir):
     print(f"Found '{build_compile_commands}', updating absolute paths")
     # We might need to change some absolute paths if we're inside
     # a docker container
-    with open(build_compile_commands, "r") as f:
+    with open(build_compile_commands) as f:
         compile_commands = json.load(f)
 
     print(f"Replacing '{basedir}' with '{newbasedir}'", flush=True)
@@ -1033,7 +1031,7 @@ def download_artifacts(pull: PullRequest, workflow_id: int):
         "Authorization": f"Bearer {pull.token}",
     }
     r = urllib3.request("GET", artifact.archive_download_url, headers=headers)
-    if r.status is not 200:
+    if r.status != 200:
         print(
             f"WARNING: Couldn't automatically download artifacts for workflow '{workflow_id}', response was: {r}: {r.reason}"
         )
@@ -1056,7 +1054,7 @@ def load_metadata() -> Optional[Metadata]:
         print(f"WARNING: Could not find metadata file ('{METADATA_FILE}')", flush=True)
         return None
 
-    with open(METADATA_FILE, "r") as metadata_file:
+    with open(METADATA_FILE) as metadata_file:
         return json.load(metadata_file)
 
 
@@ -1076,7 +1074,7 @@ def load_review(review_file: pathlib.Path) -> Optional[PRReview]:
         print(f"WARNING: Could not find review file ('{review_file}')", flush=True)
         return None
 
-    with open(review_file, "r") as review_file_handle:
+    with open(review_file) as review_file_handle:
         payload = json.load(review_file_handle)
         return payload or None
 
@@ -1307,7 +1305,7 @@ def post_annotations(pull_request: PullRequest, review: Optional[PRReview]) -> i
     }
 
     if review is None:
-        return
+        return None
 
     if review["comments"] == []:
         print("No warnings to report, LGTM!")
diff --git a/post/clang_tidy_review/clang_tidy_review/post.py b/post/clang_tidy_review/clang_tidy_review/post.py
index b3cb087..34ee65b 100755
--- a/post/clang_tidy_review/clang_tidy_review/post.py
+++ b/post/clang_tidy_review/clang_tidy_review/post.py
@@ -10,17 +10,17 @@
 import pprint
 
 from clang_tidy_review import (
+    REVIEW_FILE,
     PullRequest,
+    add_auth_arguments,
+    bool_argument,
+    download_artifacts,
+    get_auth_from_arguments,
     load_and_merge_reviews,
-    post_review,
     load_metadata,
-    strip_enclosing_quotes,
-    download_artifacts,
     post_annotations,
-    bool_argument,
-    REVIEW_FILE,
-    add_auth_arguments,
-    get_auth_from_arguments,
+    post_review,
+    strip_enclosing_quotes,
 )
 
 
diff --git a/post/clang_tidy_review/clang_tidy_review/review.py b/post/clang_tidy_review/clang_tidy_review/review.py
index dbb76f7..10da8ef 100755
--- a/post/clang_tidy_review/clang_tidy_review/review.py
+++ b/post/clang_tidy_review/clang_tidy_review/review.py
@@ -13,20 +13,19 @@
 
 from clang_tidy_review import (
     PullRequest,
+    add_auth_arguments,
+    bool_argument,
     create_review,
     fix_absolute_paths,
+    get_auth_from_arguments,
     message_group,
+    post_annotations,
     post_review,
     save_metadata,
-    strip_enclosing_quotes,
-    post_annotations,
-    bool_argument,
     set_output,
-    add_auth_arguments,
-    get_auth_from_arguments,
+    strip_enclosing_quotes,
 )
 
-
 BAD_CHARS_APT_PACKAGES_PATTERN = "[;&|($]"
 
 
diff --git a/tests/test_review.py b/tests/test_review.py
index 9fa1527..24a3d70 100644
--- a/tests/test_review.py
+++ b/tests/test_review.py
@@ -465,7 +465,7 @@ def test_decorate_comment_body():
 
 
 def test_timing_summary(monkeypatch):
-    monkeypatch.setattr(ctr, "PROFILE_DIR", str(TEST_DIR / f"src/clang-tidy-profile"))
+    monkeypatch.setattr(ctr, "PROFILE_DIR", str(TEST_DIR / "src/clang-tidy-profile"))
     profiling = ctr.load_and_merge_profiling()
     assert "time.clang-tidy.total.wall" in profiling["hello.cxx"].keys()
     assert "time.clang-tidy.total.user" in profiling["hello.cxx"].keys()

From 90eec4f794a626e3856fcb4a4b37984fd2a8382f Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 15:53:04 +0100
Subject: [PATCH 03/11] Apply ruff unsafe fixes

---
 .../clang_tidy_review/__init__.py             | 26 ++++++++-----------
 .../clang_tidy_review/post.py                 |  3 ++-
 .../clang_tidy_review/review.py               |  2 +-
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index c002d9d..4398cf0 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -6,7 +6,6 @@
 import argparse
 import base64
 import contextlib
-import datetime
 import fnmatch
 import glob
 import io
@@ -20,13 +19,12 @@
 import textwrap
 import zipfile
 from operator import itemgetter
-from typing import Any, Dict, List, Optional, TypedDict
+from typing import Dict, List, Optional, TypedDict
 
 import unidiff
 import urllib3
 import yaml
 from github import Auth, Github
-from github.GithubException import GithubException
 from github.PaginatedList import PaginatedList
 from github.Requester import Requester
 from github.WorkflowRun import WorkflowRun
@@ -313,8 +311,7 @@ def get_pr_diff(self) -> List[unidiff.PatchedFile]:
         # property to be line count within each file's diff. So we need to
         # do this little bit of faff to get a list of file-diffs with
         # their own diff_line_no range
-        diff = [unidiff.PatchSet(str(file))[0] for file in unidiff.PatchSet(diffs)]
-        return diff
+        return [unidiff.PatchSet(str(file))[0] for file in unidiff.PatchSet(diffs)]
 
     def get_pr_author(self) -> str:
         """Get the username of the PR author. This is used in google-readability-todo"""
@@ -410,9 +407,10 @@ def make_file_offset_lookup(filenames):
         # Length of each line
         line_lengths = map(len, lines)
         # Cumulative sum of line lengths => offset at end of each line
-        lookup[os.path.abspath(filename)] = [0] + list(
-            itertools.accumulate(line_lengths)
-        )
+        lookup[os.path.abspath(filename)] = [
+            0,
+            *list(itertools.accumulate(line_lengths)),
+        ]
 
     return lookup
 
@@ -1080,11 +1078,11 @@ def load_review(review_file: pathlib.Path) -> Optional[PRReview]:
 
 
 def load_and_merge_profiling() -> Dict:
-    result = dict()
+    result = {}
     for profile_file in glob.iglob(os.path.join(PROFILE_DIR, "*.json")):
         profile_dict = json.load(open(profile_file))
         filename = profile_dict["file"]
-        current_profile = result.get(filename, dict())
+        current_profile = result.get(filename, {})
         for check, timing in profile_dict["profile"].items():
             current_profile[check] = current_profile.get(check, 0.0) + timing
         result[filename] = current_profile
@@ -1116,7 +1114,7 @@ def load_and_merge_reviews(review_files: List[pathlib.Path]) -> Optional[PRRevie
 
     comments = set()
     for review in reviews:
-        comments.update(map(lambda c: HashableComment(**c), review["comments"]))
+        comments.update(HashableComment(**c) for c in review["comments"])
 
     result["comments"] = [c.__dict__ for c in sorted(comments)]
 
@@ -1166,10 +1164,8 @@ def cull_comments(pull_request: PullRequest, review, max_comments):
 
     """
 
-    unposted_comments = set(map(lambda c: HashableComment(**c), review["comments"]))
-    posted_comments = set(
-        map(lambda c: HashableComment(**c), pull_request.get_pr_comments())
-    )
+    unposted_comments = {HashableComment(**c) for c in review["comments"]}
+    posted_comments = {HashableComment(**c) for c in pull_request.get_pr_comments()}
 
     review["comments"] = [
         c.__dict__ for c in sorted(unposted_comments - posted_comments)
diff --git a/post/clang_tidy_review/clang_tidy_review/post.py b/post/clang_tidy_review/clang_tidy_review/post.py
index 34ee65b..703d37c 100755
--- a/post/clang_tidy_review/clang_tidy_review/post.py
+++ b/post/clang_tidy_review/clang_tidy_review/post.py
@@ -8,6 +8,7 @@
 import argparse
 import pathlib
 import pprint
+import sys
 
 from clang_tidy_review import (
     REVIEW_FILE,
@@ -111,4 +112,4 @@ def main() -> int:
 
 
 if __name__ == "__main__":
-    exit(main())
+    sys.exit(main())
diff --git a/post/clang_tidy_review/clang_tidy_review/review.py b/post/clang_tidy_review/clang_tidy_review/review.py
index 10da8ef..379d8c7 100755
--- a/post/clang_tidy_review/clang_tidy_review/review.py
+++ b/post/clang_tidy_review/clang_tidy_review/review.py
@@ -131,7 +131,7 @@ def main():
         with message_group(f"Installing additional packages: {apt_packages}"):
             subprocess.run(["apt-get", "update"], check=True)
             subprocess.run(
-                ["apt-get", "install", "-y", "--no-install-recommends"] + apt_packages,
+                ["apt-get", "install", "-y", "--no-install-recommends", *apt_packages],
                 check=True,
             )
 

From 52198619182e71e52ebb9a2f4ebe38c36027b789 Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 15:56:34 +0100
Subject: [PATCH 04/11] Switch `open().read()` to `pathlib.Path().read_text()`

---
 post/clang_tidy_review/clang_tidy_review/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index 4398cf0..d26183c 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -139,7 +139,7 @@ def get_auth_from_arguments(args: argparse.Namespace) -> Auth:
         elif args.private_key_base64:
             private_key = base64.b64decode(args.private_key_base64).decode("ascii")
         else:
-            private_key = open(args.private_key_file_path).read()
+            private_key = pathlib.Path(args.private_key_file_path).read_text()
         return Auth.AppAuth(args.app_id, private_key).get_installation_auth(
             args.installation_id
         )

From b03fbdb8b4da183e323f4f43dc32e3013f05c113 Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 15:59:41 +0100
Subject: [PATCH 05/11] Fix `else` after `return`

---
 .../clang_tidy_review/__init__.py               | 17 +++++++----------
 .../clang_tidy_review/clang_tidy_review/post.py |  5 +----
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index d26183c..2978a39 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -143,7 +143,7 @@ def get_auth_from_arguments(args: argparse.Namespace) -> Auth:
         return Auth.AppAuth(args.app_id, private_key).get_installation_auth(
             args.installation_id
         )
-    elif (
+    if (
         args.app_id
         or args.private_key
         or args.private_key_file_path
@@ -427,27 +427,24 @@ def get_diagnostic_file_path(clang_tidy_diagnostic, build_dir):
         file_path = clang_tidy_diagnostic["DiagnosticMessage"]["FilePath"]
         if file_path == "":
             return ""
-        elif os.path.isabs(file_path):
+        if os.path.isabs(file_path):
             return os.path.normpath(os.path.abspath(file_path))
-        elif "BuildDirectory" in clang_tidy_diagnostic:
+        if "BuildDirectory" in clang_tidy_diagnostic:
             return os.path.normpath(
                 os.path.abspath(
                     os.path.join(clang_tidy_diagnostic["BuildDirectory"], file_path)
                 )
             )
-        else:
-            return os.path.normpath(os.path.abspath(file_path))
+        return os.path.normpath(os.path.abspath(file_path))
 
     # Pre-clang-tidy-9 format
-    elif "FilePath" in clang_tidy_diagnostic:
+    if "FilePath" in clang_tidy_diagnostic:
         file_path = clang_tidy_diagnostic["FilePath"]
         if file_path == "":
             return ""
-        else:
-            return os.path.normpath(os.path.abspath(os.path.join(build_dir, file_path)))
+        return os.path.normpath(os.path.abspath(os.path.join(build_dir, file_path)))
 
-    else:
-        return ""
+    return ""
 
 
 def find_line_number_from_offset(offset_lookup, filename, offset):
diff --git a/post/clang_tidy_review/clang_tidy_review/post.py b/post/clang_tidy_review/clang_tidy_review/post.py
index 703d37c..4c337f2 100755
--- a/post/clang_tidy_review/clang_tidy_review/post.py
+++ b/post/clang_tidy_review/clang_tidy_review/post.py
@@ -105,10 +105,7 @@ def main() -> int:
             pull_request, review, args.max_comments, lgtm_comment_body, args.dry_run
         )
 
-    if args.num_comments_as_exitcode:
-        return exit_code
-    else:
-        return 0
+    return exit_code if args.num_comments_as_exitcode else 0
 
 
 if __name__ == "__main__":

From 50001b350c125614435bb9fba832266358206bb7 Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Fri, 19 Jul 2024 16:04:42 +0100
Subject: [PATCH 06/11] CI: Drop a bunch of extra ruff rules

---
 post/clang_tidy_review/pyproject.toml | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/post/clang_tidy_review/pyproject.toml b/post/clang_tidy_review/pyproject.toml
index 34cf300..2d73b4e 100644
--- a/post/clang_tidy_review/pyproject.toml
+++ b/post/clang_tidy_review/pyproject.toml
@@ -52,13 +52,8 @@ extend_exclude = "_version.py"
 extend-select = [
   "B",           # flake8-bugbear
   "I",           # isort
-  "ARG",         # flake8-unused-arguments
   "C4",          # flake8-comprehensions
   "ICN",         # flake8-import-conventions
-  "G",           # flake8-logging-format
-  "PGH",         # pygrep-hooks
-  "PIE",         # flake8-pie
-  "PL",          # pylint
   "PT",          # flake8-pytest-style
   "PTH",         # flake8-use-pathlib
   "RET",         # flake8-return
@@ -67,18 +62,5 @@ extend-select = [
   "UP",          # pyupgrade
   "YTT",         # flake8-2020
   "EXE",         # flake8-executable
-  "NPY",         # NumPy specific rules
-  "PD",          # pandas-vet
   "FURB",        # refurb
 ]
-ignore = [
-  "PLR2004",     # magic-comparison
-  "B9",          # flake8-bugbear opinionated warnings
-  "PLC0414",     # useless-import-alias
-  "PLR0913",     # too-many-arguments
-  "PLR0917",     # too-many-positional
-  "PLR0914",     # too-many-locals
-  "PLR0915",     # too-many-statements
-  "PLR0912",     # too-many-branches
-  "PTH123",      # builtin-open
-]

From afd3a850fa8d6b9bcaec39c2905dab421a54938b Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev@outlook.de>
Date: Fri, 1 Nov 2024 19:46:06 +0100
Subject: [PATCH 07/11] fix: resolve `PTH*` lints

---
 .../clang_tidy_review/__init__.py             | 51 +++++++++----------
 .../clang_tidy_review/review.py               |  7 ++-
 2 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index 2978a39..0232b57 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -7,7 +7,6 @@
 import base64
 import contextlib
 import fnmatch
-import glob
 import io
 import itertools
 import json
@@ -19,6 +18,7 @@
 import textwrap
 import zipfile
 from operator import itemgetter
+from pathlib import Path
 from typing import Dict, List, Optional, TypedDict
 
 import unidiff
@@ -251,7 +251,7 @@ def config_file_or_checks(
 def load_clang_tidy_warnings():
     """Read clang-tidy warnings from FIXES_FILE. Can be produced by build_clang_tidy_warnings"""
     try:
-        with open(FIXES_FILE) as fixes_file:
+        with Path(FIXES_FILE).open() as fixes_file:
             return yaml.safe_load(fixes_file)
     except FileNotFoundError:
         return {}
@@ -402,12 +402,12 @@ def make_file_offset_lookup(filenames):
     lookup = {}
 
     for filename in filenames:
-        with open(filename) as file:
+        with Path(filename).open() as file:
             lines = file.readlines()
         # Length of each line
         line_lengths = map(len, lines)
         # Cumulative sum of line lengths => offset at end of each line
-        lookup[os.path.abspath(filename)] = [
+        lookup[Path(filename).resolve().as_posix()] = [
             0,
             *list(itertools.accumulate(line_lengths)),
         ]
@@ -427,22 +427,21 @@ def get_diagnostic_file_path(clang_tidy_diagnostic, build_dir):
         file_path = clang_tidy_diagnostic["DiagnosticMessage"]["FilePath"]
         if file_path == "":
             return ""
-        if os.path.isabs(file_path):
-            return os.path.normpath(os.path.abspath(file_path))
+        file_path = Path(file_path)
+        if file_path.is_absolute():
+            return os.path.normpath(file_path.resolve())
         if "BuildDirectory" in clang_tidy_diagnostic:
             return os.path.normpath(
-                os.path.abspath(
-                    os.path.join(clang_tidy_diagnostic["BuildDirectory"], file_path)
-                )
+                (Path(clang_tidy_diagnostic["BuildDirectory"]) / file_path).resolve()
             )
-        return os.path.normpath(os.path.abspath(file_path))
+        return os.path.normpath(file_path.resolve())
 
     # Pre-clang-tidy-9 format
     if "FilePath" in clang_tidy_diagnostic:
         file_path = clang_tidy_diagnostic["FilePath"]
         if file_path == "":
             return ""
-        return os.path.normpath(os.path.abspath(os.path.join(build_dir, file_path)))
+        return os.path.normpath((Path(build_dir) / file_path).resolve())
 
     return ""
 
@@ -469,7 +468,7 @@ def find_line_number_from_offset(offset_lookup, filename, offset):
 def read_one_line(filename, line_offset):
     """Read a single line from a source file"""
     # Could cache the files instead of opening them each time?
-    with open(filename) as file:
+    with Path(filename).open() as file:
         file.seek(line_offset)
         return file.readline().rstrip("\n")
 
@@ -493,7 +492,7 @@ def collate_replacement_sets(diagnostic, offset_lookup):
         # from the FilePath and we'll end up looking for a path that's not in
         # the lookup dict
         # To fix this, we'll convert all the FilePaths to absolute paths
-        replacement["FilePath"] = os.path.abspath(replacement["FilePath"])
+        replacement["FilePath"] = Path(replacement["FilePath"]).resolve().as_posix()
 
         # It's possible the replacement is needed in another file?
         # Not really sure how that could come about, but let's
@@ -661,7 +660,7 @@ def fix_absolute_paths(build_compile_commands, base_dir):
     print(f"Found '{build_compile_commands}', updating absolute paths")
     # We might need to change some absolute paths if we're inside
     # a docker container
-    with open(build_compile_commands) as f:
+    with Path(build_compile_commands).open() as f:
         compile_commands = json.load(f)
 
     print(f"Replacing '{basedir}' with '{newbasedir}'", flush=True)
@@ -670,7 +669,7 @@ def fix_absolute_paths(build_compile_commands, base_dir):
         str(basedir), str(newbasedir)
     )
 
-    with open(build_compile_commands, "w") as f:
+    with Path(build_compile_commands).open("w") as f:
         f.write(modified_compile_commands)
 
 
@@ -931,7 +930,7 @@ def create_review(
 
     if files == []:
         with message_group("No files to check!"):
-            with open(REVIEW_FILE, "w") as review_file:
+            with Path(REVIEW_FILE).open("w") as review_file:
                 json.dump(
                     {
                         "body": "clang-tidy found no files to check",
@@ -947,7 +946,7 @@ def create_review(
     line_ranges = get_line_ranges(diff, files)
     if line_ranges == "[]":
         with message_group("No lines added in this PR!"):
-            with open(REVIEW_FILE, "w") as review_file:
+            with Path(REVIEW_FILE).open("w") as review_file:
                 json.dump(
                     {
                         "body": "clang-tidy found no lines added",
@@ -997,7 +996,7 @@ def create_review(
         review = create_review_file(
             clang_tidy_warnings, diff_lookup, offset_lookup, build_dir
         )
-        with open(REVIEW_FILE, "w") as review_file:
+        with Path(REVIEW_FILE).open("w") as review_file:
             json.dump(review, review_file)
 
         return review
@@ -1049,7 +1048,7 @@ def load_metadata() -> Optional[Metadata]:
         print(f"WARNING: Could not find metadata file ('{METADATA_FILE}')", flush=True)
         return None
 
-    with open(METADATA_FILE) as metadata_file:
+    with Path(METADATA_FILE).open() as metadata_file:
         return json.load(metadata_file)
 
 
@@ -1058,7 +1057,7 @@ def save_metadata(pr_number: int) -> None:
 
     metadata: Metadata = {"pr_number": pr_number}
 
-    with open(METADATA_FILE, "w") as metadata_file:
+    with Path(METADATA_FILE).open("w") as metadata_file:
         json.dump(metadata, metadata_file)
 
 
@@ -1069,15 +1068,15 @@ def load_review(review_file: pathlib.Path) -> Optional[PRReview]:
         print(f"WARNING: Could not find review file ('{review_file}')", flush=True)
         return None
 
-    with open(review_file) as review_file_handle:
+    with review_file.open() as review_file_handle:
         payload = json.load(review_file_handle)
         return payload or None
 
 
 def load_and_merge_profiling() -> Dict:
     result = {}
-    for profile_file in glob.iglob(os.path.join(PROFILE_DIR, "*.json")):
-        profile_dict = json.load(open(profile_file))
+    for profile_file in Path(PROFILE_DIR).glob("*.json"):
+        profile_dict = json.load(profile_file.open())
         filename = profile_dict["file"]
         current_profile = result.get(filename, {})
         for check, timing in profile_dict["profile"].items():
@@ -1150,7 +1149,7 @@ def get_line_ranges(diff, files):
         # Adding a copy of the line filters with backslashes allows for both cl.exe and clang.exe to work.
         if os.path.sep == "\\":
             # Converts name to backslashes for the cl.exe line filter.
-            name = os.path.join(*name.split("/"))
+            name = Path.joinpath(*name.split("/"))
             line_filter_json.append({"name": name, "lines": lines})
     return json.dumps(line_filter_json, separators=(",", ":"))
 
@@ -1196,7 +1195,7 @@ def set_output(key: str, val: str) -> bool:
         return False
 
     # append key-val pair to file
-    with open(os.environ["GITHUB_OUTPUT"], "a") as f:
+    with Path(os.environ["GITHUB_OUTPUT"]).open("a") as f:
         f.write(f"{key}={val}\n")
 
     return True
@@ -1207,7 +1206,7 @@ def set_summary(val: str) -> bool:
         return False
 
     # append key-val pair to file
-    with open(os.environ["GITHUB_STEP_SUMMARY"], "a") as f:
+    with Path(os.environ["GITHUB_STEP_SUMMARY"]).open("a") as f:
         f.write(val)
 
     return True
diff --git a/post/clang_tidy_review/clang_tidy_review/review.py b/post/clang_tidy_review/clang_tidy_review/review.py
index 379d8c7..c186736 100755
--- a/post/clang_tidy_review/clang_tidy_review/review.py
+++ b/post/clang_tidy_review/clang_tidy_review/review.py
@@ -6,10 +6,9 @@
 # See LICENSE for more information
 
 import argparse
-import os
-import pathlib
 import re
 import subprocess
+from pathlib import Path
 
 from clang_tidy_review import (
     PullRequest,
@@ -39,7 +38,7 @@ def main():
         "--clang_tidy_binary",
         help="clang-tidy binary",
         default="clang-tidy-14",
-        type=pathlib.Path,
+        type=Path,
     )
     parser.add_argument(
         "--build_dir", help="Directory with compile_commands.json", default="."
@@ -145,7 +144,7 @@ def main():
         with message_group(f"Running cmake: {cmake_command}"):
             subprocess.run(cmake_command, shell=True, check=True)
 
-    elif os.path.exists(build_compile_commands):
+    elif Path(build_compile_commands).exists():
         fix_absolute_paths(build_compile_commands, args.base_dir)
 
     pull_request = PullRequest(args.repo, args.pr, get_auth_from_arguments(args))

From 71fa1583db3d7f4ec226ef3d6c87b57be3a687e1 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev@outlook.de>
Date: Fri, 1 Nov 2024 19:48:27 +0100
Subject: [PATCH 08/11] fix: merge `with` (`SIM117`)

---
 .../clang_tidy_review/__init__.py             | 42 ++++++++++---------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index 0232b57..775327f 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -929,32 +929,34 @@ def create_review(
     files = filter_files(diff, include, exclude)
 
     if files == []:
-        with message_group("No files to check!"):
-            with Path(REVIEW_FILE).open("w") as review_file:
-                json.dump(
-                    {
-                        "body": "clang-tidy found no files to check",
-                        "event": "COMMENT",
-                        "comments": [],
-                    },
-                    review_file,
-                )
+        with message_group("No files to check!"), Path(REVIEW_FILE).open(
+            "w"
+        ) as review_file:
+            json.dump(
+                {
+                    "body": "clang-tidy found no files to check",
+                    "event": "COMMENT",
+                    "comments": [],
+                },
+                review_file,
+            )
         return None
 
     print(f"Checking these files: {files}", flush=True)
 
     line_ranges = get_line_ranges(diff, files)
     if line_ranges == "[]":
-        with message_group("No lines added in this PR!"):
-            with Path(REVIEW_FILE).open("w") as review_file:
-                json.dump(
-                    {
-                        "body": "clang-tidy found no lines added",
-                        "event": "COMMENT",
-                        "comments": [],
-                    },
-                    review_file,
-                )
+        with message_group("No lines added in this PR!"), Path(REVIEW_FILE).open(
+            "w"
+        ) as review_file:
+            json.dump(
+                {
+                    "body": "clang-tidy found no lines added",
+                    "event": "COMMENT",
+                    "comments": [],
+                },
+                review_file,
+            )
         return None
 
     print(f"Line filter for clang-tidy:\n{line_ranges}\n")

From 5e9fb02cd82bf6494d4beb0b131af93e0d3e1fa3 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev@outlook.de>
Date: Fri, 1 Nov 2024 19:49:45 +0100
Subject: [PATCH 09/11] fix: use positional args for regex (`B034`)

---
 post/clang_tidy_review/clang_tidy_review/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index 775327f..303ff7b 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -1224,7 +1224,7 @@ def decorate_check_names(comment: str) -> str:
     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)
+    return re.sub(regex, subst, comment, count=1, flags=re.MULTILINE)
 
 
 def decorate_comment(comment: PRReviewComment) -> PRReviewComment:

From 87a563ce94ff56fa79e055f3d503352b1b2c14e7 Mon Sep 17 00:00:00 2001
From: Nerixyz <nerixdev@outlook.de>
Date: Fri, 1 Nov 2024 20:31:57 +0100
Subject: [PATCH 10/11] fix: correct various types

---
 .../clang_tidy_review/__init__.py             | 33 ++++++++-----------
 .../clang_tidy_review/post.py                 |  2 +-
 .../clang_tidy_review/review.py               |  2 +-
 3 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index 303ff7b..154e951 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -19,13 +19,14 @@
 import zipfile
 from operator import itemgetter
 from pathlib import Path
-from typing import Dict, List, Optional, TypedDict
+from typing import Any, Dict, List, Optional, TypedDict
 
 import unidiff
 import urllib3
 import yaml
 from github import Auth, Github
 from github.PaginatedList import PaginatedList
+from github.PullRequest import ReviewComment
 from github.Requester import Requester
 from github.WorkflowRun import WorkflowRun
 
@@ -46,20 +47,10 @@ class Metadata(TypedDict):
     pr_number: int
 
 
-class PRReviewComment(TypedDict):
-    path: str
-    position: Optional[int]
-    body: str
-    line: Optional[int]
-    side: Optional[str]
-    start_line: Optional[int]
-    start_side: Optional[str]
-
-
 class PRReview(TypedDict):
     body: str
     event: str
-    comments: List[PRReviewComment]
+    comments: list[ReviewComment]
 
 
 class HashableComment:
@@ -125,7 +116,7 @@ def add_auth_arguments(parser: argparse.ArgumentParser):
     group_app.add_argument("--installation-id", type=int, help="app installation ID")
 
 
-def get_auth_from_arguments(args: argparse.Namespace) -> Auth:
+def get_auth_from_arguments(args: argparse.Namespace) -> Auth.Auth:
     if args.token:
         return Auth.Token(args.token)
 
@@ -260,7 +251,7 @@ def load_clang_tidy_warnings():
 class PullRequest:
     """Add some convenience functions not in PyGithub"""
 
-    def __init__(self, repo: str, pr_number: Optional[int], auth: Auth) -> None:
+    def __init__(self, repo: str, pr_number: Optional[int], auth: Auth.Auth) -> None:
         self.repo_name = repo
         self.pr_number = pr_number
         self.auth = auth
@@ -539,7 +530,7 @@ def replace_one_line(replacement_set, line_num, offset_lookup):
     line_offset = offset_lookup[filename][line_num]
 
     # List of (start, end) offsets from line_offset
-    insert_offsets = [(0, 0)]
+    insert_offsets: list[tuple[Optional[int], Optional[int]]] = [(0, 0)]
     # Read all the source lines into a dict so we only get one copy of
     # each line, though we might read the same line in multiple times
     source_lines = {}
@@ -755,7 +746,7 @@ def create_review_file(
     if "Diagnostics" not in clang_tidy_warnings:
         return None
 
-    comments: List[PRReviewComment] = []
+    comments: List[ReviewComment] = []
 
     for diagnostic in clang_tidy_warnings["Diagnostics"]:
         try:
@@ -1227,7 +1218,7 @@ def decorate_check_names(comment: str) -> str:
     return re.sub(regex, subst, comment, count=1, flags=re.MULTILINE)
 
 
-def decorate_comment(comment: PRReviewComment) -> PRReviewComment:
+def decorate_comment(comment: ReviewComment) -> ReviewComment:
     comment["body"] = decorate_check_names(comment["body"])
     return comment
 
@@ -1288,10 +1279,12 @@ def convert_comment_to_annotations(comment):
     }
 
 
-def post_annotations(pull_request: PullRequest, review: Optional[PRReview]) -> int:
+def post_annotations(
+    pull_request: PullRequest, review: Optional[PRReview]
+) -> Optional[int]:
     """Post the first 10 comments in the review as annotations"""
 
-    body = {
+    body: dict[str, Any] = {
         "name": "clang-tidy-review",
         "head_sha": pull_request.pull_request.head.sha,
         "status": "completed",
@@ -1309,7 +1302,7 @@ def post_annotations(pull_request: PullRequest, review: Optional[PRReview]) -> i
     for comment in review["comments"]:
         first_line = comment["body"].splitlines()[0]
         comments.append(
-            f"{comment['path']}:{comment.get('start_line', comment['line'])}: {first_line}"
+            f"{comment['path']}:{comment.get('start_line', comment.get('line', 0))}: {first_line}"
         )
 
     total_comments = len(review["comments"])
diff --git a/post/clang_tidy_review/clang_tidy_review/post.py b/post/clang_tidy_review/clang_tidy_review/post.py
index 4c337f2..67ff296 100755
--- a/post/clang_tidy_review/clang_tidy_review/post.py
+++ b/post/clang_tidy_review/clang_tidy_review/post.py
@@ -105,7 +105,7 @@ def main() -> int:
             pull_request, review, args.max_comments, lgtm_comment_body, args.dry_run
         )
 
-    return exit_code if args.num_comments_as_exitcode else 0
+    return (exit_code or 0) if args.num_comments_as_exitcode else 0
 
 
 if __name__ == "__main__":
diff --git a/post/clang_tidy_review/clang_tidy_review/review.py b/post/clang_tidy_review/clang_tidy_review/review.py
index c186736..9efb8e6 100755
--- a/post/clang_tidy_review/clang_tidy_review/review.py
+++ b/post/clang_tidy_review/clang_tidy_review/review.py
@@ -164,7 +164,7 @@ def main():
 
     if args.split_workflow:
         total_comments = 0 if review is None else len(review["comments"])
-        set_output("total_comments", total_comments)
+        set_output("total_comments", str(total_comments))
         print("split_workflow is enabled, not posting review")
         return
 

From 3aea09f3850d4fc1cd1aaff9aa99dbdf2613679b Mon Sep 17 00:00:00 2001
From: Peter Hill <peter.hill@york.ac.uk>
Date: Wed, 6 Nov 2024 17:20:34 +0000
Subject: [PATCH 11/11] Use `Path` in more places

---
 .../clang_tidy_review/__init__.py             | 43 +++++++++----------
 tests/test_review.py                          |  4 +-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/post/clang_tidy_review/clang_tidy_review/__init__.py b/post/clang_tidy_review/clang_tidy_review/__init__.py
index f0b0468..ef1f83e 100644
--- a/post/clang_tidy_review/clang_tidy_review/__init__.py
+++ b/post/clang_tidy_review/clang_tidy_review/__init__.py
@@ -8,7 +8,6 @@
 import contextlib
 import datetime
 import fnmatch
-import glob
 import io
 import itertools
 import json
@@ -24,7 +23,6 @@
 import tempfile
 import textwrap
 import threading
-import traceback
 import zipfile
 from operator import itemgetter
 from pathlib import Path
@@ -40,10 +38,10 @@
 from github.WorkflowRun import WorkflowRun
 
 DIFF_HEADER_LINE_LENGTH = 5
-FIXES_FILE = "clang_tidy_review.yaml"
-METADATA_FILE = "clang-tidy-review-metadata.json"
-REVIEW_FILE = "clang-tidy-review-output.json"
-PROFILE_DIR = "clang-tidy-review-profile"
+FIXES_FILE = Path("clang_tidy_review.yaml")
+METADATA_FILE = Path("clang-tidy-review-metadata.json")
+REVIEW_FILE = Path("clang-tidy-review-output.json")
+PROFILE_DIR = Path("clang-tidy-review-profile")
 MAX_ANNOTATIONS = 10
 
 
@@ -161,7 +159,7 @@ def get_auth_from_arguments(args: argparse.Namespace) -> Auth.Auth:
 def build_clang_tidy_warnings(
     base_invocation: List,
     env: dict,
-    tmpdir: str,
+    tmpdir: Path,
     task_queue: queue.Queue,
     lock: threading.Lock,
     failed_files: List,
@@ -184,7 +182,6 @@ def build_clang_tidy_warnings(
             invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env
         )
         output, err = proc.communicate()
-        end = datetime.datetime.now()
 
         if proc.returncode != 0:
             if proc.returncode < 0:
@@ -246,14 +243,15 @@ def config_file_or_checks(
     return "--config"
 
 
-def merge_replacement_files(tmpdir: str, mergefile: str):
+def merge_replacement_files(tmpdir: Path, mergefile: Path):
     """Merge all replacement files in a directory into a single file"""
     # The fixes suggested by clang-tidy >= 4.0.0 are given under
     # the top level key 'Diagnostics' in the output yaml files
     mergekey = "Diagnostics"
     merged = []
-    for replacefile in glob.iglob(os.path.join(tmpdir, "*.yaml")):
-        content = yaml.safe_load(open(replacefile, "r"))
+    for replacefile in tmpdir.glob("*.yaml"):
+        with replacefile.open() as f:
+            content = yaml.safe_load(f)
         if not content:
             continue  # Skip empty files.
         merged.extend(content.get(mergekey, []))
@@ -264,15 +262,15 @@ def merge_replacement_files(tmpdir: str, mergefile: str):
         # is actually never used inside clang-apply-replacements,
         # so we set it to '' here.
         output = {"MainSourceFile": "", mergekey: merged}
-        with open(mergefile, "w") as out:
+        with mergefile.open("w") as out:
             yaml.safe_dump(output, out)
 
 
 def load_clang_tidy_warnings(fixes_file) -> Dict:
     """Read clang-tidy warnings from fixes_file. Can be produced by build_clang_tidy_warnings"""
     try:
-        with Path(FIXES_FILE).open() as fixes_file:
-            return yaml.safe_load(fixes_file)
+        with fixes_file.open() as f:
+            return yaml.safe_load(f)
     except FileNotFoundError:
         return {}
 
@@ -672,7 +670,7 @@ def fix_absolute_paths(build_compile_commands, base_dir):
     """
 
     basedir = pathlib.Path(base_dir).resolve()
-    newbasedir = pathlib.Path().resolve()
+    newbasedir = Path.cwd()
 
     if basedir == newbasedir:
         return
@@ -992,8 +990,7 @@ def create_review(
     username = pull_request.get_pr_author() or "your name here"
 
     # Run clang-tidy with the configured parameters and produce the CLANG_TIDY_FIXES file
-    return_code = 0
-    export_fixes_dir = tempfile.mkdtemp()
+    export_fixes_dir = Path(tempfile.mkdtemp())
     env = dict(os.environ, USER=username)
     config = config_file_or_checks(clang_tidy_binary, clang_tidy_checks, config_file)
     base_invocation = [
@@ -1038,8 +1035,6 @@ def create_review(
 
         # Wait for all threads to be done.
         task_queue.join()
-        if len(failed_files):
-            return_code = 1
 
     except KeyboardInterrupt:
         # This is a sad hack. Unfortunately subprocess goes
@@ -1050,7 +1045,7 @@ def create_review(
     real_duration = datetime.datetime.now() - start
 
     # Read and parse the CLANG_TIDY_FIXES file
-    print("Writing fixes to " + FIXES_FILE + " ...")
+    print(f"Writing fixes to {FIXES_FILE} ...")
     merge_replacement_files(export_fixes_dir, FIXES_FILE)
     shutil.rmtree(export_fixes_dir)
     clang_tidy_warnings = load_clang_tidy_warnings(FIXES_FILE)
@@ -1115,9 +1110,13 @@ def download_artifacts(pull: PullRequest, workflow_id: int):
     filenames = data.namelist()
 
     metadata = (
-        json.loads(data.read(METADATA_FILE)) if METADATA_FILE in filenames else None
+        json.loads(data.read(str(METADATA_FILE)))
+        if METADATA_FILE in filenames
+        else None
+    )
+    review = (
+        json.loads(data.read(str(REVIEW_FILE))) if REVIEW_FILE in filenames else None
     )
-    review = json.loads(data.read(REVIEW_FILE)) if REVIEW_FILE in filenames else None
     return metadata, review
 
 
diff --git a/tests/test_review.py b/tests/test_review.py
index 235046b..b850de3 100644
--- a/tests/test_review.py
+++ b/tests/test_review.py
@@ -238,9 +238,7 @@ def test_line_ranges():
 
 
 def test_load_clang_tidy_warnings():
-    warnings = ctr.load_clang_tidy_warnings(
-        str(TEST_DIR / f"src/test_{ctr.FIXES_FILE}")
-    )
+    warnings = ctr.load_clang_tidy_warnings(TEST_DIR / f"src/test_{ctr.FIXES_FILE}")
 
     assert sorted(list(warnings.keys())) == ["Diagnostics", "MainSourceFile"]
     assert warnings["MainSourceFile"] == "/clang_tidy_review/src/hello.cxx"