From b02e8212c99e166925b50c3be6583f53a9a5d01d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 30 Nov 2025 13:40:33 +0000 Subject: [PATCH] [ty] Don't introduce invalid syntax when autofixing override-of-final-method (#21699) --- crates/ruff_db/src/diagnostic/mod.rs | 7 + .../resources/mdtest/final.md | 24 +- ..._possibly-undefined…_(fc7b496fd1986deb).snap | 132 ++--- ...annot_override_a_me…_(338615109711a91b).snap | 550 +++++++++--------- ...iagnostic_edge_case…_(2389d52c5ecfa2bd).snap | 2 +- ...nly_the_first_`@fin…_(9863b583f4c651c5).snap | 4 +- ...verloaded_methods_d…_(861757f48340ed92).snap | 47 +- .../src/types/diagnostic.rs | 89 ++- .../ty_python_semantic/src/types/overrides.rs | 2 +- 9 files changed, 433 insertions(+), 424 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index a8f2d09dd8..33348ddf2e 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -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) { + 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; diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index 1ccb647e4d..91c6f9273f 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -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 diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap index 41b39fef34..a137c44c51 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap @@ -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 ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Cannot_override_a_me…_(338615109711a91b).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Cannot_override_a_me…_(338615109711a91b).snap index ad51738ec5..fbb89644c6 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Cannot_override_a_me…_(338615109711a91b).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Cannot_override_a_me…_(338615109711a91b).snap @@ -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 ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Diagnostic_edge_case…_(2389d52c5ecfa2bd).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Diagnostic_edge_case…_(2389d52c5ecfa2bd).snap index 5e7f11ca02..9282af1cea 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Diagnostic_edge_case…_(2389d52c5ecfa2bd).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Diagnostic_edge_case…_(2389d52c5ecfa2bd).snap @@ -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 ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Only_the_first_`@fin…_(9863b583f4c651c5).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Only_the_first_`@fin…_(9863b583f4c651c5).snap index 342b7ec8b6..9f1c33a07d 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Only_the_first_`@fin…_(9863b583f4c651c5).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Only_the_first_`@fin…_(9863b583f4c651c5).snap @@ -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 ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloaded_methods_d…_(861757f48340ed92).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloaded_methods_d…_(861757f48340ed92).snap index 38ec75b003..f49f4ac3fc 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloaded_methods_d…_(861757f48340ed92).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloaded_methods_d…_(861757f48340ed92).snap @@ -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 ``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 782a25b98e..96aa876b50 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -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(), - ))); } } diff --git a/crates/ty_python_semantic/src/types/overrides.rs b/crates/ty_python_semantic/src/types/overrides.rs index 0cfc8c477b..517878202a 100644 --- a/crates/ty_python_semantic/src/types/overrides.rs +++ b/crates/ty_python_semantic/src/types/overrides.rs @@ -327,7 +327,7 @@ fn check_class_declaration<'db>( context, &member.name, *definition, - type_on_subclass_instance, + member.ty, superclass, class, &superclass_method,