Skip to content

Exclude irrelevant members in narrow_declared_type from union overlapping with enum #18897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 11, 2025

Conversation

sterliakov
Copy link
Collaborator

@sterliakov sterliakov commented Apr 7, 2025

Fixes #18895.

The original implementation of that block was introduced as a performance optimization in #12032. It's in fact incorrect: it produces overly optimistic meets, assuming that any match among union items makes them all relevant. As discussed in #18895, this actually results in unexpected meet behaviour, as demonstrated by

from enum import Enum
from typing_extensions import TypeIs, Literal

class Model(str, Enum):
    A = 'a'
    B = 'a'

def is_model_a(model: str) -> TypeIs[Literal[Model.A, "foo"]]:
    return True
def handle(model: Model) -> None:
    if is_model_a(model):
        reveal_type(model)  # N: Revealed type is "Union[Literal[__main__.Model.A], Literal['foo']]"


def is_int_or_list(model: object) -> TypeIs[int | list[int]]:
    return True
def compare(x: int | str) -> None:
    if is_int_or_list(x):
        reveal_type(x)  # N: Revealed type is "builtins.int"

This patch restores filtering of union members, but keeps it running before the expensive is_overlapping_types check involving expansion.

@sterliakov sterliakov marked this pull request as ready for review April 7, 2025 19:30

This comment has been minimized.

@sterliakov sterliakov changed the title Do not leak TypeGuardedType from narrow_declared_type with enums Exclude irrelevant members in narrow_declared_type from union overlapping with enum Apr 8, 2025
@sterliakov
Copy link
Collaborator Author

sterliakov commented Apr 8, 2025

@sobolevn I remember you were doing some enums recently and you reviewed #12032, may I summon you to have a look? This PR is a crash fix.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Apr 9, 2025

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Let's keep this open for a day, then merge :)

@sobolevn sobolevn merged commit b5c95d8 into python:master Apr 11, 2025
18 checks passed
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.

Internal error with TypeGuard and Literal-based union narrowing using StrEnum
2 participants