Support __bool__ with Literal in --warn-unreachable

[kreathon]

This adds support for Literal as return type of __bool__ in the reachability analysis.

Example:

class AlwaysFalse:
    def __bool__(self) -> Literal[False]: ...

if AlwaysFalse():
    x = 1                   <- this line is unreachable

Addresses #7008
Fixes parts of #15523 (Handling bool() should probably be a separate PR)

To be honest, I did not understand what the purpose of this code was / why "the branch was even taken or analzyed" before:

if ret_type and ret_type.can_be_false and not ret_type.can_be_true:
    new_t = copy_type(t)
    new_t.can_be_true = False
    return new_t

Now instead of the branch is taken with the updated type, it is marked as unreachable.

Edit: It is probably for the case that "--warn-unreachable" is not enabled. Should the type checker then continue in the unreachable block? But I guess it also never did for the other cases e.g. if False ...

Edit 2: It does not make sense to continue, because it might lead to incorrect preconditions of the blocks.

[github-actions]

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

steam.py (https://github.com/Gobot1234/steam.py)
- steam/package.py:396: error: Return value expected  [return-value]
- steam/state.py:2565: error: Item "None" of "Language | None" has no attribute "api_name"  [union-attr]

[kreathon]

MyPy Primer Analysis

- steam/package.py:396: error: Return value expected [return-value]

if self.flags & LicenseFlag.Expired:
    return
if self.time_limit is None:                      <- This code is unreachable
    return                                                <- 396
return self.time_limit - self.time_used

self.flags is an Instance of LicenceFlag (which inherits from Flags, which inherits from IntEnum).

See below for an explanation of IntEnum.

- steam/state.py:2565: error: Item "None" of "Language | None" has no attribute "api_name" [union-attr]

    player.GetGameAchievementsRequest(appid=app_id, language=(self.language or language).api_name)

The type of language is Language | None and the type of self.language is Language (which inherits from IntEnum).

See below for an explanation of IntEnum.

IntEnum

This class is a custom enum implementation which has:

def __bool__(self) -> Literal[True]:
    return True  # an enum member with a zero value would return False otherwise

I am confused by this comment, because:

>>> bool(LicenseFlag.NONE)
True

>>> bool(LicenseFlag.Renew)
True

With:

class LicenseFlag(Flags):
    """Flags for a license."""
    NONE                         = 0
    """No flags."""
    Renew                        = 1 << 0
    ...

I am not sure if this is a bug, or I am missing something ...

Edit: I will ask the steam.py maintainer.

Edit 2: Both issues were discussed on the steam.py Discord:

• self.flags & LicenseFlag.Expired creates a new LicenseFlag which will evaluate to True again
• self.language or language should be language or self.language

So the change already found two bugs 🎉

[ikonst]

I personally think we should make the type checker to go into "impossible" blocks in all but a few trivial cases (always-true constants, TYPE_CHECKING) which are pruned early on. But that's a separate discussion.

[kreathon]

I saw the comments in #15386 and I think there are also arguments for skipping. For example that someone might address issues in blocks that will be removed (e.g. is None checks or something).

On the other hand, I agree that if "--warn-unreachable" is not enabled skipping the blocks is completely confusing.

Back to the code:

I decided for UninhabitedType , because there is already:

    if not t.can_be_true:
        # All values of t are False-ish, so there are no true values in it
        return UninhabitedType(line=t.line, column=t.column)

for handling the other cases at the top. So it feels more consistent for me, even though we are skipping now blocks that were not skipped before.

For me both behaviors are fine and I can change it, if that is requested.

[hauntsaninja]

Thanks for fixing, this is clearer!