Rework narrowing logic for equality and identity #20492

hauntsaninja · 2025-12-30T09:46:32Z

Mypy does not narrow as much as it could, which results in false positives.

We would also like to narrow based on containment. The PR for that was previously reverted due to inconsistencies between narrowing via equality and via containment. This fixes the inconsistency on the equality side and paves the road for adding narrowing via containment. That is, we lay groundwork for fixing #17864 and fixing #17841

Fixes #18524
Fixes #20041
Fixes #17162
Fixes #16830
Fixes #13704
Fixes #7642
Fixes #3964

hauntsaninja · 2025-12-31T05:02:35Z

Primer is looking pretty good, with a few things I think maybe worth exploring for future PR (e.g. TypeType handling, the schemathesis / zulip false positives).

My laptop is pretty noisy, this was maybe 1.5% hit on self check. I will explore optimisations once this is merged (edit: I think on my latest dev branch that contains the whole series of changes it is down to within noise)

Changes I have ready on top of this PR:

A pure refactor, that makes some of this code easier to follow
Narrow types based on collection containment
Refactor and improve narrowing for type(x) == t
Avoid narrowing TypeType (this fixes 1-2 issues in the primer for this PR, but I think is worth looking at separately)
Improve narrowing logic for Enum int and str subclasses
Treat NotImplemented as a singleton

Things I want to explore after that are: literal coercion, checking unreachable code, narrow for __class__, some specific things like 1 2

This relates to changes made in python#20492

A5rocks · 2026-01-10T17:28:00Z

mypy/checker.py

                and not is_literal_none(expr)
+                and not is_literal_not_implemented(expr)
+                and not is_false_literal(expr)
+                and not is_true_literal(expr)


Shouldn't these be LITERAL_YES? I guess maybe they aren't now, but that seems like the right distinction.

Yeah, good point. Good thing to explore in a follow up PR

A5rocks · 2026-01-10T17:32:05Z

mypy/checker.py

-            operand_types,
-            expr_indices,
-            narrowable_operand_index_to_hash.keys(),
+            operator, operands, operand_types, expr_indices, narrowable_indices=narrowable_indices


Any deeper reason behind why specifically this arg and none of the others is consistently passed via kwarg?

No particularly deep reason

narrowable_operand_index_to_hash as a dict is much more complicated than narrowable_indices as a set, so I wanted to only have narrowable_operand_index_to_hash in the places you actually need a dict, so I did a little bit of renaming

A5rocks · 2026-01-10T17:33:17Z

mypy/checker.py

    ) -> tuple[TypeMap, TypeMap]:
        """Calculate type maps for '==', '!=', 'is' or 'is not' expression."""
        # If we haven't been able to narrow types yet, we might be dealing with a
        # explicit type(x) == some_type check


Isn't this comment in the wrong place/wrong now?

I guess so. One of the commits I have on top of this PR gets rid of this function entirely, so won't touch for now

A5rocks · 2026-01-10T17:39:22Z

mypy/checker.py

+        # custom __eq__. We should see if we can get rid of it.
+        return self.refine_away_none_in_comparison(
+            operands, operand_types, expr_indices, narrowable_indices
+        )


Is the reason this is still here because narrow_identity_equality_comparison doesn't happen for negative cases, even though it should? So something like #18574 would help?

We now hit this call much less and I want to see if we can get rid of it entirely. I need to look at primer for that

Yes, something with the effect of #18574 could still be useful! I think once I land my series of changes, the impl for it could look fairly clean...

Okay, I wrote a patch for this in my commit stack. I think it looks pretty promising (e.g. we have the fully ideal behaviour in testNarrowingEqualityDisabledForCustomEquality) and I think does let us get rid of refine_away_none_in_comparison, thanks for mentioning!

A5rocks · 2026-01-10T17:49:53Z

mypy/checker.py

+            for i in chain_indices:
+                if i not in narrowable_operand_indices:
+                    continue
+                for j, target in value_targets:


I guess one downside of this reworking is that this is O(n^2) now?

Yes, if you have a very long chained comparison (where a lot of the terms are narrowable and valid targets). Chained comparisons are pretty rare though.

A5rocks · 2026-01-10T17:52:05Z

mypy/checker.py

        Similarly, 'coerce_only_in_literal_context' controls whether we should try coercing
        expressions in the chain to a Literal type. Performing this coercion is sometimes
        too aggressive of a narrowing, depending on context.
        """


I'm a bit overwhelmed by the diff view for this function. What's the actual logic change?

Best to view this part as a rewrite, not a diff

Main logic change is more principled handling of "values" vs "types"
Previously if not is_valid_target this function didn't really do anything. Now we narrow, but only in positive case

(This also means we don't have to call refine_away_none_in_comparison anymore, which improves soundness, see the test cases with random_object)

I do have a follow up commit that improves readability here, but it does that by doing more refactoring. I wanted to keep that separate PR since it doesn't change behaviour, but would have made this diff harder to read

hauntsaninja

Thanks for taking a look, I responded to your comments

hauntsaninja · 2026-01-10T21:16:20Z

mypy/checker.py

                and not is_literal_none(expr)
+                and not is_literal_not_implemented(expr)
+                and not is_false_literal(expr)
+                and not is_true_literal(expr)


Yeah, good point. Good thing to explore in a follow up PR

hauntsaninja · 2026-01-10T21:18:29Z

mypy/checker.py

    ) -> tuple[TypeMap, TypeMap]:
        """Calculate type maps for '==', '!=', 'is' or 'is not' expression."""
        # If we haven't been able to narrow types yet, we might be dealing with a
        # explicit type(x) == some_type check


I guess so. One of the commits I have on top of this PR gets rid of this function entirely, so won't touch for now

hauntsaninja · 2026-01-10T21:20:56Z

mypy/checker.py

-            operand_types,
-            expr_indices,
-            narrowable_operand_index_to_hash.keys(),
+            operator, operands, operand_types, expr_indices, narrowable_indices=narrowable_indices


No particularly deep reason

narrowable_operand_index_to_hash as a dict is much more complicated than narrowable_indices as a set, so I wanted to only have narrowable_operand_index_to_hash in the places you actually need a dict, so I did a little bit of renaming

hauntsaninja · 2026-01-10T21:33:23Z

mypy/checker.py

+        # custom __eq__. We should see if we can get rid of it.
+        return self.refine_away_none_in_comparison(
+            operands, operand_types, expr_indices, narrowable_indices
+        )


We now hit this call much less and I want to see if we can get rid of it entirely. I need to look at primer for that

Yes, something with the effect of #18574 could still be useful! I think once I land my series of changes, the impl for it could look fairly clean...

hauntsaninja · 2026-01-10T21:45:11Z

mypy/checker.py

        Similarly, 'coerce_only_in_literal_context' controls whether we should try coercing
        expressions in the chain to a Literal type. Performing this coercion is sometimes
        too aggressive of a narrowing, depending on context.
        """


Best to view this part as a rewrite, not a diff

Main logic change is more principled handling of "values" vs "types"
Previously if not is_valid_target this function didn't really do anything. Now we narrow, but only in positive case

(This also means we don't have to call refine_away_none_in_comparison anymore, which improves soundness, see the test cases with random_object)

I do have a follow up commit that improves readability here, but it does that by doing more refactoring. I wanted to keep that separate PR since it doesn't change behaviour, but would have made this diff harder to read

hauntsaninja · 2026-01-10T21:47:08Z

mypy/checker.py

+            for i in chain_indices:
+                if i not in narrowable_operand_indices:
+                    continue
+                for j, target in value_targets:


Yes, if you have a very long chained comparison (where a lot of the terms are narrowable and valid targets). Chained comparisons are pretty rare though.

This relates to changes made in python#20492

JukkaL

Thanks, I'm glad to see so many bugs fixed! This is something I was planning to work by myself at some point but I got distracted. Left a few minor comments, looks great overall.

JukkaL · 2026-01-16T18:36:37Z

test-data/unit/check-python310.test

    match choice:
        case b.One:
-            reveal_type(choice)  # N: Revealed type is "type[T_Choice`-1]"
+            reveal_type(choice)  # N: Revealed type is "def () -> b.One"


Can choice be used as a type object when narrowed? Test this.

Will add! For context, this test case has flipped around a little recently: https://github.com/python/mypy/pull/20146/files#r2602384622

Opened #20598

JukkaL · 2026-01-16T19:01:42Z

mypy/checker.py

        )
-        if if_map == {} and else_map == {} and node is not None:
-            if_map, else_map = self.find_type_equals_check(node, expr_indices)
+        if node is not None:


node doesn't have an optional type, so is the condition always true?

This is a follow up to #20492 that is pure refactoring. I separated it out to make that change easier to review. We inline `narrow_identity_equality_comparison`, improve comments, etc. There is some further refactoring later in my commit stack too.

Requested in #20492 (comment) Regression for the Never's in #18524 (comment)

This relates to changes made in python#20492

cdce8p · 2026-01-17T11:46:30Z

Noticed a minor regression in Home Assistant with --warn-unreachable. Not sure if it's tracked already.

# mypy: warn-unreachable
from enum import IntFlag

class WeatherFeature(IntFlag):
    FORECAST_DAILY = 1
    FORECAST_HOURLY = 2
    FORECAST_TWICE_DAILY = 4

def func(supported_features: int) -> None:
    reveal_type(supported_features)
    if (supported_features & WeatherFeature.FORECAST_DAILY) == 0:
        print("Hello World")  # <-- This should work fine

note: Revealed type is "builtins.int"
error: Statement is unreachable  [unreachable]

hauntsaninja · 2026-01-17T20:21:37Z

Thanks, I have a follow up commit that will fix this case