Skip to content
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.

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

Fix --strict-equality crash for instances of a class generic over a ParamSpec #14792

Merged
merged 5 commits into from
Feb 27, 2023
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
29 changes: 28 additions & 1 deletion mypy/meet.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from itertools import chain
from typing import Callable

from mypy import join
Expand Down Expand Up @@ -342,7 +343,15 @@ def _is_overlapping_types(left: Type, right: Type) -> bool:
left_possible = get_possible_variants(left)
right_possible = get_possible_variants(right)

# We start by checking multi-variant types like Unions first. We also perform
# First handle a special case: comparing a `Parameters` to a `ParamSpecType`.
# This should always be considered an overlapping equality check.
# This needs to be done before we move on to other TypeVarLike comparisons.
if (isinstance(left, Parameters) and isinstance(right, ParamSpecType)) or (
isinstance(left, ParamSpecType) and isinstance(right, Parameters)
):
return True

# Now move on to checking multi-variant types like Unions. We also perform
# the same logic if either type happens to be a TypeVar/ParamSpec/TypeVarTuple.
#
# Handling the TypeVarLikes now lets us simulate having them bind to the corresponding
Expand Down Expand Up @@ -451,6 +460,24 @@ def _type_object_overlap(left: Type, right: Type) -> bool:
elif isinstance(right, CallableType):
right = right.fallback

if isinstance(left, Parameters):
if not isinstance(right, Parameters):
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
return False
if len(left.arg_types) == len(right.arg_types):
return all(
_is_overlapping_types(left_arg, right_arg)
for left_arg, right_arg in zip(left.arg_types, right.arg_types)
)
if not any(
isinstance(arg, TypeVarLikeType) for arg in chain(left.arg_types, right.arg_types)
):
return False
# TODO: Is this sound?
return True
if isinstance(right, Parameters):
assert not isinstance(left, (Parameters, TypeVarLikeType))
return False

if isinstance(left, LiteralType) and isinstance(right, LiteralType):
if left.value == right.value:
# If values are the same, we still need to check if fallbacks are overlapping,
Expand Down
46 changes: 46 additions & 0 deletions test-data/unit/pythoneval.test
Original file line number Diff line number Diff line change
Expand Up @@ -1924,3 +1924,49 @@ _testStarUnpackNestedUnderscore.py:10: error: List item 0 has incompatible type
_testStarUnpackNestedUnderscore.py:10: error: List item 1 has incompatible type "int"; expected "str"
_testStarUnpackNestedUnderscore.py:11: note: Revealed type is "builtins.list[builtins.str]"
_testStarUnpackNestedUnderscore.py:16: note: Revealed type is "builtins.list[builtins.object]"

[case testStrictEqualitywithParamSpec]
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
# flags: --strict-equality
from typing import Generic
from typing_extensions import Concatenate, ParamSpec

P = ParamSpec("P")

class Foo(Generic[P]): ...
class Bar(Generic[P]): ...

def bad1(foo1: Foo[[int]], foo2: Foo[[str]]) -> bool:
return foo1 == foo2
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 all of the comparisons here are technically overlapping (no error should be generated), since the actual underlying callable object could be something that accepts, say, *args, **kwargs, and it is thus compatible with all possible parameters.

Copy link
Member Author

@AlexWaygood AlexWaygood Feb 27, 2023

Choose a reason for hiding this comment

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

Following e1ff0ed, mypy now only emits an error for the case where the classes are in fact different. I've kept all the test cases for now, but let me know if you'd like some to be removed. They test distinct scenarios that could happen in user code, but they no longer test distinct code paths.


def bad2(foo1: Foo[[int, str]], foo2: Foo[[int, bytes]]) -> bool:
return foo1 == foo2

def bad3(foo1: Foo[[int]], foo2: Foo[[int, int]]) -> bool:
return foo1 == foo2

def bad4(foo: Foo[[int]], bar: Bar[[int]]) -> bool:
return foo == bar

def good1(foo1: Foo[[int]], foo2: Foo[[int]]) -> bool:
return foo1 == foo2

def good2(foo1: Foo[[int]], foo2: Foo[[bool]]) -> bool:
return foo1 == foo2
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved

def good3(foo1: Foo[[int, int]], foo2: Foo[[bool, bool]]) -> bool:
return foo1 == foo2

def good4(foo1: Foo[[int]], foo2: Foo[P], *args: P.args, **kwargs: P.kwargs) -> bool:
return foo1 == foo2

def good5(foo1: Foo[P], foo2: Foo[[int, str, bytes]], *args: P.args, **kwargs: P.kwargs) -> bool:
return foo1 == foo2

def good6(foo1: Foo[Concatenate[int, P]], foo2: Foo[[int, str, bytes]], *args: P.args, **kwargs: P.kwargs) -> bool:
return foo1 == foo2

[out]
_testStrictEqualitywithParamSpec.py:11: error: Non-overlapping equality check (left operand type: "Foo[[int]]", right operand type: "Foo[[str]]")
_testStrictEqualitywithParamSpec.py:14: error: Non-overlapping equality check (left operand type: "Foo[[int, str]]", right operand type: "Foo[[int, bytes]]")
_testStrictEqualitywithParamSpec.py:17: error: Non-overlapping equality check (left operand type: "Foo[[int]]", right operand type: "Foo[[int, int]]")
_testStrictEqualitywithParamSpec.py:20: error: Non-overlapping equality check (left operand type: "Foo[[int]]", right operand type: "Bar[[int]]")