Skip to content

Commit 0a8d425

Browse files
authored
Fix spurious unreachable and disallow-any errors from deferred passes (#13575)
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.
1 parent cf7495f commit 0a8d425

File tree

4 files changed

+49
-8
lines changed

4 files changed

+49
-8
lines changed

mypy/checker.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,9 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: str | None) ->
12031203
return_type = get_proper_type(return_type)
12041204

12051205
if self.options.warn_no_return:
1206-
if not isinstance(return_type, (NoneType, AnyType)):
1206+
if not self.current_node_deferred and not isinstance(
1207+
return_type, (NoneType, AnyType)
1208+
):
12071209
# Control flow fell off the end of a function that was
12081210
# declared to return a non-None type and is not
12091211
# entirely pass/Ellipsis/raise NotImplementedError.
@@ -2431,6 +2433,7 @@ def should_report_unreachable_issues(self) -> bool:
24312433
return (
24322434
self.in_checked_function()
24332435
and self.options.warn_unreachable
2436+
and not self.current_node_deferred
24342437
and not self.binder.is_unreachable_warning_suppressed()
24352438
)
24362439

@@ -4179,14 +4182,7 @@ def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None:
41794182
# To support local variables, we make this a definition line,
41804183
# causing assignment to set the variable's type.
41814184
var.is_inferred_def = True
4182-
# We also temporarily set current_node_deferred to False to
4183-
# make sure the inference happens.
4184-
# TODO: Use a better solution, e.g. a
4185-
# separate Var for each except block.
4186-
am_deferring = self.current_node_deferred
4187-
self.current_node_deferred = False
41884185
self.check_assignment(var, self.temp_node(t, var))
4189-
self.current_node_deferred = am_deferring
41904186
self.accept(s.handlers[i])
41914187
var = s.vars[i]
41924188
if var:

test-data/unit/check-flags.test

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,22 @@ def f() -> NoReturn: # E: Implicit return in function which does not return
366366
non_trivial_function = 1
367367
[builtins fixtures/dict.pyi]
368368

369+
[case testNoReturnImplicitReturnCheckInDeferredNode]
370+
# flags: --warn-no-return
371+
from typing import NoReturn
372+
373+
def exit() -> NoReturn: ...
374+
375+
def force_forward_reference() -> int:
376+
return 4
377+
378+
def f() -> NoReturn:
379+
x
380+
exit()
381+
382+
x = force_forward_reference()
383+
[builtins fixtures/exception.pyi]
384+
369385
[case testNoReturnNoWarnNoReturn]
370386
# flags: --warn-no-return
371387
from mypy_extensions import NoReturn

test-data/unit/check-statements.test

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,18 @@ x = f()
945945
main:10: note: Revealed type is "builtins.int"
946946
main:15: note: Revealed type is "builtins.str"
947947

948+
[case testExceptionVariableWithDisallowAnyExprInDeferredNode]
949+
# flags: --disallow-any-expr
950+
def f() -> int:
951+
x
952+
try:
953+
pass
954+
except Exception as ex:
955+
pass
956+
return 0
957+
x = f()
958+
[builtins fixtures/exception.pyi]
959+
948960
[case testArbitraryExpressionAsExceptionType]
949961
import typing
950962
a = BaseException

test-data/unit/check-unreachable-code.test

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,3 +1397,20 @@ a or a # E: Right operand of "or" is never evaluated
13971397
1 and a and 1 # E: Right operand of "and" is never evaluated
13981398
a and a # E: Right operand of "and" is never evaluated
13991399
[builtins fixtures/exception.pyi]
1400+
1401+
[case testUnreachableFlagWithTerminalBranchInDeferredNode]
1402+
# flags: --warn-unreachable
1403+
from typing import NoReturn
1404+
1405+
def assert_never(x: NoReturn) -> NoReturn: ...
1406+
1407+
def force_forward_ref() -> int:
1408+
return 4
1409+
1410+
def f(value: None) -> None:
1411+
x
1412+
if value is not None:
1413+
assert_never(value)
1414+
1415+
x = force_forward_ref()
1416+
[builtins fixtures/exception.pyi]

0 commit comments

Comments
 (0)