mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Incorrect errors caused by boolean narrowing on attributes

Open Dreamsorcerer opened this issue 3 years ago • 6 comments
trafficstars

With mypy 0.930 (compared with 0.910), it now infers the type of self._changed = False as Literal[False] instead of bool. That doesn't bother me too much, but if I add a type annotation as self._changed: bool = False, it still marks the type as Literal[False] and we get type errors in the code about unreachable code.

Code available at: https://github.com/aio-libs/aiohttp-session/pull/651/files

Dreamsorcerer avatar Dec 28 '21 00:12 Dreamsorcerer

I think I've encountered the same issue:

import enum
import os
from typing import Callable, TypeVar, Union, cast

class Undefined(enum.Enum):
    token = object()

_undefined = Undefined.token
T = TypeVar('T')

def default_clean(v: str) -> T:
    return cast(T, v)

def get_env(
    key: str,
    default: Union[str, Undefined] = _undefined,
    *,
    clean: Callable[[str], T] = default_clean,
) -> T:
    key = key.upper()
    raw = os.environ.get('BACKEND_' + key)
    if raw is None:
        raw = os.environ.get('SORNA_' + key)
    if raw is None:
        if default is _undefined:
            raise KeyError(key)
        raw = default
    return clean(raw)

def bool_env(v: str) -> bool:
    v = v.lower()
    if v in ('y', 'yes', 't', 'true', '1'):
        return True
    if v in ('n', 'no', 'f', 'false', '0'):
        return False
    raise ValueError('Unrecognized value of boolean environment variable', v)

def main(
    skip_sslcert_validation: bool = None,
) -> None:
    reveal_type(bool_env)  # "def (v: builtins.str) -> builtins.bool"
    reveal_type(get_env)   # "def [T] (key: builtins.str, default: Union[builtins.str, x-bool.Undefined] =, *, clean: def (builtins.str) -> T`-1 =) -> T`-1"
    x = (
        skip_sslcert_validation
        if skip_sslcert_validation else
        get_env('SKIP_SSLCERT_VALIDATION', 'no', clean=bool_env)  # rejected?
    )


reveal_type(bool_env)  # "def (v: builtins.str) -> builtins.bool"
reveal_type(get_env)   # "def [T] (key: builtins.str, default: Union[builtins.str, x-bool.Undefined] =, *, clean: def (builtins.str) -> T`-1 =) -> T`-1"
get_env('SKIP_SSLCERT_VALIDATION', 'no', clean=bool_env)  # accepted

This code has been accepted in prior mypy versions, now generates Argument "clean" to "get_env" has incompatible type "Callable[[str], bool]"; expected "Callable[[str], Literal[True]]" error.

achimnol avatar Dec 28 '21 07:12 achimnol

These are 2 different bugs, but they're both caused by #10389.


@Dreamsorcerer

Simplified example:

class A:
    def __init__(self) -> None:
        self.x = False
    
def change_x(a: A) -> None:
    a.x = True

def hello() -> None:
    a = A()
    assert not a.x
    change_x(a)
    assert a.x
    print('hello')  # E: Statement is unreachable
    
a = A()
reveal_type(a.x)  # N: Revealed type is "builtins.bool"

mypy is correctly inferring the type of a.x to be bool. The problem is that mypy doesn't realize that change_x changes the value of a.x, so it narrows a.x to Literal[False] after the first assert, and then uses that to assume that the second assert always fails.

This wasn't a problem before 0.930 because mypy didn't narrow booleans to literal values until #10389.

https://github.com/python/mypy/pull/11521#issuecomment-966546317 has some more information about a similar problem involving narrowing enums and some ideas for the fix.


@achimnol

Simplified example:

from typing import TypeVar

T = TypeVar('T')


def identity(x: T) -> T:
    return x

a: bool

b: bool
x = (
    b
    if b else
    identity(a)  # E: Argument 1 to "identity" has incompatible type "bool"; expected "Literal[True]"
)

reveal_type(x)  # N: Revealed type is "Literal[True]"

The value if True in the ternary expression is always True (because x = b when b is True), so mypy assigns the type Literal[True] to the value if True. It then uses that to decide that the type variable should be equal to Literal[True] in this case, causing it to show an error when you pass in a bool.

You can work around this bug by casting b to bool in the if branch or by using an if statement and an explicit bool annotation for x:

if b:
    x: bool = b
else:
    x = identity(a)

pranavrajpal avatar Dec 29 '21 01:12 pranavrajpal

mypy is correctly inferring the type of a.x to be bool. The problem is that mypy doesn't realize that change_x changes the value of a.x, so it narrows a.x to Literal[False] after the first assert, and then uses that to assume that the second assert always fails.

Ah, yes, I see. We assert it is false, then do something, and then assert it is true: https://github.com/aio-libs/aiohttp-session/blob/dependabot/pip/mypy-0.930/tests/test_session_dict.py#L83-L87

Dreamsorcerer avatar Dec 29 '21 12:12 Dreamsorcerer

I think it would probably make sense to not narrow types on attributes (especially once a method on that object is called), otherwise mypy is basically making object-oriented programming impossible...

Dreamsorcerer avatar Dec 29 '21 12:12 Dreamsorcerer

Is there a good workarround for it?

I just have the same problem where bool gets narrowed to Literal[False].

chamber.stop()
reveal_type(chamber.is_operating)
assert not chamber.is_operating

chamber.start()
reveal_type(chamber.is_operating)
assert chamber.is_operating

chamber.stop()
assert not chamber.is_operating
test.py:2: note: Revealed type is "builtins.bool"
test.py:6: note: Revealed type is "Literal[False]"
test.py:9: error: Statement is unreachable  [unreachable]
Found 1 error in 1 file (checked 1 source file)
class Chamber:
    is_operating: bool

    def __init__(self):
        self.is_operating = False

    def start():
        self.is_operating = True

    def stop():
        self.is_operating = False

9y2070m avatar Jan 10 '23 12:01 9y2070m

Cross-posting my workaround from https://github.com/python/mypy/issues/9005#issuecomment-1280985006

One work-around is using no-op assignments whenever there are side-effects, to make mypy forget its learned constraints:

assert foo.status == 'spam'
foo.refresh_from_db()
foo = foo  # <-- Work-around for https://github.com/python/mypy/issues/9005
assert foo.status == 'eggs'
#          ^^^^^^ mypy no longer complains

intgr avatar Jan 10 '23 12:01 intgr