mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Consistently avoid type-checking unreachable code

Open ikonst opened this issue 2 years ago • 5 comments

  • On module-level, now we'll skip remaining statements once unreachable. This brings the behavior in line with function-level behavior.
  • For module and function code, if --warn-unreachable is enabled, we'll emit an error, just once, on the first unreachable statement that's not a no-op statement. Previously a no-op statement would not have the "Unreachable statement" error, but the subsequent statements did not have the error either, e.g.
     raise Exception
     assert False  # no error since it's a "no-op statement"
    -foo = 42
    +foo = 42  # E: Unreachable statement
     spam = "ham"  # no error since we warn just once
    

ikonst avatar Jun 07 '23 04:06 ikonst

👋 @KotlinIsland and @sobolevn (re: #11361)

ikonst avatar Jun 09 '23 18:06 ikonst

Thinking about it more, it's strange that we avoid type checking those blocks. Having --warn-unreachable isn't much helper since (a) it's not enabled by default, and (b) even if it was, it doesn't allude to type-checking being suppressed.

In this PR I'm making function and global level consistent, but how can we know it's the consistency we want?

(From quickly testing in pyright, it narrows the expression to Never but does not suppress type checking.)

ikonst avatar Jun 12 '23 00:06 ikonst

@ikonst

(a) it's not enabled by default

It's enabled by default in basedmypy.

I'm sorry, I'm struggling a little to see all the differences in behavior made here. You are stating that it fixes #12409, but the output is identical.

The only difference I could find is your example in the OP, although within a function, which now matches the behavior observed in master for module level. I think that is a great change.

KotlinIsland avatar Jun 12 '23 09:06 KotlinIsland

@KotlinIsland Sorry, you are right. I was looking for pre-existing issues and mistook #12409 to be it :) Removing the link.

ikonst avatar Jun 12 '23 09:06 ikonst

It's enabled by default in basedmypy.

Not disagreeing with that, mind you. Rather, questioning why it should suppress type-checking: IMO that's unexpected and comes with no warning.

Even if the branch is truly unreachable, suppressing all type checks until you fix reachability effectively makes it a 'blocking error', which we are trying not to have (judging by other 'blocking error' issues).

ikonst avatar Jun 12 '23 09:06 ikonst

I think it's a good idea to explore changes to this behavior, as it's not very intuitive.

JukkaL avatar Jul 13 '23 13:07 JukkaL