Skip to content

Allow subtypes to define more overloads than their supertype #3263

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jan 15, 2018
Merged

Allow subtypes to define more overloads than their supertype #3263

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d1ec354
Fix #3262: allow subtypes to define more overloads than their supertype
refi64 Apr 26, 2017
49e7b22
Swap subtype check
refi64 Sep 15, 2017
b8a8304
Fix an old test
refi64 Sep 30, 2017
2ca49fc
Fix several overload supertype/subtype cases
refi64 Nov 7, 2017
eb36c41
Be more lenient with detecting invalid subtype overloads
refi64 Nov 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions mypy/subtypes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,19 @@ def visit_overloaded(self, left: Overloaded) -> bool:
return True
return False
elif isinstance(right, Overloaded):
# TODO: this may be too restrictive
if len(left.items()) != len(right.items()):
return False
for i in range(len(left.items())):
if not is_subtype(left.items()[i], right.items()[i], self.check_type_parameter,
# Ensure each overload in the left side is accounted for.
sub_overloads = left.items()[:]
while sub_overloads:
left_item = sub_overloads[-1]
for right_item in right.items():
if is_subtype(left_item, right_item, self.check_type_parameter,
ignore_pos_arg_names=self.ignore_pos_arg_names):
sub_overloads.pop()
break
else:
# One of the overloads was not present in the right side.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic is reversed here. It should be like this: for every item on the right (supertype) there must be an item on the left that is its subtype.

return False

return True
elif isinstance(right, UnboundType):
return True
Expand Down
44 changes: 27 additions & 17 deletions test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1419,23 +1419,6 @@ class B(A):
[out]
tmp/foo.pyi:8: error: Signature of "__add__" incompatible with supertype "A"

[case testOverloadedOperatorMethodOverrideWithSwitchedItemOrder]
from foo import *
[file foo.pyi]
from typing import overload, Any
class A:
@overload
def __add__(self, x: 'B') -> 'B': pass
@overload
def __add__(self, x: 'A') -> 'A': pass
class B(A):
@overload
def __add__(self, x: 'A') -> 'A': pass
@overload
def __add__(self, x: 'B') -> 'B': pass
[out]
tmp/foo.pyi:8: error: Signature of "__add__" incompatible with supertype "A"
Copy link
Member

@ilevkivskyi ilevkivskyi Sep 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have you removed this test? Does it fail? The semantics of overload is not 100% fixed by PEP 484, see python/typing#253, but IIUC the consensus is that order of overloads does matter, so that in this case the override is indeed incompatible. @JukkaL could you please clarify this?


[case testReverseOperatorMethodArgumentType]
from typing import Any
class A: pass
Expand Down Expand Up @@ -2494,6 +2477,33 @@ reveal_type(f(BChild())) # E: Revealed type is 'foo.B'
[builtins fixtures/classmethod.pyi]
[out]

[case testSubtypeWithMoreOverloadsThanSupertypeSucceeds]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have some tests of nasty complicated overloads involving subtyping.

  • One overload in the subtype may cover more than one overload in the supertype (contrived example, might not be a great one):
class Super:
    @overload
    def foo(a: int) -> float: ...
    @overload
    def foo(a: float) -> Number: ...
    @overload
    def foo(a: str) -> str: ...
    
class Sub(Super):
    @overload
    def foo(a: Number) -> float: ...
    @overload
    def foo(a: str) -> str: ...
  • If an overload in the supertype Y_sup accepts an overlapping set of arguments to an overload Y_sub in the subtype, Y_sub must return a subtype of Y_sup's return type. For example, I think this is an error
class Super:
    @overload
    def foo(a: Number) -> Number: ...
    @overload
    def foo(a: str) -> str: ...
    
class Sub(Super):
    @overload
    def foo(a: Number) -> Number: ...
    @overload
    def foo(a: int) -> str: ...
    @overload
    def foo(a: str) -> str: ...

(look at testPartiallyContravariantOverloadSignatures, for another reference)

There's probably even more complexity to this. There usually is.

Copy link
Contributor Author

@refi64 refi64 Apr 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sixolet FWIW my goal with this PR wasn't necessarily to 100% make this work perfectly, but it was more to allow a bit more than was originally allowed (without breaking anything, of course!).

from foo import *
[file foo.pyi]
from typing import overload


class X: pass
class Y: pass
class Z: pass


class A:
@overload
def f(self, x: X) -> X: pass
@overload
def f(self, y: Y) -> Y: pass

class B(A):
@overload
def f(self, x: X) -> X: pass
@overload
def f(self, y: Y) -> Y: pass
@overload
def f(self, z: Z) -> Z: pass
[builtins fixtures/classmethod.pyi]
[out]

[case testTypeTypeOverlapsWithObjectAndType]
from foo import *
[file foo.pyi]
Expand Down