Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error code for mutable covariant override #16399

Merged
merged 4 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,37 @@ Example:
def g(self, y: int) -> None:
pass

.. _code-mutable-override:

Check that overrides of mutable attributes are safe
---------------------------------------------------

This will enable the check for unsafe overrides of mutable attributes. For
historical reasons, and because this is a relatively common pattern in Python,
this check is not enabled by default. The example below is unsafe, and will be
flagged when this error code is enabled:

.. code-block:: python

from typing import Any

class C:
x: float
y: float
z: float

class D(C):
x: int # Error: Covariant override of a mutable attribute
# (base class "C" defined the type as "float",
# expression has type "int") [mutable-override]
y: float # OK
z: Any # OK

def f(c: C) -> None:
c.x = 1.1
d = D()
f(d)
d.x >> 1 # This will crash at runtime, because d.x is now float, not an int

.. _code-unimported-reveal:

Expand Down
21 changes: 18 additions & 3 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2041,7 +2041,6 @@ def check_method_override_for_base_with_name(
pass
elif isinstance(original_type, FunctionLike) and isinstance(typ, FunctionLike):
# Check that the types are compatible.
# TODO overloaded signatures
self.check_override(
typ,
original_type,
Expand All @@ -2056,7 +2055,6 @@ def check_method_override_for_base_with_name(
# Assume invariance for a non-callable attribute here. Note
# that this doesn't affect read-only properties which can have
# covariant overrides.
#
pass
elif (
original_node
Expand Down Expand Up @@ -2636,6 +2634,9 @@ class C(B, A[int]): ... # this is unsafe because...
first_type = get_proper_type(self.determine_type_of_member(first))
second_type = get_proper_type(self.determine_type_of_member(second))

# TODO: use more principled logic to decide is_subtype() vs is_equivalent().
# We should rely on mutability of superclass node, not on types being Callable.

# start with the special case that Instance can be a subtype of FunctionLike
call = None
if isinstance(first_type, Instance):
Expand Down Expand Up @@ -3211,14 +3212,28 @@ def check_compatibility_super(
if base_static and compare_static:
lvalue_node.is_staticmethod = True

return self.check_subtype(
ok = self.check_subtype(
compare_type,
base_type,
rvalue,
message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT,
"expression has type",
f'base class "{base.name}" defined the type as',
)
if (
ok
and codes.MUTABLE_OVERRIDE in self.options.enabled_error_codes
and self.is_writable_attribute(base_node)
):
ok = self.check_subtype(
base_type,
compare_type,
rvalue,
message_registry.COVARIANT_OVERRIDE_OF_MUTABLE_ATTRIBUTE,
f'base class "{base.name}" defined the type as',
"expression has type",
)
return ok
return True

def lvalue_type_from_base(
Expand Down
6 changes: 6 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ def __hash__(self) -> int:
"General",
default_enabled=False,
)
MUTABLE_OVERRIDE: Final[ErrorCode] = ErrorCode(
"mutable-override",
"Reject covariant overrides for mutable attributes",
"General",
default_enabled=False,
)


# Syntax errors are often blocking.
Expand Down
3 changes: 3 additions & 0 deletions mypy/message_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
INCOMPATIBLE_TYPES_IN_ASSIGNMENT: Final = ErrorMessage(
"Incompatible types in assignment", code=codes.ASSIGNMENT
)
COVARIANT_OVERRIDE_OF_MUTABLE_ATTRIBUTE: Final = ErrorMessage(
"Covariant override of a mutable attribute", code=codes.MUTABLE_OVERRIDE
)
INCOMPATIBLE_TYPES_IN_AWAIT: Final = ErrorMessage('Incompatible types in "await"')
INCOMPATIBLE_REDEFINITION: Final = ErrorMessage("Incompatible redefinition")
INCOMPATIBLE_TYPES_IN_ASYNC_WITH_AENTER: Final = (
Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -1148,3 +1148,35 @@ main:3: note: Revealed local types are:
main:3: note: x: builtins.int
main:3: error: Name "reveal_locals" is not defined [unimported-reveal]
[builtins fixtures/isinstancelist.pyi]

[case testCovariantMutableOverride]
# flags: --enable-error-code=mutable-override
from typing import Any

class C:
x: float
y: float
z: float
w: Any
@property
def foo(self) -> float: ...
@property
def bar(self) -> float: ...
@bar.setter
def bar(self, val: float) -> None: ...
baz: float
bad1: float
bad2: float
class D(C):
x: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override]
y: float
z: Any
w: float
foo: int
bar: int # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override]
def one(self) -> None:
self.baz = 5
bad1 = 5 # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override]
def other(self) -> None:
self.bad2: int = 5 # E: Covariant override of a mutable attribute (base class "C" defined the type as "float", expression has type "int") [mutable-override]
[builtins fixtures/property.pyi]