[ty] Don't introduce invalid syntax when autofixing override-of-final-method (#21699)

This commit is contained in:
Alex Waygood 2025-11-30 13:40:33 +00:00 committed by GitHub
parent 69ace00210
commit b02e8212c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 433 additions and 424 deletions

View File

@ -354,6 +354,13 @@ impl Diagnostic {
Arc::make_mut(&mut self.inner).fix = Some(fix);
}
/// If `fix` is `Some`, set the fix for this diagnostic.
pub fn set_optional_fix(&mut self, fix: Option<Fix>) {
if let Some(fix) = fix {
self.set_fix(fix);
}
}
/// Remove the fix for this diagnostic.
pub fn remove_fix(&mut self) {
Arc::make_mut(&mut self.inner).fix = None;

View File

@ -51,6 +51,10 @@ class Parent:
@final
def my_property2(self) -> int: ...
@property
@final
def my_property3(self) -> int: ...
@final
@classmethod
def class_method1(cls) -> int: ...
@ -86,6 +90,13 @@ class Child(Parent):
@property
def my_property2(self) -> int: ... # error: [override-of-final-method]
@my_property2.setter
def my_property2(self, x: int) -> None: ...
@property
def my_property3(self) -> int: ... # error: [override-of-final-method]
@my_property3.deleter
def my_proeprty3(self) -> None: ...
@classmethod
def class_method1(cls) -> int: ... # error: [override-of-final-method]
@ -230,7 +241,7 @@ class ChildOfBad(Bad):
def bar(self, x: str) -> str: ...
@overload
def bar(self, x: int) -> int: ... # error: [override-of-final-method]
@overload
def baz(self, x: str) -> str: ...
@overload
@ -461,14 +472,17 @@ class B(A):
def method1(self) -> None: ... # error: [override-of-final-method]
def method2(self) -> None: ... # error: [override-of-final-method]
def method3(self) -> None: ... # error: [override-of-final-method]
def method4(self) -> None: ... # error: [override-of-final-method]
# check that autofixes don't introduce invalid syntax
# if there are multiple statements on one line
#
# TODO: we should emit a Liskov violation here too
# error: [override-of-final-method]
method4 = 42; unrelated = 56 # fmt: skip
# Possible overrides of possibly `@final` methods...
class C(A):
if coinflip():
# TODO: the autofix here introduces invalid syntax because there are now no
# statements inside the `if:` branch
# (but it might still be a useful autofix in an IDE context?)
def method1(self) -> None: ... # error: [override-of-final-method]
else:
pass

View File

@ -49,26 +49,29 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md
35 | def method1(self) -> None: ... # error: [override-of-final-method]
36 | def method2(self) -> None: ... # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
39 |
40 | # Possible overrides of possibly `@final` methods...
41 | class C(A):
42 | if coinflip():
43 | # TODO: the autofix here introduces invalid syntax because there are now no
44 | # statements inside the `if:` branch
45 | # (but it might still be a useful autofix in an IDE context?)
46 | def method1(self) -> None: ... # error: [override-of-final-method]
47 | else:
48 | pass
49 |
50 | if coinflip():
51 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method]
52 | else:
53 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method]
54 |
55 | if coinflip():
56 | def method3(self) -> None: ... # error: [override-of-final-method]
57 | def method4(self) -> None: ... # error: [override-of-final-method]
38 |
39 | # check that autofixes don't introduce invalid syntax
40 | # if there are multiple statements on one line
41 | #
42 | # TODO: we should emit a Liskov violation here too
43 | # error: [override-of-final-method]
44 | method4 = 42; unrelated = 56 # fmt: skip
45 |
46 | # Possible overrides of possibly `@final` methods...
47 | class C(A):
48 | if coinflip():
49 | def method1(self) -> None: ... # error: [override-of-final-method]
50 | else:
51 | pass
52 |
53 | if coinflip():
54 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method]
55 | else:
56 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method]
57 |
58 | if coinflip():
59 | def method3(self) -> None: ... # error: [override-of-final-method]
60 | def method4(self) -> None: ... # error: [override-of-final-method]
```
# Diagnostics
@ -104,7 +107,7 @@ info: rule `override-of-final-method` is enabled by default
35 + # error: [override-of-final-method]
36 | def method2(self) -> None: ... # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
38 |
note: This is an unsafe fix and may change runtime behavior
```
@ -118,7 +121,6 @@ error[override-of-final-method]: Cannot override `A.method2`
36 | def method2(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
37 | def method3(self) -> None: ... # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
|
info: `A.method2` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.py:16:9
@ -140,8 +142,8 @@ info: rule `override-of-final-method` is enabled by default
- def method2(self) -> None: ... # error: [override-of-final-method]
36 + # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
39 |
38 |
39 | # check that autofixes don't introduce invalid syntax
note: This is an unsafe fix and may change runtime behavior
```
@ -154,7 +156,8 @@ error[override-of-final-method]: Cannot override `A.method3`
36 | def method2(self) -> None: ... # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
38 | def method4(self) -> None: ... # error: [override-of-final-method]
38 |
39 | # check that autofixes don't introduce invalid syntax
|
info: `A.method3` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.py:20:9
@ -174,23 +177,23 @@ info: rule `override-of-final-method` is enabled by default
36 | def method2(self) -> None: ... # error: [override-of-final-method]
- def method3(self) -> None: ... # error: [override-of-final-method]
37 + # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
39 |
40 | # Possible overrides of possibly `@final` methods...
38 |
39 | # check that autofixes don't introduce invalid syntax
40 | # if there are multiple statements on one line
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `A.method4`
--> src/mdtest_snippet.py:38:9
--> src/mdtest_snippet.py:44:5
|
36 | def method2(self) -> None: ... # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
38 | def method4(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
39 |
40 | # Possible overrides of possibly `@final` methods...
42 | # TODO: we should emit a Liskov violation here too
43 | # error: [override-of-final-method]
44 | method4 = 42; unrelated = 56 # fmt: skip
| ^^^^^^^ Overrides a definition from superclass `A`
45 |
46 | # Possible overrides of possibly `@final` methods...
|
info: `A.method4` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.py:29:9
@ -206,28 +209,19 @@ info: `A.method4` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `method4`
info: rule `override-of-final-method` is enabled by default
35 | def method1(self) -> None: ... # error: [override-of-final-method]
36 | def method2(self) -> None: ... # error: [override-of-final-method]
37 | def method3(self) -> None: ... # error: [override-of-final-method]
- def method4(self) -> None: ... # error: [override-of-final-method]
38 + # error: [override-of-final-method]
39 |
40 | # Possible overrides of possibly `@final` methods...
41 | class C(A):
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `A.method1`
--> src/mdtest_snippet.py:46:13
--> src/mdtest_snippet.py:49:13
|
44 | # statements inside the `if:` branch
45 | # (but it might still be a useful autofix in an IDE context?)
46 | def method1(self) -> None: ... # error: [override-of-final-method]
47 | class C(A):
48 | if coinflip():
49 | def method1(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
47 | else:
48 | pass
50 | else:
51 | pass
|
info: `A.method1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.py:8:9
@ -243,26 +237,17 @@ info: `A.method1` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `method1`
info: rule `override-of-final-method` is enabled by default
43 | # TODO: the autofix here introduces invalid syntax because there are now no
44 | # statements inside the `if:` branch
45 | # (but it might still be a useful autofix in an IDE context?)
- def method1(self) -> None: ... # error: [override-of-final-method]
46 + # error: [override-of-final-method]
47 | else:
48 | pass
49 |
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `A.method3`
--> src/mdtest_snippet.py:56:13
--> src/mdtest_snippet.py:59:13
|
55 | if coinflip():
56 | def method3(self) -> None: ... # error: [override-of-final-method]
58 | if coinflip():
59 | def method3(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
57 | def method4(self) -> None: ... # error: [override-of-final-method]
60 | def method4(self) -> None: ... # error: [override-of-final-method]
|
info: `A.method3` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.py:20:9
@ -277,23 +262,16 @@ info: `A.method3` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `method3`
info: rule `override-of-final-method` is enabled by default
53 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method]
54 |
55 | if coinflip():
- def method3(self) -> None: ... # error: [override-of-final-method]
56 + # error: [override-of-final-method]
57 | def method4(self) -> None: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `A.method4`
--> src/mdtest_snippet.py:57:13
--> src/mdtest_snippet.py:60:13
|
55 | if coinflip():
56 | def method3(self) -> None: ... # error: [override-of-final-method]
57 | def method4(self) -> None: ... # error: [override-of-final-method]
58 | if coinflip():
59 | def method3(self) -> None: ... # error: [override-of-final-method]
60 | def method4(self) -> None: ... # error: [override-of-final-method]
| ^^^^^^^ Overrides a definition from superclass `A`
|
info: `A.method4` is decorated with `@final`, forbidding overrides
@ -310,11 +288,5 @@ info: `A.method4` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `method4`
info: rule `override-of-final-method` is enabled by default
54 |
55 | if coinflip():
56 | def method3(self) -> None: ... # error: [override-of-final-method]
- def method4(self) -> None: ... # error: [override-of-final-method]
57 + # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```

View File

@ -28,93 +28,93 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md
14 | @final
15 | def my_property2(self) -> int: ...
16 |
17 | @final
18 | @classmethod
19 | def class_method1(cls) -> int: ...
17 | @property
18 | @final
19 | def my_property3(self) -> int: ...
20 |
21 | @classmethod
22 | @final
23 | def class_method2(cls) -> int: ...
21 | @final
22 | @classmethod
23 | def class_method1(cls) -> int: ...
24 |
25 | @final
26 | @staticmethod
27 | def static_method1() -> int: ...
25 | @classmethod
26 | @final
27 | def class_method2(cls) -> int: ...
28 |
29 | @staticmethod
30 | @final
31 | def static_method2() -> int: ...
29 | @final
30 | @staticmethod
31 | def static_method1() -> int: ...
32 |
33 | @lossy_decorator
33 | @staticmethod
34 | @final
35 | def decorated_1(self): ...
35 | def static_method2() -> int: ...
36 |
37 | @final
38 | @lossy_decorator
39 | def decorated_2(self): ...
37 | @lossy_decorator
38 | @final
39 | def decorated_1(self): ...
40 |
41 | class Child(Parent):
42 | # explicitly test the concise diagnostic message,
43 | # which is different to the verbose diagnostic summary message:
44 | #
45 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
46 | def foo(self): ...
47 | @property
48 | def my_property1(self) -> int: ... # error: [override-of-final-method]
49 |
50 | @property
51 | def my_property2(self) -> int: ... # error: [override-of-final-method]
52 |
53 | @classmethod
54 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
55 |
56 | @staticmethod
57 | def static_method1() -> int: ... # error: [override-of-final-method]
41 | @final
42 | @lossy_decorator
43 | def decorated_2(self): ...
44 |
45 | class Child(Parent):
46 | # explicitly test the concise diagnostic message,
47 | # which is different to the verbose diagnostic summary message:
48 | #
49 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
50 | def foo(self): ...
51 | @property
52 | def my_property1(self) -> int: ... # error: [override-of-final-method]
53 |
54 | @property
55 | def my_property2(self) -> int: ... # error: [override-of-final-method]
56 | @my_property2.setter
57 | def my_property2(self, x: int) -> None: ...
58 |
59 | @classmethod
60 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
61 |
62 | @staticmethod
63 | def static_method2() -> int: ... # error: [override-of-final-method]
64 |
65 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
59 | @property
60 | def my_property3(self) -> int: ... # error: [override-of-final-method]
61 | @my_property3.deleter
62 | def my_proeprty3(self) -> None: ...
63 |
64 | @classmethod
65 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
66 |
67 | @lossy_decorator
68 | def decorated_2(self): ... # TODO: should emit [override-of-final-method]
67 | @staticmethod
68 | def static_method1() -> int: ... # error: [override-of-final-method]
69 |
70 | class OtherChild(Parent): ...
71 |
72 | class Grandchild(OtherChild):
70 | @classmethod
71 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
72 |
73 | @staticmethod
74 | # TODO: we should emit a Liskov violation here too
75 | # error: [override-of-final-method]
76 | def foo(): ...
77 | @property
78 | # TODO: we should emit a Liskov violation here too
79 | # error: [override-of-final-method]
80 | def my_property1(self) -> str: ...
81 | # TODO: we should emit a Liskov violation here too
82 | # error: [override-of-final-method]
83 | class_method1 = None
84 |
85 | # Diagnostic edge case: `final` is very far away from the method definition in the source code:
86 |
87 | T = TypeVar("T")
88 |
89 | def identity(x: T) -> T: ...
90 |
91 | class Foo:
92 | @final
93 | @identity
94 | @identity
95 | @identity
96 | @identity
97 | @identity
98 | @identity
99 | @identity
100 | @identity
101 | @identity
102 | @identity
103 | @identity
74 | def static_method2() -> int: ... # error: [override-of-final-method]
75 |
76 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
77 |
78 | @lossy_decorator
79 | def decorated_2(self): ... # TODO: should emit [override-of-final-method]
80 |
81 | class OtherChild(Parent): ...
82 |
83 | class Grandchild(OtherChild):
84 | @staticmethod
85 | # TODO: we should emit a Liskov violation here too
86 | # error: [override-of-final-method]
87 | def foo(): ...
88 | @property
89 | # TODO: we should emit a Liskov violation here too
90 | # error: [override-of-final-method]
91 | def my_property1(self) -> str: ...
92 | # TODO: we should emit a Liskov violation here too
93 | # error: [override-of-final-method]
94 | class_method1 = None
95 |
96 | # Diagnostic edge case: `final` is very far away from the method definition in the source code:
97 |
98 | T = TypeVar("T")
99 |
100 | def identity(x: T) -> T: ...
101 |
102 | class Foo:
103 | @final
104 | @identity
105 | @identity
106 | @identity
@ -122,24 +122,35 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md
108 | @identity
109 | @identity
110 | @identity
111 | def bar(self): ...
112 |
113 | class Baz(Foo):
114 | def bar(self): ... # error: [override-of-final-method]
111 | @identity
112 | @identity
113 | @identity
114 | @identity
115 | @identity
116 | @identity
117 | @identity
118 | @identity
119 | @identity
120 | @identity
121 | @identity
122 | def bar(self): ...
123 |
124 | class Baz(Foo):
125 | def bar(self): ... # error: [override-of-final-method]
```
# Diagnostics
```
error[override-of-final-method]: Cannot override `Parent.foo`
--> src/mdtest_snippet.pyi:46:9
--> src/mdtest_snippet.pyi:50:9
|
44 | #
45 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
46 | def foo(self): ...
48 | #
49 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
50 | def foo(self): ...
| ^^^ Overrides a definition from superclass `Parent`
47 | @property
48 | def my_property1(self) -> int: ... # error: [override-of-final-method]
51 | @property
52 | def my_property1(self) -> int: ... # error: [override-of-final-method]
|
info: `Parent.foo` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:6:5
@ -154,28 +165,28 @@ info: `Parent.foo` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `foo`
info: rule `override-of-final-method` is enabled by default
43 | # which is different to the verbose diagnostic summary message:
44 | #
45 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
47 | # which is different to the verbose diagnostic summary message:
48 | #
49 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
- def foo(self): ...
46 +
47 | @property
48 | def my_property1(self) -> int: ... # error: [override-of-final-method]
49 |
50 +
51 | @property
52 | def my_property1(self) -> int: ... # error: [override-of-final-method]
53 |
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.my_property1`
--> src/mdtest_snippet.pyi:48:9
--> src/mdtest_snippet.pyi:52:9
|
46 | def foo(self): ...
47 | @property
48 | def my_property1(self) -> int: ... # error: [override-of-final-method]
50 | def foo(self): ...
51 | @property
52 | def my_property1(self) -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
49 |
50 | @property
53 |
54 | @property
|
info: `Parent.my_property1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:9:5
@ -192,28 +203,18 @@ info: `Parent.my_property1` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `my_property1`
info: rule `override-of-final-method` is enabled by default
44 | #
45 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`"
46 | def foo(self): ...
- @property
- def my_property1(self) -> int: ... # error: [override-of-final-method]
47 + # error: [override-of-final-method]
48 |
49 | @property
50 | def my_property2(self) -> int: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.my_property2`
--> src/mdtest_snippet.pyi:51:9
--> src/mdtest_snippet.pyi:55:9
|
50 | @property
51 | def my_property2(self) -> int: ... # error: [override-of-final-method]
54 | @property
55 | def my_property2(self) -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
52 |
53 | @classmethod
56 | @my_property2.setter
57 | def my_property2(self, x: int) -> None: ...
|
info: `Parent.my_property2` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:14:5
@ -224,181 +225,197 @@ info: `Parent.my_property2` is decorated with `@final`, forbidding overrides
15 | def my_property2(self) -> int: ...
| ------------ `Parent.my_property2` defined here
16 |
17 | @final
17 | @property
|
help: Remove the override of `my_property2`
help: Remove the getter and setter for `my_property2`
info: rule `override-of-final-method` is enabled by default
```
```
error[override-of-final-method]: Cannot override `Parent.my_property3`
--> src/mdtest_snippet.pyi:60:9
|
59 | @property
60 | def my_property3(self) -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
61 | @my_property3.deleter
62 | def my_proeprty3(self) -> None: ...
|
info: `Parent.my_property3` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:18:5
|
17 | @property
18 | @final
| ------
19 | def my_property3(self) -> int: ...
| ------------ `Parent.my_property3` defined here
20 |
21 | @final
|
help: Remove the override of `my_property3`
info: rule `override-of-final-method` is enabled by default
47 | @property
48 | def my_property1(self) -> int: ... # error: [override-of-final-method]
49 |
- @property
- def my_property2(self) -> int: ... # error: [override-of-final-method]
50 + # error: [override-of-final-method]
51 |
52 | @classmethod
53 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.class_method1`
--> src/mdtest_snippet.pyi:54:9
--> src/mdtest_snippet.pyi:65:9
|
53 | @classmethod
54 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
64 | @classmethod
65 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
55 |
56 | @staticmethod
66 |
67 | @staticmethod
|
info: `Parent.class_method1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:17:5
--> src/mdtest_snippet.pyi:21:5
|
15 | def my_property2(self) -> int: ...
16 |
17 | @final
| ------
18 | @classmethod
19 | def class_method1(cls) -> int: ...
| ------------- `Parent.class_method1` defined here
19 | def my_property3(self) -> int: ...
20 |
21 | @classmethod
21 | @final
| ------
22 | @classmethod
23 | def class_method1(cls) -> int: ...
| ------------- `Parent.class_method1` defined here
24 |
25 | @classmethod
|
help: Remove the override of `class_method1`
info: rule `override-of-final-method` is enabled by default
50 | @property
51 | def my_property2(self) -> int: ... # error: [override-of-final-method]
52 |
61 | @my_property3.deleter
62 | def my_proeprty3(self) -> None: ...
63 |
- @classmethod
- def class_method1(cls) -> int: ... # error: [override-of-final-method]
53 + # error: [override-of-final-method]
54 |
55 | @staticmethod
56 | def static_method1() -> int: ... # error: [override-of-final-method]
64 + # error: [override-of-final-method]
65 |
66 | @staticmethod
67 | def static_method1() -> int: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.static_method1`
--> src/mdtest_snippet.pyi:57:9
--> src/mdtest_snippet.pyi:68:9
|
56 | @staticmethod
57 | def static_method1() -> int: ... # error: [override-of-final-method]
67 | @staticmethod
68 | def static_method1() -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
58 |
59 | @classmethod
69 |
70 | @classmethod
|
info: `Parent.static_method1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:25:5
--> src/mdtest_snippet.pyi:29:5
|
23 | def class_method2(cls) -> int: ...
24 |
25 | @final
| ------
26 | @staticmethod
27 | def static_method1() -> int: ...
| -------------- `Parent.static_method1` defined here
27 | def class_method2(cls) -> int: ...
28 |
29 | @staticmethod
29 | @final
| ------
30 | @staticmethod
31 | def static_method1() -> int: ...
| -------------- `Parent.static_method1` defined here
32 |
33 | @staticmethod
|
help: Remove the override of `static_method1`
info: rule `override-of-final-method` is enabled by default
53 | @classmethod
54 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
55 |
64 | @classmethod
65 | def class_method1(cls) -> int: ... # error: [override-of-final-method]
66 |
- @staticmethod
- def static_method1() -> int: ... # error: [override-of-final-method]
56 + # error: [override-of-final-method]
57 |
58 | @classmethod
59 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
67 + # error: [override-of-final-method]
68 |
69 | @classmethod
70 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.class_method2`
--> src/mdtest_snippet.pyi:60:9
--> src/mdtest_snippet.pyi:71:9
|
59 | @classmethod
60 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
70 | @classmethod
71 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
61 |
62 | @staticmethod
72 |
73 | @staticmethod
|
info: `Parent.class_method2` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:22:5
--> src/mdtest_snippet.pyi:26:5
|
21 | @classmethod
22 | @final
25 | @classmethod
26 | @final
| ------
23 | def class_method2(cls) -> int: ...
27 | def class_method2(cls) -> int: ...
| ------------- `Parent.class_method2` defined here
24 |
25 | @final
28 |
29 | @final
|
help: Remove the override of `class_method2`
info: rule `override-of-final-method` is enabled by default
56 | @staticmethod
57 | def static_method1() -> int: ... # error: [override-of-final-method]
58 |
67 | @staticmethod
68 | def static_method1() -> int: ... # error: [override-of-final-method]
69 |
- @classmethod
- def class_method2(cls) -> int: ... # error: [override-of-final-method]
59 + # error: [override-of-final-method]
60 |
61 | @staticmethod
62 | def static_method2() -> int: ... # error: [override-of-final-method]
70 + # error: [override-of-final-method]
71 |
72 | @staticmethod
73 | def static_method2() -> int: ... # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.static_method2`
--> src/mdtest_snippet.pyi:63:9
--> src/mdtest_snippet.pyi:74:9
|
62 | @staticmethod
63 | def static_method2() -> int: ... # error: [override-of-final-method]
73 | @staticmethod
74 | def static_method2() -> int: ... # error: [override-of-final-method]
| ^^^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
64 |
65 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
75 |
76 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
|
info: `Parent.static_method2` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:30:5
--> src/mdtest_snippet.pyi:34:5
|
29 | @staticmethod
30 | @final
33 | @staticmethod
34 | @final
| ------
31 | def static_method2() -> int: ...
35 | def static_method2() -> int: ...
| -------------- `Parent.static_method2` defined here
32 |
33 | @lossy_decorator
36 |
37 | @lossy_decorator
|
help: Remove the override of `static_method2`
info: rule `override-of-final-method` is enabled by default
59 | @classmethod
60 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
61 |
70 | @classmethod
71 | def class_method2(cls) -> int: ... # error: [override-of-final-method]
72 |
- @staticmethod
- def static_method2() -> int: ... # error: [override-of-final-method]
62 + # error: [override-of-final-method]
63 |
64 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
65 |
73 + # error: [override-of-final-method]
74 |
75 | def decorated_1(self): ... # TODO: should emit [override-of-final-method]
76 |
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.foo`
--> src/mdtest_snippet.pyi:76:9
--> src/mdtest_snippet.pyi:87:9
|
74 | # TODO: we should emit a Liskov violation here too
75 | # error: [override-of-final-method]
76 | def foo(): ...
85 | # TODO: we should emit a Liskov violation here too
86 | # error: [override-of-final-method]
87 | def foo(): ...
| ^^^ Overrides a definition from superclass `Parent`
77 | @property
78 | # TODO: we should emit a Liskov violation here too
88 | @property
89 | # TODO: we should emit a Liskov violation here too
|
info: `Parent.foo` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:6:5
@ -413,31 +430,31 @@ info: `Parent.foo` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `foo`
info: rule `override-of-final-method` is enabled by default
70 | class OtherChild(Parent): ...
71 |
72 | class Grandchild(OtherChild):
81 | class OtherChild(Parent): ...
82 |
83 | class Grandchild(OtherChild):
- @staticmethod
- # TODO: we should emit a Liskov violation here too
- # error: [override-of-final-method]
- def foo(): ...
73 +
74 | @property
75 | # TODO: we should emit a Liskov violation here too
76 | # error: [override-of-final-method]
84 +
85 | @property
86 | # TODO: we should emit a Liskov violation here too
87 | # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.my_property1`
--> src/mdtest_snippet.pyi:80:9
--> src/mdtest_snippet.pyi:91:9
|
78 | # TODO: we should emit a Liskov violation here too
79 | # error: [override-of-final-method]
80 | def my_property1(self) -> str: ...
89 | # TODO: we should emit a Liskov violation here too
90 | # error: [override-of-final-method]
91 | def my_property1(self) -> str: ...
| ^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
81 | # TODO: we should emit a Liskov violation here too
82 | # error: [override-of-final-method]
92 | # TODO: we should emit a Liskov violation here too
93 | # error: [override-of-final-method]
|
info: `Parent.my_property1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:9:5
@ -454,92 +471,71 @@ info: `Parent.my_property1` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `my_property1`
info: rule `override-of-final-method` is enabled by default
74 | # TODO: we should emit a Liskov violation here too
75 | # error: [override-of-final-method]
76 | def foo(): ...
- @property
- # TODO: we should emit a Liskov violation here too
- # error: [override-of-final-method]
- def my_property1(self) -> str: ...
77 +
78 | # TODO: we should emit a Liskov violation here too
79 | # error: [override-of-final-method]
80 | class_method1 = None
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Parent.class_method1`
--> src/mdtest_snippet.pyi:83:5
--> src/mdtest_snippet.pyi:94:5
|
81 | # TODO: we should emit a Liskov violation here too
82 | # error: [override-of-final-method]
83 | class_method1 = None
92 | # TODO: we should emit a Liskov violation here too
93 | # error: [override-of-final-method]
94 | class_method1 = None
| ^^^^^^^^^^^^^ Overrides a definition from superclass `Parent`
84 |
85 | # Diagnostic edge case: `final` is very far away from the method definition in the source code:
95 |
96 | # Diagnostic edge case: `final` is very far away from the method definition in the source code:
|
info: `Parent.class_method1` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:17:5
--> src/mdtest_snippet.pyi:21:5
|
15 | def my_property2(self) -> int: ...
16 |
17 | @final
| ------
18 | @classmethod
19 | def class_method1(cls) -> int: ...
| ------------- `Parent.class_method1` defined here
19 | def my_property3(self) -> int: ...
20 |
21 | @classmethod
21 | @final
| ------
22 | @classmethod
23 | def class_method1(cls) -> int: ...
| ------------- `Parent.class_method1` defined here
24 |
25 | @classmethod
|
help: Remove the override of `class_method1`
info: rule `override-of-final-method` is enabled by default
80 | def my_property1(self) -> str: ...
81 | # TODO: we should emit a Liskov violation here too
82 | # error: [override-of-final-method]
- class_method1 = None
83 +
84 |
85 | # Diagnostic edge case: `final` is very far away from the method definition in the source code:
86 |
note: This is an unsafe fix and may change runtime behavior
```
```
error[override-of-final-method]: Cannot override `Foo.bar`
--> src/mdtest_snippet.pyi:114:9
--> src/mdtest_snippet.pyi:125:9
|
113 | class Baz(Foo):
114 | def bar(self): ... # error: [override-of-final-method]
124 | class Baz(Foo):
125 | def bar(self): ... # error: [override-of-final-method]
| ^^^ Overrides a definition from superclass `Foo`
|
info: `Foo.bar` is decorated with `@final`, forbidding overrides
--> src/mdtest_snippet.pyi:92:5
--> src/mdtest_snippet.pyi:103:5
|
91 | class Foo:
92 | @final
102 | class Foo:
103 | @final
| ------
93 | @identity
94 | @identity
104 | @identity
105 | @identity
|
::: src/mdtest_snippet.pyi:111:9
::: src/mdtest_snippet.pyi:122:9
|
109 | @identity
110 | @identity
111 | def bar(self): ...
120 | @identity
121 | @identity
122 | def bar(self): ...
| --- `Foo.bar` defined here
112 |
113 | class Baz(Foo):
123 |
124 | class Baz(Foo):
|
help: Remove the override of `bar`
info: rule `override-of-final-method` is enabled by default
111 | def bar(self): ...
112 |
113 | class Baz(Foo):
122 | def bar(self): ...
123 |
124 | class Baz(Foo):
- def bar(self): ... # error: [override-of-final-method]
114 + # error: [override-of-final-method]
125 + pass # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```

View File

@ -53,7 +53,7 @@ info: rule `override-of-final-method` is enabled by default
2 |
3 | class Foo(module1.Foo):
- def f(self): ... # error: [override-of-final-method]
4 + # error: [override-of-final-method]
4 + pass # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```

View File

@ -59,7 +59,7 @@ info: rule `override-of-final-method` is enabled by default
7 | class B(A):
- @final
- def f(self): ... # error: [override-of-final-method]
8 + # error: [override-of-final-method]
8 + pass # error: [override-of-final-method]
9 |
10 | class C(B):
11 | @final
@ -95,7 +95,7 @@ info: rule `override-of-final-method` is enabled by default
- @final
- # we only emit one error here, not two
- def f(self): ... # error: [override-of-final-method]
12 + # error: [override-of-final-method]
12 + pass # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```

View File

@ -58,7 +58,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md
44 | def bar(self, x: str) -> str: ...
45 | @overload
46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method]
47 |
47 |
48 | @overload
49 | def baz(self, x: str) -> str: ...
50 | @overload
@ -265,7 +265,7 @@ error[override-of-final-method]: Cannot override `Bad.bar`
45 | @overload
46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method]
| ^^^ Overrides a definition from superclass `Bad`
47 |
47 |
48 | @overload
|
info: `Bad.bar` is decorated with `@final`, forbidding overrides
@ -287,12 +287,11 @@ info: rule `override-of-final-method` is enabled by default
- def bar(self, x: str) -> str: ...
- @overload
- def bar(self, x: int) -> int: ... # error: [override-of-final-method]
43 |
43 +
44 + # error: [override-of-final-method]
45 +
45 |
46 | @overload
47 | def baz(self, x: str) -> str: ...
48 | @overload
note: This is an unsafe fix and may change runtime behavior
```
@ -319,7 +318,7 @@ help: Remove all overloads for `baz`
info: rule `override-of-final-method` is enabled by default
45 | @overload
46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method]
47 |
47 |
- @overload
- def baz(self, x: str) -> str: ...
- @overload
@ -360,12 +359,12 @@ info: rule `override-of-final-method` is enabled by default
- def f(self, x: str) -> str: ...
- @overload
- def f(self, x: int) -> int: ...
13 +
14 +
13 + pass
14 + pass
15 | # error: [override-of-final-method]
- def f(self, x: int | str) -> int | str:
- return x
16 +
16 + pass
17 |
18 | class Bad:
19 | @overload
@ -459,15 +458,6 @@ info: `Bad.f` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `f`
info: rule `override-of-final-method` is enabled by default
57 |
58 | class ChildOfBad(Bad):
59 | # TODO: these should all cause us to emit Liskov violations as well
- f = None # error: [override-of-final-method]
60 + # error: [override-of-final-method]
61 | g = None # error: [override-of-final-method]
62 | h = None # error: [override-of-final-method]
63 | i = None # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
@ -493,14 +483,6 @@ info: `Bad.g` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `g`
info: rule `override-of-final-method` is enabled by default
58 | class ChildOfBad(Bad):
59 | # TODO: these should all cause us to emit Liskov violations as well
60 | f = None # error: [override-of-final-method]
- g = None # error: [override-of-final-method]
61 + # error: [override-of-final-method]
62 | h = None # error: [override-of-final-method]
63 | i = None # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
@ -525,13 +507,6 @@ info: `Bad.h` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `h`
info: rule `override-of-final-method` is enabled by default
59 | # TODO: these should all cause us to emit Liskov violations as well
60 | f = None # error: [override-of-final-method]
61 | g = None # error: [override-of-final-method]
- h = None # error: [override-of-final-method]
62 + # error: [override-of-final-method]
63 | i = None # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```
@ -555,11 +530,5 @@ info: `Bad.i` is decorated with `@final`, forbidding overrides
|
help: Remove the override of `i`
info: rule `override-of-final-method` is enabled by default
60 | f = None # error: [override-of-final-method]
61 | g = None # error: [override-of-final-method]
62 | h = None # error: [override-of-final-method]
- i = None # error: [override-of-final-method]
63 + # error: [override-of-final-method]
note: This is an unsafe fix and may change runtime behavior
```

View File

@ -3804,6 +3804,7 @@ pub(super) fn report_overridden_final_method<'db>(
context: &InferContext<'db, '_>,
member: &str,
subclass_definition: Definition<'db>,
// N.B. the type of the *definition*, not the type on an instance of the subclass
subclass_type: Type<'db>,
superclass: ClassType<'db>,
subclass: ClassType<'db>,
@ -3811,6 +3812,23 @@ pub(super) fn report_overridden_final_method<'db>(
) {
let db = context.db();
// Some hijinks so that we emit a diagnostic on the property getter rather than the property setter
let property_getter_definition = if subclass_definition.kind(db).is_function_def()
&& let Type::PropertyInstance(property) = subclass_type
&& let Some(Type::FunctionLiteral(getter)) = property.getter(db)
{
let getter_definition = getter.definition(db);
if getter_definition.scope(db) == subclass_definition.scope(db) {
Some(getter_definition)
} else {
None
}
} else {
None
};
let subclass_definition = property_getter_definition.unwrap_or(subclass_definition);
let Some(builder) = context.report_lint(
&OVERRIDE_OF_FINAL_METHOD,
subclass_definition.focus_range(db, context.module()),
@ -3871,37 +3889,69 @@ pub(super) fn report_overridden_final_method<'db>(
diagnostic.sub(sub);
let underlying_function = match subclass_type {
Type::FunctionLiteral(function) => Some(function),
Type::BoundMethod(method) => Some(method.function(db)),
_ => None,
};
// It's tempting to autofix properties as well,
// but you'd want to delete the `@my_property.deleter` as well as the getter and the deleter,
// and we don't model property deleters at all right now.
if let Type::FunctionLiteral(function) = subclass_type {
let class_node = subclass
.class_literal(db)
.0
.body_scope(db)
.node(db)
.expect_class()
.node(context.module());
let (overloads, implementation) = function.overloads_and_implementation(db);
let overload_count = overloads.len() + usize::from(implementation.is_some());
let is_only = overload_count >= class_node.body.len();
if let Some(function) = underlying_function {
let overload_deletion = |overload: &OverloadLiteral<'db>| {
Edit::range_deletion(overload.node(db, context.file(), context.module()).range())
let range = overload.node(db, context.file(), context.module()).range();
if is_only {
Edit::range_replacement("pass".to_string(), range)
} else {
Edit::range_deletion(range)
}
};
let should_fix = overloads
.iter()
.copied()
.chain(implementation)
.all(|overload| {
class_node
.body
.iter()
.filter_map(ast::Stmt::as_function_def_stmt)
.contains(overload.node(db, context.file(), context.module()))
});
match function.overloads_and_implementation(db) {
([first_overload, rest @ ..], None) => {
diagnostic.help(format_args!("Remove all overloads for `{member}`"));
diagnostic.set_fix(Fix::unsafe_edits(
overload_deletion(first_overload),
rest.iter().map(overload_deletion),
));
diagnostic.set_optional_fix(should_fix.then(|| {
Fix::unsafe_edits(
overload_deletion(first_overload),
rest.iter().map(overload_deletion),
)
}));
}
([first_overload, rest @ ..], Some(implementation)) => {
diagnostic.help(format_args!(
"Remove all overloads and the implementation for `{member}`"
));
diagnostic.set_fix(Fix::unsafe_edits(
overload_deletion(first_overload),
rest.iter().chain([&implementation]).map(overload_deletion),
));
diagnostic.set_optional_fix(should_fix.then(|| {
Fix::unsafe_edits(
overload_deletion(first_overload),
rest.iter().chain([&implementation]).map(overload_deletion),
)
}));
}
([], Some(implementation)) => {
diagnostic.help(format_args!("Remove the override of `{member}`"));
diagnostic.set_fix(Fix::unsafe_edit(overload_deletion(&implementation)));
diagnostic.set_optional_fix(
should_fix.then(|| Fix::unsafe_edit(overload_deletion(&implementation))),
);
}
([], None) => {
// Should be impossible to get here: how would we even infer a function as a function
@ -3911,11 +3961,12 @@ pub(super) fn report_overridden_final_method<'db>(
);
}
}
} else if let Type::PropertyInstance(property) = subclass_type
&& property.setter(db).is_some()
{
diagnostic.help(format_args!("Remove the getter and setter for `{member}`"));
} else {
diagnostic.help(format_args!("Remove the override of `{member}`"));
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_deletion(
subclass_definition.full_range(db, context.module()).range(),
)));
}
}

View File

@ -327,7 +327,7 @@ fn check_class_declaration<'db>(
context,
&member.name,
*definition,
type_on_subclass_instance,
member.ty,
superclass,
class,
&superclass_method,