Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect errors caused by boolean narrowing on attributes #11853

Open
Dreamsorcerer opened this issue Dec 28, 2021 · 6 comments
Open

Incorrect errors caused by boolean narrowing on attributes #11853

Dreamsorcerer opened this issue Dec 28, 2021 · 6 comments
Labels
bug mypy got something wrong topic-type-narrowing Conditional type narrowing / binder

Comments

@Dreamsorcerer
Copy link
Contributor

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 Dreamsorcerer added the bug mypy got something wrong label Dec 28, 2021
@achimnol
Copy link
Contributor

achimnol commented Dec 28, 2021

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.

@pranavrajpal
Copy link
Contributor

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.

#11521 (comment) 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)

achimnol added a commit to lablup/backend.ai-client-py that referenced this issue Dec 29, 2021
achimnol added a commit to lablup/backend.ai-client-py that referenced this issue Dec 29, 2021
* fix: Workaround mypy's bool narrowing bug with ternery operator
  - Thanks to @pranavrajpal!
  (python/mypy#11853 (comment))
achimnol added a commit to lablup/backend.ai-client-py that referenced this issue Dec 29, 2021
* fix: Workaround mypy's bool narrowing bug with ternery operator
  - Thanks to @pranavrajpal!
  (python/mypy#11853 (comment))

Backported-From: main (22.03)
Backported-To: 21.09
achimnol added a commit to lablup/backend.ai-client-py that referenced this issue Dec 29, 2021
* fix: Workaround mypy's bool narrowing bug with ternery operator
  - Thanks to @pranavrajpal!
  (python/mypy#11853 (comment))

Backported-From: main (22.03)
Backported-To: 21.03
@Dreamsorcerer
Copy link
Contributor Author

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
Copy link
Contributor Author

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...

@9y2070m
Copy link

9y2070m commented Jan 10, 2023

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

@intgr
Copy link
Contributor

intgr commented Jan 10, 2023

Cross-posting my workaround from #9005 (comment)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

No branches or pull requests

6 participants