microsoft / pyright

Static Type Checker for Python
Other
13.38k stars 1.46k forks source link

`reportUnnecessaryIsinstance` for calls that always return false #8961

Closed BarrensZeppelin closed 1 month ago

BarrensZeppelin commented 1 month ago

Is your feature request related to a problem? Please describe. I recently changed some type aliases in a statically typed code base. In some cases I use isinstance checks to narrow unions. When I changed the type aliases, I had to manually inspect all these isinstance calls, as the type checker does not flag calls that statically can never succeed.

Describe the solution you’d like Like an isinstance call that always returns true, a call that always returns false indicates a likely bug. It would be nice if Pyright flagged such calls.

erictraut commented 1 month ago

Can you provide a concrete example? I think you'll find that most cases that you might initially think "can never succeed" actually can under certain circumstances. For example:

class A: ...
class B: ...

def func(x: A):
    if isinstance(x, B):
       # Can this ever succeed?
       print(x)

class C(A, B): ...
func(C())

Unless you're in the habit of adding @final to all of your class definitions, such a feature probably wouldn't be of much use to you.

BarrensZeppelin commented 1 month ago

I do add @final annotations to my classes. They're cheap to add and easy to remove when needed. :slightly_smiling_face:

This feature would also be useful for built-in types. Consider an example like:

# Refactored from
# type MyUnion = int | str
# to
type MyUnion = tuple[int, int] | str

def f(x: MyUnion):
    if isinstance(x, int):
        ...  # Can never succeed due to instance lay-out conflicts
erictraut commented 1 month ago

As requested, I've extended the reportUnnecessaryIsinstance check to report cases where an isinstance or issubclass call is statically determined to always return False. This typically applies only in cases where both the variable type and the filter class (the second argument) are marked @final.

Instance layout conflicts for certain builtin types are not generally computable by static type checkers. This condition cannot be determined from the information provided in typeshed stubs, and I'm reluctant to hard-code that information. So that doesn't apply in this case — at least for now.

BarrensZeppelin commented 1 month ago

Very nice, thanks!

Yeah, I guess that it's a CPython implementation detail. Makes sense. 👍

erictraut commented 1 month ago

This is addressed in pyright 1.1.381.