Don't emit Liskov violations w.r.t. grandparent class if the violation cannot be fixed without breaking Liskov w.r.t. the parent class

[t-moe]

Question

I have the following code:

from typing import Iterable
from collections import UserList

class Errors(UserList[str]):
    def __init__(self, initlist=None):
        super().__init__(initlist)

    def append(self, item: str) -> None:
        super().append(item)

    def extend(self, other: Iterable[str]) -> None:
        for item in other:
            self.append(item)

This fails with an error similar to

 uv run ty check
error[invalid-method-override]: Invalid override of method `append`
    --> error.py:8:9
     |
   6 |         super().__init__(initlist)
   7 |
   8 |     def append(self, item: str) -> None:
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `MutableSequence.append`
   9 |         super().append(item)
     |
    ::: stdlib/typing.pyi:1112:9
     |
1110 |     def __delitem__(self, index: slice) -> None: ...
1111 |     # Mixin methods
1112 |     def append(self, value: _T) -> None:
     |         ------------------------------- `MutableSequence.append` defined here
1113 |         """S.append(value) -- append value to the end of the sequence"""
     |
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default

error[invalid-method-override]: Invalid override of method `extend`
    --> error.py:11:9
     |
   9 |         super().append(item)
  10 |
  11 |     def extend(self, other: Iterable[str]) -> None:
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `MutableSequence.extend`
  12 |         for item in other:
  13 |             self.append(item)
     |
    ::: stdlib/typing.pyi:1118:9
     |
1116 |         """S.clear() -> None -- remove all items from S"""
1117 |
1118 |     def extend(self, values: Iterable[_T]) -> None:
     |         ------------------------------------------ `MutableSequence.extend` defined here
1119 |         """S.extend(iterable) -- extend sequence by appending elements from the iterable"""
     |
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
Found 2 diagnostics

What am I missing? Forgive me, if there is an obvious error hidden in plain sight. I havent written a lot of python lately..

Version

ty 0.0.2

[AlexWaygood]

Hey @t-moe, thanks for the bug report!

The issue here is with the parameter names. If I have a class hierarchy like this, then instances of Bar cannot be used as a drop-in substitute wherever instances of Foo are expected, because you can call Foo.method() with a x= keyword argument, but the same call fails on an instance of Bar:

>>> class Foo:
...     def method(self, x: int): ...
...     
>>> class Bar(Foo):
...     def method(self, y: int): ...
...     
>>> Foo().method(x=42)
>>> Bar().method(x=42)
Traceback (most recent call last):
  File "<python-input-3>", line 1, in <module>
    Bar().method(x=42)
    ~~~~~~~~~~~~^^^^^^
TypeError: Bar.method() got an unexpected keyword argument 'x'

In your code here, your parameter name item you've given to Errors.append matches the parameter name for the method on Errors's immediate parent class (UserList), but doesn't match the parameter name for the method on its grandparent class (MutableSequence). So it would be okay to use an instance of Errors wherever a UserList[str] is accepted, but it wouldn't be okay to use an instance of Errors wherever a MutableSequence[str] is accepted -- this violates the Liskov Substitution Principle.

Unfortunately... there's not much you can do about it here, in this situation! If you change your parameter names so that they match the parameter names on MutableSequence.append(), Errors will no longer violate the Liskov Substitution Principle with respect to MutableSequence, but it will now violate the Liskov Substitution Principle with respect to UserList. So you're in a bit of a bind, because of the fact that UserList itself violates the Liskov Substitution Principle.

I think there are several things we need to do here:

1. We need to improve the error message to make clear that it's the parameter names that are the problem! This is tracked in https://github.com/astral-sh/ty/issues/1644
2. Ideally I think we would split errors about bad parameter names into their own, more specific error code. Mypy doesn't emit these errors at all, and pyright doesn't emit them for dunder methods. This, like #1644, is also blocked by https://github.com/astral-sh/ty/issues/163 right now.
3. If we detect that it would actually be impossible to fix the Liskov violation without introducing a new (worse) violation, then we should probably avoid emitting the diagnostic.

[carljm]

@AlexWaygood is this issue left open in order to track item (3)? Should we re-title it to reflect that? Do we have an issue for (2)?

[AlexWaygood]

@AlexWaygood is this issue left open in order to track item (3)? Should we re-title it to reflect that? Do we have an issue for (2)?

Yes. I have retitled the issue to reflect that, and opened https://github.com/astral-sh/ty/issues/2073 for splitting the more pedantic Liskov checks into separate error codes.