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

Conversation

AlexWaygood
Copy link
Member

Fixes #14783.

Running mypy on this snippet of code currently causes a crash if you have the --strict-equality option enabled:

from typing import Generic, ParamSpec

P = ParamSpec("P")

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

def checker(foo1: Foo[[int]], foo2: Foo[[str]]) -> None:
    foo1 == foo2

This is because the overlapping-equality logic in meet.py currently does not account for the fact that left and right might both be instances of mypy.types.Parameters, leading to this assertion being tripped:

assert type(left) != type(right), f"{type(left)} vs {type(right)}"

This PR attempts to add the necessary logic to meet.py to handle instances of mypy.types.Parameters.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I think that this can be simplified a bit, but otherwise looks good.

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.

mypy/meet.py Outdated Show resolved Hide resolved
test-data/unit/pythoneval.test Show resolved Hide resolved
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JukkaL JukkaL merged commit 1853222 into python:master Feb 27, 2023
@AlexWaygood AlexWaygood deleted the paramspec-strict-equality branch February 27, 2023 18:24
koogoro pushed a commit that referenced this pull request Mar 2, 2023
… `ParamSpec` (#14792)

Fixes #14783.

Running mypy on this snippet of code currently causes a crash if you
have the `--strict-equality` option enabled:

```python
from typing import Generic, ParamSpec

P = ParamSpec("P")

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

def checker(foo1: Foo[[int]], foo2: Foo[[str]]) -> None:
    foo1 == foo2
```

This is because the overlapping-equality logic in `meet.py` currently
does not account for the fact that `left` and `right` might both be
instances of `mypy.types.Parameters`, leading to this assertion being
tripped:


https://github.com/python/mypy/blob/800e8ffdf17de9fc641fefff46389a940f147eef/mypy/meet.py#L519

This PR attempts to add the necessary logic to `meet.py` to handle
instances of `mypy.types.Parameters`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on a project using lots of generics
2 participants