Skip to content

Improve negative narrowing for literal expressions in containers - #21914

Open
Om-singhaI wants to merge 1 commit into
python:masterfrom
Om-singhaI:narrow-not-in-literal-container
Open

Improve negative narrowing for literal expressions in containers#21914
Om-singhaI wants to merge 1 commit into
python:masterfrom
Om-singhaI:narrow-not-in-literal-container

Conversation

@Om-singhaI

Copy link
Copy Markdown

A membership test against a container written out as a literal expression does not
narrow the negative branch:

from typing import Literal

def open_mode(mode: Literal["r", "w", "a"]) -> str:
    if mode in ("r", "w"):
        return "rw"
    reveal_type(mode)  # Literal['r'] | Literal['w'] | Literal['a']
    return "a"

With this change that reveal is Literal['a'].

The same container behind an annotation narrows fine, and so does the equivalent
!= chain, so today the result depends on how the container is spelled. Tuple,
list, set and dict literals all behave this way. The practical cost is that
assert_never over Literal alternatives is rejected when the branches are
written with in and accepted when they are written with ==, and that the
common VALID: Final = ("a", "b") idiom followed by if x not in VALID leaves
x at its declared type. That last one is the tuple spelling. A Final list is
still not narrowed, since it infers as list[str] and the item type is gone by
the time the container is inspected.

This is a recorded gap rather than deliberate conservatism: narrow_tuple_expression
in check-narrowing.test carries a # TODO: this should match narrow_tuple_exact
marker on exactly this behaviour, added in #21456, and #21461 extended the same gap
to list, set and dict literals. I could not find an open issue for it.

Cause

The gate that decides whether the else map can be kept asks
is_singleton_equality_type, which accepts a LiteralType. A literal written
inline never reaches that point as a LiteralType: it arrives as an Instance of
builtins.str carrying last_known_value=Literal['r']. The coercion just above
only fires for what is_literal_type_like recognises, which is LiteralType,
unions and type variables, plus a separate clause for enums. That is why enum
members and annotated containers already narrow while plain str and int
literals do not. The else map is computed correctly and then dropped on the floor.

Fix

Coerce the container item before asking whether it denotes a single value.

Only the gate coerces. The type passed to narrow_type_by_identity_equality is
left as it was, so the positive branch is unchanged: x in ["x"] for
x: str | int still reveals builtins.str, matching x == "x"
(testConsistentNarrowingEqAndIn, #17864). Coercing the item itself is the
shorter patch, but I tried it and it turns that reveal into Literal['x'] and
adds a spurious assignment error, so the coercion has to stay local to the gate.

The custom __eq__ exemption is carried over from the block above, so
testNarrowCustomEqEnumInLiteralContainer (#21703) still declines to narrow.

Soundness

x not in (a, b) should land in the same place as x != a and x != b. I checked
that by pairing every case against its != control: bool against an int
literal, int against float, a class with a custom __eq__ inside the narrowed
union, a custom __eq__ enum, a str subclass in the container, Any in the
container, a plain variable, a container mixing a literal with a variable, and
None mixed with string literals. Every not in result matches its != control
exactly. Only five of those twenty reveals move at all against master, and each one
moves from unnarrowed to whatever != already produced.

Tests

  • Updated the five literal expression expectations in
    testNarrowLiteralInLiteralContainer and removed the now stale TODO.
  • Added testNarrowNotInLiteralContainer, covering int literals, a Final
    tuple constant (which reaches the TupleType branch rather than the tuple
    expression one), a container mixing a literal with None, exhaustiveness, and
    two cases that must stay wide: a container holding a plain variable, and
    False against Literal[0, 1, 2].
  • Reverting only mypy/checker.py and keeping the test data makes both cases fail
    at nine sites. The two cases that must stay wide are identical either way.
  • Ran the narrowing, enum, literal, unreachable, isinstance, expressions,
    optional, python310, typeddict, tuples, inference, flags, statements and
    classes test files locally; all pass. black, ruff check and codespell are
    clean on the changed file, and mypy's self check reports nothing new.

I expect mypy_primer to surface new unreachable diagnostics, since code after an
exhaustive in chain now genuinely is unreachable.

`x in ('a', 'b')` left the else branch at the full declared type, while
the annotated form `x in t` for `t: tuple[Literal['a'], Literal['b']]`
already narrowed it. The gate for negative narrowing asks
`is_singleton_equality_type`, which only accepts a `LiteralType`, but a
literal written inline arrives as an `Instance` carrying a
`last_known_value`. Coerce before asking.

Only the gate coerces. The type handed to
`narrow_type_by_identity_equality` is untouched, so the positive branch
keeps matching `==`.
@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

discord.py (https://github.com/Rapptz/discord.py)
- discord/components.py:1750: error: Missing return statement  [return]

pydantic (https://github.com/pydantic/pydantic)
- pydantic/v1/types.py:704: error: Unsupported operand types for >= ("Literal['n']" and "int")  [operator]
- pydantic/v1/types.py:704: error: Unsupported operand types for >= ("Literal['N']" and "int")  [operator]
- pydantic/v1/types.py:704: error: Unsupported operand types for >= ("Literal['F']" and "int")  [operator]
- pydantic/v1/types.py:704: note: Left operand is of type "Literal['n', 'N', 'F'] | int"
- pydantic/v1/types.py:706: error: Unsupported operand types for + ("int" and "Literal['n']")  [operator]
- pydantic/v1/types.py:706: error: Unsupported operand types for + ("int" and "Literal['N']")  [operator]
- pydantic/v1/types.py:706: error: Unsupported operand types for + ("int" and "Literal['F']")  [operator]
- pydantic/v1/types.py:706: note: Right operand is of type "Literal['n', 'N', 'F'] | int"
- pydantic/v1/types.py:714: error: Argument 1 to "abs" has incompatible type "Literal['n', 'N', 'F'] | int"; expected "SupportsAbs[int]"  [arg-type]
- pydantic/v1/types.py:715: error: Argument 1 to "abs" has incompatible type "Literal['n', 'N', 'F'] | int"; expected "SupportsAbs[int]"  [arg-type]
- pydantic/v1/types.py:718: error: Argument 1 to "abs" has incompatible type "Literal['n', 'N', 'F'] | int"; expected "SupportsAbs[int]"  [arg-type]

pytest-autoprofile (https://gitlab.com/TTsangSC/pytest-autoprofile)
+ tests/test_subprocess.py:382: error: Redundant cast to "tuple[Literal[0, 1, 2], Literal[0, 1, 2]]"  [redundant-cast]

@Om-singhaI

Copy link
Copy Markdown
Author

Primer looks good. The pydantic and discord.py diffs are false positives going away: both narrow a Literal union with in and then use the else branch as an int, which is exactly the case this fixes.

The one new diagnostic is correct. pytest-autoprofile casts to tuple[Literal[0, 1, 2], Literal[0, 1, 2]] after a check that now narrows on its own, so the cast really is redundant.

I expected some new unreachable errors and there are none.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant