microsoft / pyright

Static Type Checker for Python
Other
13.04k stars 1.39k forks source link

False Negative regarding unreachable code when if condition is based on a variable set in a nested function #8382

Closed sassanh closed 1 month ago

sassanh commented 1 month ago

Describe the bug Here pyright reports logger.info is unreachable and does not consider some_variable can at least potentially be set in some_method.

Code or Screenshots

def _scope() -> None:
    some_variable: None | bool = None

    def some_method() -> None:
        nonlocal some_variable
        some_variable = True

    some_method()
    if some_variable:
        logger.info('Reachable code')
erictraut commented 1 month ago

Pyright is working as intended here, so this isn't a bug.

Static type checkers like pyright, mypy, and the TypeScript compiler all assume for purposes of local type narrowing that function calls do not have side effects on expressions that include outer-scoped variables. This is for practical reasons. If every function call invalidated all locally-narrowed types, it would be extremely cumbersome.

I'd consider the code in your example an antipattern and would avoid using this approach. If you prefer not to modify your code, you'll need to live with the consequences of type narrowing (e.g. ignore the unreachable code indication) or find some way to defeat or override the type narrowing logic in static type checkers.

sassanh commented 1 month ago

@erictraut Thanks for your detailed explanation! Now lets assume this simpler code:

def _scope() -> None:
    some_variable: bool = False

    if some_variable:
        logger.info('Reachable code')

Shouldn't my explicit type hint make some_variable of type bool instead of Literal[False] and avoid the unreachable code hint? The part that is most confusing for me is if I remove : bool, while pyright reports the type of some_variable still as Literal[False], the unreachable code hint is gone, I think it is inconsistent behavior.

erictraut commented 1 month ago

You're confusing the concept of a "type declaration" with the local type of a symbol. The bool type annotation in your example simply declares that the symbol some_variable can be assigned a value that is assignable to the type bool. It is possible to assign narrower types like Literal[False]. When you perform an assignment, the type of the symbol is locally narrowed to the assigned type. For example:

from random import random

# This declaration tells the type checker the upper bound
# of the types that are allowed to be assigned to some_variable
some_variable: float | str

some_variable = random()
reveal_type(some_variable) # float

some_variable = str(1.0)
reveal_type(some_variable) # str

some_variable = "x"
reveal_type(some_variable) # Literal["x"]

For more details about type declarations and assignability, refer to this documentation. For details about type narrowing, refer to this documentation.

sassanh commented 1 month ago

@erictraut OK, so instead of declaring types, we are declaring upper bound of the types that are allowed to be assigned, which makes sense for Python as it is not strongly typed and needs to extract as much typing information as possible, I hope I'm understanding it correctly.

But why when I remove : bool it doesn't show the unreachable code hint anymore? Both of these code snippets print the same thing: Runtime type is 'bool'

from typing import reveal_type

some_variable: bool = False

reveal_type(some_variable)

if some_variable:
    print(some_variable)
from typing import reveal_type

some_variable = False

reveal_type(some_variable)

if some_variable:
    print(some_variable)

Yet the first one show the unreachable code hint, and the second one doesn't.