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

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

Open
jwodder opened this issue Jan 11, 2022 · 7 comments
Open
Labels
bug mypy got something wrong topic-reachability Detecting unreachable code topic-type-narrowing Conditional type narrowing / binder

Comments

@jwodder
Copy link

jwodder commented Jan 11, 2022

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 48d810d
  • 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 jwodder added the bug mypy got something wrong label Jan 11, 2022
jwodder added a commit to con/tinuous that referenced this issue Jan 11, 2022
@KotlinIsland
Copy link
Contributor

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.

@sobolevn
Copy link
Member

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/

@jwodder
Copy link
Author

jwodder commented Jan 13, 2022

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?

@jbcpollak
Copy link

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.

@erictraut
Copy link

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"
}

@KotlinIsland
Copy link
Contributor

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.

@fohrloop
Copy link

fohrloop commented Mar 25, 2024

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.

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-reachability Detecting unreachable code topic-type-narrowing Conditional type narrowing / binder
Projects
None yet
Development

No branches or pull requests

7 participants