Skip to content

Commit

Permalink
Consistently avoid type-checking unreachable code (#15386)
Browse files Browse the repository at this point in the history
- 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.
  ```diff
   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
  ```
  • Loading branch information
ikonst authored Jul 13, 2023
1 parent dfea43f commit 3bf8521
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 80 deletions.
29 changes: 16 additions & 13 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,14 @@ def check_first_pass(self) -> None:
with self.tscope.module_scope(self.tree.fullname):
with self.enter_partial_types(), self.binder.top_frame_context():
for d in self.tree.defs:
if (
self.binder.is_unreachable()
and self.should_report_unreachable_issues()
and not self.is_raising_or_empty(d)
):
self.msg.unreachable_statement(d)
break
self.accept(d)
if self.binder.is_unreachable():
if not self.should_report_unreachable_issues():
break
if not self.is_noop_for_reachability(d):
self.msg.unreachable_statement(d)
break
else:
self.accept(d)

assert not self.current_node_deferred

Expand Down Expand Up @@ -2706,10 +2706,13 @@ def visit_block(self, b: Block) -> None:
return
for s in b.body:
if self.binder.is_unreachable():
if self.should_report_unreachable_issues() and not self.is_raising_or_empty(s):
if not self.should_report_unreachable_issues():
break
if not self.is_noop_for_reachability(s):
self.msg.unreachable_statement(s)
break
self.accept(s)
break
else:
self.accept(s)

def should_report_unreachable_issues(self) -> bool:
return (
Expand All @@ -2719,11 +2722,11 @@ def should_report_unreachable_issues(self) -> bool:
and not self.binder.is_unreachable_warning_suppressed()
)

def is_raising_or_empty(self, s: Statement) -> bool:
def is_noop_for_reachability(self, s: Statement) -> bool:
"""Returns 'true' if the given statement either throws an error of some kind
or is a no-op.
We use this function mostly while handling the '--warn-unreachable' flag. When
We use this function while handling the '--warn-unreachable' flag. When
that flag is present, we normally report an error on any unreachable statement.
But if that statement is just something like a 'pass' or a just-in-case 'assert False',
reporting an error would be annoying.
Expand Down
23 changes: 6 additions & 17 deletions mypyc/test-data/run-misc.test
Original file line number Diff line number Diff line change
Expand Up @@ -1108,25 +1108,14 @@ assert not C
# make the initial import fail
assert False

class C:
def __init__(self):
self.x = 1
self.y = 2
def test() -> None:
a = C()
[file driver.py]
# load native, cause PyInit to be run, create the module but don't finish initializing the globals
try:
import native
except:
pass
try:
# try accessing those globals that were never properly initialized
import native
native.test()
# should fail with AssertionError due to `assert False` in other function
except AssertionError:
pass
for _ in range(2):
try:
import native
raise RuntimeError('exception expected')
except AssertionError:
pass

[case testRepeatedUnderscoreFunctions]
def _(arg): pass
Expand Down
27 changes: 19 additions & 8 deletions test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -7725,10 +7725,14 @@ class D:
def __new__(cls) -> NoReturn: ...
def __init__(self) -> NoReturn: ...

reveal_type(A()) # N: Revealed type is "<nothing>"
reveal_type(B()) # N: Revealed type is "<nothing>"
reveal_type(C()) # N: Revealed type is "<nothing>"
reveal_type(D()) # N: Revealed type is "<nothing>"
if object():
reveal_type(A()) # N: Revealed type is "<nothing>"
if object():
reveal_type(B()) # N: Revealed type is "<nothing>"
if object():
reveal_type(C()) # N: Revealed type is "<nothing>"
if object():
reveal_type(D()) # N: Revealed type is "<nothing>"

[case testOverloadedNewAndInitNoReturn]
from typing import NoReturn, overload
Expand Down Expand Up @@ -7767,13 +7771,20 @@ class D:
def __init__(self, a: int) -> None: ...
def __init__(self, a: int = ...) -> None: ...

reveal_type(A()) # N: Revealed type is "<nothing>"
if object():
reveal_type(A()) # N: Revealed type is "<nothing>"
reveal_type(A(1)) # N: Revealed type is "__main__.A"
reveal_type(B()) # N: Revealed type is "<nothing>"

if object():
reveal_type(B()) # N: Revealed type is "<nothing>"
reveal_type(B(1)) # N: Revealed type is "__main__.B"
reveal_type(C()) # N: Revealed type is "<nothing>"

if object():
reveal_type(C()) # N: Revealed type is "<nothing>"
reveal_type(C(1)) # N: Revealed type is "__main__.C"
reveal_type(D()) # N: Revealed type is "<nothing>"

if object():
reveal_type(D()) # N: Revealed type is "<nothing>"
reveal_type(D(1)) # N: Revealed type is "__main__.D"

[case testClassScopeImportWithWrapperAndError]
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-fastparse.test
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ def g(): # E: Type signature has too many arguments
assert 1, 2
assert (1, 2) # E: Assertion is always true, perhaps remove parentheses?
assert (1, 2), 3 # E: Assertion is always true, perhaps remove parentheses?
assert ()
assert (1,) # E: Assertion is always true, perhaps remove parentheses?
assert ()
[builtins fixtures/tuple.pyi]

[case testFastParseAssertMessage]
Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -5413,7 +5413,8 @@ reveal_type(z)
[out]
tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
[out2]
tmp/c.py:2: note: Revealed type is "a.A"
tmp/b.py:2: error: Cannot determine type of "y"
tmp/c.py:2: note: Revealed type is "Any"

[case testIsInstanceAdHocIntersectionIncrementalUnreachaableToIntersection]
import c
Expand Down Expand Up @@ -5444,7 +5445,8 @@ from b import z
reveal_type(z)
[builtins fixtures/isinstance.pyi]
[out]
tmp/c.py:2: note: Revealed type is "a.A"
tmp/b.py:2: error: Cannot determine type of "y"
tmp/c.py:2: note: Revealed type is "Any"
[out2]
tmp/c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"

Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/check-inference-context.test
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,10 @@ reveal_type((lambda x, y: x + y)(1, 2)) # N: Revealed type is "builtins.int"
reveal_type((lambda s, i: s)(i=0, s='x')) # N: Revealed type is "Literal['x']?"
reveal_type((lambda s, i: i)(i=0, s='x')) # N: Revealed type is "Literal[0]?"
reveal_type((lambda x, s, i: x)(1.0, i=0, s='x')) # N: Revealed type is "builtins.float"
(lambda x, s, i: x)() # E: Too few arguments
(lambda: 0)(1) # E: Too many arguments
if object():
(lambda x, s, i: x)() # E: Too few arguments
if object():
(lambda: 0)(1) # E: Too many arguments
-- varargs are not handled, but it should not crash
reveal_type((lambda *k, s, i: i)(type, i=0, s='x')) # N: Revealed type is "Any"
reveal_type((lambda s, *k, i: i)(i=0, s='x')) # N: Revealed type is "Any"
Expand Down
12 changes: 8 additions & 4 deletions test-data/unit/check-native-int.test
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ reveal_type(meet(f32, f)) # N: Revealed type is "mypy_extensions.i32"
reveal_type(meet(f, f32)) # N: Revealed type is "mypy_extensions.i32"
reveal_type(meet(f64, f)) # N: Revealed type is "mypy_extensions.i64"
reveal_type(meet(f, f64)) # N: Revealed type is "mypy_extensions.i64"
reveal_type(meet(f32, f64)) # N: Revealed type is "<nothing>"
reveal_type(meet(f64, f32)) # N: Revealed type is "<nothing>"
if object():
reveal_type(meet(f32, f64)) # N: Revealed type is "<nothing>"
if object():
reveal_type(meet(f64, f32)) # N: Revealed type is "<nothing>"

reveal_type(meet(f, fa)) # N: Revealed type is "builtins.int"
reveal_type(meet(f32, fa)) # N: Revealed type is "mypy_extensions.i32"
Expand Down Expand Up @@ -148,8 +150,10 @@ def meet(c1: Callable[[T], None], c2: Callable[[T], None]) -> T:
def ff(x: float) -> None: pass
def fi32(x: i32) -> None: pass

reveal_type(meet(ff, fi32)) # N: Revealed type is "<nothing>"
reveal_type(meet(fi32, ff)) # N: Revealed type is "<nothing>"
if object():
reveal_type(meet(ff, fi32)) # N: Revealed type is "<nothing>"
if object():
reveal_type(meet(fi32, ff)) # N: Revealed type is "<nothing>"
[builtins fixtures/dict.pyi]

[case testNativeIntForLoopRange]
Expand Down
78 changes: 52 additions & 26 deletions test-data/unit/check-statements.test
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,16 @@ main:5: error: Exception must be derived from BaseException
class A: pass
class MyError(BaseException): pass
def f(): pass
raise BaseException
raise MyError
raise A # E: Exception must be derived from BaseException
raise object # E: Exception must be derived from BaseException
raise f # E: Exception must be derived from BaseException
if object():
raise BaseException
if object():
raise MyError
if object():
raise A # E: Exception must be derived from BaseException
if object():
raise object # E: Exception must be derived from BaseException
if object():
raise f # E: Exception must be derived from BaseException
[builtins fixtures/exception.pyi]

[case testRaiseClassObjectCustomInit]
Expand All @@ -429,18 +434,30 @@ class MyKwError(Exception):
class MyErrorWithDefault(Exception):
def __init__(self, optional=1) -> None:
...
raise BaseException
raise Exception
raise BaseException(1)
raise Exception(2)
raise MyBaseError(4)
raise MyError(5, 6)
raise MyKwError(kwonly=7)
raise MyErrorWithDefault(8)
raise MyErrorWithDefault
raise MyBaseError # E: Too few arguments for "MyBaseError"
raise MyError # E: Too few arguments for "MyError"
raise MyKwError # E: Missing named argument "kwonly" for "MyKwError"
if object():
raise BaseException
if object():
raise Exception
if object():
raise BaseException(1)
if object():
raise Exception(2)
if object():
raise MyBaseError(4)
if object():
raise MyError(5, 6)
if object():
raise MyKwError(kwonly=7)
if object():
raise MyErrorWithDefault(8)
if object():
raise MyErrorWithDefault
if object():
raise MyBaseError # E: Too few arguments for "MyBaseError"
if object():
raise MyError # E: Too few arguments for "MyError"
if object():
raise MyKwError # E: Missing named argument "kwonly" for "MyKwError"
[builtins fixtures/exception.pyi]

[case testRaiseExceptionType]
Expand Down Expand Up @@ -473,10 +490,14 @@ f: MyError
a: A
x: BaseException
del x
raise e from a # E: Exception must be derived from BaseException
raise e from e
raise e from f
raise e from x # E: Trying to read deleted variable "x"
if object():
raise e from a # E: Exception must be derived from BaseException
if object():
raise e from e
if object():
raise e from f
if object():
raise e from x # E: Trying to read deleted variable "x"
class A: pass
class MyError(BaseException): pass
[builtins fixtures/exception.pyi]
Expand All @@ -486,11 +507,16 @@ import typing
class A: pass
class MyError(BaseException): pass
def f(): pass
raise BaseException from BaseException
raise BaseException from MyError
raise BaseException from A # E: Exception must be derived from BaseException
raise BaseException from object # E: Exception must be derived from BaseException
raise BaseException from f # E: Exception must be derived from BaseException
if object():
raise BaseException from BaseException
if object():
raise BaseException from MyError
if object():
raise BaseException from A # E: Exception must be derived from BaseException
if object():
raise BaseException from object # E: Exception must be derived from BaseException
if object():
raise BaseException from f # E: Exception must be derived from BaseException
[builtins fixtures/exception.pyi]

[case testTryFinallyStatement]
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-typevar-tuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ reveal_type(f(args)) # N: Revealed type is "Tuple[builtins.int, builtins.str]"

reveal_type(f(varargs)) # N: Revealed type is "builtins.tuple[builtins.int, ...]"

f(0) # E: Argument 1 to "f" has incompatible type "int"; expected <nothing>
if object():
f(0) # E: Argument 1 to "f" has incompatible type "int"; expected <nothing>

def g(a: Tuple[Unpack[Ts]], b: Tuple[Unpack[Ts]]) -> Tuple[Unpack[Ts]]:
return a
Expand Down
8 changes: 4 additions & 4 deletions test-data/unit/check-unreachable-code.test
Original file line number Diff line number Diff line change
Expand Up @@ -873,15 +873,15 @@ def expect_str(x: str) -> str: pass
x: int
if False:
assert False
reveal_type(x)
reveal_type(x) # E: Statement is unreachable

if False:
raise Exception()
reveal_type(x)
reveal_type(x) # E: Statement is unreachable

if False:
assert_never(x)
reveal_type(x)
reveal_type(x) # E: Statement is unreachable

if False:
nonthrowing_assert_never(x) # E: Statement is unreachable
Expand All @@ -890,7 +890,7 @@ if False:
if False:
# Ignore obvious type errors
assert_never(expect_str(x))
reveal_type(x)
reveal_type(x) # E: Statement is unreachable
[builtins fixtures/exception.pyi]

[case testNeverVariants]
Expand Down
6 changes: 4 additions & 2 deletions test-data/unit/fine-grained.test
Original file line number Diff line number Diff line change
Expand Up @@ -9667,7 +9667,8 @@ reveal_type(z)
[out]
c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"
==
c.py:2: note: Revealed type is "a.A"
c.py:2: note: Revealed type is "Any"
b.py:2: error: Cannot determine type of "y"

[case testIsInstanceAdHocIntersectionFineGrainedIncrementalUnreachaableToIntersection]
import c
Expand Down Expand Up @@ -9698,7 +9699,8 @@ from b import z
reveal_type(z)
[builtins fixtures/isinstance.pyi]
[out]
c.py:2: note: Revealed type is "a.A"
b.py:2: error: Cannot determine type of "y"
c.py:2: note: Revealed type is "Any"
==
c.py:2: note: Revealed type is "a.<subclass of "A" and "B">"

Expand Down

0 comments on commit 3bf8521

Please sign in to comment.