Skip to content

Avoid false unreachable, redundant-expr, and redundant-casts warnings in loops more robustly and efficiently, and avoid multiple revealed type notes for the same line. #19118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from mypy.constraints import SUPERTYPE_OF
from mypy.erasetype import erase_type, erase_typevars, remove_instance_last_known_values
from mypy.errorcodes import TYPE_VAR, UNUSED_AWAITABLE, UNUSED_COROUTINE, ErrorCode
from mypy.errors import Errors, ErrorWatcher, report_internal_error
from mypy.errors import Errors, ErrorWatcher, LoopErrorWatcher, report_internal_error
from mypy.expandtype import expand_type
from mypy.literals import Key, extract_var_from_literal_hash, literal, literal_hash
from mypy.maptype import map_instance_to_supertype
Expand Down Expand Up @@ -600,19 +600,23 @@ def accept_loop(
# on without bound otherwise)
widened_old = len(self.widened_vars)

# Disable error types that we cannot safely identify in intermediate iteration steps:
warn_unreachable = self.options.warn_unreachable
warn_redundant = codes.REDUNDANT_EXPR in self.options.enabled_error_codes
self.options.warn_unreachable = False
self.options.enabled_error_codes.discard(codes.REDUNDANT_EXPR)

iter = 1
while True:
with self.binder.frame_context(can_skip=True, break_frame=2, continue_frame=1):
if on_enter_body is not None:
on_enter_body()

self.accept(body)
# We collect errors that indicate unreachability or redundant expressions
# in the first iteration and remove them in subsequent iterations if the
# related statement becomes reachable or non-redundant due to changes in
# partial types:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if an unreachable statement is within another unreachable statement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not exactly sure what you are asking for.

Is this an example that fits your question?

[case testAvoidUnreachableCodeInUnreachableCodeWithPartialType]
# flags: --warn-unreachable --python-version 3.11

x = None
y = None
while x is None and y is None:
    reveal_type(x)  # N: Revealed type is "None"
    reveal_type(y)  # N: Revealed type is "None"
    if x is None and y is not None:
        x = 1  # E: Statement is unreachable
        if y is None:
            y = 1

[builtins fixtures/bool.pyi]

In this case, the results are the same for 1.15 and the current state of this PR.

Copy link
Collaborator

@A5rocks A5rocks May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like this:

x = None
y = None
while True:
    if x is not None:
        print("a")
        if y is not None:
            print("b")  # this is unreachable
    x = 1

(I haven't run the PR on this so I'm not sure the result, but it should warn about unreachablity. mypy 1.15 does. I'm worried this change won't)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example should print b. Is there a typo?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited it afterwards, it shouldn't anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you were totally right. The large temporarily unreachable code section shadowed the persistent unreachable code section. I added your test case and extended the current logic a bit. It's late now, so I'll postpone double-checking until tomorrow.

Can you see any more potential pitfalls?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I again looked at it. To me, the logic appears a little special, but clear and straightforward. Of course, I cannot exclude that there are more special cases my solution does not cover. So I wait for the next review.

with LoopErrorWatcher(self.msg.errors) as watcher:
self.accept(body)
if iter == 1:
uselessness_errors = watcher.useless_statements
else:
uselessness_errors.intersection_update(watcher.useless_statements)

partials_new = sum(len(pts.map) for pts in self.partial_types)
widened_new = len(self.widened_vars)
# Perform multiple iterations if something changed that might affect
Expand All @@ -633,16 +637,21 @@ def accept_loop(
if iter == 20:
raise RuntimeError("Too many iterations when checking a loop")

# If necessary, reset the modified options and make up for the postponed error checks:
self.options.warn_unreachable = warn_unreachable
if warn_redundant:
self.options.enabled_error_codes.add(codes.REDUNDANT_EXPR)
if warn_unreachable or warn_redundant:
with self.binder.frame_context(can_skip=True, break_frame=2, continue_frame=1):
if on_enter_body is not None:
on_enter_body()

self.accept(body)
# Report those unreachable and redundant expression errors identified in all
# iteration steps:
for error_info in uselessness_errors:
context = Context(line=error_info[2], column=error_info[3])
context.end_line = error_info[4]
context.end_column = error_info[5]
self.msg.fail(error_info[1], context, code=error_info[0])
# Report all types revealed in at least one iteration step:
for note_info, types in watcher.revealed_types.items():
sorted_ = sorted(types, key=lambda typ: typ.lower())
revealed = sorted_[0] if len(types) == 1 else f"Union[{', '.join(sorted_)}]"
context = Context(line=note_info[1], column=note_info[2])
context.end_line = note_info[3]
context.end_column = note_info[4]
self.note(f'Revealed type is "{revealed}"', context)

# If exit_condition is set, assume it must be False on exit from the loop:
if exit_condition:
Expand Down
48 changes: 46 additions & 2 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from collections import defaultdict
from collections.abc import Iterable
from typing import Callable, Final, NoReturn, Optional, TextIO, TypeVar
from typing_extensions import Literal, TypeAlias as _TypeAlias
from typing_extensions import Literal, Self, TypeAlias as _TypeAlias

from mypy import errorcodes as codes
from mypy.error_formatter import ErrorFormatter
Expand Down Expand Up @@ -179,7 +179,7 @@ def __init__(
self._filter_deprecated = filter_deprecated
self._filtered: list[ErrorInfo] | None = [] if save_filtered_errors else None

def __enter__(self) -> ErrorWatcher:
def __enter__(self) -> Self:
self.errors._watchers.append(self)
return self

Expand Down Expand Up @@ -220,6 +220,50 @@ def filtered_errors(self) -> list[ErrorInfo]:
return self._filtered


class LoopErrorWatcher(ErrorWatcher):
"""Error watcher that filters and separately collects unreachable errors, redundant
expression errors, and revealed types when analysing loops iteratively to help avoid
making too-hasty reports."""

# Meaning of the tuple items: ErrorCode, message, line, column, end_line, end_column:
useless_statements: set[tuple[ErrorCode, str, int, int, int, int]]
# Meaning of the tuple items: function_or_member, line, column, end_line, end_column:
revealed_types: dict[tuple[str | None, int, int, int, int], set[str]]

def __init__(
self,
errors: Errors,
*,
filter_errors: bool | Callable[[str, ErrorInfo], bool] = False,
save_filtered_errors: bool = False,
filter_deprecated: bool = False,
) -> None:
super().__init__(
errors,
filter_errors=filter_errors,
save_filtered_errors=save_filtered_errors,
filter_deprecated=filter_deprecated,
)
self.useless_statements = set()
self.revealed_types = defaultdict(set)

def on_error(self, file: str, info: ErrorInfo) -> bool:
if info.code in (codes.UNREACHABLE, codes.REDUNDANT_EXPR):
self.useless_statements.add(
(info.code, info.message, info.line, info.column, info.end_line, info.end_column)
)
return True
if info.code == codes.MISC and info.message.startswith("Revealed type is "):
key = info.function_or_member, info.line, info.column, info.end_line, info.end_column
types = info.message.split('"')[1]
if types.startswith("Union["):
self.revealed_types[key].update(types[6:-1].split(", "))
else:
self.revealed_types[key].add(types)
return True
return super().on_error(file, info)


class Errors:
"""Container for compile errors.

Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ for var2 in [g, h, i, j, k, l]:
reveal_type(var2) # N: Revealed type is "Union[builtins.int, builtins.str]"

for var3 in [m, n, o, p, q, r]:
reveal_type(var3) # N: Revealed type is "Union[builtins.int, Any]"
reveal_type(var3) # N: Revealed type is "Union[Any, builtins.int]"

T = TypeVar("T", bound=Type[Foo])

Expand Down Expand Up @@ -1247,7 +1247,7 @@ class X(TypedDict):

x: X
for a in ("hourly", "daily"):
reveal_type(a) # N: Revealed type is "Union[Literal['hourly']?, Literal['daily']?]"
reveal_type(a) # N: Revealed type is "Union[Literal['daily']?, Literal['hourly']?]"
reveal_type(x[a]) # N: Revealed type is "builtins.int"
reveal_type(a.upper()) # N: Revealed type is "builtins.str"
c = a
Expand Down
28 changes: 25 additions & 3 deletions test-data/unit/check-narrowing.test
Original file line number Diff line number Diff line change
Expand Up @@ -2346,8 +2346,7 @@ def f() -> bool: ...

y = None
while f():
reveal_type(y) # N: Revealed type is "None" \
# N: Revealed type is "Union[builtins.int, None]"
reveal_type(y) # N: Revealed type is "Union[builtins.int, None]"
y = 1
reveal_type(y) # N: Revealed type is "Union[builtins.int, None]"

Expand All @@ -2370,7 +2369,7 @@ class A:

[builtins fixtures/primitives.pyi]

[case testAvoidFalseUnreachableInLoop]
[case testAvoidFalseUnreachableInLoop1]
# flags: --warn-unreachable --python-version 3.11

def f() -> int | None: ...
Expand All @@ -2383,6 +2382,29 @@ while x is not None or b():

[builtins fixtures/bool.pyi]

[case testAvoidFalseUnreachableInLoop2]
# flags: --warn-unreachable --python-version 3.11

y = None
while y is None:
if y is None:
y = []
y.append(1)

[builtins fixtures/list.pyi]

[case testAvoidFalseUnreachableInLoop3]
# flags: --warn-unreachable --python-version 3.11

xs: list[int | None]
y = None
for x in xs:
if x is not None:
if y is None:
y = {} # E: Need type annotation for "y" (hint: "y: Dict[<type>, <type>] = ...")

[builtins fixtures/list.pyi]

[case testAvoidFalseRedundantExprInLoop]
# flags: --enable-error-code redundant-expr --python-version 3.11

Expand Down
11 changes: 4 additions & 7 deletions test-data/unit/check-redefine2.test
Original file line number Diff line number Diff line change
Expand Up @@ -628,8 +628,7 @@ def f1() -> None:
def f2() -> None:
x = None
while int():
reveal_type(x) # N: Revealed type is "None" \
# N: Revealed type is "Union[None, builtins.str]"
reveal_type(x) # N: Revealed type is "Union[builtins.str, None]"
if int():
x = ""
reveal_type(x) # N: Revealed type is "Union[None, builtins.str]"
Expand Down Expand Up @@ -709,8 +708,7 @@ def b() -> None:
def c() -> None:
x = 0
while int():
reveal_type(x) # N: Revealed type is "builtins.int" \
# N: Revealed type is "Union[builtins.int, builtins.str, None]"
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]"
if int():
x = ""
continue
Expand Down Expand Up @@ -810,8 +808,7 @@ def f4() -> None:
x = None
break
finally:
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]" \
# N: Revealed type is "Union[builtins.int, None]"
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]"
reveal_type(x) # N: Revealed type is "Union[builtins.int, None]"
[builtins fixtures/exception.pyi]

Expand Down Expand Up @@ -927,7 +924,7 @@ class X(TypedDict):

x: X
for a in ("hourly", "daily"):
reveal_type(a) # N: Revealed type is "Union[Literal['hourly']?, Literal['daily']?]"
reveal_type(a) # N: Revealed type is "Union[Literal['daily']?, Literal['hourly']?]"
reveal_type(x[a]) # N: Revealed type is "builtins.int"
reveal_type(a.upper()) # N: Revealed type is "builtins.str"
c = a
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-typevar-tuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ from typing_extensions import Unpack

def pipeline(*xs: Unpack[Tuple[int, Unpack[Tuple[float, ...]], bool]]) -> None:
for x in xs:
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.float]"
reveal_type(x) # N: Revealed type is "Union[builtins.float, builtins.int]"
[builtins fixtures/tuple.pyi]

[case testFixedUnpackItemInInstanceArguments]
Expand Down