microsoft / pyright

Static Type Checker for Python
Other
13.47k stars 1.48k forks source link

`reportUnnecessaryComparison` false positive #9442

Closed vient closed 1 week ago

vient commented 1 week ago

Describe the bug Pyright reports an unnecessary comparison when variable is modified in another function.

Code or Screenshots

import enum

class State(enum.IntEnum):
    A = 0
    B = 1

state: State = State.A

def state_b() -> None:
    global state
    state = State.B

def f() -> None:
    if state != State.A:
        return
    state_b()
    if state != State.B:  # Condition will always evaluate to True since the types "Literal[State.A]" and "Literal[State.B]" have no overlap
        assert False

in fact, this condition will always evaluate to false.

VS Code extension or command-line Found with Pylance but then reproduced in your playground https://pyright-play.net/?pyrightVersion=1.1.388&pythonVersion=3.13&strict=true&code=JYWwDg9gTgLgBAUwHYFcQCh0GMA2BDAZwLgGUY8YEAKZNAOgEkkYBRVEASgC504%2B4AgnAC8cAAy9%2BAIRFwAjJgLlKXUsoSyyFBHQGYAJggBmcJdoD6AIyoc4AWgB8cAHIQkCHvzgBzHBEt4OKbqknxmlJrqdFIGxnBGNvZOru6e-MAm4RoAhKJalLppXnxQCDAoUEihwRbWHNUZNRG5atrRRcWEBAiwcABigd3oQA

erictraut commented 1 week ago

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

Static type checkers like pyright narrow expressions locally within a function. They do not (and generally cannot) take into account side effects from other execution contexts. Any explicit or implicit call could potentially have such a side effect, so if a static analyzer were to assume that any call or operation potentially modifies an expression's value as a side effect, it would produce many false positive errors and have a dramatic negative impact on usability.

Mutating external state as a side effect of a function call is generally considered an anti-pattern if your goal is to write robust code. I recommend modifying the code to do something like this:

def get_state():
    return state

def update_state() -> State:
    global state
    state = State.B
    return state

def f() -> None:
    state = get_state()
    if state != State.A:
        return
    state = update_state()
    if state != State.B:
        assert False