Skip to content

Using type Self allows to violate Liskov Substitution Principle #15619

Open

Description

@AntPeixe

Bug Report

By using Self in the type of a method's argument it's possible to violate the Liskov Substitution Principle without any warning.

(A clear and concise description of what the bug is.)

To Reproduce

Mypy raises no issue on this code although SubClass.update is clearly violating LSP.

from typing_extensions import Self

class BaseClass:
    foo: int
    def update(self, other: Self) -> None:
        self.foo = other.foo

class SubClass(BaseClass):
    bar: int
    def update(self, other: Self) -> None:  # problem here
        super().update(other)
        self.bar = other.bar

Expected Behavior

I'd expect that using Self would raise the warning similarly to specifying the class. Mypy raises the issue on the below.

from typing_extensions import Self

class BaseClass:
    foo: int
    def update(self, other: BaseClass) -> None:
        self.foo = other.foo

class SubClass(BaseClass):
    bar: int
    def update(self, other: SubClass) -> None:  # problem here
        super().update(other)
        self.bar = other.bar

Your Environment

  • Mypy version used: 1.4.1
  • Python version used: 3.10.8

Activity

  1. tmke8 commented on Jul 8, 2023

    @tmke8
    Contributor

    You get an error though if you try to call .update() from the first code snippet with an instance of BaseClass though. So I think that's safe enough?

    EDIT: I guess you can still violate LSP if you first cast the SubClass object to BaseClass and then call .update().

  2. jp-larose commented on Jul 11, 2023

    @jp-larose

    I think this is really a matter of intent.

    In the initial code listing, what you're telling the user of your class is the parameter to update must be an instance of the same class as the host function, and this is supposed to be true for all sub classes of BaseClass. I would consider this a special allowable exception to LSP that is explicitly documented in your code via the typehint Self. As long as all your subclasses conform to this "contract" (i.e. the parameter to update is always declared as type Self), your code works as "intended". Anyone using this code should understand that this implicitly means that instances of super classes can't be passed as a argument.

    In the second code listing, SubClass is deviating from the original contract set by BaseClass in a way that violates LSP, which is why it would make sense for mypy to flag it as an error.

    The big difference here is that in the first case, you're maintaining the same method signature, whereas in the second case you're changing it.

    Self kind of sells itself as automagically making the the type substitution, but in reality it's a little more subtle than that.

  3. ikonst commented on Jul 11, 2023

    @ikonst
    Contributor

    Self exists primarily for the type checker rather than as means of documenting intent. I agree there's a false positive here:

    def f(x: BaseClass) -> None:
      x.update(BaseClass())
    
    f(SubClass())  # attribute error getting bar
  4. lpulley commented on Sep 28, 2023

    @lpulley

    In the second code listing, SubClass is deviating from the original contract set by BaseClass in a way that violates LSP, which is why it would make sense for mypy to flag it as an error.

    I agree with this and encountered this problem as well. Here's my example, similar to the original example above:

    from typing import List
    
    from typing_extensions import Self
    
    class Base:
        def add_to(self, l: List[Self]) -> None:
            l.append(self)
    
    class Derived(Base):
        # This should not be allowed since it changes the type of `add_to`
        # incompatibly with `Base`, but mypy allows it because of `Self`!
        def add_to(self, l: List[Self]) -> None:
            l.append(self)
            
        def something_only_derived_can_do(self) -> None:
            return None

    Mypy has no problem with the above, which seems like a bug.

    It only raises the alarm when the function is actually used polymorphically(?), e.g.:

    list_of_derived: List[Derived] = []
    
    base = Base()
    # HERE is where mypy finally realizes the problem:
    base.add_to(list_of_derived)

    This error is far from the actual source of the problem -- that Derived.add_to violates LSP.

  5. jp-larose commented on Oct 24, 2023

    @jp-larose

    @lpulley The example you provided is exactly the type of situation that the Self type is intended to solve. The behaviour you're seeing from mypy is correct.

    The mistake is thinking that Self replaces the type where it's first used in a class hierarchy. But its true purpose is to say the type of it's filling is the "current runtime class". See PEP 673 – Self Type.

    In your example, when you instantiate Base, the signature of add_to is effectively add_to(self: Base, l: list[Base]) -> None. Likewise, when you instantiate Derived the signature off add_to for that object is effectively add_to(self: Derived, l: list[Derived]) -> None

    The other major point of confusion is that list[Base] is neither a supertype nor a subtype of list[Derived], that is to say list[T] is invariant, and using Self doesn't change that. See the explanation in PEP 484. This is the true reason mypy complains about base.add_to(list_of_derived).

    Type hinting and static type checking in Python is an ever moving target. That makes it difficult to keep track of all that's going on. But as far as I can tell, it's always progressing.

    Does Self break LSP? Yes, but very intentionally and very purposefully. It's an exception to the rule. And I love it!

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

    bugmypy got something wrongtopic-inheritanceInheritance and incompatible overridestopic-self-typesTypes for self

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions