Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
15 changes: 11 additions & 4 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,7 @@ def check_overload_call(self,
erased_targets = None # type: Optional[List[CallableType]]
unioned_result = None # type: Optional[Tuple[Type, Type]]
unioned_errors = None # type: Optional[MessageBuilder]
union_success = False
if any(isinstance(arg, UnionType) and len(arg.relevant_items()) > 1 # "real" union
for arg in arg_types):
erased_targets = self.overload_erased_call_targets(plausible_targets, arg_types,
Expand All @@ -1143,17 +1144,23 @@ def check_overload_call(self,
callable_name=callable_name,
object_type=object_type)
if not unioned_errors.is_errors():
# Success! Stop early.
return unioned_result
# Success! But we need to see maybe normal procedure gives a narrower type.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"need to see maybe normal procedure" -> "need to see if maybe the normal procedure"

union_success = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can simplify this if statement to union_success = not unioned_errors.is_errors(). We'd also need to change the comment above to # Record if we succeeded. Next we need to see if maybe...


# Step 3: If the union math fails, or if there was no union in the argument types,
# we fall back to checking each branch one-by-one.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably tweak this comment -- maybe just something simple like

# Step 3: We try checking each branch one-by-one

?

I don't think it matters too much, as long as we remove the "if the union math fails" bit.

inferred_result = self.infer_overload_return_type(plausible_targets, args, arg_types,
arg_kinds, arg_names, callable_name,
object_type, context, arg_messages)
if inferred_result is not None:
# Success! Stop early.
return inferred_result
# Success! Stop early by returning the best among normal and unioned.
if not union_success:
return inferred_result
else:
assert unioned_result is not None
if is_subtype(inferred_result[0], unioned_result[0]):
return inferred_result
return unioned_result
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, just had another thought -- I think it would be clearer (and a little more efficient) if we added logic to return early if union_success is true somewhere around here. Currently, we return early only if the normal algorithm also succeeds.

(The logic currently does the right thing because step 5 happens to do what we want, but step 4 and down are supposed to be failure cases.)


# Step 4: Failure. At this point, we know there is no match. We fall back to trying
# to find a somewhat plausible overload target using the erased types
Expand Down
14 changes: 14 additions & 0 deletions test-data/unit/check-overloading.test
Original file line number Diff line number Diff line change
Expand Up @@ -3611,6 +3611,20 @@ class Wrapper:

[builtins fixtures/staticmethod.pyi]

[case testUnionMathOverloadingReturnsBestType]
from typing import Union, overload

@overload
def f(x: Union[int, str]) -> int: ...
@overload
def f(x: object) -> object: ...
def f(x):
pass

x: Union[int, str]
reveal_type(f(x)) # E: Revealed type is 'builtins.int'
[out]

[case testOverloadAndSelfTypes]
from typing import overload, Union, TypeVar, Type

Expand Down