Incorrect overalp detected in __rmod__ with str argument

[zero323]

Problem observed using current HEAD (0.730+dev.d2a7f05d27d9ac5c096c61ecf0455edda91f5d5a) but can confirmed on 0.660 as well, so it is nothing new.

Flags used:

• --python-version 3.7 --strict-optional --no-site-packages --show-traceback --no-implicit-optional
• Also confirmed with no flags at all (Python 3.7)

Problem occurs:

• With __mod__ / __rmod__ pair.
• When other in __mod__ is Union[T, str], where T can be self type, or some other type (let's say int).
• When other in __rmod__ is str.
• When return type annotation for __rmod__ is provided,

Reproducible example:

from typing import Union

class Foo:
    def __mod__(self, other: Union[Foo, str]) -> Foo: ...
    def __rmod__(self, other: str) -> Foo: ...

yields

foo.py:5: error: Signatures of "__rmod__" of "Foo" and "__mod__" of "str" are unsafely overlapping

Issue is specific to __mod__ / __rmod__ pair, i.e. this works just fine:

from typing import Union

class Foo:
    def __mul__(self, other: Union[Foo, str]) -> Foo: ...
    def __rmul__(self, other: str) -> Foo: ...

and I haven't found any other problematic pair.

As far as I can tell it strictly depends on other being str. For example this works just fine

from typing import Union

class Foo:
    def __mod__(self, other: Union[Foo, int]) -> Foo: ...
    def __rmod__(self, other: int) -> Foo: ...

Additionally, to reproduce the problem, we need a return type annotation on __rmod__, and removing it is sufficient to pass type check (removing return type annotation from __mod__ has no impact at all):

from typing import Union
 
class Foo:
    def __mod__(self, other: Union[Foo, str]) -> Foo: ...          
    def __rmod__(self, other: str): ...          

though failure is not specifically caused by referencing self type, as for example this, will fail as well

from typing import Union
 
class Foo:
    def __mod__(self, other: Union[Foo, str]) -> int: ...   
    def __rmod__(self, other: str) -> int: ...   

foo.py:5: error: Signatures of "__rmod__" of "Foo" and "__mod__" of "str" are unsafely overlapping

[Michael0x2a]

I think this is a usability bug. Mypy is reporting something that's a legitimate issue/is legitimately questionable with your code, but is doing so in a fairly unhelpful way.

Basically, the problem is that Foo.__rmod__ will never actually be called: str.__mod__ is the old-style formatting operator. So, when you do something like "hello %s bar" % Foo(), the result will be a string that looks something like "hello <__main__.Foo instance at 0x103bf65a8> world".

(Actually, there are a few exceptions to this rule: for example, if you make Foo a subclass of str, Python will actually end up calling Foo.__rmod__ first before calling str.__mod__. So I think your Foo.__rmod__ type signature would actually be fine in that case...)

Regardless, mypy should do one of two things:

1. Report a more understandable error message explaining that Foo.__rmod__ is unreachable.
2. Decide to skew towards being more lenient and not report an error here.

To answer your other questions:

1. The reason why this only seems to happen with str is that str happens to define an __mod__ method that accepts anything for the RHS. Contrast this with ints, which only accept other ints (and will otherwise return NotImplemented).

2. The reason why changing the return type of Foo.__rmod__ to int or something doesn't make a difference is that no matter what __rmod__ returns, it's still never going to be called.

3. The reason why getting rid of the return type altogether (e.g. returning Any) works is probably just an artifact of how this check is done -- basically, it's internally converted into an overload, and the overload happens to be technically safe (though still redundant) if the __rmod__ were to return specifically 'str'. And Any could also happen to be str, so that's also considered safe.

   This is probably a bug? The return type doesn't really matter here so mypy shouldn't handle these two cases differently. But operators are tricky and there are probably some nuances to them that I'm forgetting...

[zero323]

Thank you for such a detailed and insightful response @Michael0x2a! In general I have no direct control over the code I annotate, but I guess this is something to push upstream (the actual module is mostly code-generated anyway).

From where I stand that's definitely an usability issue. The actual annotation is a tad more complex and results in something around:

Signatures of "__rmod__" of "Foo" and "__mod__" of "Union[K, V, T, U, str, ...]"

with little indication, where the actual problem lies. Getting an error indicating, that the code is unreachable, would be a huge improvement over that.

[tapaswenipathak]

Hi folks: I would like closing the issue, can I PR?

[Michael0x2a]

@tapaswenipathak -- sure, go for it. The method you'll want to change is either the check_overlapping_op_methods in checker.py or the is_unsafe_overlapping_op method directly below. Probably you'll want to add in a call to overload_can_never_match somewhere to detect when the __rmod__ is unreachable.

Also, it occurred to me a third option we could try exploring is by default not to report an error, and only report one if the user passes in the --warn-unreachable flag. Probably these kinds of unreachable errors aren't super-critical and are not always in the control of the end-user? Not 100% sure though -- maybe this could be something we could discuss and implement separately. For now, just fixing the error message to be more helpful would be a strict improvement over the status quo.

[A5rocks]

As of #17392, this works.