From 62810c8cc7384f2966ddbd7f3adf1b410b8469b1 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Thu, 10 Sep 2020 20:10:49 -0700 Subject: [PATCH] Add support to MyPy for first-party plugins (#10755) Similar to with Pylint plugins, we introduce a new target type `mypy_source_plugin`, along with a new option `--mypy-source-plugins`. The new target type is not strictly necessary; you could use a `python_library`. But, the new target type a) provides clearer modeling of what's going on, e.g. that this is a "root" rather than a typical library; and b) allows us to add instructions via `./pants target-types --details=mypy_source_plugin`. [ci skip-build-wheels] [ci skip-rust] --- build-support/mypy/mypy.ini | 5 +- .../mypy => pants-plugins/mypy_plugins}/BUILD | 5 +- .../mypy_plugins}/__init__.py | 0 pants-plugins/mypy_plugins/total_ordering.py | 47 +++++ pants.toml | 1 + .../python/lint/pylint/plugin_target_type.py | 8 +- .../pants/backend/python/lint/pylint/rules.py | 3 +- .../lint/pylint/rules_integration_test.py | 55 ++++-- .../backend/python/lint/pylint/subsystem.py | 7 +- .../typecheck/mypy/plugin_target_type.py | 43 +++++ .../backend/python/typecheck/mypy/register.py | 5 + .../backend/python/typecheck/mypy/rules.py | 74 ++++++-- .../typecheck/mypy/rules_integration_test.py | 162 +++++++++++++++++- .../python/typecheck/mypy/subsystem.py | 19 +- src/python/pants/mypy/plugins/BUILD | 4 - src/python/pants/mypy/plugins/__init__.py | 0 .../pants/mypy/plugins/total_ordering.py | 49 ------ .../pants/option/option_value_container.py | 3 +- 18 files changed, 378 insertions(+), 112 deletions(-) rename {src/python/pants/mypy => pants-plugins/mypy_plugins}/BUILD (62%) rename {src/python/pants/mypy => pants-plugins/mypy_plugins}/__init__.py (100%) create mode 100644 pants-plugins/mypy_plugins/total_ordering.py create mode 100644 src/python/pants/backend/python/typecheck/mypy/plugin_target_type.py delete mode 100644 src/python/pants/mypy/plugins/BUILD delete mode 100644 src/python/pants/mypy/plugins/__init__.py delete mode 100644 src/python/pants/mypy/plugins/total_ordering.py diff --git a/build-support/mypy/mypy.ini b/build-support/mypy/mypy.ini index 6b9a9a82592..597bc5fafc4 100644 --- a/build-support/mypy/mypy.ini +++ b/build-support/mypy/mypy.ini @@ -1,8 +1,7 @@ [mypy] -# TODO(#10131): Re-enable this plugin once we add support to v2 MyPy. # See: https://mypy.readthedocs.io/en/latest/extending_mypy.html#configuring-mypy-to-use-plugins -# plugins = -# pants.mypy.plugins.total_ordering +plugins = + mypy_plugins.total_ordering # Refer to https://mypy.readthedocs.io/en/latest/command_line.html for the definition of each # of these options. MyPy is frequently updated, so this file should be periodically reviewed diff --git a/src/python/pants/mypy/BUILD b/pants-plugins/mypy_plugins/BUILD similarity index 62% rename from src/python/pants/mypy/BUILD rename to pants-plugins/mypy_plugins/BUILD index e255db96c8d..58120c8145b 100644 --- a/src/python/pants/mypy/BUILD +++ b/pants-plugins/mypy_plugins/BUILD @@ -1,4 +1,7 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -python_library() +mypy_source_plugin( + name="total_ordering", + sources=["total_ordering.py"], +) diff --git a/src/python/pants/mypy/__init__.py b/pants-plugins/mypy_plugins/__init__.py similarity index 100% rename from src/python/pants/mypy/__init__.py rename to pants-plugins/mypy_plugins/__init__.py diff --git a/pants-plugins/mypy_plugins/total_ordering.py b/pants-plugins/mypy_plugins/total_ordering.py new file mode 100644 index 00000000000..77fc4056e88 --- /dev/null +++ b/pants-plugins/mypy_plugins/total_ordering.py @@ -0,0 +1,47 @@ +# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +# See: https://mypy.readthedocs.io/en/latest/extending_mypy.html#high-level-overview + +from typing import Callable, Optional, Type + +from mypy.nodes import ARG_POS, Argument, TypeInfo, Var +from mypy.plugin import ClassDefContext, Plugin +from mypy.plugins.common import add_method + + +class TotalOrderingPlugin(Plugin): + """This inserts method type stubs for the "missing" ordering methods that the `@total_ordering` + class decorator will fill in dynamically.""" + + def get_class_decorator_hook( + self, fullname: str + ) -> Optional[Callable[[ClassDefContext], None]]: + return adjust_class_def if fullname == "functools.total_ordering" else None + + +def adjust_class_def(class_def_context: ClassDefContext) -> None: + api = class_def_context.api + ordering_other_type = api.named_type("__builtins__.object") + ordering_return_type = api.named_type("__builtins__.bool") + arg = Argument( + variable=Var(name="other", type=ordering_other_type), + type_annotation=ordering_other_type, + initializer=None, + kind=ARG_POS, + ) + + type_info: TypeInfo = class_def_context.cls.info + for ordering_method_name in "__lt__", "__le__", "__gt__", "__ge__": + existing_method = type_info.get(ordering_method_name) + if existing_method is None: + add_method( + ctx=class_def_context, + name=ordering_method_name, + args=[arg], + return_type=ordering_return_type, + ) + + +def plugin(_version: str) -> Type[Plugin]: + return TotalOrderingPlugin diff --git a/pants.toml b/pants.toml index bfb574ed2a8..f014b0c471d 100644 --- a/pants.toml +++ b/pants.toml @@ -75,6 +75,7 @@ config = ["pyproject.toml", "examples/.isort.cfg"] [mypy] config = "build-support/mypy/mypy.ini" +source_plugins = ["pants-plugins/mypy_plugins:total_ordering"] [pants-releases] release_notes = """ diff --git a/src/python/pants/backend/python/lint/pylint/plugin_target_type.py b/src/python/pants/backend/python/lint/pylint/plugin_target_type.py index d33af1cbaba..7fa4a5ac7f4 100644 --- a/src/python/pants/backend/python/lint/pylint/plugin_target_type.py +++ b/src/python/pants/backend/python/lint/pylint/plugin_target_type.py @@ -7,7 +7,6 @@ class PylintPluginSources(PythonSources): required = True - expected_file_extensions = (".py",) # NB: We solely subclass this to change the docstring. @@ -26,7 +25,7 @@ class PylintSourcePlugin(Target): To load a source plugin: 1. Write your plugin. See http://pylint.pycqa.org/en/latest/how_tos/plugins.html. - 2. Define a `pylint_source_plugin` target with the plugin's Python files included in the + 2. Define a `pylint_source_plugin` target with the plugin's Python file(s) included in the `sources` field. 3. Add the parent directory of your target to the `root_patterns` option in the `[source]` scope. For example, if your plugin is at `build-support/pylint/custom_plugin.py`, add @@ -49,10 +48,7 @@ class PylintSourcePlugin(Target): third-party dependencies or are located in the same directory or a subdirectory. Other targets can depend on this target. This allows you to write a `python_tests` target for - this code. - - You can define the `provides` field to release this plugin as a distribution - (https://www.pantsbuild.org/docs/python-setup-py-goal). + this code or a `python_distribution` target to distribute the plugin externally. """ alias = "pylint_source_plugin" diff --git a/src/python/pants/backend/python/lint/pylint/rules.py b/src/python/pants/backend/python/lint/pylint/rules.py index 5b3a51c2415..c57e44de65e 100644 --- a/src/python/pants/backend/python/lint/pylint/rules.py +++ b/src/python/pants/backend/python/lint/pylint/rules.py @@ -244,8 +244,7 @@ async def pylint_lint( return LintResults([], linter_name="Pylint") plugin_target_addresses = await MultiGet( - Get(Address, AddressInput, AddressInput.parse(plugin_addr)) - for plugin_addr in pylint.source_plugins + Get(Address, AddressInput, plugin_addr) for plugin_addr in pylint.source_plugins ) plugin_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses)) linted_targets_request = Get( diff --git a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py index f6aefb7e69b..9a4152284d1 100644 --- a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py @@ -333,6 +333,7 @@ def test_plugin(self): def test_source_plugin(rule_runner: RuleRunner) -> None: # NB: We make this source plugin fairly complex by having it use transitive dependencies. # This is to ensure that we can correctly support plugins with dependencies. + # The plugin bans `print()`. rule_runner.add_to_build_file( "", dedent( @@ -350,30 +351,34 @@ def test_source_plugin(rule_runner: RuleRunner) -> None: ), ) rule_runner.create_file( - "build-support/plugins/subdir/dep.py", + "pants-plugins/plugins/subdir/dep.py", dedent( """\ from colors import red def is_print(node): _ = red("Test that transitive deps are loaded.") - return node.func.name == "print" + return hasattr(node.func, "name") and node.func.name == "print" """ ), ) rule_runner.add_to_build_file( - "build-support/plugins/subdir", "python_library(dependencies=['//:colors'])" + "pants-plugins/plugins/subdir", "python_library(dependencies=['//:colors'])" ) rule_runner.create_file( - "build-support/plugins/print_plugin.py", + "pants-plugins/plugins/print_plugin.py", dedent( """\ + '''Docstring.''' + from pylint.checkers import BaseChecker from pylint.interfaces import IAstroidChecker from subdir.dep import is_print class PrintChecker(BaseChecker): + '''Docstring.''' + __implements__ = IAstroidChecker name = "print_plugin" msgs = { @@ -381,22 +386,25 @@ class PrintChecker(BaseChecker): } def visit_call(self, node): + '''Docstring.''' if is_print(node): self.add_message("print-statement-used", node=node) + def register(linter): + '''Docstring.''' linter.register_checker(PrintChecker(linter)) """ ), ) rule_runner.add_to_build_file( - "build-support/plugins", + "pants-plugins/plugins", dedent( """\ pylint_source_plugin( name='print_plugin', sources=['print_plugin.py'], - dependencies=['//:pylint', 'build-support/plugins/subdir'], + dependencies=['//:pylint', 'pants-plugins/plugins/subdir'], ) """ ), @@ -407,18 +415,31 @@ def register(linter): load-plugins=print_plugin """ ) + + def run_pylint_with_plugin(tgt: Target) -> LintResult: + result = run_pylint( + rule_runner, + [tgt], + additional_args=[ + "--pylint-source-plugins=['pants-plugins/plugins:print_plugin']", + f"--source-root-patterns=['pants-plugins/plugins', '{PACKAGE}']", + ], + config=config_content, + ) + assert len(result) == 1 + return result[0] + target = make_target( rule_runner, [FileContent(f"{PACKAGE}/source_plugin.py", b"'''Docstring.'''\nprint()\n")] ) - result = run_pylint( - rule_runner, - [target], - additional_args=[ - "--pylint-source-plugins=['build-support/plugins:print_plugin']", - f"--source-root-patterns=['build-support/plugins', '{PACKAGE}']", - ], - config=config_content, + result = run_pylint_with_plugin(target) + assert result.exit_code == PYLINT_FAILURE_RETURN_CODE + assert f"{PACKAGE}/source_plugin.py:2:0: C9871" in result.stdout + + # Ensure that running Pylint on the plugin itself still works. + plugin_tgt = rule_runner.get_target( + Address("pants-plugins/plugins", target_name="print_plugin") ) - assert len(result) == 1 - assert result[0].exit_code == PYLINT_FAILURE_RETURN_CODE - assert f"{PACKAGE}/source_plugin.py:2:0: C9871" in result[0].stdout + result = run_pylint_with_plugin(plugin_tgt) + assert result.exit_code == 0 + assert "Your code has been rated at 10.00/10" in result.stdout diff --git a/src/python/pants/backend/python/lint/pylint/subsystem.py b/src/python/pants/backend/python/lint/pylint/subsystem.py index 760b4d3ea0b..ec92193e77f 100644 --- a/src/python/pants/backend/python/lint/pylint/subsystem.py +++ b/src/python/pants/backend/python/lint/pylint/subsystem.py @@ -1,9 +1,10 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from typing import List, Optional, cast +from typing import List, Optional, Tuple, cast from pants.backend.python.subsystems.python_tool_base import PythonToolBase +from pants.engine.addresses import AddressInput from pants.option.custom_types import file_option, shell_str, target_option @@ -66,5 +67,5 @@ def config(self) -> Optional[str]: return cast(Optional[str], self.options.config) @property - def source_plugins(self) -> List[str]: - return cast(List[str], self.options.source_plugins) + def source_plugins(self) -> Tuple[AddressInput, ...]: + return tuple(AddressInput.parse(v) for v in self.options.source_plugins) diff --git a/src/python/pants/backend/python/typecheck/mypy/plugin_target_type.py b/src/python/pants/backend/python/typecheck/mypy/plugin_target_type.py new file mode 100644 index 00000000000..eef53fa1a33 --- /dev/null +++ b/src/python/pants/backend/python/typecheck/mypy/plugin_target_type.py @@ -0,0 +1,43 @@ +# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from pants.backend.python.target_types import COMMON_PYTHON_FIELDS, PythonSources +from pants.engine.target import Dependencies, Target + + +class MyPyPluginSources(PythonSources): + required = True + + +class MyPySourcePlugin(Target): + """A MyPy plugin loaded through source code. + + To load a source plugin: + + 1. Write your plugin. See https://mypy.readthedocs.io/en/stable/extending_mypy.html. + 2. Define a `mypy_source_plugin` target with the plugin's Python file(s) included in the + `sources` field. + 3. Add `plugins = path.to.module` to your MyPy config file, using the name of the module + without source roots. For example, if your Python file is called + `pants-plugins/mypy_plugins/custom_plugin.py`, and you set `pants-plugins` as a source root, + then set `plugins = mypy_plugins.custom_plugin`. Set the `config` + option in the `[mypy]` scope to point to your MyPy config file. + 5. Set the option `source_plugins` in the `[mypy]` scope to include this target's + address, e.g. `source_plugins = ["build-support/mypy_plugins:plugin"]`. + + To instead load a third-party plugin, set the option `extra_requirements` in the `[mypy]` + scope (see https://www.pantsbuild.org/v2.0/docs/python-typecheck-goal). Set `plugins` in + your config file, like you'd do with a source plugin. + + This target type is treated similarly to a `python_library` target. For example, Python linters + and formatters will run on this target. + + You can include other targets in the `dependencies` field, including third-party requirements + and other source files (even if those source files live in a different directory). + + Other targets can depend on this target. This allows you to write a `python_tests` target for + this code or a `python_distribution` target to distribute the plugin externally. + """ + + alias = "mypy_source_plugin" + core_fields = (*COMMON_PYTHON_FIELDS, Dependencies, MyPyPluginSources) diff --git a/src/python/pants/backend/python/typecheck/mypy/register.py b/src/python/pants/backend/python/typecheck/mypy/register.py index 60ab5a41d7d..d40bac5de33 100644 --- a/src/python/pants/backend/python/typecheck/mypy/register.py +++ b/src/python/pants/backend/python/typecheck/mypy/register.py @@ -8,6 +8,11 @@ """ from pants.backend.python.typecheck.mypy import rules as mypy_rules +from pants.backend.python.typecheck.mypy.plugin_target_type import MyPySourcePlugin + + +def target_types(): + return [MyPySourcePlugin] def rules(): diff --git a/src/python/pants/backend/python/typecheck/mypy/rules.py b/src/python/pants/backend/python/typecheck/mypy/rules.py index 9bc54022142..887dbd057e6 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules.py @@ -1,12 +1,17 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import itertools from dataclasses import dataclass from pathlib import PurePath from textwrap import dedent from typing import Tuple -from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources +from pants.backend.python.target_types import ( + PythonInterpreterCompatibility, + PythonRequirementsField, + PythonSources, +) from pants.backend.python.typecheck.mypy.subsystem import MyPy from pants.backend.python.util_rules import pex_from_targets from pants.backend.python.util_rules.pex import ( @@ -23,7 +28,7 @@ ) from pants.core.goals.typecheck import TypecheckRequest, TypecheckResult, TypecheckResults from pants.core.util_rules import pants_bin -from pants.engine.addresses import Addresses +from pants.engine.addresses import Address, Addresses, AddressInput from pants.engine.fs import ( CreateDigest, Digest, @@ -104,7 +109,7 @@ def generate_args(mypy: MyPy, *, file_list_path: str) -> Tuple[str, ...]: # TODO(#10131): Improve performance, e.g. by leveraging the MyPy cache. -# TODO(#10131): Support first-party plugins and .pyi files. +# TODO(#10131): Support .pyi files. @rule(desc="Typecheck using MyPy", level=LogLevel.DEBUG) async def mypy_typecheck( request: MyPyRequest, mypy: MyPy, python_setup: PythonSetup @@ -112,11 +117,26 @@ async def mypy_typecheck( if mypy.skip: return TypecheckResults([], typechecker_name="MyPy") - transitive_targets, launcher_script = await MultiGet( - Get(TransitiveTargets, Addresses(fs.address for fs in request.field_sets)), + plugin_target_addresses = await MultiGet( + Get(Address, AddressInput, plugin_addr) for plugin_addr in mypy.source_plugins + ) + + plugin_transitive_targets_request = Get(TransitiveTargets, Addresses(plugin_target_addresses)) + typechecked_transitive_targets_request = Get( + TransitiveTargets, Addresses(fs.address for fs in request.field_sets) + ) + plugin_transitive_targets, typechecked_transitive_targets, launcher_script = await MultiGet( + plugin_transitive_targets_request, + typechecked_transitive_targets_request, Get(Digest, CreateDigest([LAUNCHER_FILE])), ) + plugin_requirements = PexRequirements.create_from_requirement_fields( + plugin_tgt[PythonRequirementsField] + for plugin_tgt in plugin_transitive_targets.closure + if plugin_tgt.has_field(PythonRequirementsField) + ) + # Interpreter constraints are tricky with MyPy: # * MyPy requires running with Python 3.5+. If run with Python 3.5-3.7, MyPy can understand # Python 2.7 and 3.4-3.7 thanks to the typed-ast library, but it can't understand 3.8+ If @@ -140,7 +160,9 @@ async def mypy_typecheck( all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields( ( tgt[PythonInterpreterCompatibility] - for tgt in transitive_targets.closure + for tgt in itertools.chain( + typechecked_transitive_targets.closure, plugin_transitive_targets.closure + ) if tgt.has_field(PythonInterpreterCompatibility) ), python_setup, @@ -156,9 +178,11 @@ async def mypy_typecheck( else: tool_interpreter_constraints = mypy.interpreter_constraints - prepared_sources_request = Get( - PythonSourceFiles, - PythonSourceFilesRequest(transitive_targets.closure), + plugin_sources_request = Get( + PythonSourceFiles, PythonSourceFilesRequest(plugin_transitive_targets.closure) + ) + typechecked_sources_request = Get( + PythonSourceFiles, PythonSourceFilesRequest(typechecked_transitive_targets.closure) ) tool_pex_request = Get( @@ -166,7 +190,9 @@ async def mypy_typecheck( PexRequest( output_filename="mypy.pex", internal_only=True, - requirements=PexRequirements(mypy.all_requirements), + requirements=PexRequirements( + itertools.chain(mypy.all_requirements, plugin_requirements) + ), interpreter_constraints=PexInterpreterConstraints(tool_interpreter_constraints), entry_point=mypy.entry_point, ), @@ -207,17 +233,27 @@ async def mypy_typecheck( ), ) - prepared_sources, tool_pex, requirements_pex, runner_pex, config_digest = await MultiGet( - prepared_sources_request, + ( + plugin_sources, + typechecked_sources, + tool_pex, + requirements_pex, + runner_pex, + config_digest, + ) = await MultiGet( + plugin_sources_request, + typechecked_sources_request, tool_pex_request, requirements_pex_request, runner_pex_request, config_digest_request, ) - srcs_snapshot = prepared_sources.source_files.snapshot + typechecked_srcs_snapshot = typechecked_sources.source_files.snapshot file_list_path = "__files.txt" - python_files = "\n".join(f for f in srcs_snapshot.files if f.endswith(".py")) + python_files = "\n".join( + f for f in typechecked_sources.source_files.snapshot.files if f.endswith(".py") + ) file_list_digest = await Get( Digest, CreateDigest([FileContent(file_list_path, python_files.encode())]), @@ -228,7 +264,8 @@ async def mypy_typecheck( MergeDigests( [ file_list_digest, - srcs_snapshot.digest, + plugin_sources.source_files.snapshot.digest, + typechecked_srcs_snapshot.digest, tool_pex.digest, requirements_pex.digest, runner_pex.digest, @@ -237,14 +274,17 @@ async def mypy_typecheck( ), ) + all_used_source_roots = sorted( + set(itertools.chain(plugin_sources.source_roots, typechecked_sources.source_roots)) + ) result = await Get( FallibleProcessResult, PexProcess( runner_pex, argv=generate_args(mypy, file_list_path=file_list_path), input_digest=merged_input_files, - extra_env={"PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots)}, - description=f"Run MyPy on {pluralize(len(srcs_snapshot.files), 'file')}.", + extra_env={"PEX_EXTRA_SYS_PATH": ":".join(all_used_source_roots)}, + description=f"Run MyPy on {pluralize(len(typechecked_srcs_snapshot.files), 'file')}.", level=LogLevel.DEBUG, ), ) diff --git a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py index 7757743c059..07408bc7ef7 100644 --- a/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py +++ b/src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py @@ -9,6 +9,7 @@ from pants.backend.python.dependency_inference import rules as dependency_inference_rules from pants.backend.python.target_types import PythonLibrary, PythonRequirementLibrary +from pants.backend.python.typecheck.mypy.plugin_target_type import MyPySourcePlugin from pants.backend.python.typecheck.mypy.rules import MyPyFieldSet, MyPyRequest from pants.backend.python.typecheck.mypy.rules import rules as mypy_rules from pants.core.goals.typecheck import TypecheckResult, TypecheckResults @@ -30,7 +31,7 @@ def rule_runner() -> RuleRunner: *dependency_inference_rules.rules(), # Used for import inference. QueryRule(TypecheckResults, (MyPyRequest, OptionsBootstrapper)), ], - target_types=[PythonLibrary, PythonRequirementLibrary], + target_types=[PythonLibrary, PythonRequirementLibrary, MyPySourcePlugin], ) @@ -237,7 +238,11 @@ def test_thirdparty_plugin(rule_runner: RuleRunner) -> None: """ ), ) - settings_file = FileContent( + # We hijack `--mypy-source-plugins` for our settings.py file to ensure that it is always used, + # even if the files we're checking don't need it. Typically, this option expects + # `mypy_source_plugin` targets, but that's not actually validated. We only want this specific + # file to be permanently included, not the whole original target, so we will use a file address. + rule_runner.create_file( f"{PACKAGE}/settings.py", dedent( """\ @@ -248,9 +253,9 @@ def test_thirdparty_plugin(rule_runner: RuleRunner) -> None: SECRET_KEY = "not so secret" MY_SETTING = URLPattern(pattern="foo", callback=lambda: None) """ - ).encode(), + ), ) - app_file = FileContent( + rule_runner.create_file( f"{PACKAGE}/app.py", dedent( """\ @@ -259,9 +264,11 @@ def test_thirdparty_plugin(rule_runner: RuleRunner) -> None: assert "forty-two" == text.slugify("forty two") assert "42" == text.slugify(42) """ - ).encode(), + ), ) - target = make_target(rule_runner, [settings_file, app_file]) + rule_runner.add_to_build_file(PACKAGE, "python_library()") + app_tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="app.py")) + config_content = dedent( """\ [mypy] @@ -274,11 +281,12 @@ def test_thirdparty_plugin(rule_runner: RuleRunner) -> None: ) result = run_mypy( rule_runner, - [target], + [app_tgt], config=config_content, additional_args=[ "--mypy-extra-requirements=django-stubs==1.5.0", "--mypy-version=mypy==0.770", + f"--mypy-source-plugins={PACKAGE}/settings.py", ], ) assert len(result) == 1 @@ -357,3 +365,143 @@ def test_works_with_python38(rule_runner: RuleRunner) -> None: assert len(result) == 1 assert result[0].exit_code == 0 assert "Success: no issues found" in result[0].stdout.strip() + + +def test_source_plugin(rule_runner: RuleRunner) -> None: + # NB: We make this source plugin fairly complex by having it use transitive dependencies. + # This is to ensure that we can correctly support plugins with dependencies. + # The plugin changes the return type of functions ending in `__overridden_by_plugin` to have a + # return type of `None`. + rule_runner.add_to_build_file( + "", + dedent( + """\ + python_requirement_library( + name='mypy', + requirements=['mypy==0.782'], + ) + + python_requirement_library( + name="more-itertools", + requirements=["more-itertools==8.4.0"], + ) + """ + ), + ) + rule_runner.create_file("pants-plugins/plugins/subdir/__init__.py") + rule_runner.create_file( + "pants-plugins/plugins/subdir/dep.py", + dedent( + """\ + from more_itertools import flatten + + def is_overridable_function(name: str) -> bool: + assert list(flatten([[1, 2], [3, 4]])) == [1, 2, 3, 4] + return name.endswith("__overridden_by_plugin") + """ + ), + ) + rule_runner.add_to_build_file("pants-plugins/plugins/subdir", "python_library()") + + # The plugin can depend on code located anywhere in the project; its dependencies need not be in + # the same directory. + rule_runner.create_file(f"{PACKAGE}/__init__.py") + rule_runner.create_file(f"{PACKAGE}/subdir/__init__.py") + rule_runner.create_file(f"{PACKAGE}/subdir/util.py", "def noop() -> None:\n pass\n") + rule_runner.add_to_build_file(f"{PACKAGE}/subdir", "python_library()") + + rule_runner.create_file("pants-plugins/plugins/__init__.py") + rule_runner.create_file( + "pants-plugins/plugins/change_return_type.py", + dedent( + """\ + from typing import Callable, Optional, Type + + from mypy.plugin import FunctionContext, Plugin + from mypy.types import NoneType, Type as MyPyType + + from plugins.subdir.dep import is_overridable_function + from project.subdir.util import noop + + noop() + + class AutoAddFieldPlugin(Plugin): + def get_function_hook( + self, fullname: str + ) -> Optional[Callable[[FunctionContext], MyPyType]]: + return hook if is_overridable_function(fullname) else None + + + def hook(ctx: FunctionContext) -> MyPyType: + return NoneType() + + + def plugin(_version: str) -> Type[Plugin]: + return AutoAddFieldPlugin + """ + ), + ) + rule_runner.add_to_build_file( + "pants-plugins/plugins", + dedent( + """\ + mypy_source_plugin( + name='change_return_type', + sources=['change_return_type.py'], + ) + """ + ), + ) + + config_content = dedent( + """\ + [mypy] + plugins = + plugins.change_return_type + """ + ) + + test_file_content = dedent( + """\ + def add(x: int, y: int) -> int: + return x + y + + + def add__overridden_by_plugin(x: int, y: int) -> int: + return x + y + + + result = add__overridden_by_plugin(1, 1) + assert add(result, 2) == 4 + """ + ).encode() + + def run_mypy_with_plugin(tgt: Target) -> TypecheckResult: + result = run_mypy( + rule_runner, + [tgt], + additional_args=[ + "--mypy-source-plugins=['pants-plugins/plugins:change_return_type']", + "--source-root-patterns=['pants-plugins', 'src/python']", + ], + config=config_content, + ) + assert len(result) == 1 + return result[0] + + target = make_target( + rule_runner, [FileContent(f"{PACKAGE}/test_source_plugin.py", test_file_content)] + ) + result = run_mypy_with_plugin(target) + assert result.exit_code == 1 + assert f"{PACKAGE}/test_source_plugin.py:10" in result.stdout + # We want to ensure we don't accidentally check the source plugin itself. + assert "(checked 2 source files)" in result.stdout + + # We also want to ensure that running MyPy on the plugin itself still works. + plugin_tgt = rule_runner.get_target( + Address("pants-plugins/plugins", target_name="change_return_type") + ) + result = run_mypy_with_plugin(plugin_tgt) + assert result.exit_code == 0 + assert "Success: no issues found in 7 source files" in result.stdout diff --git a/src/python/pants/backend/python/typecheck/mypy/subsystem.py b/src/python/pants/backend/python/typecheck/mypy/subsystem.py index 53e9ed2564e..51d13a7119f 100644 --- a/src/python/pants/backend/python/typecheck/mypy/subsystem.py +++ b/src/python/pants/backend/python/typecheck/mypy/subsystem.py @@ -4,7 +4,8 @@ from typing import Optional, Tuple, cast from pants.backend.python.subsystems.python_tool_base import PythonToolBase -from pants.option.custom_types import file_option, shell_str +from pants.engine.addresses import AddressInput +from pants.option.custom_types import file_option, shell_str, target_option class MyPy(PythonToolBase): @@ -40,6 +41,18 @@ def register_options(cls, register): advanced=True, help="Path to `mypy.ini` or alternative MyPy config file", ) + register( + "--source-plugins", + type=list, + member_type=target_option, + advanced=True, + help=( + "An optional list of `mypy_source_plugin` target addresses. This allows you to " + "load custom plugins defined in source code. Run the goal `target-types " + "--details=mypy_source_plugin` for instructions, including how to load " + "third-party plugins." + ), + ) @property def skip(self) -> bool: @@ -52,3 +65,7 @@ def args(self) -> Tuple[str, ...]: @property def config(self) -> Optional[str]: return cast(Optional[str], self.options.config) + + @property + def source_plugins(self) -> Tuple[AddressInput, ...]: + return tuple(AddressInput.parse(v) for v in self.options.source_plugins) diff --git a/src/python/pants/mypy/plugins/BUILD b/src/python/pants/mypy/plugins/BUILD deleted file mode 100644 index e255db96c8d..00000000000 --- a/src/python/pants/mypy/plugins/BUILD +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -python_library() diff --git a/src/python/pants/mypy/plugins/__init__.py b/src/python/pants/mypy/plugins/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/python/pants/mypy/plugins/total_ordering.py b/src/python/pants/mypy/plugins/total_ordering.py deleted file mode 100644 index 191726aa3f8..00000000000 --- a/src/python/pants/mypy/plugins/total_ordering.py +++ /dev/null @@ -1,49 +0,0 @@ -# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -# See: https://mypy.readthedocs.io/en/latest/extending_mypy.html#high-level-overview - -from typing import Callable, Optional, Type - -from mypy.nodes import ARG_POS, Argument, TypeInfo, Var -from mypy.plugin import ClassDefContext, Plugin -from mypy.plugins.common import add_method - - -class TotalOrderingPlugin(Plugin): - @staticmethod - def adjust_class_def(class_def_context: ClassDefContext) -> None: - # This MyPy plugin inserts method type stubs for the "missing" ordering methods the - # @total_ordering class decorator will fill in dynamically. - - api = class_def_context.api - ordering_other_type = api.named_type("__builtins__.object") - ordering_return_type = api.named_type("__builtins__.bool") - args = [ - Argument( - variable=Var(name="other", type=ordering_other_type), - type_annotation=ordering_other_type, - initializer=None, - kind=ARG_POS, - ) - ] - - type_info: TypeInfo = class_def_context.cls.info - for ordering_method_name in "__lt__", "__le__", "__gt__", "__ge__": - existing_method = type_info.get(ordering_method_name) - if existing_method is None: - add_method( - ctx=class_def_context, - name=ordering_method_name, - args=args, - return_type=ordering_return_type, - ) - - def get_class_decorator_hook( - self, fullname: str - ) -> Optional[Callable[[ClassDefContext], None]]: - return self.adjust_class_def if fullname == "functools.total_ordering" else None - - -def plugin(_version: str) -> Type[Plugin]: - return TotalOrderingPlugin diff --git a/src/python/pants/option/option_value_container.py b/src/python/pants/option/option_value_container.py index f6f62e12f98..9051ff1bc41 100644 --- a/src/python/pants/option/option_value_container.py +++ b/src/python/pants/option/option_value_container.py @@ -103,8 +103,7 @@ def _set(self, key: Key, value: RankedValue) -> None: existing_value = self._value_map.get(key) existing_rank = existing_value.rank if existing_value is not None else Rank.NONE - # TODO(#10131): Remove the ignore once we can re-enable our MyPy plugin. - if value.rank >= existing_rank: # type: ignore[operator] + if value.rank >= existing_rank: # We set values from outer scopes before values from inner scopes, so # in case of equal rank we overwrite. That way that the inner scope value wins. self._value_map[key] = value