python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.54k stars 2.84k forks source link

False positive [unreachable] with `TypeIs` #17181

Open sterliakov opened 7 months ago

sterliakov commented 7 months ago

Bug Report

Applying TypeIs to narrow type of an iterable results in too optimistic reachability analysis.

To Reproduce

from collections.abc import Iterable
from typing_extensions import TypeIs

def is_iterable_int(val: Iterable[object]) -> TypeIs[Iterable[int]]:
    return all(isinstance(item, int) for item in val)

bar: list[int] | list[str]

if is_iterable_int(bar):
    reveal_type(bar)
else:
    reveal_type(bar)

playground has more code, including TypeGuard comparison and non-iterable case that works correctly.

Expected Behavior

I'd expected both branches to be reachable and to narrow type according to the spec: list[int] in if and list[str] in else, ideally.

Actual Behavior

main.py:11: note: Revealed type is "builtins.list[builtins.int]"
main.py:13: error: Statement is unreachable  [unreachable]

Your Environment

kreathon commented 6 months ago

I think the issue is probably a general type narrowing problem of the TypeIs implementation (so not directly an issue with the reachability analysis itself). At least I think that it is a problem (otherwise it would be confusing).

Analysis

I had a quick look into the implementation and it seems that the code of the isinstance runtime check is reused. From the comments I found for the implementations it seems that this is desired:

Problem

def covers_at_runtime(item: Type, supertype: Type) -> bool:
    """Will isinstance(item, supertype) always return True at runtime?"""

    ....

    # Since runtime type checks will ignore type arguments, erase the types.
    supertype = erase_type(supertype)
    if is_proper_subtype(
        erase_type(item), supertype, ignore_promotions=True, erase_instances=True
    ):
        return True

    ...

Erasing the type argument is fine for isinstance (e.g. isinstance(x, list), but here it leads to the following issue (slightly simplified from above by using is_list_int instead of is_iterable_int):

bar: list[int] | list[str]

if is_list_int(bar):  <- here both Union members are erased to list[Any] (which is subtype of the erased TypeIs argument: list[Any])
    reveal_type(bar)
else:                 <- thus here is nothing left in the Union for this branch  and we get the type Never
    reveal_type(bar)
JelleZijlstra commented 6 months ago

Thanks for the analysis! I'd be happy to review a PR addressing this.

kreathon commented 6 months ago

I was already thinking about submitting one, but I thought it is best to wait until it is confirmed (which you did just now ;) )

I will have a look :+1:

KotlinIsland commented 5 months ago

This isn't a problem in basedmypy: playground

I think it involved updating conditional_types_with_intersection/conditional_types/restrict_subtype_away/covers_at_runtime to not erase generics.

I think the problem arises in covers_at_runtime, we can see:

# Since runtime type checks will ignore type arguments, erase the types.
kreathon commented 5 months ago

I checked the code in basedmypy and it seems that covers_at_runtime is using "only" is_proper_subtype (without erasing the generics). I think there are a few cases where this is not sufficient (because it does not handle Any as expected) for example:

(This PR should handle these cases.)