Skip to content

Commit 41911e1

Browse files
AlexWaygoodAlexWaygood
committed
Fix incorrect identification of where the overridden method was defined
1 parent bffe4a3 commit 41911e1

File tree

3 files changed

+187
-2
lines changed

3 files changed

+187
-2
lines changed

‎crates/ty_python_semantic/resources/mdtest/liskov.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ on its grandparent class. This is because type checkers will treat the child cla
142142
the grandparent class just as much as they treat it as a subtype of the parent class, so
143143
substitutability with respect to the grandparent class is just as important:
144144

145+
<!-- snapshot-diagnostics -->
146+
147+
`stub.pyi`:
148+
145149
```pyi
146150
class Grandparent:
147151
def method(self, x: int) -> None: ...
@@ -158,6 +162,27 @@ class OtherChild(Parent):
158162
def method(self, x: int) -> None: ... # error: [invalid-method-override]
159163
```
160164

165+
`other_stub.pyi`:
166+
167+
```pyi
168+
class A:
169+
def get(self, default): ...
170+
171+
class B(A):
172+
def get(self, default, /): ... # error: [invalid-method-override]
173+
174+
get = 56
175+
176+
class C(B):
177+
# `get` appears in the symbol table of `C`,
178+
# but that doesn't confuse our diagnostic...
179+
foo = get
180+
181+
class D(C):
182+
# compatible with `C.get` and `B.get`, but not with `A.get`
183+
def get(self, my_default): ... # error: [invalid-method-override]
184+
```
185+
161186
## Excluded methods
162187

163188
Certain special constructor methods are excluded from Liskov checks. None of the following classes

‎crates/ty_python_semantic/resources/mdtest/snapshots/liskov.md_-_The_Liskov_Substitut…_-_The_entire_class_hie…_(5e8fca10d966c36e).snap‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
---
2+
source: crates/ty_test/src/lib.rs
3+
expression: snapshot
4+
---
5+
---
6+
mdtest name: liskov.md - The Liskov Substitution Principle - The entire class hierarchy is checked
7+
mdtest path: crates/ty_python_semantic/resources/mdtest/liskov.md
8+
---
9+
10+
# Python source files
11+
12+
## stub.pyi
13+
14+
```
15+
1 | class Grandparent:
16+
2 | def method(self, x: int) -> None: ...
17+
3 |
18+
4 | class Parent(Grandparent):
19+
5 | def method(self, x: str) -> None: ... # error: [invalid-method-override]
20+
6 |
21+
7 | class Child(Parent):
22+
8 | # compatible with the signature of `Parent.method`, but not with `Grandparent.method`:
23+
9 | def method(self, x: str) -> None: ... # error: [invalid-method-override]
24+
10 |
25+
11 | class OtherChild(Parent):
26+
12 | # compatible with the signature of `Grandparent.method`, but not with `Parent.method`:
27+
13 | def method(self, x: int) -> None: ... # error: [invalid-method-override]
28+
```
29+
30+
## other_stub.pyi
31+
32+
```
33+
1 | class A:
34+
2 | def get(self, default): ...
35+
3 |
36+
4 | class B(A):
37+
5 | def get(self, default, /): ... # error: [invalid-method-override]
38+
6 |
39+
7 | get = 56
40+
8 |
41+
9 | class C(B):
42+
10 | # `get` appears in the symbol table of `C`,
43+
11 | # but that doesn't confuse our diagnostic...
44+
12 | foo = get
45+
13 |
46+
14 | class D(C):
47+
15 | # compatible with `C.get` and `B.get`, but not with `A.get`
48+
16 | def get(self, my_default): ... # error: [invalid-method-override]
49+
```
50+
51+
# Diagnostics
52+
53+
```
54+
error[invalid-method-override]: Invalid override of method `method`
55+
--> src/stub.pyi:2:9
56+
|
57+
1 | class Grandparent:
58+
2 | def method(self, x: int) -> None: ...
59+
| ---------------------------- `Grandparent.method` defined here
60+
3 |
61+
4 | class Parent(Grandparent):
62+
5 | def method(self, x: str) -> None: ... # error: [invalid-method-override]
63+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Grandparent.method`
64+
6 |
65+
7 | class Child(Parent):
66+
|
67+
info: This violates the Liskov Substitution Principle
68+
info: rule `invalid-method-override` is enabled by default
69+
70+
```
71+
72+
```
73+
error[invalid-method-override]: Invalid override of method `method`
74+
--> src/stub.pyi:9:9
75+
|
76+
7 | class Child(Parent):
77+
8 | # compatible with the signature of `Parent.method`, but not with `Grandparent.method`:
78+
9 | def method(self, x: str) -> None: ... # error: [invalid-method-override]
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Grandparent.method`
80+
10 |
81+
11 | class OtherChild(Parent):
82+
|
83+
::: src/stub.pyi:2:9
84+
|
85+
1 | class Grandparent:
86+
2 | def method(self, x: int) -> None: ...
87+
| ---------------------------- `Grandparent.method` defined here
88+
3 |
89+
4 | class Parent(Grandparent):
90+
|
91+
info: This violates the Liskov Substitution Principle
92+
info: rule `invalid-method-override` is enabled by default
93+
94+
```
95+
96+
```
97+
error[invalid-method-override]: Invalid override of method `method`
98+
--> src/stub.pyi:13:9
99+
|
100+
11 | class OtherChild(Parent):
101+
12 | # compatible with the signature of `Grandparent.method`, but not with `Parent.method`:
102+
13 | def method(self, x: int) -> None: ... # error: [invalid-method-override]
103+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method`
104+
|
105+
::: src/stub.pyi:5:9
106+
|
107+
4 | class Parent(Grandparent):
108+
5 | def method(self, x: str) -> None: ... # error: [invalid-method-override]
109+
| ---------------------------- `Parent.method` defined here
110+
6 |
111+
7 | class Child(Parent):
112+
|
113+
info: This violates the Liskov Substitution Principle
114+
info: rule `invalid-method-override` is enabled by default
115+
116+
```
117+
118+
```
119+
error[invalid-method-override]: Invalid override of method `get`
120+
--> src/other_stub.pyi:2:9
121+
|
122+
1 | class A:
123+
2 | def get(self, default): ...
124+
| ------------------ `A.get` defined here
125+
3 |
126+
4 | class B(A):
127+
5 | def get(self, default, /): ... # error: [invalid-method-override]
128+
| ^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `A.get`
129+
6 |
130+
7 | get = 56
131+
|
132+
info: This violates the Liskov Substitution Principle
133+
info: rule `invalid-method-override` is enabled by default
134+
135+
```
136+
137+
```
138+
error[invalid-method-override]: Invalid override of method `get`
139+
--> src/other_stub.pyi:16:9
140+
|
141+
14 | class D(C):
142+
15 | # compatible with `C.get` and `B.get`, but not with `A.get`
143+
16 | def get(self, my_default): ... # error: [invalid-method-override]
144+
| ^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `A.get`
145+
|
146+
::: src/other_stub.pyi:2:9
147+
|
148+
1 | class A:
149+
2 | def get(self, default): ...
150+
| ------------------ `A.get` defined here
151+
3 |
152+
4 | class B(A):
153+
|
154+
info: This violates the Liskov Substitution Principle
155+
info: rule `invalid-method-override` is enabled by default
156+
157+
```

‎crates/ty_python_semantic/src/types/infer/builder/liskov.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,13 @@ fn check_class_declaration<'db>(
8383
break;
8484
};
8585

86-
let class_symbol_table = place_table(db, class.class_literal(db).0.body_scope(db));
86+
let class_symbol_table = place_table(db, supercls.class_literal(db).0.body_scope(db));
8787

8888
// If the member is not defined on the class itself, skip it
89-
if class_symbol_table.symbol_by_name(&member.name).is_none() {
89+
let Some(symbol) = class_symbol_table.symbol_by_name(&member.name) else{
90+
continue;
91+
};
92+
if !(symbol.is_bound() || symbol.is_declared()) {
9093
continue;
9194
}
9295

0 commit comments

Comments
 (0)