From 71a3b280aa7d0f4d44ec642ddd29048b50c85c75 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 10 Jun 2025 07:09:11 +0200 Subject: [PATCH 1/5] Improve the handling of "iteration dependend" errors and notes in finally clauses. --- mypy/checker.py | 66 +++++++++----------- mypy/errors.py | 94 +++++++++++++++++++++++------ test-data/unit/check-narrowing.test | 19 ++++++ test-data/unit/check-redefine2.test | 3 +- 4 files changed, 123 insertions(+), 59 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 6929543db24e..0d7455fbf99d 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -25,7 +25,13 @@ 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, LoopErrorWatcher, report_internal_error +from mypy.errors import ( + Errors, + ErrorWatcher, + IterationErrorWatcher, + ÎterationDependentErrors, + 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 @@ -599,26 +605,15 @@ def accept_loop( # on without bound otherwise) widened_old = len(self.widened_vars) - # one set of `unreachable`, `redundant-expr`, and `redundant-casts` errors - # per iteration step: - uselessness_errors = [] - # one set of unreachable line numbers per iteration step: - unreachable_lines = [] - # one set of revealed types per line where `reveal_type` is used (each - # created set can grow during the iteration): - revealed_types = defaultdict(set) + iter_errors = ÎterationDependentErrors() 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() - with LoopErrorWatcher(self.msg.errors) as watcher: + with IterationErrorWatcher(self.msg.errors, iter_errors) as watcher: self.accept(body) - uselessness_errors.append(watcher.uselessness_errors) - unreachable_lines.append(watcher.unreachable_lines) - for key, values in watcher.revealed_types.items(): - revealed_types[key].update(values) partials_new = sum(len(pts.map) for pts in self.partial_types) widened_new = len(self.widened_vars) @@ -640,29 +635,10 @@ def accept_loop( if iter == 20: raise RuntimeError("Too many iterations when checking a loop") - # Report only those `unreachable`, `redundant-expr`, and `redundant-casts` - # errors that could not be ruled out in any iteration step: - persistent_uselessness_errors = set() - for candidate in set(itertools.chain(*uselessness_errors)): - if all( - (candidate in errors) or (candidate[2] in lines) - for errors, lines in zip(uselessness_errors, unreachable_lines) - ): - persistent_uselessness_errors.add(candidate) - for error_info in persistent_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 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) + for error_info in watcher.yield_error_infos(): + self.msg.fail(*error_info[:2], code=error_info[2]) + for note_info in watcher.yield_note_infos(): + self.note(*note_info) # If exit_condition is set, assume it must be False on exit from the loop: if exit_condition: @@ -4974,6 +4950,9 @@ def type_check_raise(self, e: Expression, s: RaiseStmt, optional: bool = False) def visit_try_stmt(self, s: TryStmt) -> None: """Type check a try statement.""" + + iter_errors = None + # Our enclosing frame will get the result if the try/except falls through. # This one gets all possible states after the try block exited abnormally # (by exception, return, break, etc.) @@ -4988,7 +4967,9 @@ def visit_try_stmt(self, s: TryStmt) -> None: self.visit_try_without_finally(s, try_frame=bool(s.finally_body)) if s.finally_body: # First we check finally_body is type safe on all abnormal exit paths - self.accept(s.finally_body) + iter_errors = ÎterationDependentErrors() + with IterationErrorWatcher(self.msg.errors, iter_errors) as watcher: + self.accept(s.finally_body) if s.finally_body: # Then we try again for the more restricted set of options @@ -5002,8 +4983,15 @@ def visit_try_stmt(self, s: TryStmt) -> None: # type checks in both contexts, but only the resulting types # from the latter context affect the type state in the code # that follows the try statement.) + assert iter_errors is not None if not self.binder.is_unreachable(): - self.accept(s.finally_body) + with IterationErrorWatcher(self.msg.errors, iter_errors) as watcher: + self.accept(s.finally_body) + + for error_info in watcher.yield_error_infos(): + self.msg.fail(*error_info[:2], code=error_info[2]) + for note_info in watcher.yield_note_infos(): + self.msg.note(*note_info) def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None: """Type check a try statement, ignoring the finally block. diff --git a/mypy/errors.py b/mypy/errors.py index 6aa19ed7c5a0..42cc8ed8e3d6 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -5,12 +5,14 @@ import traceback from collections import defaultdict from collections.abc import Iterable -from typing import Callable, Final, NoReturn, Optional, TextIO, TypeVar -from typing_extensions import Literal, Self, TypeAlias as _TypeAlias +from itertools import chain +from typing import Callable, Final, Iterator, NoReturn, Optional, TextIO, TypeVar +from typing_extensions import Literal, NamedTuple, Self, TypeAlias as _TypeAlias from mypy import errorcodes as codes from mypy.error_formatter import ErrorFormatter from mypy.errorcodes import IMPORT, IMPORT_NOT_FOUND, IMPORT_UNTYPED, ErrorCode, mypy_error_codes +from mypy.nodes import Context from mypy.options import Options from mypy.scope import Scope from mypy.util import DEFAULT_SOURCE_OFFSET, is_typeshed_file @@ -220,23 +222,44 @@ def filtered_errors(self) -> list[ErrorInfo]: return self._filtered -class LoopErrorWatcher(ErrorWatcher): - """Error watcher that filters and separately collects `unreachable` errors, - `redundant-expr` and `redundant-casts` 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: - uselessness_errors: set[tuple[ErrorCode, str, int, int, int, int]] +class ÎterationDependentErrors: + """An `ÎterationDependentErrors` instance serves to collect the `unreachable`, + `redundant-expr`, and `redundant-casts` errors, as well as the revealed types, + handled by the individual `IterationErrorWatcher` instances sequentially applied to + the same code section.""" + + # One set of `unreachable`, `redundant-expr`, and `redundant-casts` errors per + # iteration step. Meaning of the tuple items: ErrorCode, message, line, column, + # end_line, end_column. + uselessness_errors: list[set[tuple[ErrorCode, str, int, int, int, int]]] + + # One set of unreachable line numbers per iteration step. Not only the lines where + # the error report occurs but really all unreachable lines. + unreachable_lines: list[set[int]] - # Meaning of the tuple items: function_or_member, line, column, end_line, end_column: + # One set of revealed types for each `reveal_type` statement. Each created set can + # grow during the iteration. 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]] - # Not only the lines where the error report occurs but really all unreachable lines: - unreachable_lines: set[int] + def __init__(self) -> None: + self.uselessness_errors = [] + self.unreachable_lines = [] + self.revealed_types = defaultdict(set) + + +class IterationErrorWatcher(ErrorWatcher): + """Error watcher that filters and separately collects `unreachable` errors, + `redundant-expr` and `redundant-casts` errors, and revealed types when analysing + code sections iteratively to help avoid making too-hasty reports.""" + + iteration_dependent_errors: ÎterationDependentErrors def __init__( self, errors: Errors, + iteration_dependent_errors: ÎterationDependentErrors, *, filter_errors: bool | Callable[[str, ErrorInfo], bool] = False, save_filtered_errors: bool = False, @@ -248,31 +271,66 @@ def __init__( save_filtered_errors=save_filtered_errors, filter_deprecated=filter_deprecated, ) - self.uselessness_errors = set() - self.unreachable_lines = set() - self.revealed_types = defaultdict(set) + self.iteration_dependent_errors = iteration_dependent_errors + iteration_dependent_errors.uselessness_errors.append(set()) + iteration_dependent_errors.unreachable_lines.append(set()) def on_error(self, file: str, info: ErrorInfo) -> bool: + """Filter out the "iteration-dependent" errors and notes and store their + information to handle them after iteration is completed.""" + + iter_errors = self.iteration_dependent_errors if info.code in (codes.UNREACHABLE, codes.REDUNDANT_EXPR, codes.REDUNDANT_CAST): - self.uselessness_errors.add( + iter_errors.uselessness_errors[-1].add( (info.code, info.message, info.line, info.column, info.end_line, info.end_column) ) if info.code == codes.UNREACHABLE: - self.unreachable_lines.update(range(info.line, info.end_line + 1)) + iter_errors.unreachable_lines[-1].update(range(info.line, info.end_line + 1)) 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(", ")) + iter_errors.revealed_types[key].update(types[6:-1].split(", ")) else: - self.revealed_types[key].add(types) + iter_errors.revealed_types[key].add(types) return True return super().on_error(file, info) + def yield_error_infos(self) -> Iterator[tuple[str, Context, ErrorCode]]: + """Report only those `unreachable`, `redundant-expr`, and `redundant-casts` + errors that could not be ruled out in any iteration step.""" + + persistent_uselessness_errors = set() + iter_errors = self.iteration_dependent_errors + for candidate in set(chain(*iter_errors.uselessness_errors)): + if all( + (candidate in errors) or (candidate[2] in lines) + for errors, lines in zip( + iter_errors.uselessness_errors, iter_errors.unreachable_lines + ) + ): + persistent_uselessness_errors.add(candidate) + for error_info in persistent_uselessness_errors: + context = Context(line=error_info[2], column=error_info[3]) + context.end_line = error_info[4] + context.end_column = error_info[5] + yield error_info[1], context, error_info[0] + + def yield_note_infos(self) -> Iterator[tuple[str, Context]]: + """Yield all types revealed in at least one iteration step.""" + + for note_info, types in self.iteration_dependent_errors.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] + yield f'Revealed type is "{revealed}"', context + class Errors: """Container for compile errors. diff --git a/test-data/unit/check-narrowing.test b/test-data/unit/check-narrowing.test index 47ad62248fe0..b4bde8d5d68e 100644 --- a/test-data/unit/check-narrowing.test +++ b/test-data/unit/check-narrowing.test @@ -2453,6 +2453,25 @@ while x is not None and b(): [builtins fixtures/primitives.pyi] +[case testAvoidFalseUnreachableInFinally] +# flags: --allow-redefinition-new --local-partial-types --warn-unreachable +def f() -> None: + try: + x = 1 + if int(): + x = "" + return + if int(): + x = None + return + finally: + reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]" + if isinstance(x, str): + reveal_type(x) # N: Revealed type is "builtins.str" + reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str, None]" + +[builtins fixtures/isinstancelist.pyi] + [case testNarrowingTypeVarMultiple] from typing import TypeVar diff --git a/test-data/unit/check-redefine2.test b/test-data/unit/check-redefine2.test index 1062be6976c0..924e66584669 100644 --- a/test-data/unit/check-redefine2.test +++ b/test-data/unit/check-redefine2.test @@ -791,8 +791,7 @@ def f3() -> None: x = "" return finally: - reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str]" \ - # N: Revealed type is "builtins.int" + reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str]" reveal_type(x) # N: Revealed type is "builtins.int" def f4() -> None: From fc1c79075d1957d869b620fe3b3d9d7df99e9148 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 10 Jun 2025 15:34:29 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- mypy/errors.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mypy/errors.py b/mypy/errors.py index 42cc8ed8e3d6..48ea5a17d3b1 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -4,10 +4,10 @@ import sys import traceback from collections import defaultdict -from collections.abc import Iterable +from collections.abc import Iterable, Iterator from itertools import chain -from typing import Callable, Final, Iterator, NoReturn, Optional, TextIO, TypeVar -from typing_extensions import Literal, NamedTuple, Self, TypeAlias as _TypeAlias +from typing import Callable, Final, NoReturn, Optional, TextIO, TypeVar +from typing_extensions import Literal, Self, TypeAlias as _TypeAlias from mypy import errorcodes as codes from mypy.error_formatter import ErrorFormatter @@ -222,7 +222,6 @@ def filtered_errors(self) -> list[ErrorInfo]: return self._filtered - class ÎterationDependentErrors: """An `ÎterationDependentErrors` instance serves to collect the `unreachable`, `redundant-expr`, and `redundant-casts` errors, as well as the revealed types, From 2ddab269cd9dd7a6c447c956fd90d781d79755e7 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Tue, 10 Jun 2025 19:10:33 +0200 Subject: [PATCH 3/5] fix typo --- mypy/checker.py | 6 +++--- mypy/errors.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 58b88b94c6b2..2cd23de14edf 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -30,7 +30,7 @@ Errors, ErrorWatcher, IterationErrorWatcher, - ÎterationDependentErrors, + IterationDependentErrors, report_internal_error, ) from mypy.expandtype import expand_type @@ -605,7 +605,7 @@ def accept_loop( # on without bound otherwise) widened_old = len(self.widened_vars) - iter_errors = ÎterationDependentErrors() + iter_errors = IterationDependentErrors() iter = 1 while True: with self.binder.frame_context(can_skip=True, break_frame=2, continue_frame=1): @@ -4942,7 +4942,7 @@ def visit_try_stmt(self, s: TryStmt) -> None: self.visit_try_without_finally(s, try_frame=bool(s.finally_body)) if s.finally_body: # First we check finally_body is type safe on all abnormal exit paths - iter_errors = ÎterationDependentErrors() + iter_errors = IterationDependentErrors() with IterationErrorWatcher(self.msg.errors, iter_errors) as watcher: self.accept(s.finally_body) diff --git a/mypy/errors.py b/mypy/errors.py index ba978aaf61bd..30e99286bb47 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -221,8 +221,8 @@ def filtered_errors(self) -> list[ErrorInfo]: return self._filtered -class ÎterationDependentErrors: - """An `ÎterationDependentErrors` instance serves to collect the `unreachable`, +class IterationDependentErrors: + """An `IterationDependentErrors` instance serves to collect the `unreachable`, `redundant-expr`, and `redundant-casts` errors, as well as the revealed types, handled by the individual `IterationErrorWatcher` instances sequentially applied to the same code section.""" @@ -252,12 +252,12 @@ class IterationErrorWatcher(ErrorWatcher): `redundant-expr` and `redundant-casts` errors, and revealed types when analysing code sections iteratively to help avoid making too-hasty reports.""" - iteration_dependent_errors: ÎterationDependentErrors + iteration_dependent_errors: IterationDependentErrors def __init__( self, errors: Errors, - iteration_dependent_errors: ÎterationDependentErrors, + iteration_dependent_errors: IterationDependentErrors, *, filter_errors: bool | Callable[[str, ErrorInfo], bool] = False, save_filtered_errors: bool = False, From 0b6f6b96eba700315e6aa734787ddd1538c382ca Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 10 Jun 2025 17:12:31 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 2cd23de14edf..1a24597c163b 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -29,8 +29,8 @@ ErrorInfo, Errors, ErrorWatcher, - IterationErrorWatcher, IterationDependentErrors, + IterationErrorWatcher, report_internal_error, ) from mypy.expandtype import expand_type From af8b672ab2a89630be56c1494bea0877cad7c507 Mon Sep 17 00:00:00 2001 From: Christoph Tyralla Date: Fri, 20 Jun 2025 08:11:11 +0200 Subject: [PATCH 5/5] Support the "or syntax" for revealed unions. --- mypy/checker.py | 4 ++-- mypy/errors.py | 9 +++++++-- test-data/unit/check-union-error-syntax.test | 21 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index c79b4383f18e..f929178e374e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -637,7 +637,7 @@ def accept_loop( for error_info in watcher.yield_error_infos(): self.msg.fail(*error_info[:2], code=error_info[2]) - for note_info in watcher.yield_note_infos(): + for note_info in watcher.yield_note_infos(self.options): self.note(*note_info) # If exit_condition is set, assume it must be False on exit from the loop: @@ -4965,7 +4965,7 @@ def visit_try_stmt(self, s: TryStmt) -> None: for error_info in watcher.yield_error_infos(): self.msg.fail(*error_info[:2], code=error_info[2]) - for note_info in watcher.yield_note_infos(): + for note_info in watcher.yield_note_infos(self.options): self.msg.note(*note_info) def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None: diff --git a/mypy/errors.py b/mypy/errors.py index 30e99286bb47..41a4de639236 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -318,12 +318,17 @@ def yield_error_infos(self) -> Iterator[tuple[str, Context, ErrorCode]]: context.end_column = error_info[5] yield error_info[1], context, error_info[0] - def yield_note_infos(self) -> Iterator[tuple[str, Context]]: + def yield_note_infos(self, options: Options) -> Iterator[tuple[str, Context]]: """Yield all types revealed in at least one iteration step.""" for note_info, types in self.iteration_dependent_errors.revealed_types.items(): sorted_ = sorted(types, key=lambda typ: typ.lower()) - revealed = sorted_[0] if len(types) == 1 else f"Union[{', '.join(sorted_)}]" + if len(types) == 1: + revealed = sorted_[0] + elif options.use_or_syntax(): + revealed = " | ".join(sorted_) + else: + revealed = 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] diff --git a/test-data/unit/check-union-error-syntax.test b/test-data/unit/check-union-error-syntax.test index 3c541173a891..d41281b774e1 100644 --- a/test-data/unit/check-union-error-syntax.test +++ b/test-data/unit/check-union-error-syntax.test @@ -55,3 +55,24 @@ from typing import Literal, Union x : Union[Literal[1], None] x = 3 # E: Incompatible types in assignment (expression has type "Literal[3]", variable has type "Optional[Literal[1]]") [builtins fixtures/tuple.pyi] + +[case testUnionSyntaxRecombined] +# flags: --python-version 3.10 --force-union-syntax --allow-redefinition-new --local-partial-types +# The following revealed type is recombined because the finally body is visited twice. +try: + x = 1 + x = "" +finally: + reveal_type(x) # N: Revealed type is "Union[builtins.int, builtins.str]" +[builtins fixtures/isinstancelist.pyi] + +[case testOrSyntaxRecombined] +# flags: --python-version 3.10 --no-force-union-syntax --allow-redefinition-new --local-partial-types +# The following revealed type is recombined because the finally body is visited twice. +# ToDo: Improve this recombination logic, especially (but not only) for the "or syntax". +try: + x = 1 + x = "" +finally: + reveal_type(x) # N: Revealed type is "builtins.int | builtins.str | builtins.str" +[builtins fixtures/isinstancelist.pyi]