Add typing hints to github_tools with mypy enforcement (#771)

WORKSPACE

@@ -6,6 +6,10 @@ workspace(name = "carbon")
 
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
+###############################################################################
+# Python rules
+###############################################################################
+
 rules_python_version = "0.3.0"
 
 # Add Bazel's python rules and set up pip.
@@ -26,6 +30,46 @@ pip_install(
     requirements = "//github_tools:requirements.txt",
 )
 
+###############################################################################
+# Python mypy rules
+###############################################################################
+
+# NOTE: https://github.com/bazelbuild/bazel/issues/4948 tracks bazel supporting
+# typing directly. If it's added, we will probably want to switch.
+
+# Add mypy
+mypy_integration_version = "0.2.0"
+
+http_archive(
+    name = "mypy_integration",
+    sha256 = "621df076709dc72809add1f5fe187b213fee5f9b92e39eb33851ab13487bd67d",
+    strip_prefix = "bazel-mypy-integration-%s" % mypy_integration_version,
+    urls = [
+        "https://github.com/thundergolfer/bazel-mypy-integration/archive/refs/tags/%s.tar.gz" % mypy_integration_version,
+    ],
+)
+
+load(
+    "@mypy_integration//repositories:repositories.bzl",
+    mypy_integration_repositories = "repositories",
+)
+
+mypy_integration_repositories()
+
+load("@mypy_integration//:config.bzl", "mypy_configuration")
+
+mypy_configuration("//bazel/mypy:mypy.ini")
+
+load("@mypy_integration//repositories:deps.bzl", mypy_integration_deps = "deps")
+
+mypy_integration_deps(
+    mypy_requirements_file = "//bazel/mypy:version.txt",
+)
+
+###############################################################################
+# C++ rules
+###############################################################################
+
 # Configure the bootstrapped Clang and LLVM toolchain for Bazel.
 load(
     "//bazel/cc_toolchains:clang_configuration.bzl",
@@ -34,6 +78,10 @@ load(
 
 configure_clang_toolchain(name = "bazel_cc_toolchain")
 
+###############################################################################
+# LLVM libraries
+###############################################################################
+
 local_repository(
     name = "llvm_bazel",
     path = "third_party/llvm-bazel/llvm-bazel",
@@ -61,6 +109,10 @@ load("@llvm_bazel//:zlib.bzl", "llvm_zlib_system")
 # We require successful detection and use of a system zlib library.
 llvm_zlib_system(name = "llvm_zlib")
 
+###############################################################################
+# Flex/Bison rules
+###############################################################################
+
 # TODO: Can switch to a normal release version when it includes:
 # https://github.com/jmillikin/rules_m4/commit/b504241407916d1d6d72c66a766daacf9603cf8b
 rules_m4_version = "b504241407916d1d6d72c66a766daacf9603cf8b"
@@ -115,6 +167,10 @@ load("@rules_bison//bison:bison.bzl", "bison_register_toolchains")
 # fix them anyways.
 bison_register_toolchains(extra_copts = ["-w"])
 
+###############################################################################
+# Example conversion repositories
+###############################################################################
+
 local_repository(
     name = "brotli",
     path = "third_party/examples/brotli/original",

bazel/mypy/BUILD

@@ -0,0 +1,5 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+# Empty BUILD file to allow access to Bazel extensions.

bazel/mypy/mypy.ini

@@ -0,0 +1,18 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+[mypy]
+disallow_untyped_decorators = true
+warn_unused_configs = true
+disallow_subclassing_any = true
+disallow_untyped_calls = true
+disallow_untyped_defs = true
+disallow_incomplete_defs = true
+check_untyped_defs = true
+no_implicit_optional = true
+warn_redundant_casts = true
+warn_unused_ignores = true
+warn_return_any = true
+ignore_missing_imports = false
+pretty = true

bazel/mypy/version.txt

@@ -0,0 +1,7 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+# Synced with:
+# https://github.com/thundergolfer/bazel-mypy-integration/blob/master/third_party/requirements.txt
+mypy==0.790

github_tools/BUILD

@@ -4,6 +4,7 @@
 
 load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
 load("@py_deps//:requirements.bzl", "requirement")
+load("@mypy_integration//:mypy.bzl", "mypy_test")
 
 py_library(
     name = "github_helpers",
@@ -25,6 +26,11 @@ py_binary(
     deps = ["github_helpers"],
 )
 
+mypy_test(
+    name = "pr_comments_mypy_test",
+    deps = [":pr_comments"],
+)
+
 py_test(
     name = "pr_comments_test",
     srcs = ["pr_comments_test.py"],
@@ -42,6 +48,11 @@ py_binary(
     ],
 )
 
+mypy_test(
+    name = "update_label_access_mypy_test",
+    deps = [":update_label_access"],
+)
+
 py_test(
     name = "update_label_access_test",
     srcs = ["update_label_access_test.py"],

github_tools/github_helpers.py

@@ -9,11 +9,14 @@ Exceptions. See /LICENSE for license information.
 SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
+import argparse
 import os
+from typing import Dict, Generator, Optional, Tuple
 
 # https://pypi.org/project/gql/
-import gql
-import gql.transport.requests
+# gql is missing type annotations, so disable checking on around it.
+import gql  # type: ignore
+import gql.transport.requests  # type: ignore
 
 _ENV_TOKEN = "GITHUB_ACCESS_TOKEN"
 
@@ -25,7 +28,9 @@ PAGINATION = """pageInfo {
 totalCount"""
 
 
-def add_access_token_arg(parser, permissions):
+def add_access_token_arg(
+    parser: argparse.ArgumentParser, permissions: str
+) -> None:
     """Adds a flag to set the access token."""
     access_token = os.environ.get(_ENV_TOKEN, default=None)
     parser.add_argument(
@@ -42,7 +47,7 @@ def add_access_token_arg(parser, permissions):
 class Client(object):
     """A GitHub GraphQL client."""
 
-    def __init__(self, parsed_args):
+    def __init__(self, parsed_args: argparse.Namespace):
         """Connects to GitHub."""
         transport = gql.transport.requests.RequestsHTTPTransport(
             url="https://api.github.com/graphql",
@@ -50,11 +55,16 @@ class Client(object):
         )
         self._client = gql.Client(transport=transport)
 
-    def execute(self, query):
+    def execute(self, query: str) -> Dict:
         """Runs a query."""
-        return self._client.execute(gql.gql(query))
+        return self._client.execute(gql.gql(query))  # type: ignore
 
-    def execute_and_paginate(self, query, path, first_page=None):
+    def execute_and_paginate(
+        self,
+        query: str,
+        path: Tuple[str, ...],
+        first_page: Optional[Dict] = None,
+    ) -> Generator[Dict, None, None]:
         """Runs a query with pagination.
 
         Arguments:

github_tools/github_helpers_test.py

@@ -9,7 +9,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 import unittest
 from unittest import mock
 
-from carbon.github_tools import github_helpers
+from github_tools import github_helpers
 
 
 _TEST_QUERY = """

github_tools/pr_comments.py

@@ -14,18 +14,19 @@ import hashlib
 import os
 import importlib.util
 import textwrap
+from typing import Any, Dict, Callable, List, Optional, Tuple
 
 
 # Do some extra work to support direct runs.
 try:
-    from carbon.github_tools import github_helpers
+    from github_tools import github_helpers
 except ImportError:
     github_helpers_spec = importlib.util.spec_from_file_location(
         "github_helpers",
         os.path.join(os.path.dirname(__file__), "github_helpers.py"),
     )
     github_helpers = importlib.util.module_from_spec(github_helpers_spec)
-    github_helpers_spec.loader.exec_module(github_helpers)
+    github_helpers_spec.loader.exec_module(github_helpers)  # type: ignore
 
 
 # The main query, into which other queries are composed.
@@ -111,7 +112,7 @@ _QUERY_REVIEW_THREADS = """
 class _Comment(object):
     """A comment, either on a review thread or top-level on the PR."""
 
-    def __init__(self, author, timestamp, body):
+    def __init__(self, author: str, timestamp: str, body: str):
         self.author = author
         self.timestamp = datetime.datetime.strptime(
             timestamp, "%Y-%m-%dT%H:%M:%SZ"
@@ -119,7 +120,7 @@ class _Comment(object):
         self.body = body
 
     @staticmethod
-    def from_raw_comment(raw_comment):
+    def from_raw_comment(raw_comment: Dict) -> "_Comment":
         """Creates the comment from a raw comment dict."""
         return _Comment(
             raw_comment["author"]["login"],
@@ -128,7 +129,7 @@ class _Comment(object):
         )
 
     @staticmethod
-    def _rewrap(content):
+    def _rewrap(content: str) -> str:
         """Rewraps a comment to fit in 80 columns with an indent."""
         lines = []
         for line in content.split("\n"):
@@ -145,7 +146,7 @@ class _Comment(object):
             )
         return "\n".join(lines)
 
-    def format(self, long):
+    def format(self, long: bool) -> str:
         """Formats the comment."""
         if long:
             return "%s%s at %s:\n%s" % (
@@ -166,7 +167,7 @@ class _Comment(object):
 class _PRComment(_Comment):
     """A comment on the top-level PR."""
 
-    def __init__(self, raw_comment):
+    def __init__(self, raw_comment: Dict):
         super().__init__(
             raw_comment["author"]["login"],
             raw_comment["createdAt"],
@@ -174,23 +175,23 @@ class _PRComment(_Comment):
         )
         self.url = raw_comment["url"]
 
-    def __lt__(self, other):
+    def __lt__(self, other: "_PRComment") -> bool:
         return self.timestamp < other.timestamp
 
-    def format(self, long):
+    def format(self, long: bool) -> str:
         return "%s\n%s" % (self.url, super().format(long))
 
 
 class _Thread(object):
     """A review thread on a line of code."""
 
-    def __init__(self, parsed_args, thread):
-        self.is_resolved = thread["isResolved"]
+    def __init__(self, parsed_args: argparse.Namespace, thread: Dict):
+        self.is_resolved: bool = thread["isResolved"]
 
         comments = thread["comments"]["nodes"]
         first_comment = comments[0]
-        self.line = first_comment["originalPosition"]
-        self.path = first_comment["path"]
+        self.line: int = first_comment["originalPosition"]
+        self.path: str = first_comment["path"]
 
         # Link to the comment in the commit; GitHub features work better there
         # than in the conversation view. The diff_url allows viewing changes
@@ -211,10 +212,10 @@ class _Thread(object):
             "pr_num": parsed_args.pr_num,
             "repo": parsed_args.repo,
         }
-        self.url = template % format_dict
+        self.url: str = template % format_dict
         format_dict["head"] = "..HEAD"
         format_dict["line_side"] = "L"
-        self.diff_url = template % format_dict
+        self.diff_url: str = template % format_dict
 
         self.comments = [
             _Comment.from_raw_comment(comment)
@@ -229,13 +230,13 @@ class _Thread(object):
                 )
             )
 
-    def __lt__(self, other):
+    def __lt__(self, other: "_Thread") -> bool:
         """Sort threads by line then timestamp."""
         if self.line != other.line:
-            return self.line < other.line
+            return bool(self.line < other.line)
         return self.comments[0].timestamp < other.comments[0].timestamp
 
-    def format(self, long):
+    def format(self, long: bool) -> str:
         """Formats the review thread with comments."""
         lines = []
         lines.append(
@@ -252,7 +253,7 @@ class _Thread(object):
             lines.append(comment.format(long))
         return "\n".join(lines)
 
-    def has_comment_from(self, comments_from):
+    def has_comment_from(self, comments_from: str) -> bool:
         """Returns true if comments has a comment from comments_from."""
         for comment in self.comments:
             if comment.author == comments_from:
@@ -260,7 +261,7 @@ class _Thread(object):
         return False
 
 
-def _parse_args(args=None):
+def _parse_args(args: Optional[List[str]] = None) -> argparse.Namespace:
     """Parses command-line arguments and flags."""
     parser = argparse.ArgumentParser(description="Lists comments on a PR.")
     parser.add_argument(
@@ -303,7 +304,9 @@ def _parse_args(args=None):
     return parser.parse_args(args=args)
 
 
-def _query(parsed_args, field_name=None):
+def _query(
+    parsed_args: argparse.Namespace, field_name: Optional[str] = None
+) -> str:
     """Returns a query for the passed field_name, or all by default."""
     print(".", end="", flush=True)
     format = {
@@ -332,14 +335,22 @@ def _query(parsed_args, field_name=None):
     return _QUERY % format
 
 
-def _accumulate_pr_comment(parsed_args, comments, raw_comment):
+def _accumulate_pr_comment(
+    parsed_args: argparse.Namespace,
+    comments: List[_PRComment],
+    raw_comment: Dict,
+) -> None:
     """Collects top-level comments and reviews."""
     # Elide reviews that have no top-level comment body.
     if raw_comment["body"]:
         comments.append(_PRComment(raw_comment))
 
 
-def _accumulate_thread(parsed_args, threads_by_path, raw_thread):
+def _accumulate_thread(
+    parsed_args: argparse.Namespace,
+    threads_by_path: Dict[str, List[_Thread]],
+    raw_thread: Dict,
+) -> None:
     """Adds threads to threads_by_path for later sorting."""
     thread = _Thread(parsed_args, raw_thread)
 
@@ -366,8 +377,13 @@ def _accumulate_thread(parsed_args, threads_by_path, raw_thread):
 
 
 def _paginate(
-    field_name, accumulator, parsed_args, client, main_result, output
-):
+    field_name: str,
+    accumulator: Callable[[argparse.Namespace, Any, Dict], None],
+    parsed_args: argparse.Namespace,
+    client: github_helpers.Client,
+    main_result: Dict,
+    output: Any,
+) -> None:
     """Paginates through the given field_name, accumulating results."""
     query = _query(parsed_args, field_name=field_name)
     path = ("repository", "pullRequest", field_name)
@@ -377,7 +393,9 @@ def _paginate(
         accumulator(parsed_args, output, node)
 
 
-def _fetch_comments(parsed_args):
+def _fetch_comments(
+    parsed_args: argparse.Namespace,
+) -> Tuple[List[_PRComment], Dict[str, List[_Thread]]]:
     """Fetches comments and review threads from GitHub."""
     # Each _query call will print a '.' for progress.
     print(
@@ -394,7 +412,7 @@ def _fetch_comments(parsed_args):
     pull_request = main_result["repository"]["pullRequest"]
 
     # Paginate comments, reviews, and review threads.
-    comments = []
+    comments: List[_PRComment] = []
     _paginate(
         "comments",
         _accumulate_pr_comment,
@@ -412,7 +430,7 @@ def _fetch_comments(parsed_args):
         main_result,
         comments,
     )
-    threads_by_path = {}
+    threads_by_path: Dict[str, List[_Thread]] = {}
     _paginate(
         "reviewThreads",
         _accumulate_thread,
@@ -433,7 +451,7 @@ def _fetch_comments(parsed_args):
     return comments, threads_by_path
 
 
-def main():
+def main() -> None:
     parsed_args = _parse_args()
     comments, threads_by_path = _fetch_comments(parsed_args)
 

github_tools/pr_comments_test.py

@@ -10,8 +10,8 @@ import os
 import unittest
 from unittest import mock
 
-from carbon.github_tools import github_helpers
-from carbon.github_tools import pr_comments
+from github_tools import github_helpers
+from github_tools import pr_comments
 
 
 class TestPRComments(unittest.TestCase):

github_tools/update_label_access.py

@@ -12,12 +12,13 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
 import argparse
+from typing import List, Optional, Set
 
 # https://github.com/PyGithub/PyGithub
 # GraphQL is preferred, but falling back to pygithub for unsupported mutations.
-import github
+import github  # type: ignore
 
-from carbon.github_tools import github_helpers
+from github_tools import github_helpers
 
 # The organization to mirror members from.
 _ORG = "carbon-language"
@@ -65,14 +66,14 @@ query {
 _TEAM_MEMBER_PATH = ("organization", "team", "members")
 
 
-def _parse_args(args=None):
+def _parse_args(args: Optional[List[str]] = None) -> argparse.Namespace:
     """Parses command-line arguments and flags."""
     parser = argparse.ArgumentParser(description=__doc__)
     github_helpers.add_access_token_arg(parser, "admin:org, repo")
     return parser.parse_args(args=args)
 
 
-def _load_org_members(client):
+def _load_org_members(client: github_helpers.Client) -> Set[str]:
     """Loads org members."""
     print("Loading %s..." % _ORG)
     org_members = set()
@@ -94,7 +95,7 @@ def _load_org_members(client):
     return org_members
 
 
-def _load_team_members(client):
+def _load_team_members(client: github_helpers.Client) -> Set[str]:
     """Load team members."""
     print("Loading %s..." % _TEAM)
     team_members = set()
@@ -106,7 +107,9 @@ def _load_team_members(client):
     return team_members
 
 
-def _update_team(gh, org_members, team_members):
+def _update_team(
+    gh: github.Github, org_members: Set[str], team_members: Set[str]
+) -> None:
     """Updates the team if needed.
 
     This switches to pygithub because GraphQL lacks equivalent mutation support.
@@ -125,7 +128,7 @@ def _update_team(gh, org_members, team_members):
             gh_team.remove_membership(gh.get_user(member))
 
 
-def main():
+def main() -> None:
     parsed_args = _parse_args()
     print("Connecting...")
     client = github_helpers.Client(parsed_args)

github_tools/update_label_access_test.py

@@ -10,10 +10,10 @@ import os
 import unittest
 from unittest import mock
 
-import github
+import github  # type: ignore
 
-from carbon.github_tools import github_helpers
-from carbon.github_tools import update_label_access
+from github_tools import github_helpers
+from github_tools import update_label_access
 
 
 class TestUpdateLabelAccess(unittest.TestCase):