diff --git a/proposals/scripts/BUILD b/proposals/scripts/BUILD index aaaa0e834ecf..00fdb8736ab3 100644 --- a/proposals/scripts/BUILD +++ b/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"], ) diff --git a/proposals/scripts/new_proposal.py b/proposals/scripts/new_proposal.py index 0048347254f7..0000045b6306 100755 --- a/proposals/scripts/new_proposal.py +++ b/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( diff --git a/proposals/scripts/new_proposal_test.py b/proposals/scripts/new_proposal_test.py index 59de82fcf24f..b5d9f4ec7d15 100644 --- a/proposals/scripts/new_proposal_test.py +++ b/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__": diff --git a/proposals/scripts/proposals.py b/proposals/scripts/proposal_list.py similarity index 86% rename from proposals/scripts/proposals.py rename to proposals/scripts/proposal_list.py index a08c17cc4f9c..525528a5fa04 100644 --- a/proposals/scripts/proposals.py +++ b/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() diff --git a/proposals/scripts/proposals_test.py b/proposals/scripts/proposal_list_test.py similarity index 79% rename from proposals/scripts/proposals_test.py rename to proposals/scripts/proposal_list_test.py index 3a047d09906e..f6db4f31ccac 100644 --- a/proposals/scripts/proposals_test.py +++ b/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], ( diff --git a/proposals/scripts/update_proposal_list.py b/proposals/scripts/update_proposal_list.py index ded97da9d221..6f387ebd1c23 100755 --- a/proposals/scripts/update_proposal_list.py +++ b/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("\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()