From 3bf85217386806b0f68bf8857b61379ae2f6ad1e Mon Sep 17 00:00:00 2001 From: Ilya Priven Date: Thu, 13 Jul 2023 06:42:39 -0700 Subject: [PATCH] Consistently avoid type-checking unreachable code (#15386) - 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 ``` --- mypy/checker.py | 29 ++++---- mypyc/test-data/run-misc.test | 23 ++---- test-data/unit/check-classes.test | 27 ++++--- test-data/unit/check-fastparse.test | 2 +- test-data/unit/check-incremental.test | 6 +- test-data/unit/check-inference-context.test | 6 +- test-data/unit/check-native-int.test | 12 ++-- test-data/unit/check-statements.test | 78 ++++++++++++++------- test-data/unit/check-typevar-tuple.test | 3 +- test-data/unit/check-unreachable-code.test | 8 +-- test-data/unit/fine-grained.test | 6 +- 11 files changed, 120 insertions(+), 80 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 71c9746ce24f..e2ff8a6ec2a4 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -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 @@ -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 ( @@ -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. diff --git a/mypyc/test-data/run-misc.test b/mypyc/test-data/run-misc.test index fd0eb5022236..c40e0fc55f0e 100644 --- a/mypyc/test-data/run-misc.test +++ b/mypyc/test-data/run-misc.test @@ -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 diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 69227e50f6fa..957eb9214d7c 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -7725,10 +7725,14 @@ class D: def __new__(cls) -> NoReturn: ... def __init__(self) -> NoReturn: ... -reveal_type(A()) # N: Revealed type is "" -reveal_type(B()) # N: Revealed type is "" -reveal_type(C()) # N: Revealed type is "" -reveal_type(D()) # N: Revealed type is "" +if object(): + reveal_type(A()) # N: Revealed type is "" +if object(): + reveal_type(B()) # N: Revealed type is "" +if object(): + reveal_type(C()) # N: Revealed type is "" +if object(): + reveal_type(D()) # N: Revealed type is "" [case testOverloadedNewAndInitNoReturn] from typing import NoReturn, overload @@ -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 "" +if object(): + reveal_type(A()) # N: Revealed type is "" reveal_type(A(1)) # N: Revealed type is "__main__.A" -reveal_type(B()) # N: Revealed type is "" + +if object(): + reveal_type(B()) # N: Revealed type is "" reveal_type(B(1)) # N: Revealed type is "__main__.B" -reveal_type(C()) # N: Revealed type is "" + +if object(): + reveal_type(C()) # N: Revealed type is "" reveal_type(C(1)) # N: Revealed type is "__main__.C" -reveal_type(D()) # N: Revealed type is "" + +if object(): + reveal_type(D()) # N: Revealed type is "" reveal_type(D(1)) # N: Revealed type is "__main__.D" [case testClassScopeImportWithWrapperAndError] diff --git a/test-data/unit/check-fastparse.test b/test-data/unit/check-fastparse.test index 2e4473c2716b..132a34503b89 100644 --- a/test-data/unit/check-fastparse.test +++ b/test-data/unit/check-fastparse.test @@ -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] diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 5203b0828122..cd009887a5b5 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -5413,7 +5413,8 @@ reveal_type(z) [out] tmp/c.py:2: note: Revealed type is "a." [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 @@ -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." diff --git a/test-data/unit/check-inference-context.test b/test-data/unit/check-inference-context.test index 13cfec46eeab..59f515490964 100644 --- a/test-data/unit/check-inference-context.test +++ b/test-data/unit/check-inference-context.test @@ -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" diff --git a/test-data/unit/check-native-int.test b/test-data/unit/check-native-int.test index 1e945d0af27d..1129512694f4 100644 --- a/test-data/unit/check-native-int.test +++ b/test-data/unit/check-native-int.test @@ -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 "" -reveal_type(meet(f64, f32)) # N: Revealed type is "" +if object(): + reveal_type(meet(f32, f64)) # N: Revealed type is "" +if object(): + reveal_type(meet(f64, f32)) # N: Revealed type is "" reveal_type(meet(f, fa)) # N: Revealed type is "builtins.int" reveal_type(meet(f32, fa)) # N: Revealed type is "mypy_extensions.i32" @@ -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 "" -reveal_type(meet(fi32, ff)) # N: Revealed type is "" +if object(): + reveal_type(meet(ff, fi32)) # N: Revealed type is "" +if object(): + reveal_type(meet(fi32, ff)) # N: Revealed type is "" [builtins fixtures/dict.pyi] [case testNativeIntForLoopRange] diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index 8a232d52968f..023e2935a158 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -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] @@ -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] @@ -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] @@ -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] diff --git a/test-data/unit/check-typevar-tuple.test b/test-data/unit/check-typevar-tuple.test index e1fae05eac63..1024f90ee6b7 100644 --- a/test-data/unit/check-typevar-tuple.test +++ b/test-data/unit/check-typevar-tuple.test @@ -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 +if object(): + f(0) # E: Argument 1 to "f" has incompatible type "int"; expected def g(a: Tuple[Unpack[Ts]], b: Tuple[Unpack[Ts]]) -> Tuple[Unpack[Ts]]: return a diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index b2fd44043435..1db2a16e2e1c 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -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 @@ -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] diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index 7d854bab424c..11a8f03590f7 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -9667,7 +9667,8 @@ reveal_type(z) [out] c.py:2: note: Revealed type is "a." == -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 @@ -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."