Skip to content
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

False positive [unreachable] with TypeIs #17181

Open
sterliakov opened this issue Apr 27, 2024 · 5 comments
Open

False positive [unreachable] with TypeIs #17181

sterliakov opened this issue Apr 27, 2024 · 5 comments
Labels
bug mypy got something wrong

Comments

@sterliakov
Copy link
Contributor

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 sterliakov added the bug mypy got something wrong label Apr 27, 2024
@kreathon
Copy link
Contributor

kreathon commented May 9, 2024

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
Copy link
Member

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

@kreathon
Copy link
Contributor

kreathon commented May 9, 2024

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 👍

@KotlinIsland
Copy link
Contributor

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
Copy link
Contributor

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

4 participants