mypy icon indicating copy to clipboard operation
mypy copied to clipboard

False positive `safe-super` errors for properties defined in protocol classes using inheritance

Open nph opened this issue 2 years ago • 7 comments

Bug Report

Empty properties defined in a protocol class that is subclassed by another protocol are incorrectly classified by mypy as abstract methods - this can then result in a safe-super error if the parent property is accessed via super() within a class that is type hinted with the child protocol, e.g. when type hinting self in mixin classes (as described here).

To Reproduce

from __future__ import annotations

from typing import Protocol, Optional, NoReturn


class Dataset(Protocol):
    def validate(self) -> Optional[NoReturn]:
        ...

    @property
    def has_schema(self) -> bool:
        ...


class TSDataset(Dataset, Protocol):
    @property
    def num_rows(self) -> int:
        ...


class TSDatasetMixin:
    def check_schema(self: TSDataset) -> None:
        super().validate()
        if super().has_schema:
            print('Dataset has a schema')

Expected Behavior

No errors.

Actual Behavior

mypy_safe_super_protocol.py:24: error: Call to abstract method "has_schema" of "Dataset" with trivial body via super() is unsafe  [safe-super]
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: 1.0.1
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.8.0

nph avatar Feb 22 '23 05:02 nph

I think I might be able to add to this with this code

from abc import ABC, abstractmethod
from typing import Any


class Interface(ABC):
    @abstractmethod
    def method(self) -> Any:
        pass


class Level1_1(Interface):
    def method(self) -> str:
        return ""


class Level1_2(Interface):
    def method(self) -> str:
        return ""


class Level2(Level1_1, Level1_2):
    def method(self) -> str:
        reveal_type(super(Level1_1, self).method())  # Line 23
        reveal_type(super(Level1_2, self).method())  # Line 24
        super(Level1_1, self).method()
        super(Level1_2, self).method()
        return "TEST"

We get

clone.py:23: note: Revealed type is builtins.str
clone.py:24: note: Revealed type is "Any"
Success: no issues found in 1 source file^

Which I believe should both be the builtins.str mypy is not picking up the Level1_2 (I think alway the last class that inherits from Interface) method

paperclipstudio avatar Feb 24 '23 19:02 paperclipstudio

Thanks @paperclipstudio. I believe mypy is actually correct here. The expression:

super(Level1_1, self).method()

will call method in Level1_2 - hence the revealed type of str. Whereas the expression:

super(Level1_2, self).method()

will call method in Interface.

nph avatar Feb 24 '23 21:02 nph

Yes, Sorry my mistake, thanks for the help.

paperclipstudio avatar Feb 27 '23 13:02 paperclipstudio

I also want to add that this also happens even if we directly call a protocol's method in a class that inherits from that protocol.. Here's an example that produces the same problem:

_my_protocol.py

from typing import Protocol


class MyProtocol(Protocol):
    def my_method(self) -> dict[str, int | str]:
        ...

my_mixin.py

from typing import TYPE_CHECKING

from ._my_protocol import MyProtocol

if TYPE_CHECKING:
    _Base = MyProtocol
else:
    _Base = object


class MyMixin(_Base):
    def my_method(self) -> dict[str, int | str]:
        return super().my_method()

Error: error: Call to abstract method "my_method" of "MyProtocol" with trivial body via super() is unsafe [safe-super]

ebram96 avatar Nov 08 '23 23:11 ebram96

Hi @ebram96 - I think mypy is correct in this case. Your MyMixin class is a regular class and not a Protocol class (as it does not explicitly inherit from Protocol itself) and since MyProtocol does not provide an implementation of my_method, mypy is correct to raise a safe-super error for the super().my_method() call. PEP544 gives a similar example of a safe-super error for the BadColor class.

nph avatar Nov 09 '23 05:11 nph

I have a variant of this error for a mixin class using super() against a protocol:

from typing import Protocol


class BaseClassProtocol(Protocol):
    def some_method(self) -> str | None: ...


class SubClassProtocol(BaseClassProtocol, Protocol):
    @property
    def some_flag(self) -> str: ...


class MyMixin:
    def some_method(self: SubClassProtocol) -> str | None:
        if self.some_flag:  # `self` type is defined to access this
            return 'some_flag'
        return super().some_method()  # But this is flagged for `safe_super`

Playground URL: https://mypy-play.net/?mypy=latest&python=3.12&gist=7129a6792261d89c8e3b414a6f12e0f8

jace avatar Jun 28 '24 11:06 jace

The fix from #12344 / #14082 doesn't work to stop the safe_super error, but replacing the method definition with Callable type makes the error go away:

from collections.abc import Callable
from typing import Protocol

class MixinProtocol(Protocol):
    some_method: Callable

    @property
    def some_flag(self) -> str: ...

class MyMixin:
    def some_method(self: MixinProtocol) -> str | None:
        if self.some_flag:
            return 'some_flag'
        return super().some_method()  # No error now!

jace avatar Jun 28 '24 11:06 jace