[llvm] [Workflow] make code-format-helper.py mypy-safe (NFC) (PR #69691)

[Ryan Prichard via llvm-commits]

https://github.com/rprichard updated https://github.com/llvm/llvm-project/pull/69691

>From 8a96d1d654e7fd8825eade605936c6cb6385b93e Mon Sep 17 00:00:00 2001
From: Ryan Prichard <rprichard at google.com>
Date: Fri, 20 Oct 2023 01:15:02 -0700
Subject: [PATCH 1/2] [Workflow] make code-format-helper.py mypy-safe (NFC)

Fix type errors that mypy reports with code-format-helper.py.

Add a few return type annotations and change `param: [str]` to
`param: list[str]`.

Leave a few required FormatHelper members missing instead of defining a
placeholder:
 - FormatHelper.name
 - FormatHelper.friendly_name
 - FormatHelper.format_run: NotImplementedError() instead of `pass`
---
 llvm/utils/git/code-format-helper.py | 39 ++++++++++++++++++----------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/llvm/utils/git/code-format-helper.py b/llvm/utils/git/code-format-helper.py
index 8d3c30b309d015d..f02415bfbe0d201 100644
--- a/llvm/utils/git/code-format-helper.py
+++ b/llvm/utils/git/code-format-helper.py
@@ -20,14 +20,21 @@
 
 class FormatHelper:
     COMMENT_TAG = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->"
-    name = "unknown"
+    name: str
+    friendly_name: str
 
     @property
     def comment_tag(self) -> str:
         return self.COMMENT_TAG.replace("fmt", self.name)
 
-    def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
-        pass
+    @property
+    def instructions(self) -> str:
+        raise NotImplementedError()
+
+    def format_run(
+        self, changed_files: list[str], args: argparse.Namespace
+    ) -> str | None:
+        raise NotImplementedError()
 
     def pr_comment_text(self, diff: str) -> str:
         return f"""
@@ -66,7 +73,7 @@ def find_comment(
                 return comment
         return None
 
-    def update_pr(self, diff: str, args: argparse.Namespace):
+    def update_pr(self, diff: str, args: argparse.Namespace) -> None:
         repo = github.Github(args.token).get_repo(args.repo)
         pr = repo.get_issue(args.issue_number).as_pull_request()
 
@@ -78,7 +85,7 @@ def update_pr(self, diff: str, args: argparse.Namespace):
         else:
             pr.as_issue().create_comment(pr_text)
 
-    def update_pr_success(self, args: argparse.Namespace):
+    def update_pr_success(self, args: argparse.Namespace) -> None:
         repo = github.Github(args.token).get_repo(args.repo)
         pr = repo.get_issue(args.issue_number).as_pull_request()
 
@@ -91,7 +98,7 @@ def update_pr_success(self, args: argparse.Namespace):
 """
             )
 
-    def run(self, changed_files: [str], args: argparse.Namespace):
+    def run(self, changed_files: list[str], args: argparse.Namespace) -> bool:
         diff = self.format_run(changed_files, args)
         if diff:
             self.update_pr(diff, args)
@@ -106,11 +113,11 @@ class ClangFormatHelper(FormatHelper):
     friendly_name = "C/C++ code formatter"
 
     @property
-    def instructions(self):
+    def instructions(self) -> str:
         return " ".join(self.cf_cmd)
 
     @cached_property
-    def libcxx_excluded_files(self):
+    def libcxx_excluded_files(self) -> list[str]:
         with open("libcxx/utils/data/ignore_format.txt", "r") as ifd:
             return [excl.strip() for excl in ifd.readlines()]
 
@@ -120,7 +127,7 @@ def should_be_excluded(self, path: str) -> bool:
             return True
         return False
 
-    def filter_changed_files(self, changed_files: [str]) -> [str]:
+    def filter_changed_files(self, changed_files: list[str]) -> list[str]:
         filtered_files = []
         for path in changed_files:
             _, ext = os.path.splitext(path)
@@ -129,10 +136,12 @@ def filter_changed_files(self, changed_files: [str]) -> [str]:
                     filtered_files.append(path)
         return filtered_files
 
-    def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
+    def format_run(
+        self, changed_files: list[str], args: argparse.Namespace
+    ) -> str | None:
         cpp_files = self.filter_changed_files(changed_files)
         if not cpp_files:
-            return
+            return None
         cf_cmd = [
             "git-clang-format",
             "--diff",
@@ -159,7 +168,7 @@ class DarkerFormatHelper(FormatHelper):
     def instructions(self):
         return " ".join(self.darker_cmd)
 
-    def filter_changed_files(self, changed_files: [str]) -> [str]:
+    def filter_changed_files(self, changed_files: list[str]) -> list[str]:
         filtered_files = []
         for path in changed_files:
             name, ext = os.path.splitext(path)
@@ -168,10 +177,12 @@ def filter_changed_files(self, changed_files: [str]) -> [str]:
 
         return filtered_files
 
-    def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None:
+    def format_run(
+        self, changed_files: list[str], args: argparse.Namespace
+    ) -> str | None:
         py_files = self.filter_changed_files(changed_files)
         if not py_files:
-            return
+            return None
         darker_cmd = [
             "darker",
             "--check",

>From 07fe3c011b8d412cda3d7f9aab6d94307aa10897 Mon Sep 17 00:00:00 2001
From: Ryan Prichard <rprichard at google.com>
Date: Fri, 20 Oct 2023 01:43:57 -0700
Subject: [PATCH 2/2] Also annotate DarkerFormatHelper.instructions return type

---
 llvm/utils/git/code-format-helper.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/utils/git/code-format-helper.py b/llvm/utils/git/code-format-helper.py
index f02415bfbe0d201..742fa5a2eeb9456 100644
--- a/llvm/utils/git/code-format-helper.py
+++ b/llvm/utils/git/code-format-helper.py
@@ -165,7 +165,7 @@ class DarkerFormatHelper(FormatHelper):
     friendly_name = "Python code formatter"
 
     @property
-    def instructions(self):
+    def instructions(self) -> str:
         return " ".join(self.darker_cmd)
 
     def filter_changed_files(self, changed_files: list[str]) -> list[str]: