Add mypy annotations to proposals scripts (#783)

Renames proposals to proposal_list because the module name conflicts with the directory name

proposals/scripts/BUILD

@@ -3,18 +3,19 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
+load("@mypy_integration//:mypy.bzl", "mypy_test")
 
 py_library(
-    name = "proposals",
-    srcs = ["proposals.py"],
+    name = "proposal_list",
+    srcs = ["proposal_list.py"],
 )
 
 py_test(
-    name = "proposals_test",
-    srcs = ["proposals_test.py"],
+    name = "proposal_list_test",
+    srcs = ["proposal_list_test.py"],
     data = ["//proposals:md_files"],
     python_version = "PY3",
-    deps = [":proposals"],
+    deps = [":proposal_list"],
 )
 
 py_binary(
@@ -31,9 +32,19 @@ py_test(
     deps = [":new_proposal"],
 )
 
+mypy_test(
+    name = "new_proposal_mypy_test",
+    deps = [":new_proposal"],
+)
+
 # This is a directly runnable script, but should not be run via bazel.
 py_library(
     name = "update_proposal_list",
     srcs = ["update_proposal_list.py"],
-    deps = [":proposals"],
+    deps = [":proposal_list"],
+)
+
+mypy_test(
+    name = "update_proposal_list_mypy_test",
+    deps = [":update_proposal_list"],
 )

proposals/scripts/new_proposal.py

@@ -15,6 +15,7 @@ import shlex
 import shutil
 import subprocess
 import sys
+from typing import List, Optional
 
 _PROMPT = """This will:
   - Create and switch to a new branch named '%s'.
@@ -37,12 +38,7 @@ _LINK_TEMPLATE = """Proposal links (add links as proposal evolves):
 """
 
 
-def _exit(error):
-    """Wraps sys.exit for testing."""
-    sys.exit(error)
-
-
-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="Generates a branch and PR for a new proposal with the "
@@ -75,23 +71,24 @@ def _parse_args(args=None):
     return parser.parse_args(args=args)
 
 
-def _calculate_branch(parsed_args):
+def _calculate_branch(parsed_args: argparse.Namespace) -> str:
     """Returns the branch name."""
     if parsed_args.branch:
+        assert isinstance(parsed_args.branch, str)
         return parsed_args.branch
     # Only use the first 20 chars of the title for branch names.
     return "proposal-%s" % (parsed_args.title.lower().replace(" ", "-")[0:20])
 
 
-def _find_tool(tool):
+def _find_tool(tool: str) -> str:
     """Checks if a tool is present."""
     tool_path = shutil.which(tool)
     if not tool_path:
-        _exit("ERROR: Missing the '%s' command-line tool." % tool)
+        exit("ERROR: Missing the '%s' command-line tool." % tool)
     return tool_path
 
 
-def _fill_template(template_path, title, pr_num):
+def _fill_template(template_path: str, title: str, pr_num: int) -> str:
     """Fills out template TODO fields."""
     with open(template_path) as template_file:
         content = template_file.read()
@@ -105,16 +102,19 @@ def _fill_template(template_path, title, pr_num):
     return content
 
 
-def _get_proposals_dir(parsed_args):
+def _get_proposals_dir(parsed_args: argparse.Namespace) -> str:
     """Returns the path to the proposals directory."""
     if parsed_args.proposals_dir:
+        assert isinstance(parsed_args.proposals_dir, str)
         return parsed_args.proposals_dir
     return os.path.realpath(
         os.path.join(os.path.dirname(__file__), "../../proposals")
     )
 
 
-def _run(argv, check=True, get_stdout=False):
+def _run(
+    argv: List[str], check: bool = True, get_stdout: bool = False
+) -> Optional[str]:
     """Runs a command."""
     cmd = " ".join([shlex.quote(x) for x in argv])
     print("\n+ RUNNING: %s" % cmd, file=sys.stderr)
@@ -129,23 +129,25 @@ def _run(argv, check=True, get_stdout=False):
         out = stdout.decode("utf-8")
         print(out, end="")
     if check and p.returncode != 0:
-        _exit("ERROR: Command failed: %s" % cmd)
+        exit("ERROR: Command failed: %s" % cmd)
     if get_stdout:
         return out
+    return None
 
 
-def _run_pr_create(argv):
+def _run_pr_create(argv: List[str]) -> int:
     """Runs a command and returns the PR#."""
     out = _run(argv, get_stdout=True)
+    assert out is not None
     match = re.search(
         r"^https://github.com/[^/]+/[^/]+/pull/(\d+)$", out, re.MULTILINE
     )
     if not match:
-        _exit("ERROR: Failed to find PR# in output.")
-    return int(match[1])
+        exit("ERROR: Failed to find PR# in output.")
+    return int(match.group(1))
 
 
-def main():
+def main() -> None:
     parsed_args = _parse_args()
     title = parsed_args.title
     branch = _calculate_branch(parsed_args)
@@ -162,14 +164,14 @@ def main():
     # Verify there are no uncommitted changes.
     p = subprocess.run([git_bin, "diff-index", "--quiet", "HEAD", "--"])
     if p.returncode != 0:
-        _exit("ERROR: There are uncommitted changes in your git repo.")
+        exit("ERROR: There are uncommitted changes in your git repo.")
 
     # Prompt before proceeding.
     response = "?"
     while response not in ("y", "n", ""):
         response = input(_PROMPT % (branch, title)).lower()
     if response == "n":
-        _exit("ERROR: Cancelled")
+        exit("ERROR: Cancelled")
 
     # Create a proposal branch.
     _run(

proposals/scripts/new_proposal_test.py

@@ -10,17 +10,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import os
 import unittest
-from unittest import mock
 
-from carbon.proposals.scripts import new_proposal
-
-
-class FakeExitError(Exception):
-    pass
-
-
-def _fake_exit(message):
-    raise FakeExitError(message)
+from proposals.scripts import new_proposal
 
 
 class TestNewProposal(unittest.TestCase):
@@ -63,11 +54,7 @@ class TestNewProposal(unittest.TestCase):
         new_proposal._run(["true"])
 
     def test_run_failure(self):
-        with mock.patch(
-            "carbon.proposals.scripts.new_proposal._exit",
-            side_effect=_fake_exit,
-        ):
-            self.assertRaises(FakeExitError, new_proposal._run, ["false"])
+        self.assertRaises(SystemExit, new_proposal._run, ["false"])
 
 
 if __name__ == "__main__":

proposals/scripts/proposals.py → proposals/scripts/proposal_list.py

@@ -9,9 +9,10 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 import os
 import re
 import sys
+from typing import List, Tuple
 
 
-def get_title(parent_dir, entry):
+def get_title(parent_dir: str, entry: str) -> str:
     """Returns the title from the requested file."""
     path = os.path.join(parent_dir, entry)
     with open(path) as md:
@@ -21,11 +22,11 @@ def get_title(parent_dir, entry):
         return titles[0][2:-1]
 
 
-def get_path():
+def get_path() -> str:
     return os.path.realpath(os.path.join(os.path.dirname(__file__), ".."))
 
 
-def get_list(proposals_path):
+def get_list(proposals_path: str) -> List[Tuple[str, str]]:
     proposals = []
     proposals_list = os.listdir(proposals_path)
     proposals_list.sort()

proposals/scripts/proposals_test.py → proposals/scripts/proposal_list_test.py

@@ -1,4 +1,4 @@
-"""Tests for proposals.py."""
+"""Tests for proposal_list.py."""
 
 __copyright__ = """
 Part of the Carbon Language project, under the Apache License v2.0 with LLVM
@@ -8,13 +8,13 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import unittest
 
-from carbon.proposals.scripts import proposals
+from proposals.scripts import proposal_list
 
 
 class TestProposal(unittest.TestCase):
     def test_get_path(self):
-        proposals_path = proposals.get_path()
-        p = proposals.get_list(proposals_path)
+        proposals_path = proposal_list.get_path()
+        p = proposal_list.get_list(proposals_path)
         self.assertEqual(
             p[0],
             (

proposals/scripts/update_proposal_list.py

@@ -16,21 +16,22 @@ import sys
 
 # Do some extra work to support direct runs.
 try:
-    from carbon.proposals.scripts import proposals
+    from proposals.scripts import proposal_list
 except ImportError:
-    proposals_spec = importlib.util.spec_from_file_location(
-        "proposals", os.path.join(os.path.dirname(__file__), "proposals.py")
+    proposal_list_spec = importlib.util.spec_from_file_location(
+        "proposal_list",
+        os.path.join(os.path.dirname(__file__), "proposal_list.py"),
     )
-    proposals = importlib.util.module_from_spec(proposals_spec)
-    proposals_spec.loader.exec_module(proposals)
+    proposal_list = importlib.util.module_from_spec(proposal_list_spec)
+    proposal_list_spec.loader.exec_module(proposal_list)  # type: ignore
 
 if __name__ == "__main__":
-    proposals_path = proposals.get_path()
+    proposals_path = proposal_list.get_path()
 
     with io.StringIO() as out:
         out.write("<!-- Generated by ./scripts/update_proposal_list.py -->\n\n")
         results = out.getvalue()
-        for title, filename in proposals.get_list(proposals_path):
+        for title, filename in proposal_list.get_list(proposals_path):
             out.write("-   [%s](%s)\n" % (title, filename))
         toc = out.getvalue()