Skip to content

Commit 55c4067

Browse files
authored
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)
Fixes #18606 Closes #18511 Improves #18991 Fixes #19170 This change is an improvement over 9685171. Besides fixing the regressions reported in #18606 and #19170 and removing the duplicates reported in #18511, it should significantly reduce the performance regression reported in #18991. At least running `Measure-command {python runtests.py self}` on my computer (with removed cache) is 10 % faster.
1 parent b025bda commit 55c4067

File tree

6 files changed

+162
-33
lines changed

6 files changed

+162
-33
lines changed

mypy/checker.py

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from mypy.constraints import SUPERTYPE_OF
2626
from mypy.erasetype import erase_type, erase_typevars, remove_instance_last_known_values
2727
from mypy.errorcodes import TYPE_VAR, UNUSED_AWAITABLE, UNUSED_COROUTINE, ErrorCode
28-
from mypy.errors import Errors, ErrorWatcher, report_internal_error
28+
from mypy.errors import Errors, ErrorWatcher, LoopErrorWatcher, report_internal_error
2929
from mypy.expandtype import expand_type
3030
from mypy.literals import Key, extract_var_from_literal_hash, literal, literal_hash
3131
from mypy.maptype import map_instance_to_supertype
@@ -599,19 +599,27 @@ def accept_loop(
599599
# on without bound otherwise)
600600
widened_old = len(self.widened_vars)
601601

602-
# Disable error types that we cannot safely identify in intermediate iteration steps:
603-
warn_unreachable = self.options.warn_unreachable
604-
warn_redundant = codes.REDUNDANT_EXPR in self.options.enabled_error_codes
605-
self.options.warn_unreachable = False
606-
self.options.enabled_error_codes.discard(codes.REDUNDANT_EXPR)
607-
602+
# one set of `unreachable`, `redundant-expr`, and `redundant-casts` errors
603+
# per iteration step:
604+
uselessness_errors = []
605+
# one set of unreachable line numbers per iteration step:
606+
unreachable_lines = []
607+
# one set of revealed types per line where `reveal_type` is used (each
608+
# created set can grow during the iteration):
609+
revealed_types = defaultdict(set)
608610
iter = 1
609611
while True:
610612
with self.binder.frame_context(can_skip=True, break_frame=2, continue_frame=1):
611613
if on_enter_body is not None:
612614
on_enter_body()
613615

614-
self.accept(body)
616+
with LoopErrorWatcher(self.msg.errors) as watcher:
617+
self.accept(body)
618+
uselessness_errors.append(watcher.uselessness_errors)
619+
unreachable_lines.append(watcher.unreachable_lines)
620+
for key, values in watcher.revealed_types.items():
621+
revealed_types[key].update(values)
622+
615623
partials_new = sum(len(pts.map) for pts in self.partial_types)
616624
widened_new = len(self.widened_vars)
617625
# Perform multiple iterations if something changed that might affect
@@ -632,16 +640,29 @@ def accept_loop(
632640
if iter == 20:
633641
raise RuntimeError("Too many iterations when checking a loop")
634642

635-
# If necessary, reset the modified options and make up for the postponed error checks:
636-
self.options.warn_unreachable = warn_unreachable
637-
if warn_redundant:
638-
self.options.enabled_error_codes.add(codes.REDUNDANT_EXPR)
639-
if warn_unreachable or warn_redundant:
640-
with self.binder.frame_context(can_skip=True, break_frame=2, continue_frame=1):
641-
if on_enter_body is not None:
642-
on_enter_body()
643-
644-
self.accept(body)
643+
# Report only those `unreachable`, `redundant-expr`, and `redundant-casts`
644+
# errors that could not be ruled out in any iteration step:
645+
persistent_uselessness_errors = set()
646+
for candidate in set(itertools.chain(*uselessness_errors)):
647+
if all(
648+
(candidate in errors) or (candidate[2] in lines)
649+
for errors, lines in zip(uselessness_errors, unreachable_lines)
650+
):
651+
persistent_uselessness_errors.add(candidate)
652+
for error_info in persistent_uselessness_errors:
653+
context = Context(line=error_info[2], column=error_info[3])
654+
context.end_line = error_info[4]
655+
context.end_column = error_info[5]
656+
self.msg.fail(error_info[1], context, code=error_info[0])
657+
658+
# Report all types revealed in at least one iteration step:
659+
for note_info, types in revealed_types.items():
660+
sorted_ = sorted(types, key=lambda typ: typ.lower())
661+
revealed = sorted_[0] if len(types) == 1 else f"Union[{', '.join(sorted_)}]"
662+
context = Context(line=note_info[1], column=note_info[2])
663+
context.end_line = note_info[3]
664+
context.end_column = note_info[4]
665+
self.note(f'Revealed type is "{revealed}"', context)
645666

646667
# If exit_condition is set, assume it must be False on exit from the loop:
647668
if exit_condition:

mypy/errors.py

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from collections import defaultdict
77
from collections.abc import Iterable
88
from typing import Callable, Final, NoReturn, Optional, TextIO, TypeVar
9-
from typing_extensions import Literal, TypeAlias as _TypeAlias
9+
from typing_extensions import Literal, Self, TypeAlias as _TypeAlias
1010

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

182-
def __enter__(self) -> ErrorWatcher:
182+
def __enter__(self) -> Self:
183183
self.errors._watchers.append(self)
184184
return self
185185

@@ -220,6 +220,60 @@ def filtered_errors(self) -> list[ErrorInfo]:
220220
return self._filtered
221221

222222

223+
class LoopErrorWatcher(ErrorWatcher):
224+
"""Error watcher that filters and separately collects `unreachable` errors,
225+
`redundant-expr` and `redundant-casts` errors, and revealed types when analysing
226+
loops iteratively to help avoid making too-hasty reports."""
227+
228+
# Meaning of the tuple items: ErrorCode, message, line, column, end_line, end_column:
229+
uselessness_errors: set[tuple[ErrorCode, str, int, int, int, int]]
230+
231+
# Meaning of the tuple items: function_or_member, line, column, end_line, end_column:
232+
revealed_types: dict[tuple[str | None, int, int, int, int], set[str]]
233+
234+
# Not only the lines where the error report occurs but really all unreachable lines:
235+
unreachable_lines: set[int]
236+
237+
def __init__(
238+
self,
239+
errors: Errors,
240+
*,
241+
filter_errors: bool | Callable[[str, ErrorInfo], bool] = False,
242+
save_filtered_errors: bool = False,
243+
filter_deprecated: bool = False,
244+
) -> None:
245+
super().__init__(
246+
errors,
247+
filter_errors=filter_errors,
248+
save_filtered_errors=save_filtered_errors,
249+
filter_deprecated=filter_deprecated,
250+
)
251+
self.uselessness_errors = set()
252+
self.unreachable_lines = set()
253+
self.revealed_types = defaultdict(set)
254+
255+
def on_error(self, file: str, info: ErrorInfo) -> bool:
256+
257+
if info.code in (codes.UNREACHABLE, codes.REDUNDANT_EXPR, codes.REDUNDANT_CAST):
258+
self.uselessness_errors.add(
259+
(info.code, info.message, info.line, info.column, info.end_line, info.end_column)
260+
)
261+
if info.code == codes.UNREACHABLE:
262+
self.unreachable_lines.update(range(info.line, info.end_line + 1))
263+
return True
264+
265+
if info.code == codes.MISC and info.message.startswith("Revealed type is "):
266+
key = info.function_or_member, info.line, info.column, info.end_line, info.end_column
267+
types = info.message.split('"')[1]
268+
if types.startswith("Union["):
269+
self.revealed_types[key].update(types[6:-1].split(", "))
270+
else:
271+
self.revealed_types[key].add(types)
272+
return True
273+
274+
return super().on_error(file, info)
275+
276+
223277
class Errors:
224278
"""Container for compile errors.
225279

test-data/unit/check-inference.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ for var2 in [g, h, i, j, k, l]:
343343
reveal_type(var2) # N: Revealed type is "Union[builtins.int, builtins.str]"
344344

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

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

@@ -1247,7 +1247,7 @@ class X(TypedDict):
12471247

12481248
x: X
12491249
for a in ("hourly", "daily"):
1250-
reveal_type(a) # N: Revealed type is "Union[Literal['hourly']?, Literal['daily']?]"
1250+
reveal_type(a) # N: Revealed type is "Union[Literal['daily']?, Literal['hourly']?]"
12511251
reveal_type(x[a]) # N: Revealed type is "builtins.int"
12521252
reveal_type(a.upper()) # N: Revealed type is "builtins.str"
12531253
c = a

test-data/unit/check-narrowing.test

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,8 +2346,7 @@ def f() -> bool: ...
23462346

23472347
y = None
23482348
while f():
2349-
reveal_type(y) # N: Revealed type is "None" \
2350-
# N: Revealed type is "Union[builtins.int, None]"
2349+
reveal_type(y) # N: Revealed type is "Union[builtins.int, None]"
23512350
y = 1
23522351
reveal_type(y) # N: Revealed type is "Union[builtins.int, None]"
23532352

@@ -2370,7 +2369,42 @@ class A:
23702369

23712370
[builtins fixtures/primitives.pyi]
23722371

2373-
[case testAvoidFalseUnreachableInLoop]
2372+
[case testPersistentUnreachableLinesNestedInInpersistentUnreachableLines]
2373+
# flags: --warn-unreachable --python-version 3.11
2374+
2375+
x = None
2376+
y = None
2377+
while True:
2378+
if x is not None:
2379+
if y is not None:
2380+
reveal_type(y) # E: Statement is unreachable
2381+
x = 1
2382+
2383+
[builtins fixtures/bool.pyi]
2384+
2385+
[case testAvoidFalseRedundantCastInLoops]
2386+
# flags: --warn-redundant-casts
2387+
2388+
from typing import Callable, cast, Union
2389+
2390+
ProcessorReturnValue = Union[str, int]
2391+
Processor = Callable[[str], ProcessorReturnValue]
2392+
2393+
def main_cast(p: Processor) -> None:
2394+
ed: ProcessorReturnValue
2395+
ed = cast(str, ...)
2396+
while True:
2397+
ed = p(cast(str, ed))
2398+
2399+
def main_no_cast(p: Processor) -> None:
2400+
ed: ProcessorReturnValue
2401+
ed = cast(str, ...)
2402+
while True:
2403+
ed = p(ed) # E: Argument 1 has incompatible type "Union[str, int]"; expected "str"
2404+
2405+
[builtins fixtures/bool.pyi]
2406+
2407+
[case testAvoidFalseUnreachableInLoop1]
23742408
# flags: --warn-unreachable --python-version 3.11
23752409

23762410
def f() -> int | None: ...
@@ -2383,6 +2417,29 @@ while x is not None or b():
23832417

23842418
[builtins fixtures/bool.pyi]
23852419

2420+
[case testAvoidFalseUnreachableInLoop2]
2421+
# flags: --warn-unreachable --python-version 3.11
2422+
2423+
y = None
2424+
while y is None:
2425+
if y is None:
2426+
y = []
2427+
y.append(1)
2428+
2429+
[builtins fixtures/list.pyi]
2430+
2431+
[case testAvoidFalseUnreachableInLoop3]
2432+
# flags: --warn-unreachable --python-version 3.11
2433+
2434+
xs: list[int | None]
2435+
y = None
2436+
for x in xs:
2437+
if x is not None:
2438+
if y is None:
2439+
y = {} # E: Need type annotation for "y" (hint: "y: Dict[<type>, <type>] = ...")
2440+
2441+
[builtins fixtures/list.pyi]
2442+
23862443
[case testAvoidFalseRedundantExprInLoop]
23872444
# flags: --enable-error-code redundant-expr --python-version 3.11
23882445

test-data/unit/check-redefine2.test

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -628,8 +628,7 @@ def f1() -> None:
628628
def f2() -> None:
629629
x = None
630630
while int():
631-
reveal_type(x) # N: Revealed type is "None" \
632-
# N: Revealed type is "Union[None, builtins.str]"
631+
reveal_type(x) # N: Revealed type is "Union[builtins.str, None]"
633632
if int():
634633
x = ""
635634
reveal_type(x) # N: Revealed type is "Union[None, builtins.str]"
@@ -709,8 +708,7 @@ def b() -> None:
709708
def c() -> None:
710709
x = 0
711710
while int():
712-
reveal_type(x) # N: Revealed type is "builtins.int" \
713-
# N: Revealed type is "Union[builtins.int, builtins.str, None]"
711+
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]"
714712
if int():
715713
x = ""
716714
continue
@@ -810,8 +808,7 @@ def f4() -> None:
810808
x = None
811809
break
812810
finally:
813-
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]" \
814-
# N: Revealed type is "Union[builtins.int, None]"
811+
reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]"
815812
reveal_type(x) # N: Revealed type is "Union[builtins.int, None]"
816813
[builtins fixtures/exception.pyi]
817814

@@ -927,7 +924,7 @@ class X(TypedDict):
927924

928925
x: X
929926
for a in ("hourly", "daily"):
930-
reveal_type(a) # N: Revealed type is "Union[Literal['hourly']?, Literal['daily']?]"
927+
reveal_type(a) # N: Revealed type is "Union[Literal['daily']?, Literal['hourly']?]"
931928
reveal_type(x[a]) # N: Revealed type is "builtins.int"
932929
reveal_type(a.upper()) # N: Revealed type is "builtins.str"
933930
c = a

test-data/unit/check-typevar-tuple.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -989,7 +989,7 @@ from typing_extensions import Unpack
989989

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

995995
[case testFixedUnpackItemInInstanceArguments]

0 commit comments

Comments
 (0)