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]

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 🎉

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.

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.