Skip to content

Commit

Permalink
Force-enable strict optional only when checking for unsafe overlaps (#…
Browse files Browse the repository at this point in the history
…5252)

Resolves #5246

Previously, we force-enabled strict optional checks when performing all
overload definition checks. This ended up causing mypy to miss some
errors in definitions when `--no-strict-optional` mode is enabled.

This commit will now force-enable strict optional only when checking if
overloads are unsafely overlapping: we use the existing mode when
checking if one variant completely eclipses another or when checking the
implementation.
  • Loading branch information
Michael0x2a authored and ilevkivskyi committed Jun 21, 2018
1 parent a75fa88 commit bd9e93b
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 74 deletions.
147 changes: 73 additions & 74 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,82 +414,81 @@ def _visit_overloaded_func_def(self, defn: OverloadedFuncDef) -> None:
def check_overlapping_overloads(self, defn: OverloadedFuncDef) -> None:
# At this point we should have set the impl already, and all remaining
# items are decorators
#
# Note: we force mypy to check overload signatures in strict-optional mode
# so we don't incorrectly report errors when a user tries typing an overload
# that happens to have a 'if the argument is None' fallback.
#
# For example, the following is fine in strict-optional mode but would throw
# the unsafe overlap error when strict-optional is disabled:
#
# @overload
# def foo(x: None) -> int: ...
# @overload
# def foo(x: str) -> str: ...
#
# See Python 2's map function for a concrete example of this kind of overload.
with experiments.strict_optional_set(True):
is_descriptor_get = defn.info is not None and defn.name() == "__get__"
for i, item in enumerate(defn.items):
# TODO overloads involving decorators
assert isinstance(item, Decorator)
sig1 = self.function_type(item.func)
assert isinstance(sig1, CallableType)

for j, item2 in enumerate(defn.items[i + 1:]):
assert isinstance(item2, Decorator)
sig2 = self.function_type(item2.func)
assert isinstance(sig2, CallableType)

if not are_argument_counts_overlapping(sig1, sig2):
continue
is_descriptor_get = defn.info is not None and defn.name() == "__get__"
for i, item in enumerate(defn.items):
# TODO overloads involving decorators
assert isinstance(item, Decorator)
sig1 = self.function_type(item.func)
assert isinstance(sig1, CallableType)

for j, item2 in enumerate(defn.items[i + 1:]):
assert isinstance(item2, Decorator)
sig2 = self.function_type(item2.func)
assert isinstance(sig2, CallableType)

if not are_argument_counts_overlapping(sig1, sig2):
continue

if overload_can_never_match(sig1, sig2):
self.msg.overloaded_signature_will_never_match(
i + 1, i + j + 2, item2.func)
elif (not is_descriptor_get
and is_unsafe_overlapping_overload_signatures(sig1, sig2)):
self.msg.overloaded_signatures_overlap(
i + 1, i + j + 2, item.func)

if defn.impl:
if isinstance(defn.impl, FuncDef):
impl_type = defn.impl.type
elif isinstance(defn.impl, Decorator):
impl_type = defn.impl.var.type
else:
assert False, "Impl isn't the right type"
if overload_can_never_match(sig1, sig2):
self.msg.overloaded_signature_will_never_match(
i + 1, i + j + 2, item2.func)
elif not is_descriptor_get:
# Note: we force mypy to check overload signatures in strict-optional mode
# so we don't incorrectly report errors when a user tries typing an overload
# that happens to have a 'if the argument is None' fallback.
#
# For example, the following is fine in strict-optional mode but would throw
# the unsafe overlap error when strict-optional is disabled:
#
# @overload
# def foo(x: None) -> int: ...
# @overload
# def foo(x: str) -> str: ...
#
# See Python 2's map function for a concrete example of this kind of overload.
with experiments.strict_optional_set(True):
if is_unsafe_overlapping_overload_signatures(sig1, sig2):
self.msg.overloaded_signatures_overlap(
i + 1, i + j + 2, item.func)

if defn.impl:
if isinstance(defn.impl, FuncDef):
impl_type = defn.impl.type
elif isinstance(defn.impl, Decorator):
impl_type = defn.impl.var.type
else:
assert False, "Impl isn't the right type"

# This can happen if we've got an overload with a different
# decorator too -- we gave up on the types.
if impl_type is None or isinstance(impl_type, AnyType):
return
assert isinstance(impl_type, CallableType)

# Is the overload alternative's arguments subtypes of the implementation's?
if not is_callable_compatible(impl_type, sig1,
is_compat=is_subtype,
ignore_return=True):
self.msg.overloaded_signatures_arg_specific(i + 1, defn.impl)

# Repeat the same unification process 'is_callable_compatible'
# internally performs so we can examine the return type separately.
if impl_type.variables:
# Note: we set 'ignore_return=True' because 'unify_generic_callable'
# normally checks the arguments and return types with differing variance.
#
# This is normally what we want, but for checking the validity of overload
# implementations, we actually want to use the same variance for both.
#
# TODO: Patch 'is_callable_compatible' and 'unify_generic_callable'?
# somehow so we can customize the variance in all different sorts
# of ways? This would let us infer more constraints, letting us
# infer more precise types.
impl_type = unify_generic_callable(impl_type, sig1, ignore_return=True)

# Is the overload alternative's return type a subtype of the implementation's?
if impl_type is not None and not is_subtype(sig1.ret_type, impl_type.ret_type):
self.msg.overloaded_signatures_ret_specific(i + 1, defn.impl)
# This can happen if we've got an overload with a different
# decorator too -- we gave up on the types.
if impl_type is None or isinstance(impl_type, AnyType):
return
assert isinstance(impl_type, CallableType)

# Is the overload alternative's arguments subtypes of the implementation's?
if not is_callable_compatible(impl_type, sig1,
is_compat=is_subtype,
ignore_return=True):
self.msg.overloaded_signatures_arg_specific(i + 1, defn.impl)

# Repeat the same unification process 'is_callable_compatible'
# internally performs so we can examine the return type separately.
if impl_type.variables:
# Note: we set 'ignore_return=True' because 'unify_generic_callable'
# normally checks the arguments and return types with differing variance.
#
# This is normally what we want, but for checking the validity of overload
# implementations, we actually want to use the same variance for both.
#
# TODO: Patch 'is_callable_compatible' and 'unify_generic_callable'?
# somehow so we can customize the variance in all different sorts
# of ways? This would let us infer more constraints, letting us
# infer more precise types.
impl_type = unify_generic_callable(impl_type, sig1, ignore_return=True)

# Is the overload alternative's return type a subtype of the implementation's?
if impl_type is not None and not is_subtype(sig1.ret_type, impl_type.ret_type):
self.msg.overloaded_signatures_ret_specific(i + 1, defn.impl)

# Here's the scoop about generators and coroutines.
#
Expand Down
46 changes: 46 additions & 0 deletions test-data/unit/check-overloading.test
Original file line number Diff line number Diff line change
Expand Up @@ -3797,3 +3797,49 @@ def relpath(path: str) -> str: ...
@overload
def relpath(path: unicode) -> unicode: ...
[out]

[case testOverloadsWithNoneComingSecondAreAlwaysFlaggedInNoStrictOptional]
# flags: --no-strict-optional
from typing import overload

@overload
def none_first(x: None) -> None: ...
@overload
def none_first(x: int) -> int: ...
def none_first(x: int) -> int:
return x

@overload
def none_second(x: int) -> int: ...
@overload
def none_second(x: None) -> None: ... # E: Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader
def none_second(x: int) -> int:
return x

[case testOverloadsWithNoneComingSecondIsOkInStrictOptional]
# flags: --strict-optional
from typing import overload, Optional

@overload
def none_first(x: None) -> None: ...
@overload
def none_first(x: int) -> int: ...
def none_first(x: Optional[int]) -> Optional[int]:
return x

@overload
def none_second(x: int) -> int: ...
@overload
def none_second(x: None) -> None: ...
def none_second(x: Optional[int]) -> Optional[int]:
return x

@overload
def none_loose_impl(x: None) -> None: ...
@overload
def none_loose_impl(x: int) -> int: ...
def none_loose_impl(x: int) -> int:
return x
[out]
main:22: error: Overloaded function implementation does not accept all possible arguments of signature 1
main:22: error: Overloaded function implementation cannot produce return type of signature 1

0 comments on commit bd9e93b

Please sign in to comment.