Merge pull request #137 from Nerixyz/chore/more-linting

Fix remaining Ruff lints and (most) type errors

Author: ZedThree

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,13 +18,15 @@
 import textwrap
 import zipfile
 from operator import itemgetter
-from typing import Dict, List, Optional, TypedDict
+from pathlib import Path
+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)
 
@@ -251,7 +242,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 {}
@@ -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
@@ -402,12 +393,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 +418,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 +459,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 +483,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
@@ -540,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 = {}
@@ -661,7 +651,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 +660,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)
 
 
@@ -756,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:
@@ -930,32 +920,34 @@ def create_review(
     files = filter_files(diff, include, exclude)
 
     if files == []:
-        with message_group("No files to check!"):
-            with open(REVIEW_FILE, "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 open(REVIEW_FILE, "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")
@@ -997,7 +989,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 +1041,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 +1050,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 +1061,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 +1142,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 +1188,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 +1199,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
@@ -1223,10 +1215,10 @@ 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:
+def decorate_comment(comment: ReviewComment) -> ReviewComment:
     comment["body"] = decorate_check_names(comment["body"])
     return comment
 
@@ -1287,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",
@@ -1308,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"])

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__":

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))
@@ -165,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