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

Unions of TypedDicts don't work as expected #5930

Closed
JukkaL opened this issue Nov 21, 2018 · 1 comment · Fixed by #6560
Closed

Unions of TypedDicts don't work as expected #5930

JukkaL opened this issue Nov 21, 2018 · 1 comment · Fixed by #6560
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-typed-dict topic-union-types

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 21, 2018

Operations on unions of typed dicts seem to use the fallback types, resulting in unexpected types:

from mypy_extensions import TypedDict
from typing import Union

class A(TypedDict):
    x: int
    y: str

class B(TypedDict):
    x: str

d: Union[A, B]
reveal_type(d['x'])  # builtins.object (expected: Union[int, str])
reveal_type(d.get('x'))  # builtins.object (expected: Union[int, str, None])
@grantwwu
Copy link

grantwwu commented Mar 20, 2019

Here is an additional example, noting that you don't need conflicting types to hit this issue:

from mypy_extensions import TypedDict
from typing import Union


class A(TypedDict):
    x: int
    y: str


class B(TypedDict):
    x: int


d: Union[A, B]
reveal_type(d["x"]) # Revealed type is 'builtins.object*' (expected: int)

ilevkivskyi added a commit that referenced this issue Jul 4, 2019
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
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-typed-dict topic-union-types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants