Changing order of isinstance checks breaks inferred type

[pirate]

This works fine:

def __add__(self, other: object) -> 'Currency':
    if isinstance(other, self.__class__) and isinstance(other, Currency):
        return self.__class__(other.amt + self.amt)
    return NotImplemented

but this does not:

def __add__(self, other: object) -> 'Currency':
    if isinstance(other, Currency) and isinstance(other, self.__class__):   # only expression order is flipped
        return self.__class__(other.amt + self.amt)
    return NotImplemented

throws: error: "object" has no attribute "amt"

Full code to reproduce it locally: https://gist.github.com/pirate/b3ef5a25449285dadc366dd926cebcea

The intention is to check that other is the same class as self (to prevent adding BTC to USD by accident). That check alone should imply that other is a subclass of Currency (since the __add__ method is defined on Currency), but it looks like I have to explicitly make both assertions, and make the Currency assertion after the self.__class__ one in order for mypy to narrow the type correctly.

Correct me if I'm mistaken, but I think there may be two bugs here:

• The order of isinstance checks shouldn't matter (especially not in an and expression)
• The isinstance(other, Currency) should not even be necessary, as it's implied that other is a Currency if other.__class__ == self.__class__

This issue discusses a vaguely similar type-narrowing problem, but I'm not sure if it's actually related: #2776

[ilevkivskyi]

It is indeed close to #2776 And is also another hint that we need (at least internal) intersection type.

[gvanrossum]

Hm, the direct cause seems to me that mypy doesn't understand enough about self.__class__ -- it just knows it is a class, it doesn't know it is Type[Currency]. So isinstance(other, self.__class__) ends up being equivalent to isinstance(other, object). Plus, there's something wrong with the interpretation of and, seemingly it throws away the left operand in cases like this.

[ilevkivskyi]

Hm, the direct cause seems to me that mypy doesn't understand enough about self.__class__ -- it just knows it is a class, it doesn't know it is Type[Currency].

Maybe this can be fixed by reapplying python/typeshed#1549? IIRC the issue caused by this reverted typeshed PR was solved some time ago. (Although this will not help in more complex situations with multiple inheritance like in #2776, that needs at least synthetic TypeInfos as proposed in #2776 (comment).)

[pirate]

Possibly fixed by #8305?

[97littleleaf11]

@pirate Thanks for pointing out! All three cases wouldn't generate errors now!