Set up type checking with pyrefly

.github/workflows/ci.yml

@@ -45,6 +45,10 @@ jobs:
         run: |
           python -m poetry install
 
+      - name: Run typecheck
+        run: |
+          python -m poetry run pyrefly check
+
       - name: Run tests
         run: |
           python -m poetry run python -m unittest

kernel_patches_daemon/branch_worker.py

@@ -37,7 +37,11 @@
 from github.Repository import Repository
 from github.WorkflowJob import WorkflowJob
 
-from kernel_patches_daemon.config import EmailConfig, SERIES_TARGET_SEPARATOR
+from kernel_patches_daemon.config import (
+    EmailConfig,
+    PRCommentsForwardingConfig,
+    SERIES_TARGET_SEPARATOR,
+)
 from kernel_patches_daemon.github_connector import GithubConnector
 from kernel_patches_daemon.github_logs import GithubLogExtractor
 from kernel_patches_daemon.patchwork import Patchwork, Series, Subject
@@ -388,8 +392,10 @@ def reply_email_recipients(
     """
     tos = msg.get_all("To", [])
     ccs = msg.get_all("Cc", [])
+    # pyrefly: ignore  # implicit-import
     cc_list = [a for (_, a) in email.utils.getaddresses(tos + ccs)]
 
+    # pyrefly: ignore  # implicit-import, bad-argument-type
     (_, sender_address) = email.utils.parseaddr(msg.get("From"))
     to_list = [sender_address]
 
@@ -405,25 +411,27 @@ def reply_email_recipients(
 
 
 async def send_pr_comment_email(
-    config: EmailConfig, msg: EmailMessage, body: str
+    email_config: EmailConfig, msg: EmailMessage, body: str
 ) -> Optional[str]:
     """
     This function forwards a pull request comment (`body`) as an email reply to the original
     patch email message `msg`. It extracts recipients from the `msg`, applies allowlist, denylist,
     and always_cc as configured, and sets Subject and In-Reply-To based on the `msg`
 
     Args:
-        config: EmailConfig with PRCommentsForwardingConfig
+        email_config: EmailConfig with PRCommentsForwardingConfig
         msg: the original EmailMessage we are replying to (the patch submission)
         body: the content of the reply we are sending
 
     Returns:
         Message-Id of the sent email, or None if it wasn't sent
     """
-    if config is None or not config.is_pr_comment_forwarding_enabled():
+    if email_config.pr_comments_forwarding is None:
+        return
+    cfg: PRCommentsForwardingConfig = email_config.pr_comments_forwarding
+    if not cfg.enabled:
         return
 
-    cfg = config.pr_comments_forwarding
     (to_list, cc_list) = reply_email_recipients(
         msg, allowlist=cfg.recipient_allowlist, denylist=cfg.recipient_denylist
     )
@@ -432,10 +440,11 @@ async def send_pr_comment_email(
     if not to_list and not cc_list:
         return
 
+    # pyrefly: ignore  # unsupported-operation
     subject = "Re: " + msg.get("Subject")
     in_reply_to = msg.get("Message-Id")
 
-    return await send_email(config, to_list, cc_list, subject, body, in_reply_to)
+    return await send_email(email_config, to_list, cc_list, subject, body, in_reply_to)
 
 
 def pr_has_label(pr: PullRequest, label: str) -> bool:
@@ -459,6 +468,7 @@ def _uniq_tmp_folder(
     sha.update(f"{url}/{branch}".encode("utf-8"))
     # pyre-fixme[6]: For 1st argument expected `PathLike[Variable[AnyStr <: [str,
     #  bytes]]]` but got `Optional[str]`.
+    # pyrefly: ignore  # no-matching-overload
     repo_name = remove_unsafe_chars(os.path.basename(url))
     return os.path.join(base_directory, f"pw_sync_{repo_name}_{sha.hexdigest()}")
 
@@ -504,6 +514,7 @@ def parse_pr_ref(ref: str) -> Dict[str, Any]:
 
     tmp = res["series"].split("/", maxsplit=1)
     if len(tmp) >= 2 and tmp[1].isdigit():
+        # pyrefly: ignore  # unsupported-operation
         res["series_id"] = int(tmp[1])
 
     return res
@@ -604,6 +615,7 @@ def _is_outdated_pr(pr: PullRequest) -> bool:
     # that have it as a parent will also be changed.
     commit = commits[commits.totalCount - 1]
     last_modified = dateutil.parser.parse(commit.stats.last_modified)
+    # pyrefly: ignore  # unsupported-operation
     age = datetime.now(timezone.utc) - last_modified
     logger.info(f"Pull request {pr} has age {age}")
     return age > PULL_REQUEST_TTL
@@ -727,16 +739,20 @@ def update_e2e_test_branch_and_update_pr(
         # Now that we have an updated base branch, create a dummy commit on top
         # so that we can actually create a pull request (this path tests
         # upstream directly, without any mailbox patches applied).
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.checkout("-B", branch_name)
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.commit("--allow-empty", "--message", "Dummy commit")
 
         title = f"[test] {branch_name}"
 
         # Force push only if there is no branch or code changes.
         pushed = False
+        # pyrefly: ignore  # missing-attribute
         if branch_name not in self.branches or self.repo_local.git.diff(
             branch_name, f"remotes/origin/{branch_name}"
         ):
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.push("--force", "origin", branch_name)
             pushed = True
 
@@ -750,18 +766,25 @@ def can_do_sync(self) -> bool:
 
     def do_sync(self) -> None:
         # fetch most recent upstream
+        # pyrefly: ignore  # missing-attribute
         if UPSTREAM_REMOTE_NAME in [x.name for x in self.repo_local.remotes]:
+            # pyrefly: ignore  # missing-attribute
             urls = list(self.repo_local.remote(UPSTREAM_REMOTE_NAME).urls)
             if urls != [self.upstream_url]:
                 logger.warning(f"remote upstream set to track {urls}, re-creating")
+                # pyrefly: ignore  # missing-attribute
                 self.repo_local.delete_remote(UPSTREAM_REMOTE_NAME)
+                # pyrefly: ignore  # missing-attribute
                 self.repo_local.create_remote(UPSTREAM_REMOTE_NAME, self.upstream_url)
         else:
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.create_remote(UPSTREAM_REMOTE_NAME, self.upstream_url)
+        # pyrefly: ignore  # missing-attribute
         upstream_repo = self.repo_local.remote(UPSTREAM_REMOTE_NAME)
         upstream_repo.fetch(self.upstream_branch)
         upstream_branch = getattr(upstream_repo.refs, self.upstream_branch)
         _reset_repo(self.repo_local, f"{UPSTREAM_REMOTE_NAME}/{self.upstream_branch}")
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.push(
             "--force", "origin", f"{upstream_branch}:refs/heads/{self.repo_branch}"
         )
@@ -801,6 +824,7 @@ def fetch_repo_branch(self) -> None:
         """
         Fetch the repository branch of interest only once
         """
+        # pyrefly: ignore  # bad-assignment
         self.repo_local = self.fetch_repo(
             self.repo_dir, self.repo_url, self.repo_branch
         )
@@ -824,24 +848,31 @@ def _update_pr_base_branch(self, base_branch: str):
         self._add_ci_files()
 
         try:
+            # pyrefly: ignore  # missing-attribute
             diff = self.repo_local.git.diff(f"remotes/origin/{base_branch}")
         except git.exc.GitCommandError:
             # The remote may not exist, in which case we want to push.
             diff = True
 
         if diff:
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.checkout("-B", base_branch)
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.push("--force", "origin", f"refs/heads/{base_branch}")
         else:
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.checkout("-B", base_branch, f"origin/{base_branch}")
 
     def _create_dummy_commit(self, branch_name: str) -> None:
         """
         Reset branch, create dummy commit
         """
         _reset_repo(self.repo_local, f"{UPSTREAM_REMOTE_NAME}/{self.upstream_branch}")
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.checkout("-B", branch_name)
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.commit("--allow-empty", "--message", "Dummy commit")
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.push("--force", "origin", branch_name)
 
     def _close_pr(self, pr: PullRequest) -> None:
@@ -914,6 +945,7 @@ async def _comment_series_pr(
 
         if not pr and can_create and not close:
             # If there is no merge conflict and no change, ignore the series
+            # pyrefly: ignore  # missing-attribute
             if not has_merge_conflict and not self.repo_local.git.diff(
                 self.repo_pr_base_branch, branch_name
             ):
@@ -1010,9 +1042,12 @@ def _add_ci_files(self) -> None:
         """
         if Path(f"{self.ci_repo_dir}/.github").exists():
             execute_command(f"cp --archive {self.ci_repo_dir}/.github {self.repo_dir}")
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.add("--force", ".github")
         execute_command(f"cp --archive {self.ci_repo_dir}/* {self.repo_dir}")
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.add("--all", "--force")
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.commit("--all", "--message", "adding ci files")
 
     async def try_apply_mailbox_series(
@@ -1022,17 +1057,20 @@ async def try_apply_mailbox_series(
         # The pull request will be created against `repo_pr_base_branch`. So
         # prepare it for that.
         self._update_pr_base_branch(self.repo_pr_base_branch)
+        # pyrefly: ignore  # missing-attribute
         self.repo_local.git.checkout("-B", branch_name)
 
         # Apply series
         patch_content = await series.get_patch_binary_content()
         with temporary_patch_file(patch_content) as tmp_patch_file:
             try:
+                # pyrefly: ignore  # missing-attribute
                 self.repo_local.git.am("--3way", istream=tmp_patch_file)
             except git.exc.GitCommandError as e:
                 logger.warning(
                     f"Failed complete 3-way merge series {series.id} patch into {branch_name} branch: {e}"
                 )
+                # pyrefly: ignore  # missing-attribute
                 conflict = self.repo_local.git.diff()
                 return (False, e, conflict)
         return (True, None, None)
@@ -1051,6 +1089,7 @@ async def apply_push_comment(
             # In other words, patchwork could be reporting a relevant
             # status (ie. !accepted) while the series has already been
             # merged and pushed.
+            # pyrefly: ignore  # bad-argument-type
             if await _series_already_applied(self.repo_local, series):
                 logger.info(f"Series {series.url} already applied to tree")
                 raise NewPRWithNoChangeException(self.repo_pr_base_branch, branch_name)
@@ -1074,6 +1113,7 @@ async def apply_push_comment(
         if branch_name in self.branches and (
             branch_name not in self.all_prs  # NO PR yet
             or _is_branch_changed(
+                # pyrefly: ignore  # bad-argument-type
                 self.repo_local,
                 f"remotes/origin/{self.repo_pr_base_branch}",
                 f"remotes/origin/{branch_name}",
@@ -1089,10 +1129,12 @@ async def apply_push_comment(
                 can_create=True,
             )
             assert pr
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.push("--force", "origin", branch_name)
 
             # Metadata inside `pr` may be stale from the force push; refresh it
             pr.update()
+            # pyrefly: ignore  # missing-attribute
             wanted_sha = self.repo_local.head.commit.hexsha
             for _ in range(30):
                 if pr.head.sha == wanted_sha:
@@ -1106,9 +1148,11 @@ async def apply_push_comment(
             return pr
         # we don't have a branch, also means no PR, push first then create PR
         elif branch_name not in self.branches:
+            # pyrefly: ignore  # missing-attribute
             if not self.repo_local.git.diff(self.repo_pr_base_branch, branch_name):
                 # raise an exception so it bubbles up to the caller.
                 raise NewPRWithNoChangeException(self.repo_pr_base_branch, branch_name)
+            # pyrefly: ignore  # missing-attribute
             self.repo_local.git.push("--force", "origin", branch_name)
             return await self._comment_series_pr(
                 series,
@@ -1179,9 +1223,11 @@ def closed_prs(self) -> List[Any]:
         # closed prs are last resort to re-open expired PRs
         # and also required for branch expiration
         if not self._closed_prs:
+            # pyrefly: ignore  # bad-assignment
             self._closed_prs = list(
                 self.repo.get_pulls(state="closed", base=self.repo_pr_base_branch)
             )
+        # pyrefly: ignore  # bad-return
         return self._closed_prs
 
     def filter_closed_pr(self, head: str) -> Optional[PullRequest]:
@@ -1223,6 +1269,7 @@ async def sync_checks(self, pr: PullRequest, series: Series) -> None:
         # completed ones. The reason being that the information that pending
         # ones are present is very much relevant for status reporting.
         for run in self.repo.get_workflow_runs(
+            # pyrefly: ignore  # bad-argument-type
             actor=self.user_login,
             head_sha=pr.head.sha,
         ):
@@ -1293,6 +1340,8 @@ async def evaluate_ci_result(
             logger.info("No email configuration present; skipping sending...")
             return
 
+        email_cfg: EmailConfig = self.email_config
+
         if status in (Status.PENDING, Status.SKIPPED):
             return
 
@@ -1329,7 +1378,7 @@ async def evaluate_ci_result(
             subject = await get_ci_email_subject(series)
             ctx = build_email_body_context(self.repo, pr, status, series, inline_logs)
             body = furnish_ci_email_body(ctx)
-            await send_ci_results_email(self.email_config, series, subject, body)
+            await send_ci_results_email(email_cfg, series, subject, body)
             bump_email_status_counters(status)
 
     def expire_branches(self) -> None:
@@ -1379,12 +1428,15 @@ async def submit_pr_summary(
         pr_summary_report.add(1)
 
     async def forward_pr_comments(self, pr: PullRequest, series: Series):
-        if (
-            self.email_config is None
-            or not self.email_config.is_pr_comment_forwarding_enabled()
-        ):
+        # The checks and local variables are needed to make type checker happy
+        if self.email_config is None:
+            return
+        email_cfg: EmailConfig = self.email_config
+        if email_cfg.pr_comments_forwarding is None:
+            return
+        cfg: PRCommentsForwardingConfig = email_cfg.pr_comments_forwarding
+        if not cfg.enabled:
             return
-        cfg = self.email_config.pr_comments_forwarding
 
         comments = pr.get_issue_comments()
 
@@ -1421,6 +1473,7 @@ async def forward_pr_comments(self, pr: PullRequest, series: Series):
             subject = match.group(1)
             patch = series.patch_by_subject(subject)
             if not patch:
+                # pyrefly: ignore  # deprecated
                 logger.warn(
                     f"Ignoring PR comment {comment.html_url}, could not find relevant patch on patchwork"
                 )
@@ -1441,13 +1494,14 @@ async def forward_pr_comments(self, pr: PullRequest, series: Series):
 
             # and forward the target comment via email
             sent_msg_id = await send_pr_comment_email(
-                self.email_config, patch_msg, comment.body
+                email_cfg, patch_msg, comment.body
             )
             if sent_msg_id is not None:
                 logger.info(
                     f"Forwarded PR comment {comment.html_url} via email, Message-Id: {sent_msg_id}"
                 )
             else:
+                # pyrefly: ignore  # deprecated
                 logger.warn(
                     f"Failed to forward PR comment in reply to {msg_id}, no recipients"
                 )

kernel_patches_daemon/config.py

@@ -168,12 +168,6 @@ def from_json(cls, json: Dict) -> "EmailConfig":
             ),
         )
 
-    def is_pr_comment_forwarding_enabled(self) -> bool:
-        if self.pr_comments_forwarding is not None:
-            return self.pr_comments_forwarding.enabled
-        else:
-            return False
-
 
 @dataclass
 class PatchworksConfig:

kernel_patches_daemon/daemon.py

@@ -53,6 +53,7 @@ def reset_github_sync(self) -> bool:
 
     async def submit_metrics(self) -> None:
         if self.metrics_logger is None:
+            # pyrefly: ignore  # deprecated
             logger.warn(
                 "Not submitting run metrics because metrics logger is not configured"
             )
@@ -119,7 +120,9 @@ async def start_async(self) -> None:
 
         loop = asyncio.get_event_loop()
 
+        # pyrefly: ignore  # bad-argument-type
         loop.add_signal_handler(signal.SIGTERM, self.stop)
+        # pyrefly: ignore  # bad-argument-type
         loop.add_signal_handler(signal.SIGINT, self.stop)
 
         self._task = asyncio.create_task(self.worker.run())