python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.26k stars 2.79k forks source link

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

Open nph opened 1 year ago

nph commented 1 year ago

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

paperclipstudio commented 1 year ago

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

nph commented 1 year ago

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.

paperclipstudio commented 1 year ago

Yes, Sorry my mistake, thanks for the help.

ebram96 commented 10 months ago

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]

nph commented 10 months ago

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.

jace commented 3 months ago

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 commented 3 months ago

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!