Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
99725c9
Fix `--strict-equality` for iteratively visited code.
tyralla Aug 10, 2025
016eda6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 10, 2025
2182e5d
add testAvoidFalseNonOverlappingEqualityCheckInLoop3
tyralla Aug 10, 2025
edf63d7
respect that other error watcher may want to filter comparison-overla…
tyralla Aug 11, 2025
6c3b489
respect that other error watcher may want to filter comparison-overla…
tyralla Aug 11, 2025
ab4d567
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Aug 23, 2025
9842064
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Sep 7, 2025
1d69935
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Oct 9, 2025
d3b9ebd
remove unnecessary blank line
tyralla Oct 31, 2025
d1ba2c6
refactor: introduce the named tuple `NonOverlapErrorInfo`
tyralla Nov 15, 2025
fc00f9c
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Nov 15, 2025
b40c1e6
fix last commit
tyralla Nov 15, 2025
4e474cb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 15, 2025
f741645
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Nov 24, 2025
ee852fd
Merge branch 'master' into fix_strict_quality_in_loops
hauntsaninja Nov 28, 2025
f00e86c
fix typo
tyralla Dec 4, 2025
189f8b3
`from_iterable` instead of `*`
tyralla Dec 4, 2025
b4455ff
Merge branch 'master' into fix_strict_quality_in_loops
tyralla Dec 5, 2025
ef9f650
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 5, 2025
129ff47
fix merge
tyralla Dec 5, 2025
02dfe62
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 5, 2025
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
56 changes: 50 additions & 6 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 Callable, Iterable, Iterator
from itertools import chain
from typing import Final, Literal, NoReturn, TextIO, TypeAlias as _TypeAlias, TypeVar
from typing import Final, Literal, NamedTuple, NoReturn, TextIO, TypeAlias as _TypeAlias, TypeVar
from typing_extensions import Self

from mypy import errorcodes as codes
Expand Down Expand Up @@ -229,11 +229,19 @@ def filtered_errors(self) -> list[ErrorInfo]:
return self._filtered


class NonOverlapErrorInfo(NamedTuple):
line: int
column: int
end_line: int | None
end_column: int | None
kind: str


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."""
`redundant-expr`, and `redundant-casts` errors, as well as the revealed types and
non-overlapping 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,
Expand All @@ -249,9 +257,13 @@ class IterationDependentErrors:
# end_line, end_column:
revealed_types: dict[tuple[int, int, int | None, int | None], list[Type]]

# One dictionary of non-overlapping types per iteration step:
nonoverlapping_types: list[dict[NonOverlapErrorInfo, tuple[Type, Type]]]

def __init__(self) -> None:
self.uselessness_errors = []
self.unreachable_lines = []
self.nonoverlapping_types = []
self.revealed_types = defaultdict(list)

def yield_uselessness_error_infos(self) -> Iterator[tuple[str, Context, ErrorCode]]:
Expand All @@ -271,6 +283,36 @@ def yield_uselessness_error_infos(self) -> Iterator[tuple[str, Context, ErrorCod
context.end_column = error_info[5]
yield error_info[1], context, error_info[0]

def yield_nonoverlapping_types(
self,
) -> Iterator[tuple[tuple[list[Type], list[Type]], str, Context]]:
"""Report expressions where non-overlapping types were detected for all iterations
were the expression was reachable."""

selected = set()
for candidate in set(chain.from_iterable(self.nonoverlapping_types)):
if all(
(candidate in nonoverlap) or (candidate.line in lines)
for nonoverlap, lines in zip(self.nonoverlapping_types, self.unreachable_lines)
):
selected.add(candidate)

persistent_nonoverlaps: dict[NonOverlapErrorInfo, tuple[list[Type], list[Type]]] = (
defaultdict(lambda: ([], []))
)
for nonoverlaps in self.nonoverlapping_types:
for candidate, (left, right) in nonoverlaps.items():
Copy link
Collaborator

Choose a reason for hiding this comment

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

selected is a strict subset of nonoverlaps, right? How big is it usually, relative to nonoverlaps size? If it normally includes a relatively small portion of those, it might be faster to swap these checks (iterate over selected and do left, right = nonoverlaps[candidate] for each).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

selected is only a subset of each nonoverlaps item if the relevant lines are reachable in all iterations. So, I am not really sure about the potential benefits of such a switch. However, maybe performance isn't overly critical here, since the loop's body is only executed if there is, in fact, an overlap issue to report?

if candidate in selected:
types = persistent_nonoverlaps[candidate]
types[0].append(left)
types[1].append(right)

for error_info, types in persistent_nonoverlaps.items():
context = Context(line=error_info.line, column=error_info.column)
context.end_line = error_info.end_line
context.end_column = error_info.end_column
yield (types[0], types[1]), error_info.kind, context

def yield_revealed_type_infos(self) -> Iterator[tuple[list[Type], Context]]:
"""Yield all types revealed in at least one iteration step."""

Expand All @@ -283,8 +325,9 @@ def yield_revealed_type_infos(self) -> Iterator[tuple[list[Type], Context]]:

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."""
`redundant-expr` and `redundant-casts` errors, and revealed types and
non-overlapping types when analysing code sections iteratively to help avoid
making too-hasty reports."""

iteration_dependent_errors: IterationDependentErrors

Expand All @@ -305,6 +348,7 @@ def __init__(
)
self.iteration_dependent_errors = iteration_dependent_errors
iteration_dependent_errors.uselessness_errors.append(set())
iteration_dependent_errors.nonoverlapping_types.append({})
iteration_dependent_errors.unreachable_lines.append(set())

def on_error(self, file: str, info: ErrorInfo) -> bool:
Expand Down
26 changes: 25 additions & 1 deletion mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
ErrorWatcher,
IterationDependentErrors,
IterationErrorWatcher,
NonOverlapErrorInfo,
)
from mypy.nodes import (
ARG_NAMED,
Expand Down Expand Up @@ -1624,6 +1625,26 @@ def incompatible_typevar_value(
)

def dangerous_comparison(self, left: Type, right: Type, kind: str, ctx: Context) -> None:
# In loops (and similar cases), the same expression might be analysed multiple
# times and thereby confronted with different types. We only want to raise a
# `comparison-overlap` error if it occurs in all cases and therefore collect the
# respective types of the current iteration here so that we can report the error
# later if it is persistent over all iteration steps:
for watcher in self.errors.get_watchers():
if watcher._filter:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move this logic into the watcher itself? This access to private followed by special-casing on watcher type is a bit painful to read, IMO this would be clearer as

for watcher in self.errors.get_watchers():
    if watcher.store_nonoverlapping_types(ctx, kind, left, right):
        return

Where store_nonoverlapping_types (not the best name) is a no-op in ErrorWatcher, overridden with this block in IterationErrorWatcher

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ough, you really need both break and return, sorry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to excuse. I am obviously using things a little differently from how they were originally intended. Hence, I highly appreciate any thoughts on improving readability.

break
if isinstance(watcher, IterationErrorWatcher):
watcher.iteration_dependent_errors.nonoverlapping_types[-1][
NonOverlapErrorInfo(
line=ctx.line,
column=ctx.column,
end_line=ctx.end_line,
end_column=ctx.end_column,
kind=kind,
)
] = (left, right)
return

left_str = "element" if kind == "container" else "left operand"
right_str = "container item" if kind == "container" else "right operand"
message = "Non-overlapping {} check ({} type: {}, {} type: {})"
Expand Down Expand Up @@ -2513,8 +2534,11 @@ def match_statement_inexhaustive_match(self, typ: Type, context: Context) -> Non
def iteration_dependent_errors(self, iter_errors: IterationDependentErrors) -> None:
for error_info in iter_errors.yield_uselessness_error_infos():
self.fail(*error_info[:2], code=error_info[2])
msu = mypy.typeops.make_simplified_union
for nonoverlaps, kind, context in iter_errors.yield_nonoverlapping_types():
self.dangerous_comparison(msu(nonoverlaps[0]), msu(nonoverlaps[1]), kind, context)
for types, context in iter_errors.yield_revealed_type_infos():
self.reveal_type(mypy.typeops.make_simplified_union(types), context)
self.reveal_type(msu(types), context)


def quote_type_string(type_string: str) -> str:
Expand Down
35 changes: 35 additions & 0 deletions test-data/unit/check-narrowing.test
Original file line number Diff line number Diff line change
Expand Up @@ -2446,6 +2446,41 @@ while x is not None and b():
x = f()
[builtins fixtures/primitives.pyi]

[case testAvoidFalseNonOverlappingEqualityCheckInLoop1]
# flags: --allow-redefinition-new --local-partial-types --strict-equality

x = 1
while True:
if x == str():
break
x = str()
if x == int(): # E: Non-overlapping equality check (left operand type: "str", right operand type: "int")
break
[builtins fixtures/primitives.pyi]

[case testAvoidFalseNonOverlappingEqualityCheckInLoop2]
# flags: --allow-redefinition-new --local-partial-types --strict-equality

class A: ...
class B: ...
class C: ...

x = A()
while True:
if x == C(): # E: Non-overlapping equality check (left operand type: "Union[A, B]", right operand type: "C")
break
x = B()
[builtins fixtures/primitives.pyi]

[case testAvoidFalseNonOverlappingEqualityCheckInLoop3]
# flags: --strict-equality

for y in [1.0]:
if y is not None or y != "None":
...

[builtins fixtures/primitives.pyi]

[case testNarrowPromotionsInsideUnions1]

from typing import Union
Expand Down