Skip to content

Commit

Permalink
Add a mypy plugin to support @total_ordering. (#9525)
Browse files Browse the repository at this point in the history
Without this, when using a comparison method synthesized by the
`@total_ordering` class decorator, MyPy will incorrectly observe
something like:
```
src/python/pants/option/option_value_container.py:108:12: error: Unsupported
left operand type for >= ("Rank")  [operator]
            if new_rank >= existing_rank:
               ^
Found 1 error in 1 file (checked 40 source files)
```

Here, just `__lt__` is explicitly defined in the "Rank" class and
`@total_ordering` generates `__ge__` which MyPy is ignorant of.

Also, in order to get the in-repo plugin working, fix the mypy task to
upgrade its interpreter when `--include-requirements` is turned on and
remove type checking tags from mypy example targets which are only ever
used in integration tests where tags are not used as filters anyhow.
  • Loading branch information
jsirois authored Apr 13, 2020
1 parent 45bf1da commit 2d5ce99
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 4 deletions.
14 changes: 14 additions & 0 deletions 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ coverage>=4.5,<4.6
dataclasses==0.6
docutils==0.14
fasteners==0.15.0

# The MyPy requirement should be maintained in lockstep with the requirement the Pants repo uses
# for the mypy task since it configures custom MyPy plugins. That requirement can be found via:
#
# ./pants \
# --backend-packages=pants.contrib.mypy \
# options \
# --output-format=json \
# --scope=mypy \
# --name=version \
# | jq -r '."mypy.version".value'
#
mypy==0.761

Markdown==2.1.1
packaging==16.8
parameterized==0.6.1
Expand Down
5 changes: 3 additions & 2 deletions build-support/bin/mypy.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ def main() -> None:
# still want to run MyPy against it so that we can enforce the type hints that may be there
# already and we can make sure that we don't revert in adding code that MyPy flags as an
# error.
"--tag=type_checked,partially_type_checked",
"--tag=+type_checked,+partially_type_checked,-nolint",
"--backend-packages=pants.contrib.mypy",
"--mypy-config=build-support/mypy/mypy.ini",
"lint",
"lint.mypy",
"--include-requirements",
*globs,
],
check=True,
Expand Down
4 changes: 4 additions & 0 deletions build-support/mypy/mypy.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
[mypy]
# See: https://mypy.readthedocs.io/en/latest/extending_mypy.html#configuring-mypy-to-use-plugins
plugins =
pants.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
# for any new behavior that we can opt-in to.
Expand Down
2 changes: 0 additions & 2 deletions contrib/mypy/examples/src/python/mypy_plugin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ python_library(
dependencies=[
':django-stubs',
],
tags = {'partially_type_checked'},
)

python_library(
Expand All @@ -31,7 +30,6 @@ python_library(
':django',
':settings',
],
tags = {'partially_type_checked'},
)

python_library(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ def execute(self):
extra_pexes.append(
self.context.products.get_data(ResolveRequirements.REQUIREMENTS_PEX)
)
mypy_interpreter = interpreter_for_targets
else:
self.context.log.warn(
f"The --include-requirements option is set, but the current target's requirements have "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def cmdline(cls, *, include_requirements):
cmd = [
"--backend-packages=pants.contrib.mypy",
f'--mypy-config={cls.example_dir / "mypy.ini"}',
"--mypy-version=mypy==0.720",
"lint.mypy",
]
if include_requirements:
Expand Down
Empty file.
9 changes: 9 additions & 0 deletions src/python/pants/mypy/plugins/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
dependencies=[
'3rdparty/python:mypy',
],
tags = {'type_checked'},
)
Empty file.
49 changes: 49 additions & 0 deletions src/python/pants/mypy/plugins/total_ordering.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# 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

0 comments on commit 2d5ce99

Please sign in to comment.