Skip to content

Commit fec59ed

Browse files
ilevkivskyiMichael0x2a
authored andcommitted
Allow overloaded overrides if they don't extend domain (#5505)
Fixes #4985 Mypy previously disallowed introducing overloads for operator methods in overrides because this can interact unsafely with reverse methods. However it looks like this is safe for the situations where the domain of the override is not extended. In fact the same unsafety exists for non-overloaded op-methods (see example in #4985 (comment)), but it looks like we are fine with this, I have found few existing tests explicitly testing for this.
1 parent 402d734 commit fec59ed

File tree

3 files changed

+84
-10
lines changed

3 files changed

+84
-10
lines changed

mypy/checker.py

+26-6
Original file line numberDiff line numberDiff line change
@@ -1341,6 +1341,20 @@ def check_method_override_for_base_with_name(
13411341
self.msg.signature_incompatible_with_supertype(
13421342
defn.name(), name, base.name(), context)
13431343

1344+
def get_op_other_domain(self, tp: FunctionLike) -> Optional[Type]:
1345+
if isinstance(tp, CallableType):
1346+
if tp.arg_kinds and tp.arg_kinds[0] == ARG_POS:
1347+
return tp.arg_types[0]
1348+
return None
1349+
elif isinstance(tp, Overloaded):
1350+
raw_items = [self.get_op_other_domain(it) for it in tp.items()]
1351+
items = [it for it in raw_items if it]
1352+
if items:
1353+
return UnionType.make_simplified_union(items)
1354+
return None
1355+
else:
1356+
assert False, "Need to check all FunctionLike subtypes here"
1357+
13441358
def check_override(self, override: FunctionLike, original: FunctionLike,
13451359
name: str, name_in_super: str, supertype: str,
13461360
original_class_or_static: bool,
@@ -1357,15 +1371,18 @@ def check_override(self, override: FunctionLike, original: FunctionLike,
13571371
"""
13581372
# Use boolean variable to clarify code.
13591373
fail = False
1374+
op_method_wider_note = False
13601375
if not is_subtype(override, original, ignore_pos_arg_names=True):
13611376
fail = True
1362-
elif (not isinstance(original, Overloaded) and
1363-
isinstance(override, Overloaded) and
1364-
self.is_forward_op_method(name)):
1365-
# Operator method overrides cannot introduce overloading, as
1377+
elif isinstance(override, Overloaded) and self.is_forward_op_method(name):
1378+
# Operator method overrides cannot extend the domain, as
13661379
# this could be unsafe with reverse operator methods.
1367-
fail = True
1368-
1380+
original_domain = self.get_op_other_domain(original)
1381+
override_domain = self.get_op_other_domain(override)
1382+
if (original_domain and override_domain and
1383+
not is_subtype(override_domain, original_domain)):
1384+
fail = True
1385+
op_method_wider_note = True
13691386
if isinstance(original, FunctionLike) and isinstance(override, FunctionLike):
13701387
if original_class_or_static and not override_class_or_static:
13711388
fail = True
@@ -1427,6 +1444,9 @@ def erase_override(t: Type) -> Type:
14271444
# Fall back to generic incompatibility message.
14281445
self.msg.signature_incompatible_with_supertype(
14291446
name, name_in_super, supertype, node)
1447+
if op_method_wider_note:
1448+
self.note("Overloaded operator methods can't have wider argument types"
1449+
" in overrides", node)
14301450

14311451
def visit_class_def(self, defn: ClassDef) -> None:
14321452
"""Type check a class definition."""

test-data/unit/check-classes.test

+5-4
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,8 @@ from typing import overload
15711571
class A:
15721572
def __add__(self, x: int) -> int: pass
15731573
class B(A):
1574-
@overload # E: Signature of "__add__" incompatible with supertype "A"
1574+
@overload # E: Signature of "__add__" incompatible with supertype "A" \
1575+
# N: Overloaded operator methods can't have wider argument types in overrides
15751576
def __add__(self, x: int) -> int: pass
15761577
@overload
15771578
def __add__(self, x: str) -> str: pass
@@ -1665,7 +1666,8 @@ class A:
16651666
@overload
16661667
def __add__(self, x: str) -> 'A': pass
16671668
class B(A):
1668-
@overload
1669+
@overload # E: Signature of "__add__" incompatible with supertype "A" \
1670+
# N: Overloaded operator methods can't have wider argument types in overrides
16691671
def __add__(self, x: int) -> A: pass
16701672
@overload
16711673
def __add__(self, x: str) -> A: pass
@@ -1830,8 +1832,7 @@ class Num1:
18301832
def __radd__(self, other: Num1) -> Num1: ...
18311833

18321834
class Num2(Num1):
1833-
# TODO: This should not be an error. See https://github.com/python/mypy/issues/4985
1834-
@overload # E: Signature of "__add__" incompatible with supertype "Num1"
1835+
@overload
18351836
def __add__(self, other: Num2) -> Num2: ...
18361837
@overload
18371838
def __add__(self, other: Num1) -> Num2: ...

test-data/unit/check-overloading.test

+53
Original file line numberDiff line numberDiff line change
@@ -4125,6 +4125,59 @@ main:11: error: Argument 7 to "f" has incompatible type "Union[int, str]"; expec
41254125
main:11: error: Argument 8 to "f" has incompatible type "Union[int, str]"; expected "int"
41264126
main:11: note: Not all union combinations were tried because there are too many unions
41274127

4128+
[case testSafeDunderOverlapInSubclass]
4129+
from typing import overload
4130+
4131+
class A:
4132+
def __add__(self, x : 'A') -> 'A': ...
4133+
4134+
class B(A):
4135+
@overload
4136+
def __add__(self, x : 'B') -> 'B': ...
4137+
@overload
4138+
def __add__(self, x : 'A') -> 'A' : ...
4139+
def __add__(self, x):
4140+
pass
4141+
[out]
4142+
4143+
[case testUnsafeDunderOverlapInSubclass]
4144+
from typing import overload
4145+
4146+
class A:
4147+
def __add__(self, x : 'A') -> 'A':
4148+
if isinstance(x, A):
4149+
return A()
4150+
else:
4151+
return NotImplemented
4152+
4153+
# This is unsafe override because of the problem below
4154+
class B(A):
4155+
@overload # E: Signature of "__add__" incompatible with supertype "A" \
4156+
# N: Overloaded operator methods can't have wider argument types in overrides
4157+
def __add__(self, x : 'Other') -> 'B' : ...
4158+
@overload
4159+
def __add__(self, x : 'A') -> 'A': ...
4160+
def __add__(self, x):
4161+
if isinstance(x, Other):
4162+
return B()
4163+
elif isinstance(x, A):
4164+
return A()
4165+
else:
4166+
return NotImplemented
4167+
4168+
class Other:
4169+
def __radd__(self, x: 'A') -> 'Other':
4170+
if isinstance(x, A):
4171+
return Other()
4172+
else:
4173+
return NotImplemented
4174+
4175+
actually_b: A = B()
4176+
reveal_type(actually_b + Other()) # E: Revealed type is '__main__.Other'
4177+
# Runtime type is B, this is why we report the error on overriding.
4178+
[builtins fixtures/isinstance.pyi]
4179+
[out]
4180+
41284181
[case testOverloadErrorMessageManyMatches]
41294182
from typing import overload
41304183

0 commit comments

Comments
 (0)