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

False positive "Statement is unreachable" when checking if typevar is None #18126

Open
Cnoor0171 opened this issue Nov 7, 2024 · 1 comment · May be fixed by #18138
Open

False positive "Statement is unreachable" when checking if typevar is None #18126

Cnoor0171 opened this issue Nov 7, 2024 · 1 comment · May be fixed by #18138
Labels
bug mypy got something wrong topic-overlap Overlapping equality check topic-type-variables

Comments

@Cnoor0171
Copy link

Cnoor0171 commented Nov 7, 2024

Bug Report

Mypy doesn't seem to like it when you check if an argument with typevar type is checked to see if it is None. It incorrectly reports error: Statement is unreachable [unreachable]

To Reproduce

https://mypy-play.net/?mypy=latest&python=3.12&flags=strict%2Cwarn-unreachable&gist=ceb8193f19f8b53a55dea70fad802469

import typing as t

T = t.TypeVar("T")

def foo(arg: T) -> None:
    if arg is None:
        return None
    # Do something with arg
    return None


foo(None) ## Allowed
foo(123)

Run mypy with --warn-unreachable flag

Expected Behavior

No errors

Actual Behavior

$ mypy .vscode/scratch.py
.vscode/scratch.py: note: In function "foo":
.vscode/scratch.py:9: error: Statement is unreachable  [unreachable]
Found 1 error in 1 file (checked 1 source file)

The error seems to only happen when checking if arg is None, but not with other types. For example, I tired changing if arg is None: line to the following and these were the results.

  1. if arg is True: -> no errors
  2. if isinstance(arg, int): -> no errors
  3. if isinstance(arg, type(None)): -> unreachable error
  4. if arg == None: -> No error

Also, changing the type to T | None removes the error as well.

Your Environment

  • Mypy version used: mypy 1.13.0 (compiled: yes)
  • Mypy command-line flags:
  • Mypy configuration options from mypy.ini (and other config files): warn_unreachable = True
  • Python version used: 3.12
@Cnoor0171 Cnoor0171 added the bug mypy got something wrong label Nov 7, 2024
@Cnoor0171 Cnoor0171 changed the title False positive " Statement is unreachable" when checking if typevar is None False positive "Statement is unreachable" when checking if typevar is None Nov 7, 2024
@sterliakov
Copy link
Contributor

sterliakov commented Nov 10, 2024

There's something wrong with this logic. When checking for narrowing in if clause by is None, we explicitly request to not consider bare TypeVar overlapping with None. The obvious decision is to stop asking for that, but... There's a test asserting this behaviour (introduced in #5476, and if clause is indeed unreachable - using a non-trivial body gives an [unreachable] diagnostic with --warn-unreachable):

[case testNoneCheckDoesNotNarrowWhenUsingTypeVars]
# Note: this test (and the following one) are testing checker.conditional_type_map:
# if you set the 'prohibit_none_typevar_overlap' keyword argument to False when calling
# 'is_overlapping_types', the binder will incorrectly infer that 'out' has a type of
# Union[T, None] after the if statement.
from typing import TypeVar
T = TypeVar('T')
def foo(x: T) -> T:
out = None
out = x
if out is None:
pass
return out
[builtins fixtures/isinstance.pyi]

I don't think that this approach is sound: narrowing a lone TypeVar is a common problem, it shouldn't erroneously mark code as unreachable.

binder builds a Union of types after all non-terminating branches when exiting a frame, so here it produces None | T (None from the if clause since it's reachable, and T from a pass-through).

I tried to modify the binder to keep track of whether every type in frame was updated via an isinstance check or by assignment, and this seems to work as expected. I'm not sure, however, that this approach covers all corner cases. We already know that TypeVar + isinstance causes a lot of pain (#9424 and many others), so this probably won't make things marginally worse.

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-overlap Overlapping equality check topic-type-variables
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants