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

Unsoundness in subclassing Mappings with TypedDicts #5927

Closed
mrkmndz opened this issue Nov 20, 2018 · 1 comment
Closed

Unsoundness in subclassing Mappings with TypedDicts #5927

mrkmndz opened this issue Nov 20, 2018 · 1 comment

Comments

@mrkmndz
Copy link
Contributor

mrkmndz commented Nov 20, 2018

In the course of implementing TypedDicts into Pyre, I came across an issue with how mypy is treating the relationship between TypedDicts and Mappings.
It seems like the implemented semantics are to treat a T1 = TypedDict[ {k_1: v_1, k_2: v:2, ... k_n: v:n} ] as a subclass of T2 = Mapping[str, Union[v_1, v_2, ... v_n]]. However, this causes a problem when combined with the structural subtyping rule that T3 = TypedDict[ {k_1: v_1, k_2: v:2, ... k_n: v:n, k': v'} ] is a subclass of T1, which implies that T3 is a subclass of T2. When you call __getitem__ on a T2 that's actually a T3 with key k', you would unexpectedly get a v'. This can lead to an undetected potential AttributeError as in the following example:

from typing import Mapping
from mypy_extensions import TypedDict

class Y():
    def ya(self)-> int:
        return 7

class Z():
    def za(self) -> int:
        return 8

Foo = TypedDict('Foo', {'visible': Y, 'hidden': Z})

Bar = TypedDict('Bar', {'visible': Y})

def access(x:Mapping[str, Y]) -> None:
    if "hidden" in x:
        x["hidden"].ya()

def hide(x:Bar) -> None:
    access(x)

def f(x: Foo) -> None:
    hide(x)

td: Foo = {'visible': Y(), 'hidden': Z()}
f(td)

This passes mypy typechecking but when run gives an attribute error.

I think we're going to end up having TypedDicts just subclass Mapping[str, Any] for this reason.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 20, 2018

Yeah, we are aware of the problem -- and I actually started working on a fix earlier today :-). I'm trying out what happens if make all typed dicts have Mapping[str, object] as a supertype. This would be safer, but it may require users to add some extra casts.

JukkaL added a commit that referenced this issue Nov 21, 2018
The previous fallback mapping item type was join of the value
types, which is unsafe because of structural subtyping.

Fixes #5927.
JukkaL added a commit that referenced this issue Nov 21, 2018
The previous fallback mapping item type was join of the value
types, which is unsafe because of structural subtyping.

Fixes #5927.
JukkaL added a commit that referenced this issue Nov 22, 2018
The previous fallback mapping item type was join of the value
types, which is unsafe because of structural subtyping.

Fixes #5927.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants