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.

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

Jump to bottom

Several plugin hooks don't work on unions #6117

Closed
ilevkivskyi opened this issue Dec 30, 2018 · 2 comments · Fixed by #6560
Closed

Several plugin hooks don't work on unions #6117

ilevkivskyi opened this issue Dec 30, 2018 · 2 comments · Fixed by #6560
Assignees
@ilevkivskyi
Labels
bug mypy got something wrong priority-0-high topic-plugins The plugin API and ideas for new plugins topic-typed-dict topic-union-types

Comments

@ilevkivskyi
Copy link
Member

Some plugin hooks (in particular for TypedDicts) are not called if the underlying "base" type is a union. For example:

from typing import Union
from mypy_extensions import TypedDict

class TDA(TypedDict):
    a: int

class TDB(TypedDict):
    a: int

td: Union[TDA, TDB]

reveal_type(td['a'])  # Revealed type is 'builtins.object*'
reveal_type(td.pop('a'))  # Revealed type is 'builtins.object'
                          # Argument 1 to "pop" of "TypedDict" has incompatible type "str"; expected "NoReturn"

This affects both "builtin" plugins and user defined plugins.
@ilevkivskyi ilevkivskyi added bug mypy got something wrong topic-typed-dict priority-0-high topic-union-types topic-plugins The plugin API and ideas for new plugins labels Dec 30, 2018
@ilevkivskyi ilevkivskyi mentioned this issue Jan 4, 2019
Open
@ilevkivskyi
Copy link
Member Author

It actually looks like the only plugin hook that is broken is get_method_signature_hook(), other hooks seem to work, but this one is still important to fix, I will make a PR now.

@ilevkivskyi ilevkivskyi self-assigned this Feb 25, 2019
@ilevkivskyi
Copy link
Member Author

It is a bit more tricky than I thought (also get_method_hook() may be also broken for unions). I will continue next weekend.

@ilevkivskyi ilevkivskyi mentioned this issue Mar 18, 2019
Merged
ilevkivskyi added a commit that referenced this issue Jul 4, 2019
@ilevkivskyi
Support method plugin hooks on unions (#6560)
72734f2 
Fixes #6117
Fixes #5930

Currently both our plugin method hooks don't work with unions. This PR fixes this with three things:
* Moves a bit of logic from `visit_call_expr_inner()` (which is a long method already) to `check_call_expr_with_callee_type()` (which is a short method).
* Special-cases unions in `check_call_expr_with_callee_type()` (normal method calls) and `check_method_call_by_name()` (dunder/operator method calls).
* Adds some clarifying comments and a docstring.

The week point is interaction with binder, but IMO this is the best we can have for now. I left a comment mentioning that check for overlap should be consistent in two functions.

In general, I don't like special-casing, but I spent several days thinking of other solutions, and it looks like special-casing unions in couple more places is the only reasonable way to fix unions-vs-plugins interactions.

This PR may interfere with #6558 that fixes an "opposite" problem, hopefully they will work together unmodified, so that accessing union of literals on union of typed dicts works. Whatever PR lands second, should add a test for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ilevkivskyi ilevkivskyi

Labels
bug mypy got something wrong priority-0-high topic-plugins The plugin API and ideas for new plugins topic-typed-dict topic-union-types
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support method plugin hooks on unions ilevkivskyi/mypy
1 participant
@ilevkivskyi