From 3eb30a87931a0bdd00943b43319a26317f57fb80 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Wed, 31 Aug 2022 20:59:44 -0700 Subject: [PATCH] Fix spurious unreachable and disallow-any errors from deferred passes This diff: - Fixes #8129 - Fixes #13043 - Fixes #13167 For more concise repros of these various issues, see the modified test files. But in short, there were two broad categories of errors: 1. Within the deferred pass, we tend to infer 'Any' for the types of different variables instead of the actual type. This interacts badly with our unreachable and disallow-any checks and causes spurious errors. Arguably, the better way of handling this error is to only collect errors during the final pass. I briefly experimented with this approach, but was unable to find a clean, efficient, and non-disruptive way of implementing this. So, I settled for sprinkling in a few more `not self.current_node_deferred` checks. 2. The 'self.msg.disallowed_any_type(...)` call is normally guarded behind a `not self.chk.current_node_deferred` check. However, we were bypassing this check for `except` block assignments because we were deliberately setting that flag to False to work around some bug. For more context, see #2290. It appears we no longer need this patch anymore. I'm not entirely sure why, but I'm guessing we tightened and fixed the underlying problem with deferred passes some time during the past half-decade. --- mypy/checker.py | 12 ++++-------- test-data/unit/check-flags.test | 16 ++++++++++++++++ test-data/unit/check-statements.test | 12 ++++++++++++ test-data/unit/check-unreachable-code.test | 17 +++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 1f4cb8bc7b3a..dbd1adfb42e3 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -1203,7 +1203,9 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) -> return_type = get_proper_type(return_type) if self.options.warn_no_return: - if not isinstance(return_type, (NoneType, AnyType)): + if not self.current_node_deferred and not isinstance( + return_type, (NoneType, AnyType) + ): # Control flow fell off the end of a function that was # declared to return a non-None type and is not # entirely pass/Ellipsis/raise NotImplementedError. @@ -2431,6 +2433,7 @@ def should_report_unreachable_issues(self) -> bool: return ( self.in_checked_function() and self.options.warn_unreachable + and not self.current_node_deferred and not self.binder.is_unreachable_warning_suppressed() ) @@ -4179,14 +4182,7 @@ def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None: # To support local variables, we make this a definition line, # causing assignment to set the variable's type. var.is_inferred_def = True - # We also temporarily set current_node_deferred to False to - # make sure the inference happens. - # TODO: Use a better solution, e.g. a - # separate Var for each except block. - am_deferring = self.current_node_deferred - self.current_node_deferred = False self.check_assignment(var, self.temp_node(t, var)) - self.current_node_deferred = am_deferring self.accept(s.handlers[i]) var = s.vars[i] if var: diff --git a/test-data/unit/check-flags.test b/test-data/unit/check-flags.test index 11229465eac4..9fdd5ea2232c 100644 --- a/test-data/unit/check-flags.test +++ b/test-data/unit/check-flags.test @@ -366,6 +366,22 @@ def f() -> NoReturn: # E: Implicit return in function which does not return non_trivial_function = 1 [builtins fixtures/dict.pyi] +[case testNoReturnImplicitReturnCheckInDeferredNode] +# flags: --warn-no-return +from typing import NoReturn + +def exit() -> NoReturn: ... + +def force_forward_reference() -> int: + return 4 + +def f() -> NoReturn: + x + exit() + +x = force_forward_reference() +[builtins fixtures/exception.pyi] + [case testNoReturnNoWarnNoReturn] # flags: --warn-no-return from mypy_extensions import NoReturn diff --git a/test-data/unit/check-statements.test b/test-data/unit/check-statements.test index c26e9672056b..9b571cb20c0d 100644 --- a/test-data/unit/check-statements.test +++ b/test-data/unit/check-statements.test @@ -945,6 +945,18 @@ x = f() main:10: note: Revealed type is "builtins.int" main:15: note: Revealed type is "builtins.str" +[case testExceptionVariableWithDisallowAnyExprInDeferredNode] +# flags: --disallow-any-expr +def f() -> int: + x + try: + pass + except Exception as ex: + pass + return 0 +x = f() +[builtins fixtures/exception.pyi] + [case testArbitraryExpressionAsExceptionType] import typing a = BaseException diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index 289d042d8790..64736e55e2dd 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -1397,3 +1397,20 @@ a or a # E: Right operand of "or" is never evaluated 1 and a and 1 # E: Right operand of "and" is never evaluated a and a # E: Right operand of "and" is never evaluated [builtins fixtures/exception.pyi] + +[case testUnreachableFlagWithTerminalBranchInDeferredNode] +# flags: --warn-unreachable +from typing import NoReturn + +def assert_never(x: NoReturn) -> NoReturn: ... + +def force_forward_ref() -> int: + return 4 + +def f(value: None) -> None: + x + if value is not None: + assert_never(value) + +x = force_forward_ref() +[builtins fixtures/exception.pyi]