8000 Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). by tyralla · Pull Request #14975 · python/mypy · GitHub
[go: up one dir, main page]
Skip to content

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). #14975

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

Closed
wants to merge 10 commits into from
Closed

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943). #14975

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4b1fe1c
Fix type argument inference for overloaded functions with explicit se…
tyralla Mar 29, 2023
f354153
revert first attempt
tyralla Apr 10, 2023
589e0c6
try filtering in `bind_self` instead
tyralla Apr 10, 2023
9c107b3
black
tyralla Apr 10, 2023
b143796
flake
tyralla Apr 10, 2023
6b1573f
Simplify `checker.TypeChecker.bind_and_map_method` and remove "xfail"…
tyralla Apr 14, 2023
f1414f2
Add `testSelfTypeOverrideCompatibilitySelfTypeVar` (not my work: take…
tyralla Apr 14, 2023
6e5476a
ignore_type_vars=False for testing
tyralla Apr 15, 2023
3097ac2
Also filter externally defined self types (preliminary approach)
tyralla Apr 15, 2023
33a1f96
reverse ignore_type_vars=False
tyralla Apr 15, 2023
8000
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
Next Next commit
Fix type argument inference for overloaded functions with explicit se… 
…lf types (Fixes #14943).

When finding no compatible callable right away, try to let method `ConstraintBuilderVisitor.find_matching_overload_item` return the first overloaded function with the suitable self type.

Checked by test case `TestOverloadedMethodWithExplictSelfTypes`.
  • Loading branch information
@tyralla
tyralla committed Mar 29, 2023
commit 4b1fe1c6f9d75a852c666b4110abec6aa0c4552c
11 changes: 8 additions & 3 deletions mypy/constraints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from mypy.argmap import ArgTypeExpander
from mypy.erasetype import erase_typevars
from mypy.maptype import map_instance_to_supertype
from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind
from mypy.nodes import ARG_OPT, ARG_POS, CONTRAVARIANT, COVARIANT, ArgKind, FuncDef
from mypy.types import (
TUPLE_LIKE_INSTANCE_NAMES,
AnyType,
Expand Down Expand Up @@ -1138,8 +1138,13 @@ def find_matching_overload_item(overloaded: Overloaded, template: CallableType)
item, template, is_compat=mypy.subtypes.is_subtype, ignore_return=True
):
return item
# Fall back to the first item if we can't find a match. This is totally arbitrary --
# maybe we should just bail out at this point.
# Try to return the first item with the correct self type (fixes issue 14943).
for item in items:
if isinstance(item.definition, FuncDef) and isinstance(item.definition.type, CallableType):
if item.bound_args and item.definition.type.arg_types:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The definition attribute is only supposed to be used for error messages, and it's not guaranteed to be set. It cannot be reliably used for type checking. It's not serialized, in particular, but there may well be other contexts where it's lost that I'm not aware of.

Also, the bound_args attribute is unfortunately not documented (or the documentation seems incorrect). Again, I'm not sure if we can expect it to be always set.

Here's an alternative approach that might work. When we bind the self type to an overloaded method (in mypy.typeops.bind_self), what about filtering the overload items at that point to those matching the self type (only when explicit self types are used)? This filtering would happen in an earlier stage, and I believe we have access to all the necessary information without using undocumented or inconsistently set attributes. The result of the filtering could be a non-overloaded callable type or an overloaded type with fewer items.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for your feedback. Using two fragile attributes in five new lines - what a negative hit rate...

Your suggestion targets code sections I did not encounter yet. My first naive implementation of your suggestion solved the original issue but broke many other things. So far, I could not find any information to distinguish implicit and explicit self types within bind_self. Is there some obvious way I am missing?

I would take a second, more thorough try within the next few days. Please do not hesitate to fix this yourself if you already have a good solution in mind and if you want that issue fixed before the 1.2 release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried to filter within bind_self as suggested. All tests pass (on my computer). Let's see what Mypy primer says.

I am still not overly happy with my approach for the following reasons:

  1. I added the ignore_type_vars option to is_subtype, which seems like a huge change for such a specific problem to me.
  2. I found no way to distinguish explicit self types from implicit self types.
  3. The result is still always an overloaded type (changing this within bind_self would be simple but requires changing its signature and eventually many code sections that use bind_self).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

3. The result is still always an overloaded type (changing this within bind_self would be simple but requires changing its signature and eventually many code sections that use bind_self).

I played around with returning a single non-overloaded callable for a few minutes. Maybe it's not a good idea because it changes the signatures in error reports and notes. On the other hand, I did not encounter a case where an overloaded type with a single item caused a problem (but I did not actively search for such a problem).

if item.bound_args[0] == item.definition.type.arg_types[0]:
return item
# Give up and just return the first of all items.
return items[0]


Expand Down
39 changes: 39 additions & 0 deletions test-data/unit/check-protocols.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4020,3 +4020,42 @@ class P(Protocol):

[file lib.py]
class C: ...

[case TestOverloadedMethodWithExplictSelfTypes]
from typing import Generic, overload, Protocol, TypeVar, Union

AnyStr = TypeVar("AnyStr", str, bytes)
T_co = TypeVar("T_co", covariant=True)
T_contra = TypeVar("T_contra", contravariant=True)

class SupportsRead(Protocol[T_co]):
def read(self) -> T_co: ...

class SupportsWrite(Protocol[T_contra]):
def write(self, s: T_contra) -> int: ...

class Input(Generic[AnyStr]):
def read(self) -> AnyStr: ...

class Output(Generic[AnyStr]):
@overload
def write(self: Output[str], s: str) -> int: ...
@overload
def write(self: Output[bytes], s: bytes) -> int: ...
def write(self, s: Union[str, bytes]) -> int: ...

def f(src: SupportsRead[AnyStr], dst: SupportsWrite[AnyStr]) -> None: ...

def g1(a: Input[bytes], b: Output[bytes]) -> None:
f(a, b)

def g2(a: Input[bytes], b: Output[bytes]) -> None:
f(a, b)

def g3(a: Input[str], b: Output[bytes]) -> None:
f(a, b) # E: Cannot infer type argument 1 of "f"

def g4(a: Input[bytes], b: Output[str]) -> None:
f(a, b) # E: Cannot infer type argument 1 of "f"

[builtins fixtures/tuple.pyi]
0