Detect impossible condition caused by user defined __bool__

[graingert]

Eg with the following:

from typing import *

class Foo:
    def __bool__(self) -> Literal[True]:
        return True

def some_foo() -> Foo:
    ...

def throw_error():
    raise Exception


if __name__ == '__main__':
    print(some_foo() or throw_error())
    #                   ^^^^^^^^^^^^^  impossible condition, unreachable

mypy should be able to detect that the call to throw_error is unreachable

[ilevkivskyi]

Well mypy doesn't detect even simpler unreachable function calls like True or print(). It may be a useful feature to warn about unreachable code.

Related to #2395

[ilevkivskyi]

#6990 is another example where this may be useful.

[Michael0x2a]

FWIW I've started sort of looking into adding in some kind of flag for this during the past week (I'm already poking around this corner of the codebase for literal and enum reachability stuff anyways).

I think in principle it should be straightforward: to avoid false-positives with things like sys.platform checks, we don't alert on blocks that were marked as unreachable as a part of semantic analysis and only look at results from the type-checking phase.

We'll also probably want to adjust mypy so it distinguishes between intentionally and unintentionally unreachable blocks. E.g. if somebody does if False: ... that was probably intentional so we'll probably need to do something different there. Probably we'll need to refactor find_isinstance_check a bit to make this all work cleanly, but I was maybe thinking of doing that anyways.

TBH I think the hardest/most annoying part will be constructing an understandable error message for subexpressions that were skipped due to being unreachable, just due to the "line-by-line" nature of error reporting in mypy.

Possibly adding in a "fyi this expression is always True/always False" error message might be sufficient instead, as opposed to indicating specific ignored subexpressions. Not sure, still need to do more poking around.

[ilevkivskyi]

TBH I think the hardest/most annoying part will be constructing an understandable error message for subexpressions that were skipped due to being unreachable, just due to the "line-by-line" nature of error reporting in mypy.

On Python 3.8 there is a simple solution for that: ast.get_source_segment() gives exact source code for every AST node. But maybe a more generic error message would be sufficient.

E.g. if somebody does if False: ... that was probably intentional

On the other hand adding a # type: ignore to that code of switching it to if typing.TYPE_CHECKING: ... is probably a fine solution too.

[graingert]

There are better ways to stop code executing than `if False`

…

On Wed, 19 Jun 2019, 18:47 Ivan Levkivskyi, ***@***.***> wrote: TBH I think the hardest/most annoying part will be constructing an understandable error message for subexpressions that were skipped due to being unreachable, just due to the "line-by-line" nature of error reporting in mypy. On Python 3.8 there is a simple solution for that: ast.get_source_segment() gives exact source code for every AST node. But maybe a more generic error message would be sufficient. E.g. if somebody does if False: ... that was probably intentional On the other hand adding a # type: ignore to that code of switching it to if typing.TYPE_CHECKING: ... is probably a fine solution too. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7008?email_source=notifications&email_token=AADFATGXR2YJJHFE7MUITDDP3JWMLA5CNFSM4HYYMF42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYCUNNA#issuecomment-503662260>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADFATANVYUWKTOBP7OLMNTP3JWMLANCNFSM4HYYMF4Q> .

[ilevkivskyi]

It looks like #7454 will help with this. @Michael0x2a is there something else apart from better error location needed for this issue? If not, I would propose to close this one.

[ilevkivskyi]

Actually I think -> Literal[True] (currently not supported by --warn-unreachable) part is enough to keep this open.