Skip to content

Commit ec973f2

Browse files
committed
[ty] Check method definitions on subclasses for Liskov violations
1 parent 9e80e5a commit ec973f2

13 files changed

+792
-50
lines changed

crates/ty_python_semantic/resources/mdtest/attributes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,6 +1904,7 @@ we only consider the attribute assignment to be valid if the assigned attribute
19041904
from typing import Literal
19051905

19061906
class Date:
1907+
# error: [invalid-method-override]
19071908
def __setattr__(self, name: Literal["day", "month", "year"], value: int) -> None:
19081909
pass
19091910

crates/ty_python_semantic/resources/mdtest/class/super.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,8 +501,8 @@ class A[T]:
501501
return a
502502

503503
class B[T](A[T]):
504-
def f(self, b: T) -> T:
505-
return super().f(b)
504+
def f(self, a: T) -> T:
505+
return super().f(a)
506506
```
507507

508508
## Invalid Usages

crates/ty_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ class GtReturnType: ...
2424
class GeReturnType: ...
2525

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

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

3333
def __lt__(self, other: A) -> LtReturnType:
@@ -66,10 +66,10 @@ class GtReturnType: ...
6666
class GeReturnType: ...
6767

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

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

7575
def __lt__(self, other: B) -> LtReturnType:
@@ -111,10 +111,10 @@ class GtReturnType: ...
111111
class GeReturnType: ...
112112

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

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

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

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

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

143141
# Because `object.__eq__` and `object.__ne__` accept `object` in typeshed,
@@ -180,10 +178,10 @@ class GtReturnType: ...
180178
class GeReturnType: ...
181179

182180
class A:
183-
def __eq__(self, other: A) -> A:
181+
def __eq__(self, other: A) -> A: # error: [invalid-method-override]
184182
return A()
185183

186-
def __ne__(self, other: A) -> A:
184+
def __ne__(self, other: A) -> A: # error: [invalid-method-override]
187185
return A()
188186

189187
def __lt__(self, other: A) -> A:
@@ -199,22 +197,23 @@ class A:
199197
return A()
200198

201199
class B(A):
202-
def __eq__(self, other: A) -> EqReturnType:
200+
201+
def __eq__(self, other: A) -> EqReturnType: # error: [invalid-method-override]
203202
return EqReturnType()
204203

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

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

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

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

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

220219
reveal_type(A() == B()) # revealed: EqReturnType
@@ -243,10 +242,10 @@ class A:
243242
return A()
244243

245244
class B(A):
246-
def __lt__(self, other: int) -> B:
245+
def __lt__(self, other: int) -> B: # error: [invalid-method-override]
247246
return B()
248247

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

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

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

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

301299
reveal_type(A() == A()) # revealed: bool

crates/ty_python_semantic/resources/mdtest/comparison/tuples.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,10 @@ class GtReturnType: ...
155155
class GeReturnType: ...
156156

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

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

164164
def __lt__(self, o: A) -> LtReturnType:
@@ -386,6 +386,7 @@ class NotBoolable:
386386
__bool__: None = None
387387

388388
class A:
389+
# error: [invalid-method-override]
389390
def __eq__(self, other) -> NotBoolable:
390391
return NotBoolable()
391392

crates/ty_python_semantic/resources/mdtest/expression/len.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ class UnknownLengthSubclassWithDunderLenOverridden(tuple[int, ...]):
103103
reveal_type(len(UnknownLengthSubclassWithDunderLenOverridden())) # revealed: Literal[42]
104104

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

110109
reveal_type(len(FixedLengthSubclassWithDunderLenOverridden((1,)))) # revealed: Literal[42]
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
# The Liskov Substitution Principle
2+
3+
The Liskov Substitution Principle provides the basis for many of the assumptions a type checker
4+
generally makes about types in Python:
5+
6+
> Subtype Requirement: Let ⁠`ϕ(x)`⁠ be a property provable about objects ⁠`x`⁠ of type `T`. Then
7+
> `ϕ(y)`⁠ should be true for objects ⁠`y` of type `S` where `S` is a subtype of `T`.
8+
9+
In order for a type checker's assumptions to be sound, it is crucial for the type checker to enforce
10+
the Liskov Substitution Principle on code that it checks. In practice, this usually manifests as
11+
three checks for a type checker to perform when it checks a subclass `B` of a class `A`:
12+
13+
1. Read-only attributes should only ever be overridden covariantly: if a property `A.p` resolves to
14+
`int` when accessed, accessing `B.p` should either resolve to `int` or a subtype of `int`.
15+
1. Method return types should only ever be overidden covariantly: if a method `A.f` returns `int`
16+
when called, calling `B.f` should also resolve to `int or a subtype of`int\`.
17+
1. Method parameters should only ever be overridden contravariantly: if a method `A.f` can be called
18+
with an argument of type `bool`, then the method `B.f` must also be callable with type `bool`
19+
(though it is permitted for the override to also accept other types)
20+
1. Mutable attributes should only ever be overridden invariantly: if a mutable attribute `A.attr`
21+
resolves to type `str`, it can only be overidden on a subclass with exactly the same type.
22+
23+
## Method return types
24+
25+
<!-- snapshot-diagnostics -->
26+
27+
```pyi
28+
class Super:
29+
def method(self) -> int: ...
30+
31+
class Sub1(Super):
32+
def method(self) -> int: ... # fine
33+
34+
class Sub2(Super):
35+
def method(self) -> bool: ... # fine: `bool` is a subtype of `int`
36+
37+
class Sub3(Super):
38+
def method(self) -> object: ... # error: [invalid-method-override]
39+
40+
class Sub4(Super):
41+
def method(self) -> str: ... # error: [invalid-method-override]
42+
```
43+
44+
## Method parameters
45+
46+
<!-- snapshot-diagnostics -->
47+
48+
```pyi
49+
class Super:
50+
def method(self, x: int, /): ...
51+
52+
class Sub1(Super):
53+
def method(self, x: int, /): ... # fine
54+
55+
class Sub2(Super):
56+
def method(self, x: object, /): ... # fine: `method` still accepts any argument of type `bool`
57+
58+
class Sub4(Super):
59+
def method(self, x: int | str, /): ... # fine
60+
61+
class Sub5(Super):
62+
def method(self, x: int): ... # fine: `x` can still be passed positionally
63+
64+
class Sub6(Super):
65+
# fine: `method()` can still be called with just a single argument
66+
def method(self, x: int, *args): ...
67+
68+
class Sub7(Super):
69+
def method(self, x: int, **kwargs): ... # fine
70+
71+
class Sub8(Super):
72+
def method(self, x: int, *args, **kwargs): ... # fine
73+
74+
class Sub9(Super):
75+
def method(self, x: int, extra_positional_arg=42, /): ... # fine
76+
77+
class Sub10(Super):
78+
def method(self, x: int, extra_pos_or_kw_arg=42): ... # fine
79+
80+
class Sub11(Super):
81+
def method(self, x: int, *, extra_kw_only_arg=42): ... # fine
82+
83+
class Sub12(Super):
84+
# Some calls permitted by the superclass are now no longer allowed
85+
# (the method can no longer be passed any arguments!)
86+
def method(self, /): ... # error: [invalid-method-override]
87+
88+
class Sub13(Super):
89+
# Some calls permitted by the superclass are now no longer allowed
90+
# (the method can no longer be passed with exactly one argument!)
91+
def method(self, x, y, /): ... # error: [invalid-method-override]
92+
93+
class Sub14(Super):
94+
# Some calls permitted by the superclass are now no longer allowed
95+
# (x can no longer be passed positionally!)
96+
def method(self, /, *, x): ... # error: [invalid-method-override]
97+
98+
class Sub15(Super):
99+
# Some calls permitted by the superclass are now no longer allowed
100+
# (x can no longer be passed any integer -- it now requires a bool!)
101+
def method(self, x: bool, /): ... # error: [invalid-method-override]
102+
103+
class Super2:
104+
def method2(self, x): ...
105+
106+
class Sub16(Super2):
107+
def method2(self, x, /): ... # error: [invalid-method-override]
108+
109+
class Sub17(Super2):
110+
def method2(self, *, x): ... # error: [invalid-method-override]
111+
112+
class Super3:
113+
def method3(self, *, x): ...
114+
115+
class Sub18(Super3):
116+
def method3(self, x): ... # fine: `x` can still be used as a keyword argument
117+
118+
class Sub19(Super3):
119+
def method3(self, x, /): ... # error: [invalid-method-override]
120+
121+
class Super4:
122+
def method(self, *args: int, **kwargs: str): ...
123+
124+
class Sub20(Super4):
125+
def method(self, *args: object, **kwargs: object): ... # fine
126+
127+
class Sub21(Super4):
128+
def method(self, *args): ... # error: [invalid-method-override]
129+
130+
class Sub22(Super4):
131+
def method(self, **kwargs): ... # error: [invalid-method-override]
132+
133+
class Sub23(Super4):
134+
def method(self, x, *args, y, **kwargs): ... # error: [invalid-method-override]
135+
```
136+
137+
## The entire class hierarchy is checked
138+
139+
If a child class's method definition is Liskov-compatible with the method definition on its parent
140+
class, Liskov compatibility must also nonetheless be checked with respect to the method definition
141+
on its grandparent class. This is because type checkers will treat the child class as a subtype of
142+
the grandparent class just as much as they treat it as a subtype of the parent class, so
143+
substitutability with respect to the grandparent class is just as important:
144+
145+
```pyi
146+
class Grandparent:
147+
def method(self, x: int) -> None: ...
148+
149+
class Parent(Grandparent):
150+
def method(self, x: str) -> None: ... # error: [invalid-method-override]
151+
152+
class Child(Parent):
153+
# compatible with the signature of `Parent.method`, but not with `Grandparent.method`:
154+
def method(self, x: str) -> None: ... # error: [invalid-method-override]
155+
156+
class OtherChild(Parent):
157+
# compatible with the signature of `Grandparent.method`, but not with `Parent.method`:
158+
def method(self, x: int) -> None: ... # error: [invalid-method-override]
159+
```
160+
161+
## Excluded methods
162+
163+
Certain special constructor methods are excluded from Liskov checks: `__new__` and `__init__`. None
164+
of the following classes cause us to emit any errors, therefore:
165+
166+
```pyi
167+
from typing_extensions import Self
168+
169+
class Grandparent: ...
170+
class Parent(Grandparent):
171+
def __new__(cls, x: int) -> Self: ...
172+
def __init__(self, x: int) -> None: ...
173+
174+
class Child(Parent):
175+
def __new__(cls, x: str, y: str) -> Self: ...
176+
def __init__(self, x: str, y: str) -> Self: ...
177+
```

crates/ty_python_semantic/resources/mdtest/protocols.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3073,18 +3073,15 @@ from typing import Protocol
30733073
from ty_extensions import static_assert, is_subtype_of, is_equivalent_to, is_disjoint_from
30743074

30753075
class HasRepr(Protocol):
3076-
# TODO: we should emit a diagnostic here complaining about a Liskov violation
3077-
# (it incompatibly overrides `__repr__` from `object`, a supertype of `HasRepr`)
3076+
# error: [invalid-method-override]
30783077
def __repr__(self) -> object: ...
30793078

30803079
class HasReprRecursive(Protocol):
3081-
# TODO: we should emit a diagnostic here complaining about a Liskov violation
3082-
# (it incompatibly overrides `__repr__` from `object`, a supertype of `HasReprRecursive`)
3080+
# error: [invalid-method-override]
30833081
def __repr__(self) -> "HasReprRecursive": ...
30843082

30853083
class HasReprRecursiveAndFoo(Protocol):
3086-
# TODO: we should emit a diagnostic here complaining about a Liskov violation
3087-
# (it incompatibly overrides `__repr__` from `object`, a supertype of `HasReprRecursiveAndFoo`)
3084+
# error: [invalid-method-override]
30883085
def __repr__(self) -> "HasReprRecursiveAndFoo": ...
30893086
foo: int
30903087

0 commit comments

Comments
 (0)