mypy icon indicating copy to clipboard operation
mypy copied to clipboard

False positive [unreachable] with `TypeIs`

Open sterliakov opened this issue 1 year ago • 5 comments

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

  • Mypy version used: 1.10.0 and master (playground)
  • Mypy command-line flags: --warn-unreachable
  • Mypy configuration options from mypy.ini (and other config files): N/A
  • Python version used: 3.11

sterliakov avatar Apr 27 '24 01:04 sterliakov

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:

  • https://github.com/python/mypy/pull/16898
  • https://github.com/microsoft/pyright/pull/7777

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)

kreathon avatar May 09 '24 15:05 kreathon

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

JelleZijlstra avatar May 09 '24 15:05 JelleZijlstra

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:

kreathon avatar May 09 '24 15:05 kreathon

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.

KotlinIsland avatar Jun 16 '24 07:06 KotlinIsland

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:

  • https://mypy-play.net/?mypy=basedmypy-latest&python=3.12&gist=9b46a06f86eaac33cc4deeaf1e678551
  • https://mypy-play.net/?mypy=basedmypy-latest&python=3.12&gist=bd55748d6840238c409efdf75741acd2 (I think the type[Any] instead of type[main.A] is a different story)

(This PR should handle these cases.)

kreathon avatar Jun 16 '24 09:06 kreathon