From 107848e30f5c93a3d9d476af8b2e8af6d88d66f0 Mon Sep 17 00:00:00 2001 From: Jon Meow Date: Mon, 9 May 2022 09:54:12 -0700 Subject: [PATCH] Switch mypy approach to pre-commit (#1234) I'm treating my PR to mypy bazel integration as dead in the water. Abandoning that approach, this seems to work reasonably well as a replacement. It's actually a slightly newer version of mypy too -- the bazel integration was stuck on an old version. Note I don't think we're losing much: we shouldn't be writing too much python. --- .pre-commit-config.yaml | 29 ++++++++ WORKSPACE | 47 ------------- bazel/check_deps/BUILD | 21 ------ bazel/mypy/BUILD | 5 -- bazel/mypy/mypy.ini | 18 ----- bazel/mypy/version.txt | 7 -- explorer/BUILD | 16 ----- explorer/syntax/BUILD | 8 --- github_tools/BUILD | 13 ---- github_tools/github_helpers.py | 6 +- github_tools/pr_comments.py | 1 + github_tools/update_label_access.py | 8 +-- migrate_cpp/BUILD | 8 --- proposals/scripts/BUILD | 8 --- scripts/BUILD | 100 ---------------------------- scripts/create_compdb.py | 2 +- scripts/fix_cc_deps.py | 2 +- scripts/forbid_llvm_googletest.py | 2 +- scripts/run_bazel.py | 2 +- scripts/run_buildifier.py | 2 +- scripts/run_buildozer.py | 2 +- 21 files changed, 43 insertions(+), 264 deletions(-) delete mode 100644 bazel/mypy/BUILD delete mode 100644 bazel/mypy/mypy.ini delete mode 100644 bazel/mypy/version.txt delete mode 100644 scripts/BUILD diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 48841cf409d5..d25424900f53 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -85,6 +85,35 @@ repos: rev: cbeb4c9c4137cff1568659fcc48e8b85cddd0c8d # frozen: 4.0.1 hooks: - id: flake8 + - repo: https://github.com/pre-commit/mirrors-mypy + rev: 'v0.950' + hooks: + - id: mypy + args: + - --disallow-untyped-decorators + - --warn-unused-configs + - --disallow-subclassing-any + - --disallow-untyped-calls + - --disallow-untyped-defs + - --disallow-incomplete-defs + - --check-untyped-defs + - --no-implicit-optional + - --warn-redundant-casts + - --warn-unused-ignores + - --warn-return-any + - --pretty + # This should match the requirements added in the WORKSPACE pip_install. + additional_dependencies: + - gql >= 2.0.0, < 3.0.0 + - PyGitHub + # TODO: should probably fix gen_rtti. + exclude: | + (?x)^( + explorer/gen_rtti\.py| + proposals/.*| + .*/lit\.cfg\.py| + .*_test\.py + )$ - repo: https://github.com/codespell-project/codespell rev: 01da43d753cdff4ea7dff952b4f5317450af31e3 # frozen: v2.1.0 hooks: diff --git a/WORKSPACE b/WORKSPACE index e21fbd72390e..1eed10c48fc5 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,53 +30,6 @@ 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 = "e5f8071f33eca637cd90bf70cb45f749e63bf2ca" - -# TODO: Can switch back to the official repo when it includes: -# https://github.com/thundergolfer/bazel-mypy-integration/pull/43 -#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, -# ], -#) - -http_archive( - name = "mypy_integration", - sha256 = "481ec6f0953a84a36b8103286f04c4cd274ae689060099085c02ac187d007592", - strip_prefix = "bazel-mypy-integration-%s" % mypy_integration_version, - urls = [ - "https://github.com/jonmeow/bazel-mypy-integration/archive/%s.zip" % 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 ############################################################################### diff --git a/bazel/check_deps/BUILD b/bazel/check_deps/BUILD index 19bd01c8d36f..6cc534327b14 100644 --- a/bazel/check_deps/BUILD +++ b/bazel/check_deps/BUILD @@ -2,8 +2,6 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@mypy_integration//:mypy.bzl", "mypy_test") - # The filegroups establishing root rules for `genquery` invocations can be # updated by running: # @@ -38,22 +36,3 @@ py_test( data = [":non_test_cc_deps.txt"], main = "check_non_test_cc_deps.py", ) - -mypy_test( - name = "check_non_test_cc_deps_mypy_test", - include_imports = True, - deps = [":check_non_test_cc_deps"], -) - -# Note that this script expects to be run directly and not via Bazel as it in -# turn invokes Bazel. -py_binary( - name = "update_roots", - srcs = ["update_roots.py"], -) - -mypy_test( - name = "update_roots_mypy_test", - include_imports = True, - deps = [":update_roots"], -) diff --git a/bazel/mypy/BUILD b/bazel/mypy/BUILD deleted file mode 100644 index 74906e61dd59..000000000000 --- a/bazel/mypy/BUILD +++ /dev/null @@ -1,5 +0,0 @@ -# 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. diff --git a/bazel/mypy/mypy.ini b/bazel/mypy/mypy.ini deleted file mode 100644 index fe26ab421a69..000000000000 --- a/bazel/mypy/mypy.ini +++ /dev/null @@ -1,18 +0,0 @@ -# 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 diff --git a/bazel/mypy/version.txt b/bazel/mypy/version.txt deleted file mode 100644 index c650849ae6bd..000000000000 --- a/bazel/mypy/version.txt +++ /dev/null @@ -1,7 +0,0 @@ -# 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 diff --git a/explorer/BUILD b/explorer/BUILD index 5a64f3dc97d6..4b2c3d8edf89 100644 --- a/explorer/BUILD +++ b/explorer/BUILD @@ -2,8 +2,6 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@mypy_integration//:mypy.bzl", "mypy_test") - package(default_visibility = [ "//bazel/check_deps:__pkg__", "//explorer:__subpackages__", @@ -43,17 +41,3 @@ py_binary( name = "gen_rtti", srcs = ["gen_rtti.py"], ) - -# This script should be run directly; the target is only provided for the -# mypy_test. -py_binary( - name = "update_checks_mypy_wrapper", - srcs = ["update_checks.py"], - main = "update_checks.py", -) - -mypy_test( - name = "update_checks_mypy_test", - include_imports = True, - deps = [":update_checks_mypy_wrapper"], -) diff --git a/explorer/syntax/BUILD b/explorer/syntax/BUILD index 7ced2a7cc076..f65aa3c56542 100644 --- a/explorer/syntax/BUILD +++ b/explorer/syntax/BUILD @@ -2,8 +2,6 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@mypy_integration//:mypy.bzl", "mypy_test") - package(default_visibility = [ "//explorer:__pkg__", "//explorer/fuzzing:__pkg__", @@ -139,12 +137,6 @@ py_test( deps = ["format_grammar_lib"], ) -mypy_test( - name = "format_grammar_mypy_test", - include_imports = True, - deps = [":format_grammar_lib"], -) - cc_test( name = "unimplemented_example_test", srcs = ["unimplemented_example_test.cpp"], diff --git a/github_tools/BUILD b/github_tools/BUILD index 7ace40825703..639360317885 100644 --- a/github_tools/BUILD +++ b/github_tools/BUILD @@ -3,7 +3,6 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception load("@py_deps//:requirements.bzl", "requirement") -load("@mypy_integration//:mypy.bzl", "mypy_test") py_library( name = "github_helpers", @@ -25,12 +24,6 @@ py_binary( deps = ["github_helpers"], ) -mypy_test( - name = "pr_comments_mypy_test", - include_imports = True, - deps = [":pr_comments"], -) - py_test( name = "pr_comments_test", srcs = ["pr_comments_test.py"], @@ -48,12 +41,6 @@ py_binary( ], ) -mypy_test( - name = "update_label_access_mypy_test", - include_imports = True, - deps = [":update_label_access"], -) - py_test( name = "update_label_access_test", srcs = ["update_label_access_test.py"], diff --git a/github_tools/github_helpers.py b/github_tools/github_helpers.py index e94499f86765..fb3715aa45fb 100644 --- a/github_tools/github_helpers.py +++ b/github_tools/github_helpers.py @@ -14,8 +14,8 @@ import os from typing import Dict, Generator, Optional, Tuple # https://pypi.org/project/gql/ -import gql -import gql.transport.requests +import gql # type: ignore +import gql.transport.requests # type: ignore _ENV_TOKEN = "GITHUB_ACCESS_TOKEN" @@ -52,7 +52,7 @@ class Client(object): url="https://api.github.com/graphql", headers={"Authorization": "bearer %s" % parsed_args.access_token}, ) - self._client = gql.Client(transport=transport) # type: ignore + self._client = gql.Client(transport=transport) def execute(self, query: str) -> Dict: """Runs a query.""" diff --git a/github_tools/pr_comments.py b/github_tools/pr_comments.py index c010cf16c3ea..7e02ae810c57 100755 --- a/github_tools/pr_comments.py +++ b/github_tools/pr_comments.py @@ -25,6 +25,7 @@ except ImportError: "github_helpers", os.path.join(os.path.dirname(__file__), "github_helpers.py"), ) + assert github_helpers_spec is not None github_helpers = importlib.util.module_from_spec(github_helpers_spec) github_helpers_spec.loader.exec_module(github_helpers) # type: ignore diff --git a/github_tools/update_label_access.py b/github_tools/update_label_access.py index 7d74f9011fb6..8c32e25163cb 100644 --- a/github_tools/update_label_access.py +++ b/github_tools/update_label_access.py @@ -114,18 +114,18 @@ def _update_team( This switches to pygithub because GraphQL lacks equivalent mutation support. """ - gh_team = gh.get_organization(_ORG).get_team_by_slug(_TEAM) # type: ignore + gh_team = gh.get_organization(_ORG).get_team_by_slug(_TEAM) add_members = org_members - team_members if add_members: print("Adding members: %s" % ", ".join(add_members)) for member in add_members: - gh_team.add_membership(gh.get_user(member)) # type: ignore + gh_team.add_membership(gh.get_user(member)) remove_members = team_members - org_members if remove_members: print("Removing members: %s" % ", ".join(remove_members)) for member in remove_members: - gh_team.remove_membership(gh.get_user(member)) # type: ignore + gh_team.remove_membership(gh.get_user(member)) def main() -> None: @@ -136,7 +136,7 @@ def main() -> None: org_members = _load_org_members(client) team_members = _load_team_members(client) if org_members != team_members: - gh = github.Github(parsed_args.access_token) # type: ignore + gh = github.Github(parsed_args.access_token) _update_team(gh, org_members, team_members) print("Done!") diff --git a/migrate_cpp/BUILD b/migrate_cpp/BUILD index 9059a783a362..be283cafb847 100644 --- a/migrate_cpp/BUILD +++ b/migrate_cpp/BUILD @@ -2,8 +2,6 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@mypy_integration//:mypy.bzl", "mypy_test") - py_binary( name = "migrate_cpp", srcs = ["migrate_cpp.py"], @@ -13,9 +11,3 @@ py_binary( ], python_version = "PY3", ) - -mypy_test( - name = "migrate_cpp_mypy_test", - include_imports = True, - deps = [":migrate_cpp"], -) diff --git a/proposals/scripts/BUILD b/proposals/scripts/BUILD index b438be6bdf42..a9349ab4fbd3 100644 --- a/proposals/scripts/BUILD +++ b/proposals/scripts/BUILD @@ -2,8 +2,6 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@mypy_integration//:mypy.bzl", "mypy_test") - py_binary( name = "new_proposal", srcs = ["new_proposal.py"], @@ -17,9 +15,3 @@ py_test( python_version = "PY3", deps = [":new_proposal"], ) - -mypy_test( - name = "new_proposal_mypy_test", - include_imports = True, - deps = [":new_proposal"], -) diff --git a/scripts/BUILD b/scripts/BUILD deleted file mode 100644 index b9db9c19356b..000000000000 --- a/scripts/BUILD +++ /dev/null @@ -1,100 +0,0 @@ -# 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 - -load("@mypy_integration//:mypy.bzl", "mypy_test") - -py_library( - name = "scripts_utils", - srcs = ["scripts_utils.py"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "create_compdb", - testonly = 1, - srcs = ["create_compdb.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "create_compdb_mypy_test", - include_imports = True, - deps = [":create_compdb"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "fix_cc_deps", - testonly = 1, - srcs = ["fix_cc_deps.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "fix_cc_deps_mypy_test", - include_imports = True, - deps = [":fix_cc_deps"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "forbid_llvm_googletest", - testonly = 1, - srcs = ["forbid_llvm_googletest.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "forbid_llvm_googletest_mypy_test", - include_imports = True, - deps = [":forbid_llvm_googletest"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "run_bazel", - testonly = 1, - srcs = ["run_bazel.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "run_bazel_mypy_test", - include_imports = True, - deps = [":run_bazel"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "run_buildifier", - testonly = 1, - srcs = ["run_buildifier.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "run_buildifier_mypy_test", - include_imports = True, - deps = [":run_buildifier"], -) - -# Note that this Python script is intended to be run directly, and not through -# Bazel. We have a rule for it so we can type check it. -py_binary( - name = "run_buildozer", - testonly = 1, - srcs = ["run_buildozer.py"], - deps = [":scripts_utils"], -) - -mypy_test( - name = "run_buildozer_mypy_test", - include_imports = True, - deps = [":run_buildozer"], -) diff --git a/scripts/create_compdb.py b/scripts/create_compdb.py index 5990928e2e7e..ae0c8f249e9b 100755 --- a/scripts/create_compdb.py +++ b/scripts/create_compdb.py @@ -29,7 +29,7 @@ import subprocess import sys from pathlib import Path -import scripts_utils # type: ignore +import scripts_utils scripts_utils.chdir_repo_root() directory = Path.cwd() diff --git a/scripts/fix_cc_deps.py b/scripts/fix_cc_deps.py index 7452c2067b7e..28d83ed7933e 100755 --- a/scripts/fix_cc_deps.py +++ b/scripts/fix_cc_deps.py @@ -19,7 +19,7 @@ import subprocess from typing import Callable, Dict, List, NamedTuple, Set, Tuple from xml.etree import ElementTree -import scripts_utils # type: ignore +import scripts_utils # Maps external repository names to a method translating bazel labels to file diff --git a/scripts/forbid_llvm_googletest.py b/scripts/forbid_llvm_googletest.py index 14c27c76fc55..1e29dac5820b 100755 --- a/scripts/forbid_llvm_googletest.py +++ b/scripts/forbid_llvm_googletest.py @@ -20,7 +20,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import subprocess -import scripts_utils # type: ignore +import scripts_utils _MESSAGE = """\ Dependencies on @llvm-project//llvm:gtest are forbidden, but a dependency path diff --git a/scripts/run_bazel.py b/scripts/run_bazel.py index 53c0abda5450..2f42e984def4 100755 --- a/scripts/run_bazel.py +++ b/scripts/run_bazel.py @@ -15,7 +15,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import os import sys -import scripts_utils # type: ignore +import scripts_utils def main() -> None: diff --git a/scripts/run_buildifier.py b/scripts/run_buildifier.py index 61a8fb33021f..0cea5caceb0a 100755 --- a/scripts/run_buildifier.py +++ b/scripts/run_buildifier.py @@ -11,7 +11,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import os import sys -import scripts_utils # type: ignore +import scripts_utils def main() -> None: diff --git a/scripts/run_buildozer.py b/scripts/run_buildozer.py index d3e7b939b2c4..2c67fd0556d6 100755 --- a/scripts/run_buildozer.py +++ b/scripts/run_buildozer.py @@ -15,7 +15,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import os import sys -import scripts_utils # type: ignore +import scripts_utils def main() -> None: