Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -6674,8 +6674,8 @@ def comparison_type_narrowing_helper(self, node: ComparisonExpr) -> tuple[TypeMa
# If we have found non-trivial restrictions from the regular comparisons,
# then return soon. Otherwise try to infer restrictions involving `len(x)`.
# TODO: support regular and len() narrowing in the same chain.
if any(m != ({}, {}) for m in partial_type_maps):
return reduce_conditional_maps(partial_type_maps)
if any(len(m[0]) or len(m[1]) for m in partial_type_maps):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TBH I don't understand how is this different from what it was before. Is this just some drive-by performance micro-optimization?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, just a micro optimisation

return reduce_conditional_maps(partial_type_maps, use_meet=True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am in favour of this, but just as a warning, this may have some subtle side-effect w.r.t. Any, so you may want to add some tests involving Any as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added some tests, but couldn't find one with a behaviour change. If there is one, I think it should result in fewer Any's, and we don't see anything on primer, so that's good

else:
# Use meet for `and` maps to get correct results for chained checks
# like `if 1 < len(x) < 4: ...`
Expand Down
14 changes: 14 additions & 0 deletions test-data/unit/check-narrowing.test
Original file line number Diff line number Diff line change
Expand Up @@ -3264,6 +3264,20 @@ def bad_but_should_pass(has_key: bool, key: bool, s: tuple[bool, ...]) -> None:
reveal_type(key) # N: Revealed type is "builtins.bool"
[builtins fixtures/primitives.pyi]

[case testNarrowChainedComparisonMeet]
# flags: --strict-equality --warn-unreachable
from __future__ import annotations

def f1(a: str | None, b: str | None) -> None:
if None is not a == b:
reveal_type(a) # N: Revealed type is "builtins.str"
reveal_type(b) # N: Revealed type is "builtins.str | None"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should also be str. I will fix this in a subsequent PR (this one is not a regression, it will need net new code)


if (None is not a) and (a == b):
reveal_type(a) # N: Revealed type is "builtins.str"
reveal_type(b) # N: Revealed type is "builtins.str"
[builtins fixtures/primitives.pyi]

[case testNarrowTypeObject]
# flags: --strict-equality --warn-unreachable
from typing import Any
Expand Down
Loading