isinstance(x, SomeClass) doesn't imply x is not None

[dnaaun]

I think this is a bug, but I wasn't sure so I started a discussion instead of an issue.

As far as I know, None is the only Python object that satisfies isinstance(x, object) and x is None. So asserting isinstance(x, SomeClass) where SomeClass is not object should automatically imply that x is not None.¹

Here's a minimal reproduction:

from typing import Optional, Union


def func(arg1: Union[str, int]) -> None:
    if isinstance(arg1, str):
        if arg1 is None:
            x = 1
        else:
            x = 1000
        reveal_type(x) # reveals Literal[1, 1000]. I believe it should be Literal[1000].

If this seems obscure/unlikely to happen in real life, that's because it's my attempt to distill a real life scenario into a minimal repro. The real life situation is more like:

from typing import Optional, Union
import random
from library_without_types import NoneExistent # type: ignore


def func(arg1: Union[str, NoneExistent]) -> None:
    var1: Optional[str] = "some string" if random.random() > 0.5 else None
    if isinstance(arg1, str):
        if var1 is None:
            var1 = arg1
        reveal_type(var1) # reveals str | None, I think it should be str

That NonExistent is actually the BeautifulSoup class from the bs4 library, but I don't think that matters.

---

¹ I suppose it's possible that classes override __instancecheck__, but I think pyright doesn't consider __instancecheck__ at all.

[erictraut]

In the first code snippet, you have annotated arg1 with the type annotation Union[str, int]. That means it must either be a str or an int. It cannot be a None. Therefore the conditional statement if arg1 is None is nonsensical in this context, and pyright narrows the type of arg1 to Never within the if branch.

In the second code snippet, pyright will do the right thing if it knows the type of NoneExistent. However, it has no knowledge of this type because you've imported it from a "library_without_types". Without this type knowledge, it has no way of knowing whether it derives from str. It conservatively assume that it might, so the isinstance type narrowing logic doesn't narrow the type of arg1. The type is left as str | Unknown.

If I replace the import with a minimal definition for NoneExistent, the type checker produces the results that you expect.

from typing import Optional, Union

class NoneExistent:
    pass

def func(arg1: Union[str, NoneExistent]) -> None:
    if isinstance(arg1, str):
        var1: Optional[str] = None
        if var1 is None:
            var1 = arg1
        reveal_type(var1)  # reveals str

This is a good demonstration of why it's so important to have good type information for the libraries you import.

[dnaaun]

Thanks for the quick reply, and for correcting my mistaken attempt at distilling my problem into a minimal repro.

This is a good demonstration of why it's so important to have good type information for the libraries you import.

Agreed that a well annotated third-party library is the cure for my current typing woe.

However, I'm not sure that the issue that I indicated in the title is solved. Check the following out:

from typing import Optional, Union

def func(arg1: Optional[str]) -> None:
    if isinstance(arg1, str):
        reveal_type(arg1) # reveals str, makes sense.
        if arg1 is None:
            x = 1
        else:
            x = 1000
        reveal_type(x) # reveals Literal[1, 1000]. I believe it should be Literal[1000].

Again, I agree that this is not relevant to my actual issue, but I just thought I might as welll point it out in the spirit of contribution.

[erictraut]

Thanks for the clarification. This is expected behavior.

At the risk of providing more information than you're probably interested in... Pyright builds a code flow graph early in its analysis (in the "binding" stage). This occurs prior to knowing the types of any symbols in the code. It uses the code flow graph to determine those types in a later phase.

Type analysis — and in particular, type narrowing — does not affect the code flow graph. Even though pyright is able to determine later that arg1 is narrowed to type Never within the "if" block, the code flow graph still indicates that x can be assigned either 1 or 1000.

We have added a few specific cases where type information can influence the code flow graph by introducing "gates" in the graph, but these are very expensive to evaluate because they create dependencies (sometimes long dependency chains) between different symbols.

Since the code in your example above is nonsensical to begin with (arg1 can never be None in this case because it's already known to be an instance of str), it wouldn't make sense to add special (expensive) logic to the type analyzer to handle this case.

I've thought about adding a diagnostic check that reports nonsensical conditional statements (like if arg1 is None in the example above). I'll have to give that more thought to see if I can come up with a general rule that wouldn't produce false positives.

[dnaaun]

Thanks for detailed explanation.

a diagnostic check that reports nonsensical conditional statements

That would indeed be useful. Even without this feature, I'm yet to come across a type checker that has the combination of speed, stability, ease-of-install and ease-of-use, and feature completeness (ie, all the PEPs, recursive type aliases) that pyright has. You've done a great job.