mypy icon indicating copy to clipboard operation
mypy copied to clipboard

warn-unreachable false-positive when method updates attribute after assert

Open jwodder opened this issue 3 years ago • 12 comments

Consider the following code:

from dataclasses import dataclass


@dataclass
class Foo:
    value: int
    modified: bool = False

    def update(self) -> None:
        self.value += 1
        self.modified = True


foo = Foo(42)
assert not foo.modified
foo.update()
assert foo.modified
print("Reached")

When this is run, the print("Reached") line is reached, but mypy doesn't seem to realize that. Running mypy --warn-unreachable --pretty on the above code gives:

unreachable03.py:18: error: Statement is unreachable
    print("Reached")
    ^
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: both v0.931 and the GitHub version as of commit 48d810d5ed25f898d4f220a7476b6b267c289b9a
  • Mypy command-line flags: --warn-unreachable --pretty
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.9.9
  • Operating system and version: macOS 11.6

jwodder avatar Jan 11 '22 16:01 jwodder

In this scenario mypy pretty much assumes everything is immutable and only takes into consideration explicit mutation, not side effects from any function calls.

This is because otherwise most narrowing would be invalidated instantly.

And to check each function to see if and what variables it mutates would be extremely slow and complicated.

KotlinIsland avatar Jan 13 '22 05:01 KotlinIsland

Yes, I agree with @KotlinIsland. This is intentional. You can use assert wrapper to not trigger bool -> Literal[True] inference. For example: https://github.com/sobolevn/safe-assert/

sobolevn avatar Jan 13 '22 10:01 sobolevn

Methods that mutate attributes are surely a common occurrence, and I feel that assuming that they don't exist for the purpose of detecting unreachable code is the wrong thing to do, as it would lead to too many false positives. I don't know what sort of narrowing would be invalidated by not assuming everything is immutable, but could it be possible to only "invalidate" it for warn-unreachable purposes?

jwodder avatar Jan 13 '22 13:01 jwodder

I agree with @jwodder - this is common and its a bad assumption variables can't change. Typescript for example also has type narrowing and does not have this issue: https://www.typescriptlang.org/docs/handbook/2/narrowing.html

TypeScript follows possible paths of execution that our programs can take to analyze the most specific possible type of a value at a given position. It looks at these special checks (called type guards) and assignments, and the process of refining types to more specific types than declared is called narrowing.

jbcpollak avatar Jan 19 '22 16:01 jbcpollak

TypeScript also assumes that variables are not changed "behind the back" of the type checker in another execution scope. Without this assumption, it wouldn't be feasible to apply type narrowing to any member access expressions.

class Foo {
    bar: number | string = 0;

    setBarToString() {
        this.bar = '';
    }
}

function test() {
    const f = new Foo();
    f.bar = 1;

    const val1 = f.bar; // Type of "val1" is revealed as "number"

    f.setBarToString();

    const val2 = f.bar; // Type of "val2" is still revealed as "number"
}

erictraut avatar Jan 19 '22 17:01 erictraut

Kotlin handles this correctly by only narrowing val properties, and leaving var properties un-narrowed. Kotlin also narrows on so called 'scope functions'(the last part of this example)

class Foo(
    val foo: Any,
    var bar: Any,
)

fun test() {
    val f = Foo(1, 1);

    if (f.foo is String) {
        val v2 = f.foo // Type of "v2" is revealed as "String"
    }
    if (f.bar is String) {
        val v2 = f.bar // Type of "v2" is still revealed as "Any"
    }
    (f.bar as? String)?.let {
        val v2 = it // Type of "v2" is now revealed as "String"
    }
}

This way Kotlin is able to maintain perfect correctness while still achieving useful narrowing.

KotlinIsland avatar Jan 19 '22 22:01 KotlinIsland

I asked about this in SO where user sytech introduced a workaround using type guard functions using TypeGuard. They're essentially functions that must return a boolean, and if they return True the first positional argument (or first after self in methods) is guaranteed to have type TYPE if the function return type annotation is TypeGuard[TYPE].

I wrote two helper functions, object_is and is_instance. You then would use them with assert statements. For example:

  • assert foo.modified -> assert object_is(foo.modified, True)
  • assert foo.bar is None -> assert object_is(foo.bar, None)
  • assert isintance(foo.baz, int) -> assert is_instance(foo.baz, int)

Full example:

import sys
from typing import Type, TypeVar

if sys.version_info < (3, 10):
    from typing_extensions import TypeGuard
else:
    from typing import TypeGuard

from dataclasses import dataclass


@dataclass
class Foo:
    value: int
    modified: bool = False

    def update(self) -> None:
        self.value += 1
        self.modified = True


_T = TypeVar("_T")


def object_is(obj, value: _T) -> TypeGuard[_T]:
    return obj is value


def is_instance(val, klass: Type[_T]) -> TypeGuard[_T]:
    return isinstance(val, klass)


foo = Foo(42)
assert object_is(foo.modified, False)
foo.update()
assert object_is(foo.modified, True)
print("Reached")

This uses typing-extensions to make the solution to work on older versions, as TypeGuard was introduced in Python 3.10. I tested this with mypy 1.9.0 and it works well on Python 3.7 to 3.12.

fohrloop avatar Mar 25 '24 21:03 fohrloop

This problem continues to plague otherwise valid code. Take the following reproducer (which even uses @property, something commonly used to indicate "this variable access runs code"):

class Foo:
    @property
    def bool_prop(self) -> bool:
        pass


f = Foo()
assert f.bool_prop
assert not f.bool_prop
x = 0
$ mypy mypy_bug.py
mypy_bug.py: note: At top level:
mypy_bug.py:19:1: error: Statement is unreachable  [unreachable]
    x = 0
    ^~~~~
Found 1 error in 1 file (checked 1 source file)

Mypy's decision to not address this on the grounds that type narrowing would be broken is strange to say the least.

Python type checking already relies entirely on promises by the programmer. If my code says def foo() -> Bar, then mypy must believe that is true. In fact, if it is not, I can even lie to mypy:

def foo() -> Bar:
    return typing.cast(Bar, 12345)

And mypy is required to believe me! To now have mypy turn around and say "You have typed your function one way, but I can't take your word for it" is counterintuitive.

Also it seems that with this feature mypy is now attempting to prove the value of certain variables. This is not (IMO) within the purview of a type checker. In these cases the types are all correct. It is not up to mypy to also decide if the values are correct, especially if it cannot definitively prove they are wrong.

Jacobfaib avatar Mar 25 '25 18:03 Jacobfaib

mypy is very unlikely to make any changes here.

If you want an easy way to reset what mypy thinks it knows about the attributes of a variable, you can do foo = foo or one of several other workarounds.

hauntsaninja avatar Mar 26 '25 00:03 hauntsaninja

@hauntsaninja inserting foo.modified = foo.modified before the assert foo.modified doesn't help (same error: unreachable). safe_assert, object_is, operator.is_ worked in my case.

zed avatar Mar 28 '25 15:03 zed

I said foo = foo

hauntsaninja avatar Mar 28 '25 20:03 hauntsaninja

Interestingly enough changing from assert not foo.modified to assert foo.modified == False fixes it as well.

I don't think it's well behaved to assume that a method call on an object (foo.update() in this case) can't change its properties (foo.modified in this case).

Tested with mypy 1.15 and 1.16.

rnestler avatar Jun 05 '25 15:06 rnestler