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

(🐞) Inconsistent reachability depending if module level or not #12409

Open
KotlinIsland opened this issue Mar 21, 2022 · 1 comment
Open
Labels
bug mypy got something wrong topic-reachability Detecting unreachable code

Comments

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Mar 21, 2022

Really strange behavior here, when this statement is on the module level it creates a silently unreachable branch, but when in a scope it doesn't create unreachable

import sys


def foo() -> None:
    assert sys.platform == "AMONGUS"
    a # error, reachable ❌

cond: bool
if cond:
    assert sys.platform == "AMONGUS"
    a # error, reachable ❌

assert sys.platform == "AMONGUS"
a  # no error, silent unreachable ✅

playground

Related #11438
Related #11361

@KotlinIsland KotlinIsland added the bug mypy got something wrong label Mar 21, 2022
@JelleZijlstra JelleZijlstra added the topic-reachability Detecting unreachable code label Mar 21, 2022
@ikonst
Copy link
Contributor

ikonst commented Jun 12, 2023

We give special treatment to top-level assert statements here. This was discussed in #5308 and implemented in #5894.

The term "unreachable" is overloaded and refers to either:

  • code that cannot be soundly reached
    if False:
      a = 42  # E: Statement is unreachable
    if True:
      raise Exception
      a = 42  # E: Statement is unreachable
    assert False
    a = 42  # E: Statement is unreachable
  • code unreachable on current version/platform
    if sys.version_info < (3,):
       a = 42
    if sys.platform == 'foobar':
       a = 42
    assert sys.platform == 'foobar'
    a = 42

Maybe it'd be good to refer to the latter as something different than "reachability"

So, except for this hack, which is intended to reduce the indent in this case

 import sys

-if sys.platform == 'windows':
-  <rest of file>
+assert sys.platform == 'windows'
+<rest of file>

we don't treat platform/version checks as reachability cues at all (otherwise you'd have to play error whack-a-mole, since if your code has an if-else for two platforms, one would always be unreachable).

💡 Here's an idea: The #5894 feature is intended to designate an entire module as unfit on other platforms. One clearly has to import sys before this assertion, but it's probably not needed to execute any other statements except for imports, so how about:

  • Continue working as-is if it appears after import statements only.
  • If other statements were seen before it, then it'll be an error, e.g.
import sys

def f() -> None: ...

assert sys.platform == 'windows'  # E: Module-level platform assertions must appear at the beginning of the module (before other statements)

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
Projects
None yet
Development

No branches or pull requests

3 participants