Allow booleans to be narrowed to literal types #10389

ethan-leba · 2021-04-30T12:56:18Z

Description

This PR allows boolean types to be narrowed to literals, in the same fashion
that Enums can be narrowed.

Test Plan

New unit tests included!

ethan-leba · 2021-04-30T13:52:05Z

mypy/checker.py

-        if isinstance(target, LiteralType) and target.is_enum_literal():
-            enum_name = target.fallback.type.fullname
+        if (isinstance(target, LiteralType) and
+                (target.is_enum_literal() or target.fallback.type.fullname == "builtins.bool")):


is there a better way to check if the type is a bool than looking for this string?

isinstance(target.value, bool)?

JukkaL

Thanks for the PR! Left some comments (not a full review).

Note that since we'll hopefully have a release soon, we probably won't merge this PR until after the release branch has been cut, since changes to type inference are a bit risky just before a release.

JukkaL · 2021-05-05T15:30:07Z

test-data/unit/check-narrowing.test


+if bool_val is not False:


Should if bool_val and if not bool_val also do narrowing? These can't be overridden for bool.

Yeah, that seems useful, i'll look into it 👍

JelleZijlstra · 2021-05-06T16:47:59Z

The homeassistant line 414 error seems like a bug. The code is at https://github.com/home-assistant/core/blob/dev/homeassistant/config_entries.py#L403. There is an assertion that result is a bool, but somehow mypy still thinks it's Any according to the error message.

ethan-leba · 2021-05-09T13:11:34Z

The homeassistant line 414 error seems like a bug. The code is at https://github.com/home-assistant/core/blob/dev/homeassistant/config_entries.py#L403. There is an assertion that result is a bool, but somehow mypy still thinks it's Any according to the error message.

It looks the error is stemming from the type binder here:
https://github.com/python/mypy/blob/master/mypy/binder.py#L201

A (narrowed) Any type which does not have the same type on each side of a
conditional will be converted back to Any. Not sure about the context of this
decision, is this something we want to change?

No longer only works on enums

ethan-leba · 2021-08-09T13:49:20Z

@JukkaL ping

ethan-leba · 2021-09-28T20:40:16Z

Hi @hauntsaninja, is there any interest in re-reviewing this PR? If so, I can rebase, etc.

hauntsaninja · 2021-09-29T05:28:01Z

Thanks, yes! This PR is great and something I'm very interested in merging (although I would feel more comfortable doing so if @JukkaL had time to okay).

I fixed the broken test and merge conflict, hopefully it's enough to get CI green.

github-actions · 2021-09-29T06:29:51Z

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

websockets (https://github.com/aaugustin/websockets.git)
+ src/websockets/legacy/protocol.py:1076: error: unused "type: ignore" comment

pandas (https://github.com/pandas-dev/pandas.git)
+ pandas/io/stata.py:1423: error: Incompatible types in assignment (expression has type "Union[str, str]", variable has type "Union[Literal['>'], Literal['<']]")  [assignment]

isort (https://github.com/pycqa/isort.git)
+ isort/api.py:181: error: Redundant cast to "TextIO"
+ isort/api.py:299: error: Redundant cast to "TextIO"
+ isort/api.py:469: error: Redundant cast to "TextIO"

poetry (https://github.com/python-poetry/poetry.git)
- poetry/mixology/failure.py:143: error: Item "RootCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:143: error: Item "NoVersionsCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:143: error: Item "DependencyCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:143: error: Item "PythonCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:143: error: Item "PlatformCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:143: error: Item "PackageNotFoundCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "other"
- poetry/mixology/failure.py:144: error: Item "RootCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "conflict"
- poetry/mixology/failure.py:144: error: Item "NoVersionsCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "conflict"
- poetry/mixology/failure.py:144: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)
+ poetry/mixology/failure.py:143: note: (Skipping most remaining errors due to unresolved imports or missing stubs; fix these first)

hauntsaninja · 2021-11-07T20:35:17Z

Thanks again for this PR, super useful!

Dreamsorcerer · 2021-12-29T12:44:13Z

This change seems to break basic object-oriented programming (but, could be a wider issue with type narrowing). e.g.

assert not thing.frozen
thing.freeze()
assert thing.frozen  # Mypy: thing.frozen is Literal[False]

I'd suggest disabling this feature for attributes, or if not too complex, reset the type narrowing after a method of that object is called.

ethan-leba changed the title ~~Boolean narrowing~~ Allow booleans to be narrowed to literal types Apr 30, 2021