python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.18k stars 2.77k forks source link

Reachable code marked as unreachable in generic class with None type #8871

Open badochov opened 4 years ago

badochov commented 4 years ago
T = TypeVar("T")

class A(Generic[T]):
    def __init__(self, a: T) -> None:
        self.a = a

    def __str__(self) -> str:
        if self.a is None:
            return "Foo"
        return "Bar"

Running mypy on this code with warn_unreachable = True results in error that return "Foo" is unreachable. I wasn't able to find anywhere that NoneType is not correct type for the TypeVar and extending this class with NoneType doesn't result in error as well.

class B(A[None]): ...
beezee commented 4 years ago

T is completely unbound so I personally don't find this surprising. When you say T in this context , you're really saying "within the body of class A, I know nothing about type T, including whether or not it could be None.

I'd expect type of self.a to be Optional[T] if you are performing this check, no?

badochov commented 4 years ago

As you said I don't know whether or not this type T is None or not and so doesn't MyPy so IMO it shouldn't assume that it can't be None. Type of self.a should be T because subclass can set T as None.

beezee commented 4 years ago

I'd rephrase "mypy shouldn't assume that T can't be None" as "mypy should assume that T can be None", which I think is more illustrative of my point here. Unbound type variables represent universal quantification, meaning "for all t in T, where T is the ambient set of all things" - the only thing you can assume about an arbitrary t in the ambient set of all things is that it exists, whereas optionality is a refinement, "for all t in T, where T is the ambient set of all optional things" - for that I'd expect a type of Optional[T].

To try and illustrate further, if you can check T for None, why can't you check it for int? Why not for str? At that point, what you have is type Any, not T.

Does this make sense?

badochov commented 4 years ago

OK I see you point. But then I think that mypy should not enable one to create subclass of generic class with None type.

beezee commented 4 years ago

That would defeat the purpose of subclassing generics.

The whole point is that in the superclass you know nothing about T except for how it relates to other things. Your example doesn't particularly illustrate this because you only reference T in a single covariant position. A more clear demonstration might be

class A(Generic[T]):
    def __init__(self, a: T) -> None:
        self.a = a
    @abstractmethod
    def show(self, t: T) -> str:
        pass
    def __str__(self): str:
       return self.show(self.a)

Now when you subclass and unify T with None - that is precisely where you have knowledge of T, and can provide an implementation of the show method that does what you want.

This is ultimately the benefit of type variables - explicit declaration of what you know about a type and nothing more - the behavior you're looking for is closer to a union, (of which optional is, Option[A] = Union[A, None]) and the biggest union out there is Any (union of all types).

JukkaL commented 4 years ago

I think that this is a false positive (and a potential false negative if mypy doesn't warn about unreachable code).

beezee commented 4 years ago

@JukkaL in what way is it a false positive? Shouldn't I expect to get a typecheck error if I try to pass None to any subclass of A whose T is unified with any type that doesn't admit None as a value? Shouldn't that allow me to implement the body of a function with input type T unified with any type that doesn't admit None as a value with some guarantee that the value won't be None?

JukkaL commented 4 years ago

The code is reachable if T substituted with a type that includes None. T has no bound so it may contain arbitrary values.

Anyway, reachability checks are based on heuristics and there's no one obviously correct answer always

beezee commented 4 years ago

Your point about heuristics makes sense in general - in this case I feel like I'm missing something about why the code should be reachable in a context where T is not bound and has not yet been substituted with any type. Is there something about mypy/python that makes this true? Sorry, not trying to be difficult, just trying to understand.

KotlinIsland commented 3 years ago
T = TypeVar("T")

def bar(a: T) -> None:
    if a is not None:
        print("not None")
    else:
        print("is None")  # Statement is unreachable

bar(None)