Skip to content

Calling callable in an if-condition makes mypy think branch in unreachable #3605

Closed

Description

@ilinum

In the following code, mypy thinking callable(o) always returns False. This might be related to #3603.

def f(o: object) -> None:
    if callable(o):
        # this branch is not typechecked
        o()
        1 + 'boom'  # no error from mypy
        o()

f(lambda: print('hello'))

Here is the output from mypy and python.

$ mypy n.py 
$ python3 n.py 
hello
Traceback (most recent call last):
  File "n.py", line 8, in <module>
    f(lambda: print('hello'))
  File "n.py", line 5, in f
    1 + 'boom'
TypeError: unsupported operand type(s) for +: 'int' and 'str'

Activity

  1. msullivan commented on Dec 6, 2017

    @msullivan
    Collaborator

    The key problem here is that the conditional type-checking based on callable (as introduced to fix #2627) is pretty fundamentally unsound.

    We can't really ever conclude that something can't be callable, short of using whole-program information about the entire class hierarchy. Even in the example given in that bug of testing a type Union[str, Callable[[str], int]], there can be subclasses of str that are callable, so it isn't safe to conclude that callable being true implies the type is Callable[[str], int].

    I'm not sure if there is anything sound that we can do in response to callable.

    We can soundly assume that the type is Callable, but that has its own brand of unsoundness by introducing a bunch of Anys into play. That might still be the right thing; I think the only other option is to do nothing for callable.

  2. JukkaL commented on Dec 6, 2017

    @JukkaL
    Collaborator

    For constructs that we can't handle safely, we sometimes try to do something that avoids false positives and still does some useful type checking, even if it's not type safe. In the original example, inferring type Callable (that is, Callable[..., Any]) as the type after the callable check would be my pick. At least we would catch some unsafe code and wouldn't require a cast. Casts are best avoided if possible, since they aren't checked at runtime and often mask bugs.

    In cases like Union[str, Callable[[str], int]] the reasoning could go like this:

    def f(o: Union[str, Callable[[str], int]]) -> None:
        if callable(o):
            # Could be a subclass of 'str' that is callable or Callable[[str], int].
            # We don't know the precise type in the first case, so we could fall
            # back to Callable.
            #
            # We could then infer type Union[Callable[[str], int], Callable].
            ...
        else:
            # Can only be 'str' here.
            ...

    However, elsewhere we sometimes make unsafe assumptions about inheritance. For example, we sometimes assume that there is no multiple inheritance from some classes A and B, even though we cannot be sure that it's the case. In analogy it would be reasonable in the latter example to infer just Callable[[str], int] as the type of o after the callable check and assume that no str subclass is callable.

    Eventually I'd like to have optional strictness flags that reject this kind of type unsafety at the cost of some extra required refactoring. I don't think that this is urgent right now, however. There still tends to be a lot of Any types in large type checked programs due to missing or incompletely typed typeshed stubs (and various other reasons). Edge cases such as the ones discussed here only add a very little extra unsafety in practice. My experience suggests that users dislike having to refactor their existing code, so introducing some unsafety is occasionally okay -- but these are often very delicate design decisions.

  3. ilevkivskyi commented on Dec 6, 2017

    @ilevkivskyi
    Member

    For example, we sometimes assume that there is no multiple inheritance from some classes A and B, even though we cannot be sure that it's the case.

    It looks like Intersection may be a more long-term solution for such cases and also for callable(). It doesn't have many use cases for a user facing API, but IIRC there are lots of corner cases that will be fixed by having internal intersection types. I am sure this was already proposed somewhere, but can't remember where.

  4. ilevkivskyi commented on Dec 6, 2017

    @ilevkivskyi
    Member

    OK, found it #3603 (comment)

  5. msullivan commented on Dec 8, 2017

    @msullivan
    Collaborator

    I started hacking up the beginnings of a fix to this based on assuming the type of o is a Callable as in the example Jukka gave.

    My naive implementation of this didn't work for what I think turns out is actually a pretty fundamental reason: the type maps returned by find_isinstance_check are restrictions that are used to narrow_declared_type, which computes the meet of object and Callable, which is None.

    I had had it in my head that I wanted to just override the previous type with Callable, which would work fine for object but probably be the wrong thing for more specific types.

    So I think that gaining any useful information from callable being true is going to require some form of intersection? I think that if it was only callable we cared about, a super special cased version of the proposal in #3603 that just made up a new TypeInfo that added a __call__ method would probably suffice.

  6. gvanrossum commented on Dec 8, 2017

    @gvanrossum
    Member
  7. added 3 commits that reference this issue on Dec 12, 2017
    3df1996
    450afb1
    9aba159
  8. added a commit that references this issue on Dec 19, 2017
    38337a8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions