Skip to content

Commit

Permalink
Fix spurious unreachable and disallow-any errors from deferred passes
Browse files Browse the repository at this point in the history
This diff:

- Fixes python#8129
- Fixes python#13043
- Fixes python#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 python#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.
  • Loading branch information
Michael0x2a committed Sep 2, 2022
1 parent 38eb6e8 commit 3eb30a8
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 8 deletions.
12 changes: 4 additions & 8 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
)

Expand Down Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions test-data/unit/check-statements.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions test-data/unit/check-unreachable-code.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]

0 comments on commit 3eb30a8

Please sign in to comment.