Skip to content

[ty] Check method definitions on subclasses for Liskov violations #21436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Nov 23, 2025
Merged

[ty] Check method definitions on subclasses for Liskov violations #21436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7570094
[ty] Check method definitions on subclasses for Liskov violations
AlexWaygood Nov 13, 2025
4f3d256
fixes
AlexWaygood Nov 13, 2025
7f053ea
Fix incorrect identification of where the overridden method was defined
AlexWaygood Nov 14, 2025
e127c25
disambiguate class names where necessary
AlexWaygood Nov 14, 2025
96b2535
try to optimize
AlexWaygood Nov 14, 2025
e1764c7
regen
AlexWaygood Nov 14, 2025
935640d
qualified-name cleanup
AlexWaygood Nov 17, 2025
6d296a7
more reliable subdiagnostic
AlexWaygood Nov 17, 2025
da0cdfc
snapshots and docs
AlexWaygood Nov 17, 2025
cd2d59c
more subdiagnostics and docs
AlexWaygood Nov 17, 2025
3522623
move submodule
AlexWaygood Nov 17, 2025
65f1a20
remove done TODO
AlexWaygood Nov 17, 2025
7f918d7
add submodule doc-comment
AlexWaygood Nov 17, 2025
e4b2174
Merge branch 'main' into alex/basic-liskov
AlexWaygood Nov 18, 2025
4f18e49
further improve diagnostics
AlexWaygood Nov 18, 2025
a5f0983
Merge branch 'main' into alex/basic-liskov
AlexWaygood Nov 18, 2025
8c03f3f
fix bugs, add many more tests, hopefully optimize?
AlexWaygood Nov 20, 2025
7f7169a
Merge branch 'main' into alex/basic-liskov
AlexWaygood Nov 20, 2025
7d57fee
moar tests
AlexWaygood Nov 21, 2025
76adbcb
Apply suggestions from code review
AlexWaygood Nov 23, 2025
d586e66
Merge branch 'main' into alex/basic-liskov
AlexWaygood Nov 23, 2025
da211a9
Merge branch 'alex/basic-liskov' of https://github.com/astral-sh/ruff…
AlexWaygood Nov 23, 2025
1d6c816
more review comments
AlexWaygood Nov 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
222 changes: 154 additions & 68 deletions crates/ty/docs/rules.md
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions crates/ty_python_semantic/resources/mdtest/attributes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1904,6 +1904,7 @@ we only consider the attribute assignment to be valid if the assigned attribute
from typing import Literal

class Date:
# error: [invalid-method-override]
Copy link
Contributor

@carljm carljm Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary to decide in this PR, but I wonder if we should special-case ignoring object.__setattr__ for Liskov purposes (and separately implement validation of a correct __setattr__ signature). TBH I'm not even sure why typeshed has object.__setattr__, and I don't think there is any soundness need for subclasses of object to respect Liskov compatibility with the object.__setattr__ definition in typeshed.

Copy link
Member Author

@AlexWaygood AlexWaygood Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you're right.

I was also wondering about special-case ignoring object.__hash__. It's obviously common to define unhashable classes in Python, and it's often the correct thing to do to add __hash__ = None if your class is mutable. Having a type checker yell at you every time you have to do that has never felt helpful to me.

I can propose them both (either together or separately) as followups. I'd rather keep them out of this PR, since either special case (while, IMO, well-motivated) would be a deviation from the behaviour of other type checkers.

def __setattr__(self, name: Literal["day", "month", "year"], value: int) -> None:
pass

Expand Down
4 changes: 2 additions & 2 deletions crates/ty_python_semantic/resources/mdtest/class/super.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,8 @@ class A[T]:
return a

class B[T](A[T]):
def f(self, b: T) -> T:
return super().f(b)
def f(self, a: T) -> T:
return super().f(a)
```

## Invalid Usages
Expand Down
43 changes: 20 additions & 23 deletions crates/ty_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ class GtReturnType: ...
class GeReturnType: ...

class A:
def __eq__(self, other: A) -> EqReturnType:
def __eq__(self, other: A) -> EqReturnType: # error: [invalid-method-override]
return EqReturnType()

def __ne__(self, other: A) -> NeReturnType:
def __ne__(self, other: A) -> NeReturnType: # error: [invalid-method-override]
return NeReturnType()

def __lt__(self, other: A) -> LtReturnType:
Expand Down Expand Up @@ -66,10 +66,10 @@ class GtReturnType: ...
class GeReturnType: ...

class A:
def __eq__(self, other: B) -> EqReturnType:
def __eq__(self, other: B) -> EqReturnType: # error: [invalid-method-override]
return EqReturnType()

def __ne__(self, other: B) -> NeReturnType:
def __ne__(self, other: B) -> NeReturnType: # error: [invalid-method-override]
return NeReturnType()

def __lt__(self, other: B) -> LtReturnType:
Expand Down Expand Up @@ -111,10 +111,10 @@ class GtReturnType: ...
class GeReturnType: ...

class A:
def __eq__(self, other: B) -> EqReturnType:
def __eq__(self, other: B) -> EqReturnType: # error: [invalid-method-override]
return EqReturnType()

def __ne__(self, other: B) -> NeReturnType:
def __ne__(self, other: B) -> NeReturnType: # error: [invalid-method-override]
return NeReturnType()

def __lt__(self, other: B) -> LtReturnType:
Expand All @@ -132,12 +132,10 @@ class A:
class Unrelated: ...

class B:
# To override builtins.object.__eq__ and builtins.object.__ne__
# TODO these should emit an invalid override diagnostic
def __eq__(self, other: Unrelated) -> B:
def __eq__(self, other: Unrelated) -> B: # error: [invalid-method-override]
return B()

def __ne__(self, other: Unrelated) -> B:
def __ne__(self, other: Unrelated) -> B: # error: [invalid-method-override]
return B()

# Because `object.__eq__` and `object.__ne__` accept `object` in typeshed,
Expand Down Expand Up @@ -180,10 +178,10 @@ class GtReturnType: ...
class GeReturnType: ...

class A:
def __eq__(self, other: A) -> A:
def __eq__(self, other: A) -> A: # error: [invalid-method-override]
return A()

def __ne__(self, other: A) -> A:
def __ne__(self, other: A) -> A: # error: [invalid-method-override]
return A()

def __lt__(self, other: A) -> A:
Expand All @@ -199,22 +197,22 @@ class A:
return A()

class B(A):
def __eq__(self, other: A) -> EqReturnType:
def __eq__(self, other: A) -> EqReturnType: # error: [invalid-method-override]
return EqReturnType()

def __ne__(self, other: A) -> NeReturnType:
def __ne__(self, other: A) -> NeReturnType: # error: [invalid-method-override]
return NeReturnType()

def __lt__(self, other: A) -> LtReturnType:
def __lt__(self, other: A) -> LtReturnType: # error: [invalid-method-override]
return LtReturnType()

def __le__(self, other: A) -> LeReturnType:
def __le__(self, other: A) -> LeReturnType: # error: [invalid-method-override]
return LeReturnType()

def __gt__(self, other: A) -> GtReturnType:
def __gt__(self, other: A) -> GtReturnType: # error: [invalid-method-override]
return GtReturnType()

def __ge__(self, other: A) -> GeReturnType:
def __ge__(self, other: A) -> GeReturnType: # error: [invalid-method-override]
return GeReturnType()

reveal_type(A() == B()) # revealed: EqReturnType
Expand Down Expand Up @@ -243,10 +241,10 @@ class A:
return A()

class B(A):
def __lt__(self, other: int) -> B:
def __lt__(self, other: int) -> B: # error: [invalid-method-override]
return B()

def __gt__(self, other: int) -> B:
def __gt__(self, other: int) -> B: # error: [invalid-method-override]
return B()

reveal_type(A() < B()) # revealed: A
Expand Down Expand Up @@ -291,11 +289,10 @@ Please refer to the [docs](https://docs.python.org/3/reference/datamodel.html#ob
from __future__ import annotations

class A:
# TODO both these overrides should emit invalid-override diagnostic
def __eq__(self, other: int) -> A:
def __eq__(self, other: int) -> A: # error: [invalid-method-override]
return A()

def __ne__(self, other: int) -> A:
def __ne__(self, other: int) -> A: # error: [invalid-method-override]
return A()

reveal_type(A() == A()) # revealed: bool
Expand Down
5 changes: 3 additions & 2 deletions crates/ty_python_semantic/resources/mdtest/comparison/tuples.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ class GtReturnType: ...
class GeReturnType: ...

class A:
def __eq__(self, o: object) -> EqReturnType:
def __eq__(self, o: object) -> EqReturnType: # error: [invalid-method-override]
return EqReturnType()

def __ne__(self, o: object) -> NeReturnType:
def __ne__(self, o: object) -> NeReturnType: # error: [invalid-method-override]
return NeReturnType()

def __lt__(self, o: A) -> LtReturnType:
Expand Down Expand Up @@ -386,6 +386,7 @@ class NotBoolable:
__bool__: None = None

class A:
# error: [invalid-method-override]
def __eq__(self, other) -> NotBoolable:
return NotBoolable()

Expand Down
3 changes: 1 addition & 2 deletions crates/ty_python_semantic/resources/mdtest/expression/len.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ class UnknownLengthSubclassWithDunderLenOverridden(tuple[int, ...]):
reveal_type(len(UnknownLengthSubclassWithDunderLenOverridden())) # revealed: Literal[42]

class FixedLengthSubclassWithDunderLenOverridden(tuple[int]):
# TODO: we should complain about this as a Liskov violation (incompatible override)
def __len__(self) -> Literal[42]:
def __len__(self) -> Literal[42]: # error: [invalid-method-override]
return 42

reveal_type(len(FixedLengthSubclassWithDunderLenOverridden((1,)))) # revealed: Literal[42]
Expand Down
Loading
Loading