From 0e025d6505211eb953e0153f0843d1714058d354 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 21:16:00 +0000 Subject: [PATCH 01/13] [ty] Implement `typing.final` for methods --- .../resources/mdtest/final.md | 272 ++++++++++- ..._a_me\342\200\246_(338615109711a91b).snap" | 414 ++++++++++++++++ ..._case\342\200\246_(2389d52c5ecfa2bd).snap" | 52 ++ ...ods_d\342\200\246_(861757f48340ed92).snap" | 457 ++++++++++++++++++ crates/ty_python_semantic/src/types/class.rs | 5 + .../src/types/diagnostic.rs | 92 +++- .../ty_python_semantic/src/types/function.rs | 45 +- crates/ty_python_semantic/src/types/liskov.rs | 102 ++-- .../e2e__commands__debug_command.snap | 1 + ty.schema.json | 10 + 10 files changed, 1415 insertions(+), 35 deletions(-) create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index e37cf41c8cedf..851c7857cef66 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -1,6 +1,6 @@ # Tests for the `@typing(_extensions).final` decorator -## Cannot subclass +## Cannot subclass a class decorated with `@final` Don't do this: @@ -29,3 +29,273 @@ class H( G, ): ... ``` + +## Cannot override a method decorated with `@final` + + + +```pyi +from typing_extensions import final, Callable, TypeVar + +def lossy_decorator(fn: Callable) -> Callable: ... + +class Parent: + @final + def foo(self): ... + @final + @property + def my_property1(self) -> int: ... + @property + @final + def my_property2(self) -> int: ... + @final + @classmethod + def class_method1(cls) -> int: ... + @staticmethod + @final + def static_method1() -> int: ... + @final + @classmethod + def class_method2(cls) -> int: ... + @staticmethod + @final + def static_method2() -> int: ... + @lossy_decorator + @final + def decorated_1(self): ... + @final + @lossy_decorator + def decorated_2(self): ... + +class Child(Parent): + def foo(self): ... # error: [override-of-final-method] + @property + def my_property1(self) -> int: ... # error: [override-of-final-method] + @property + def my_property2(self) -> int: ... # error: [override-of-final-method] + @classmethod + def class_method1(cls) -> int: ... # error: [override-of-final-method] + @staticmethod + def static_method1() -> int: ... # error: [override-of-final-method] + @classmethod + def class_method2(cls) -> int: ... # error: [override-of-final-method] + @staticmethod + def static_method2() -> int: ... # error: [override-of-final-method] + def decorated_1(self): ... # TODO: should emit [override-of-final-method] + @lossy_decorator + def decorated_2(self): ... # TODO: should emit [override-of-final-method] + +class OtherChild(Parent): ... + +class Grandchild(OtherChild): + @staticmethod + # TODO: we should emit a Liskov violation here too + # error: [override-of-final-method] + def foo(): ... + @property + # TODO: we should emit a Liskov violation here too + # error: [override-of-final-method] + def my_property1(self) -> str: ... + # TODO: we should emit a Liskov violation here too + # error: [override-of-final-method] + class_method1 = None + +# Diagnostic edge case: `final` is very far away from the method definition in the source code: + +T = TypeVar("T") + +def identity(x: T) -> T: ... + +class Foo: + @final + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + def bar(self): ... + +class Baz(Foo): + def bar(self): ... # error: [override-of-final-method] +``` + +## Diagnostic edge case: superclass with `@final` method has the same name as the subclass + + + +`module1.py`: + +```py +from typing import final + +class Foo: + @final + def f(self): ... +``` + +`module2.py`: + +```py +import module1 + +class Foo(module1.Foo): + def f(self): ... # error: [override-of-final-method] +``` + +## Overloaded methods decorated with `@final` + +In a stub file, `@final` should be applied to the first overload. In a runtime file, `@final` should +only be applied to the implementation function. + + + +`stub.pyi`: + +```pyi +from typing import final, overload + +class Good: + @overload + @final + def bar(self, x: str) -> str: ... + @overload + def bar(self, x: int) -> int: ... + @final + @overload + def baz(self, x: str) -> str: ... + @overload + def baz(self, x: int) -> int: ... + +class ChildOfGood(Good): + @overload + 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 + def baz(self, x: int) -> int: ... # error: [override-of-final-method] + +class Bad: + @overload + def bar(self, x: str) -> str: ... + @overload + @final + # error: [invalid-overload] + def bar(self, x: int) -> int: ... + @overload + def baz(self, x: str) -> str: ... + @final + @overload + # error: [invalid-overload] + def baz(self, x: int) -> int: ... + +class ChildOfBad(Bad): + @overload + 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 + def baz(self, x: int) -> int: ... # error: [override-of-final-method] +``` + +`main.py`: + +```py +from typing import overload, final + +class Good: + @overload + def f(self, x: str) -> str: ... + @overload + def f(self, x: int) -> int: ... + @final + def f(self, x: int | str) -> int | str: + return x + +class ChildOfGood(Good): + @overload + def f(self, x: str) -> str: ... + @overload + def f(self, x: int) -> int: ... + # error: [override-of-final-method] + def f(self, x: int | str) -> int | str: + return x + +class Bad: + @overload + @final + def f(self, x: str) -> str: ... + @overload + def f(self, x: int) -> int: ... + # error: [invalid-overload] + def f(self, x: int | str) -> int | str: + return x + + @final + @overload + def g(self, x: str) -> str: ... + @overload + def g(self, x: int) -> int: ... + # error: [invalid-overload] + def g(self, x: int | str) -> int | str: + return x + + @overload + def h(self, x: str) -> str: ... + @overload + @final + def h(self, x: int) -> int: ... + # error: [invalid-overload] + def h(self, x: int | str) -> int | str: + return x + + @overload + def i(self, x: str) -> str: ... + @final + @overload + def i(self, x: int) -> int: ... + # error: [invalid-overload] + def i(self, x: int | str) -> int | str: + return x + +class ChildOfBad(Bad): + # TODO: these should all cause us to emit Liskov violations as well + f = None # error: [override-of-final-method] + g = None # error: [override-of-final-method] + h = None # error: [override-of-final-method] + i = None # error: [override-of-final-method] +``` + +## Edge case: the function is decorated with `@final` but originally defined elsewhere + +As of 2025-11-26, pyrefly emits a diagnostic on this, but mypy and pyright do not: + +```py +from typing import final + +class A: + @final + def method(self): ... + +class B: + method = A.method + +class C(B): + def method(self): ... # no diagnostic +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" new file mode 100644 index 0000000000000..e3a2545fad885 --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" @@ -0,0 +1,414 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - Cannot override a method decorated with `@final` +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## mdtest_snippet.pyi + +``` + 1 | from typing_extensions import final, Callable, TypeVar + 2 | + 3 | def lossy_decorator(fn: Callable) -> Callable: ... + 4 | + 5 | class Parent: + 6 | @final + 7 | def foo(self): ... + 8 | @final + 9 | @property +10 | def my_property1(self) -> int: ... +11 | @property +12 | @final +13 | def my_property2(self) -> int: ... +14 | @final +15 | @classmethod +16 | def class_method1(cls) -> int: ... +17 | @staticmethod +18 | @final +19 | def static_method1() -> int: ... +20 | @final +21 | @classmethod +22 | def class_method2(cls) -> int: ... +23 | @staticmethod +24 | @final +25 | def static_method2() -> int: ... +26 | @lossy_decorator +27 | @final +28 | def decorated_1(self): ... +29 | @final +30 | @lossy_decorator +31 | def decorated_2(self): ... +32 | +33 | class Child(Parent): +34 | def foo(self): ... # error: [override-of-final-method] +35 | @property +36 | def my_property1(self) -> int: ... # error: [override-of-final-method] +37 | @property +38 | def my_property2(self) -> int: ... # error: [override-of-final-method] +39 | @classmethod +40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +41 | @staticmethod +42 | def static_method1() -> int: ... # error: [override-of-final-method] +43 | @classmethod +44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +45 | @staticmethod +46 | def static_method2() -> int: ... # error: [override-of-final-method] +47 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +48 | @lossy_decorator +49 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] +50 | +51 | class OtherChild(Parent): ... +52 | +53 | class Grandchild(OtherChild): +54 | @staticmethod +55 | # TODO: we should emit a Liskov violation here too +56 | # error: [override-of-final-method] +57 | def foo(): ... +58 | @property +59 | # TODO: we should emit a Liskov violation here too +60 | # error: [override-of-final-method] +61 | def my_property1(self) -> str: ... +62 | # TODO: we should emit a Liskov violation here too +63 | # error: [override-of-final-method] +64 | class_method1 = None +65 | +66 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: +67 | +68 | T = TypeVar("T") +69 | +70 | def identity(x: T) -> T: ... +71 | +72 | class Foo: +73 | @final +74 | @identity +75 | @identity +76 | @identity +77 | @identity +78 | @identity +79 | @identity +80 | @identity +81 | @identity +82 | @identity +83 | @identity +84 | @identity +85 | @identity +86 | @identity +87 | @identity +88 | @identity +89 | @identity +90 | @identity +91 | @identity +92 | def bar(self): ... +93 | +94 | class Baz(Foo): +95 | def bar(self): ... # error: [override-of-final-method] +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `Parent.foo` + --> src/mdtest_snippet.pyi:34:9 + | +33 | class Child(Parent): +34 | def foo(self): ... # error: [override-of-final-method] + | ^^^ Override of `foo` from superclass `Parent` +35 | @property +36 | 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 + | +5 | class Parent: +6 | @final + | ------ +7 | def foo(self): ... + | --- `Parent.foo` defined here +8 | @final +9 | @property + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.my_property1` + --> src/mdtest_snippet.pyi:36:9 + | +34 | def foo(self): ... # error: [override-of-final-method] +35 | @property +36 | def my_property1(self) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` +37 | @property +38 | def my_property2(self) -> int: ... # error: [override-of-final-method] + | +info: `Parent.my_property1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:8:5 + | + 6 | @final + 7 | def foo(self): ... + 8 | @final + | ------ + 9 | @property +10 | def my_property1(self) -> int: ... + | ------------ `Parent.my_property1` defined here +11 | @property +12 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.my_property2` + --> src/mdtest_snippet.pyi:38:9 + | +36 | def my_property1(self) -> int: ... # error: [override-of-final-method] +37 | @property +38 | def my_property2(self) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^ Override of `my_property2` from superclass `Parent` +39 | @classmethod +40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] + | +info: `Parent.my_property2` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:12:5 + | +10 | def my_property1(self) -> int: ... +11 | @property +12 | @final + | ------ +13 | def my_property2(self) -> int: ... + | ------------ `Parent.my_property2` defined here +14 | @final +15 | @classmethod + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.class_method1` + --> src/mdtest_snippet.pyi:40:9 + | +38 | def my_property2(self) -> int: ... # error: [override-of-final-method] +39 | @classmethod +40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` +41 | @staticmethod +42 | def static_method1() -> int: ... # error: [override-of-final-method] + | +info: `Parent.class_method1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:14:5 + | +12 | @final +13 | def my_property2(self) -> int: ... +14 | @final + | ------ +15 | @classmethod +16 | def class_method1(cls) -> int: ... + | ------------- `Parent.class_method1` defined here +17 | @staticmethod +18 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.static_method1` + --> src/mdtest_snippet.pyi:42:9 + | +40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +41 | @staticmethod +42 | def static_method1() -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^^ Override of `static_method1` from superclass `Parent` +43 | @classmethod +44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + | +info: `Parent.static_method1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:18:5 + | +16 | def class_method1(cls) -> int: ... +17 | @staticmethod +18 | @final + | ------ +19 | def static_method1() -> int: ... + | -------------- `Parent.static_method1` defined here +20 | @final +21 | @classmethod + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.class_method2` + --> src/mdtest_snippet.pyi:44:9 + | +42 | def static_method1() -> int: ... # error: [override-of-final-method] +43 | @classmethod +44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^ Override of `class_method2` from superclass `Parent` +45 | @staticmethod +46 | def static_method2() -> int: ... # error: [override-of-final-method] + | +info: `Parent.class_method2` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:20:5 + | +18 | @final +19 | def static_method1() -> int: ... +20 | @final + | ------ +21 | @classmethod +22 | def class_method2(cls) -> int: ... + | ------------- `Parent.class_method2` defined here +23 | @staticmethod +24 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.static_method2` + --> src/mdtest_snippet.pyi:46:9 + | +44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +45 | @staticmethod +46 | def static_method2() -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^^ Override of `static_method2` from superclass `Parent` +47 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +48 | @lossy_decorator + | +info: `Parent.static_method2` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:24:5 + | +22 | def class_method2(cls) -> int: ... +23 | @staticmethod +24 | @final + | ------ +25 | def static_method2() -> int: ... + | -------------- `Parent.static_method2` defined here +26 | @lossy_decorator +27 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.foo` + --> src/mdtest_snippet.pyi:57:9 + | +55 | # TODO: we should emit a Liskov violation here too +56 | # error: [override-of-final-method] +57 | def foo(): ... + | ^^^ Override of `foo` from superclass `Parent` +58 | @property +59 | # TODO: we should emit a Liskov violation here too + | +info: `Parent.foo` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:6:5 + | +5 | class Parent: +6 | @final + | ------ +7 | def foo(self): ... + | --- `Parent.foo` defined here +8 | @final +9 | @property + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.my_property1` + --> src/mdtest_snippet.pyi:61:9 + | +59 | # TODO: we should emit a Liskov violation here too +60 | # error: [override-of-final-method] +61 | def my_property1(self) -> str: ... + | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` +62 | # TODO: we should emit a Liskov violation here too +63 | # error: [override-of-final-method] + | +info: `Parent.my_property1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:8:5 + | + 6 | @final + 7 | def foo(self): ... + 8 | @final + | ------ + 9 | @property +10 | def my_property1(self) -> int: ... + | ------------ `Parent.my_property1` defined here +11 | @property +12 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Parent.class_method1` + --> src/mdtest_snippet.pyi:64:5 + | +62 | # TODO: we should emit a Liskov violation here too +63 | # error: [override-of-final-method] +64 | class_method1 = None + | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` +65 | +66 | # 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:14:5 + | +12 | @final +13 | def my_property2(self) -> int: ... +14 | @final + | ------ +15 | @classmethod +16 | def class_method1(cls) -> int: ... + | ------------- `Parent.class_method1` defined here +17 | @staticmethod +18 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Foo.bar` + --> src/mdtest_snippet.pyi:95:9 + | +94 | class Baz(Foo): +95 | def bar(self): ... # error: [override-of-final-method] + | ^^^ Override of `bar` from superclass `Foo` + | +info: `Foo.bar` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:73:5 + | +72 | class Foo: +73 | @final + | ------ +74 | @identity +75 | @identity + | + ::: src/mdtest_snippet.pyi:92:9 + | +90 | @identity +91 | @identity +92 | def bar(self): ... + | --- `Foo.bar` defined here +93 | +94 | class Baz(Foo): + | +info: rule `override-of-final-method` is enabled by default + +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" new file mode 100644 index 0000000000000..4ab0b1aa3e6ad --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" @@ -0,0 +1,52 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - Diagnostic edge case: superclass with `@final` method has the same name as the subclass +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## module1.py + +``` +1 | from typing import final +2 | +3 | class Foo: +4 | @final +5 | def f(self): ... +``` + +## module2.py + +``` +1 | import module1 +2 | +3 | class Foo(module1.Foo): +4 | def f(self): ... # error: [override-of-final-method] +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `module1.Foo.f` + --> src/module2.py:4:9 + | +3 | class Foo(module1.Foo): +4 | def f(self): ... # error: [override-of-final-method] + | ^ Override of `f` from superclass `module1.Foo` + | +info: `module1.Foo.f` is decorated with `@final`, forbidding overrides + --> src/module1.py:4:5 + | +3 | class Foo: +4 | @final + | ------ +5 | def f(self): ... + | - `module1.Foo.f` defined here + | +info: rule `override-of-final-method` is enabled by default + +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" new file mode 100644 index 0000000000000..033b734eb29b8 --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" @@ -0,0 +1,457 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - Overloaded methods decorated with `@final` +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## stub.pyi + +``` + 1 | from typing import final, overload + 2 | + 3 | class Good: + 4 | @overload + 5 | @final + 6 | def bar(self, x: str) -> str: ... + 7 | @overload + 8 | def bar(self, x: int) -> int: ... + 9 | @final +10 | @overload +11 | def baz(self, x: str) -> str: ... +12 | @overload +13 | def baz(self, x: int) -> int: ... +14 | +15 | class ChildOfGood(Good): +16 | @overload +17 | def bar(self, x: str) -> str: ... +18 | @overload +19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +20 | @overload +21 | def baz(self, x: str) -> str: ... +22 | @overload +23 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] +24 | +25 | class Bad: +26 | @overload +27 | def bar(self, x: str) -> str: ... +28 | @overload +29 | @final +30 | # error: [invalid-overload] +31 | def bar(self, x: int) -> int: ... +32 | @overload +33 | def baz(self, x: str) -> str: ... +34 | @final +35 | @overload +36 | # error: [invalid-overload] +37 | def baz(self, x: int) -> int: ... +38 | +39 | class ChildOfBad(Bad): +40 | @overload +41 | def bar(self, x: str) -> str: ... +42 | @overload +43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +44 | @overload +45 | def baz(self, x: str) -> str: ... +46 | @overload +47 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] +``` + +## main.py + +``` + 1 | from typing import overload, final + 2 | + 3 | class Good: + 4 | @overload + 5 | def f(self, x: str) -> str: ... + 6 | @overload + 7 | def f(self, x: int) -> int: ... + 8 | @final + 9 | def f(self, x: int | str) -> int | str: +10 | return x +11 | +12 | class ChildOfGood(Good): +13 | @overload +14 | def f(self, x: str) -> str: ... +15 | @overload +16 | def f(self, x: int) -> int: ... +17 | # error: [override-of-final-method] +18 | def f(self, x: int | str) -> int | str: +19 | return x +20 | +21 | class Bad: +22 | @overload +23 | @final +24 | def f(self, x: str) -> str: ... +25 | @overload +26 | def f(self, x: int) -> int: ... +27 | # error: [invalid-overload] +28 | def f(self, x: int | str) -> int | str: +29 | return x +30 | +31 | @final +32 | @overload +33 | def g(self, x: str) -> str: ... +34 | @overload +35 | def g(self, x: int) -> int: ... +36 | # error: [invalid-overload] +37 | def g(self, x: int | str) -> int | str: +38 | return x +39 | +40 | @overload +41 | def h(self, x: str) -> str: ... +42 | @overload +43 | @final +44 | def h(self, x: int) -> int: ... +45 | # error: [invalid-overload] +46 | def h(self, x: int | str) -> int | str: +47 | return x +48 | +49 | @overload +50 | def i(self, x: str) -> str: ... +51 | @final +52 | @overload +53 | def i(self, x: int) -> int: ... +54 | # error: [invalid-overload] +55 | def i(self, x: int | str) -> int | str: +56 | return x +57 | +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] +61 | g = None # error: [override-of-final-method] +62 | h = None # error: [override-of-final-method] +63 | i = None # error: [override-of-final-method] +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `Good.bar` + --> src/stub.pyi:19:9 + | +17 | def bar(self, x: str) -> str: ... +18 | @overload +19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Override of `bar` from superclass `Good` +20 | @overload +21 | def baz(self, x: str) -> str: ... + | +info: `Good.bar` is decorated with `@final`, forbidding overrides + --> src/stub.pyi:5:5 + | +3 | class Good: +4 | @overload +5 | @final + | ------ +6 | def bar(self, x: str) -> str: ... + | --- `Good.bar` defined here +7 | @overload +8 | def bar(self, x: int) -> int: ... + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Good.baz` + --> src/stub.pyi:23:9 + | +21 | def baz(self, x: str) -> str: ... +22 | @overload +23 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Override of `baz` from superclass `Good` +24 | +25 | class Bad: + | +info: `Good.baz` is decorated with `@final`, forbidding overrides + --> src/stub.pyi:9:5 + | + 7 | @overload + 8 | def bar(self, x: int) -> int: ... + 9 | @final + | ------ +10 | @overload +11 | def baz(self, x: str) -> str: ... + | --- `Good.baz` defined here +12 | @overload +13 | def baz(self, x: int) -> int: ... + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the first overload + --> src/stub.pyi:27:9 + | +25 | class Bad: +26 | @overload +27 | def bar(self, x: str) -> str: ... + | --- First overload defined here +28 | @overload +29 | @final +30 | # error: [invalid-overload] +31 | def bar(self, x: int) -> int: ... + | ^^^ +32 | @overload +33 | def baz(self, x: str) -> str: ... + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the first overload + --> src/stub.pyi:33:9 + | +31 | def bar(self, x: int) -> int: ... +32 | @overload +33 | def baz(self, x: str) -> str: ... + | --- First overload defined here +34 | @final +35 | @overload +36 | # error: [invalid-overload] +37 | def baz(self, x: int) -> int: ... + | ^^^ +38 | +39 | class ChildOfBad(Bad): + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.bar` + --> src/stub.pyi:43:9 + | +41 | def bar(self, x: str) -> str: ... +42 | @overload +43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Override of `bar` from superclass `Bad` +44 | @overload +45 | def baz(self, x: str) -> str: ... + | +info: `Bad.bar` is decorated with `@final`, forbidding overrides + --> src/stub.pyi:27:9 + | +25 | class Bad: +26 | @overload +27 | def bar(self, x: str) -> str: ... + | --- `Bad.bar` defined here +28 | @overload +29 | @final + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.baz` + --> src/stub.pyi:47:9 + | +45 | def baz(self, x: str) -> str: ... +46 | @overload +47 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Override of `baz` from superclass `Bad` + | +info: `Bad.baz` is decorated with `@final`, forbidding overrides + --> src/stub.pyi:33:9 + | +31 | def bar(self, x: int) -> int: ... +32 | @overload +33 | def baz(self, x: str) -> str: ... + | --- `Bad.baz` defined here +34 | @final +35 | @overload + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Good.f` + --> src/main.py:18:9 + | +16 | def f(self, x: int) -> int: ... +17 | # error: [override-of-final-method] +18 | def f(self, x: int | str) -> int | str: + | ^ Override of `f` from superclass `Good` +19 | return x + | +info: `Good.f` is decorated with `@final`, forbidding overrides + --> src/main.py:8:5 + | + 6 | @overload + 7 | def f(self, x: int) -> int: ... + 8 | @final + | ------ + 9 | def f(self, x: int | str) -> int | str: + | - `Good.f` defined here +10 | return x + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the overload implementation + --> src/main.py:28:9 + | +26 | def f(self, x: int) -> int: ... +27 | # error: [invalid-overload] +28 | def f(self, x: int | str) -> int | str: + | - + | | + | Implementation defined here +29 | return x + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the overload implementation + --> src/main.py:37:9 + | +35 | def g(self, x: int) -> int: ... +36 | # error: [invalid-overload] +37 | def g(self, x: int | str) -> int | str: + | - + | | + | Implementation defined here +38 | return x + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the overload implementation + --> src/main.py:46:9 + | +44 | def h(self, x: int) -> int: ... +45 | # error: [invalid-overload] +46 | def h(self, x: int | str) -> int | str: + | - + | | + | Implementation defined here +47 | return x + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[invalid-overload]: `@final` decorator should be applied only to the overload implementation + --> src/main.py:55:9 + | +53 | def i(self, x: int) -> int: ... +54 | # error: [invalid-overload] +55 | def i(self, x: int | str) -> int | str: + | - + | | + | Implementation defined here +56 | return x + | +info: rule `invalid-overload` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.f` + --> src/main.py:60:5 + | +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] + | ^ Override of `f` from superclass `Bad` +61 | g = None # error: [override-of-final-method] +62 | h = None # error: [override-of-final-method] + | +info: `Bad.f` is decorated with `@final`, forbidding overrides + --> src/main.py:28:9 + | +26 | def f(self, x: int) -> int: ... +27 | # error: [invalid-overload] +28 | def f(self, x: int | str) -> int | str: + | - `Bad.f` defined here +29 | return x + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.g` + --> src/main.py:61:5 + | +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] + | ^ Override of `g` from superclass `Bad` +62 | h = None # error: [override-of-final-method] +63 | i = None # error: [override-of-final-method] + | +info: `Bad.g` is decorated with `@final`, forbidding overrides + --> src/main.py:37:9 + | +35 | def g(self, x: int) -> int: ... +36 | # error: [invalid-overload] +37 | def g(self, x: int | str) -> int | str: + | - `Bad.g` defined here +38 | return x + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.h` + --> src/main.py:62:5 + | +60 | f = None # error: [override-of-final-method] +61 | g = None # error: [override-of-final-method] +62 | h = None # error: [override-of-final-method] + | ^ Override of `h` from superclass `Bad` +63 | i = None # error: [override-of-final-method] + | +info: `Bad.h` is decorated with `@final`, forbidding overrides + --> src/main.py:46:9 + | +44 | def h(self, x: int) -> int: ... +45 | # error: [invalid-overload] +46 | def h(self, x: int | str) -> int | str: + | - `Bad.h` defined here +47 | return x + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `Bad.i` + --> src/main.py:63:5 + | +61 | g = None # error: [override-of-final-method] +62 | h = None # error: [override-of-final-method] +63 | i = None # error: [override-of-final-method] + | ^ Override of `i` from superclass `Bad` + | +info: `Bad.i` is decorated with `@final`, forbidding overrides + --> src/main.py:55:9 + | +53 | def i(self, x: int) -> int: ... +54 | # error: [invalid-overload] +55 | def i(self, x: int | str) -> int | str: + | - `Bad.i` defined here +56 | return x + | +info: rule `override-of-final-method` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 033f326c39a7e..7d7985bd9bc98 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -497,6 +497,11 @@ impl<'db> ClassType<'db> { class_literal.name(db) } + pub(super) fn qualified_name(self, db: &'db dyn Db) -> QualifiedClassName<'db> { + let (class_literal, _) = self.class_literal(db); + class_literal.qualified_name(db) + } + pub(crate) fn known(self, db: &'db dyn Db) -> Option { let (class_literal, _) = self.class_literal(db); class_literal.known(db) diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 44b54d0c417ed..ea1767e65ab6c 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -100,6 +100,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&POSSIBLY_MISSING_IMPORT); registry.register_lint(&POSSIBLY_UNRESOLVED_REFERENCE); registry.register_lint(&SUBCLASS_OF_FINAL_CLASS); + registry.register_lint(&OVERRIDE_OF_FINAL_METHOD); registry.register_lint(&TYPE_ASSERTION_FAILURE); registry.register_lint(&TOO_MANY_POSITIONAL_ARGUMENTS); registry.register_lint(&UNAVAILABLE_IMPLICIT_SUPER_ARGUMENTS); @@ -1613,6 +1614,33 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for methods on subclasses that override superclass methods decorated with `@final`. + /// + /// ## Why is this bad? + /// Decorating a method with `@final` declares to the type checker that it should not be + /// overridden on any subclass. + /// + /// ## Example + /// + /// ```python + /// from typing import final + /// + /// class A: + /// @final + /// def foo(self): ... + /// + /// class B(A): + /// def foo(self): ... # Error raised here + /// ``` + pub(crate) static OVERRIDE_OF_FINAL_METHOD = { + summary: "detects subclasses of final classes", + status: LintStatus::stable("0.0.1-alpha.29"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for methods that are decorated with `@override` but do not override any method in a superclass. @@ -3620,7 +3648,7 @@ pub(super) fn report_invalid_method_override<'db>( let overridden_method = if class_name == superclass_name { format!( "{superclass}.{member}", - superclass = superclass.class_literal(db).0.qualified_name(db), + superclass = superclass.qualified_name(db), ) } else { format!("{superclass_name}.{member}") @@ -3763,6 +3791,68 @@ pub(super) fn report_invalid_method_override<'db>( } } +pub(super) fn report_overridden_final_method<'db>( + context: &InferContext<'db, '_>, + member: &str, + subclass_definition: Definition<'db>, + superclass: ClassType<'db>, + subclass: ClassType<'db>, + superclass_method: FunctionType<'db>, +) { + let db = context.db(); + + let Some(builder) = context.report_lint( + &OVERRIDE_OF_FINAL_METHOD, + subclass_definition.focus_range(db, context.module()), + ) else { + return; + }; + + let superclass_name = if superclass.name(db) == subclass.name(db) { + superclass.qualified_name(db).to_string() + } else { + superclass.name(db).to_string() + }; + + let mut diagnostic = + builder.into_diagnostic(format_args!("Cannot override `{superclass_name}.{member}`")); + diagnostic.set_primary_message(format_args!( + "Override of `{member}` from superclass `{superclass_name}`" + )); + diagnostic.set_concise_message(format_args!( + "Cannot override final member `{member}` from superclass `{superclass_name}`" + )); + + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + format_args!( + "`{superclass_name}.{member}` is decorated with `@final`, forbidding overrides" + ), + ); + + let superclass_function_literal = if superclass_method.file(db).is_stub(db) { + superclass_method.first_overload(db) + } else { + superclass_method.literal(db).last_definition(db) + }; + + sub.annotate( + Annotation::secondary(Span::from( + superclass_function_literal + .focus_range(db, &parsed_module(db, superclass_method.file(db)).load(db)), + )) + .message(format_args!("`{superclass_name}.{member}` defined here")), + ); + + if let Some(decorator_span) = + superclass_function_literal.find_known_decorator_span(db, KnownFunction::Final) + { + sub.annotate(Annotation::secondary(decorator_span)); + } + + diagnostic.sub(sub); +} + /// This function receives an unresolved `from foo import bar` import, /// where `foo` can be resolved to a module but that module does not /// have a `bar` member or submodule. diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 3c440ba2a2702..827eadb2a3b6b 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -83,7 +83,7 @@ use crate::types::{ ClassLiteral, ClassType, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType, Truthiness, Type, TypeContext, TypeMapping, TypeRelation, - UnionBuilder, binding_type, walk_signature, + UnionBuilder, binding_type, definition_expression_type, walk_signature, }; use crate::{Db, FxOrderSet, ModuleName, resolve_module}; @@ -278,7 +278,7 @@ impl<'db> OverloadLiteral<'db> { || is_implicit_classmethod(self.name(db)) } - pub(super) fn node<'ast>( + fn node<'ast>( self, db: &dyn Db, file: File, @@ -294,6 +294,41 @@ impl<'db> OverloadLiteral<'db> { self.body_scope(db).node(db).expect_function().node(module) } + /// Iterate through the decorators on this function, returning the span of the first one + /// that matches the given predicate. + pub(super) fn find_decorator_span( + self, + db: &'db dyn Db, + predicate: impl Fn(Type<'db>) -> bool, + ) -> Option { + let definition = self.definition(db); + let file = definition.file(db); + self.node(db, file, &parsed_module(db, file).load(db)) + .decorator_list + .iter() + .find(|decorator| { + predicate(definition_expression_type( + db, + definition, + &decorator.expression, + )) + }) + .map(|decorator| Span::from(file).with_range(decorator.range)) + } + + /// Iterate through the decorators on this function, returning the span of the first one + /// that matches the given [`KnownFunction`]. + pub(super) fn find_known_decorator_span( + self, + db: &'db dyn Db, + needle: KnownFunction, + ) -> Option { + self.find_decorator_span(db, |ty| { + ty.as_function_literal() + .is_some_and(|f| f.is_known(db, needle)) + }) + } + /// Returns the [`FileRange`] of the function's name. pub(crate) fn focus_range(self, db: &dyn Db, module: &ParsedModuleRef) -> FileRange { FileRange::new( @@ -905,6 +940,12 @@ impl<'db> FunctionType<'db> { self.literal(db).iter_overloads_and_implementation(db) } + pub(crate) fn first_overload(self, db: &'db dyn Db) -> OverloadLiteral<'db> { + self.iter_overloads_and_implementation(db) + .next() + .expect("A function must have at least one overload/implementation") + } + /// Typed externally-visible signature for this function. /// /// This is the signature as seen by external callers, possibly modified by decorators and/or diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index 75b9f11a01f39..e1bc768191b05 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -7,13 +7,15 @@ use rustc_hash::FxHashSet; use crate::{ place::Place, - semantic_index::place_table, + semantic_index::{place_table, use_def_map}, types::{ ClassBase, ClassLiteral, ClassType, KnownClass, Type, class::CodeGeneratorKind, context::InferContext, - definition_expression_type, - diagnostic::{INVALID_EXPLICIT_OVERRIDE, report_invalid_method_override}, + diagnostic::{ + INVALID_EXPLICIT_OVERRIDE, report_invalid_method_override, + report_overridden_final_method, + }, function::{FunctionDecorators, KnownFunction}, ide_support::{MemberWithDefinition, all_declarations_and_bindings}, }, @@ -43,11 +45,6 @@ fn check_class_declaration<'db>( let MemberWithDefinition { member, definition } = member; - // TODO: Check Liskov on non-methods too - let Type::FunctionLiteral(function) = member.ty else { - return; - }; - let Some(definition) = definition else { return; }; @@ -80,6 +77,7 @@ fn check_class_declaration<'db>( let mut has_dynamic_superclass = false; let mut has_typeddict_in_mro = false; let mut liskov_diagnostic_emitted = false; + let mut overridden_final_method = None; for class_base in class.iter_mro(db).skip(1) { let superclass = match class_base { @@ -96,11 +94,15 @@ fn check_class_declaration<'db>( }; let (superclass_literal, superclass_specialization) = superclass.class_literal(db); - let superclass_symbol_table = place_table(db, superclass_literal.body_scope(db)); + let superclass_scope = superclass_literal.body_scope(db); + let superclass_symbol_table = place_table(db, superclass_scope); + let superclass_symbol_id = superclass_symbol_table.symbol_id(&member.name); + let mut method_kind = MethodKind::default(); // If the member is not defined on the class itself, skip it - if let Some(superclass_symbol) = superclass_symbol_table.symbol_by_name(&member.name) { + if let Some(id) = superclass_symbol_id { + let superclass_symbol = superclass_symbol_table.symbol(id); if !(superclass_symbol.is_bound() || superclass_symbol.is_declared()) { continue; } @@ -119,12 +121,6 @@ fn check_class_declaration<'db>( subclass_overrides_superclass_declaration = true; - // Only one Liskov diagnostic should be emitted per each invalid override, - // even if it overrides multiple superclasses incorrectly! - if liskov_diagnostic_emitted { - continue; - } - let Place::Defined(superclass_type, _, _) = Type::instance(db, superclass) .member(db, &member.name) .place @@ -133,6 +129,48 @@ fn check_class_declaration<'db>( break; }; + overridden_final_method = overridden_final_method.or_else(|| { + let superclass_symbol_id = superclass_symbol_id?; + + // TODO: `@final` should be more like a type qualifier: + // we should also recognise `@final`-decorated methods that don't end up + // as being function- or property-types (because they're wrapped by other + // decorators that transform the type into something else). + let underlying_function = match superclass + .own_class_member(db, None, &member.name) + .ignore_possibly_undefined()? + { + Type::BoundMethod(method) => method.function(db), + Type::FunctionLiteral(function) => function, + Type::PropertyInstance(property) => property.getter(db)?.as_function_literal()?, + _ => return None, + }; + + if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) { + use_def_map(db, superclass_scope) + .end_of_scope_symbol_bindings(superclass_symbol_id) + .next()? + .binding + .definition()? + .kind(db) + .is_function_def() + .then_some((superclass, underlying_function)) + } else { + None + } + }); + + // Only one Liskov diagnostic should be emitted per each invalid override, + // even if it overrides multiple superclasses incorrectly! + if liskov_diagnostic_emitted { + continue; + } + + // TODO: Check Liskov on non-methods too + let Type::FunctionLiteral(subclass_function) = member.ty else { + continue; + }; + let Some(superclass_type_as_callable) = superclass_type .try_upcast_to_callable(db) .map(|callables| callables.into_type(db)) @@ -149,7 +187,7 @@ fn check_class_declaration<'db>( &member.name, class, *definition, - function, + subclass_function, superclass, superclass_type, method_kind, @@ -187,13 +225,11 @@ fn check_class_declaration<'db>( && function.has_known_decorator(db, FunctionDecorators::OVERRIDE) { let function_literal = if context.in_stub() { - function - .iter_overloads_and_implementation(db) - .next() - .expect("There should always be at least one overload or implementation") + function.first_overload(db) } else { function.literal(db).last_definition(db) }; + if let Some(builder) = context.report_lint( &INVALID_EXPLICIT_OVERRIDE, function_literal.focus_range(db, context.module()), @@ -202,17 +238,10 @@ fn check_class_declaration<'db>( "Method `{}` is decorated with `@override` but does not override anything", member.name )); - if let Some(decorator) = function_literal - .node(db, context.file(), context.module()) - .decorator_list - .iter() - .find(|decorator| { - definition_expression_type(db, *definition, &decorator.expression) - .as_function_literal() - .is_some_and(|function| function.is_known(db, KnownFunction::Override)) - }) + if let Some(decorator_span) = + function_literal.find_known_decorator_span(db, KnownFunction::Override) { - diagnostic.annotate(Annotation::secondary(context.span(decorator))); + diagnostic.annotate(Annotation::secondary(decorator_span)); } diagnostic.info(format_args!( "No `{member}` definitions were found on any superclasses of `{class}`", @@ -221,6 +250,17 @@ fn check_class_declaration<'db>( )); } } + + if let Some((superclass, superclass_method)) = overridden_final_method { + report_overridden_final_method( + context, + &member.name, + *definition, + superclass, + class, + superclass_method, + ); + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap index 0676f15fd711e..5505fbd5f20b9 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap @@ -83,6 +83,7 @@ Settings: Settings { "no-matching-overload": Error (Default), "non-subscriptable": Error (Default), "not-iterable": Error (Default), + "override-of-final-method": Error (Default), "parameter-already-assigned": Error (Default), "positional-only-parameter-as-kwarg": Error (Default), "possibly-missing-attribute": Warning (Default), diff --git a/ty.schema.json b/ty.schema.json index 919b0e8dfcfa2..8a19ce44cfc48 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -863,6 +863,16 @@ } ] }, + "override-of-final-method": { + "title": "detects subclasses of final classes", + "description": "## What it does\nChecks for methods on subclasses that override superclass methods decorated with `@final`.\n\n## Why is this bad?\nDecorating a method with `@final` declares to the type checker that it should not be\noverridden on any subclass.\n\n## Example\n\n```python\nfrom typing import final\n\nclass A:\n @final\n def foo(self): ...\n\nclass B(A):\n def foo(self): ... # Error raised here\n```", + "default": "error", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "parameter-already-assigned": { "title": "detects multiple arguments for the same parameter", "description": "## What it does\nChecks for calls which provide more than one argument for a single parameter.\n\n## Why is this bad?\nProviding multiple values for a single parameter will raise a `TypeError` at runtime.\n\n## Examples\n\n```python\ndef f(x: int) -> int: ...\n\nf(1, x=2) # Error raised here\n```", From b885cb7284e8d7be580a7ec9b928c41c4c07fbc5 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 21:49:28 +0000 Subject: [PATCH 02/13] fix unrelated mdtest failure --- crates/ty_python_semantic/resources/mdtest/call/union.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/resources/mdtest/call/union.md b/crates/ty_python_semantic/resources/mdtest/call/union.md index 4f374ac754673..45ccf78ed34c8 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/union.md +++ b/crates/ty_python_semantic/resources/mdtest/call/union.md @@ -277,6 +277,6 @@ def _(flag: bool): x = f({"x": 1}) reveal_type(x) # revealed: int - # error: [invalid-argument-type] "Argument to function `f` is incorrect: Expected `T`, found `dict[str, int] & dict[Unknown | str, Unknown | int]`" + # error: [invalid-argument-type] "Argument to function `f` is incorrect: Expected `T`, found `dict[Unknown | str, Unknown | int] & dict[str, int]`" f({"y": 1}) ``` From 3ca26759992d5776722bd40eee0bedcb507ab24c Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 22:04:23 +0000 Subject: [PATCH 03/13] another edge case --- .../resources/mdtest/final.md | 13 +++++++ crates/ty_python_semantic/src/types/liskov.rs | 35 +++++++++++-------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index 851c7857cef66..026df292c525a 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -299,3 +299,16 @@ class B: class C(B): def method(self): ... # no diagnostic ``` + +## Constructor methods are also checked + +```py +from typing import final + +class A: + @final + def __init__(self) -> None: ... + +class B(A): + def __init__(self) -> None: ... # error: [override-of-final-method] +``` diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index e1bc768191b05..bd19f575bbaf7 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -49,24 +49,9 @@ fn check_class_declaration<'db>( return; }; - // Constructor methods are not checked for Liskov compliance - if matches!( - &*member.name, - "__init__" | "__new__" | "__post_init__" | "__init_subclass__" - ) { - return; - } - let (literal, specialization) = class.class_literal(db); let class_kind = CodeGeneratorKind::from_class(db, literal, specialization); - // Synthesized `__replace__` methods on dataclasses are not checked - if &member.name == "__replace__" - && matches!(class_kind, Some(CodeGeneratorKind::DataclassLike(_))) - { - return; - } - let Place::Defined(type_on_subclass_instance, _, _) = Type::instance(db, class).member(db, &member.name).place else { @@ -160,6 +145,11 @@ fn check_class_declaration<'db>( } }); + // ********************************************************** + // Everything below this point in the loop + // is about Liskov Substitution Principle checks + // ********************************************************** + // Only one Liskov diagnostic should be emitted per each invalid override, // even if it overrides multiple superclasses incorrectly! if liskov_diagnostic_emitted { @@ -171,6 +161,21 @@ fn check_class_declaration<'db>( continue; }; + // Constructor methods are not checked for Liskov compliance + if matches!( + &*member.name, + "__init__" | "__new__" | "__post_init__" | "__init_subclass__" + ) { + continue; + } + + // Synthesized `__replace__` methods on dataclasses are not checked + if &member.name == "__replace__" + && matches!(class_kind, Some(CodeGeneratorKind::DataclassLike(_))) + { + continue; + } + let Some(superclass_type_as_callable) = superclass_type .try_upcast_to_callable(db) .map(|callables| callables.into_type(db)) From 47d07e2176d2b978a9190498c87c0ef54b04b83c Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 22:09:24 +0000 Subject: [PATCH 04/13] another edge case --- .../resources/mdtest/final.md | 23 ++++++ ...`@fin\342\200\246_(9863b583f4c651c5).snap" | 81 +++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index 026df292c525a..e5ce6fd0f2edb 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -312,3 +312,26 @@ class A: class B(A): def __init__(self) -> None: ... # error: [override-of-final-method] ``` + +## Only the first `@final` violation is reported + +(Don't do this.) + + + +```py +from typing import final + +class A: + @final + def f(self): ... + +class B(A): + @final + def f(self): ... # error: [override-of-final-method] + +class C(B): + @final + # we only emit one error here, not two + def f(self): ... # error: [override-of-final-method] +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" new file mode 100644 index 0000000000000..aa69ded0b6f9c --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" @@ -0,0 +1,81 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - Only the first `@final` violation is reported +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | from typing import final + 2 | + 3 | class A: + 4 | @final + 5 | def f(self): ... + 6 | + 7 | class B(A): + 8 | @final + 9 | def f(self): ... # error: [override-of-final-method] +10 | +11 | class C(B): +12 | @final +13 | # we only emit one error here, not two +14 | def f(self): ... # error: [override-of-final-method] +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `A.f` + --> src/mdtest_snippet.py:9:9 + | + 7 | class B(A): + 8 | @final + 9 | def f(self): ... # error: [override-of-final-method] + | ^ Override of `f` from superclass `A` +10 | +11 | class C(B): + | +info: `A.f` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:4:5 + | +3 | class A: +4 | @final + | ------ +5 | def f(self): ... + | - `A.f` defined here +6 | +7 | class B(A): + | +info: rule `override-of-final-method` is enabled by default + +``` + +``` +error[override-of-final-method]: Cannot override `B.f` + --> src/mdtest_snippet.py:14:9 + | +12 | @final +13 | # we only emit one error here, not two +14 | def f(self): ... # error: [override-of-final-method] + | ^ Override of `f` from superclass `B` + | +info: `B.f` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:8:5 + | + 7 | class B(A): + 8 | @final + | ------ + 9 | def f(self): ... # error: [override-of-final-method] + | - `B.f` defined here +10 | +11 | class C(B): + | +info: rule `override-of-final-method` is enabled by default + +``` From 486caf5d322f47cc9ff867d215b241d8192c8d6d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 22:18:02 +0000 Subject: [PATCH 05/13] another edge case --- .../resources/mdtest/final.md | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index e5ce6fd0f2edb..d1b3c8b9040a9 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -335,3 +335,38 @@ class C(B): # we only emit one error here, not two def f(self): ... # error: [override-of-final-method] ``` + +## For when you just really want to drive the point home + +```py +from typing import final, Final + +@final +@final +@final +@final +@final +@final +class A: + @final + @final + @final + @final + @final + def method(self): ... + +@final +@final +@final +@final +@final +class B: + method: Final = A.method + +class C(A): # error: [subclass-of-final-class] + def method(self): ... # error: [override-of-final-method] + +class D(B): # error: [subclass-of-final-class] + # TODO: we should emit a diagnostic here + def method(self): ... +``` From 76ccc48855d3498f6088463c53b19386cfde9440 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 26 Nov 2025 22:43:46 +0000 Subject: [PATCH 06/13] try to optimize --- crates/ty_python_semantic/src/types/liskov.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index bd19f575bbaf7..c37721283112f 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -49,15 +49,15 @@ fn check_class_declaration<'db>( return; }; - let (literal, specialization) = class.class_literal(db); - let class_kind = CodeGeneratorKind::from_class(db, literal, specialization); - let Place::Defined(type_on_subclass_instance, _, _) = Type::instance(db, class).member(db, &member.name).place else { return; }; + let (literal, specialization) = class.class_literal(db); + let class_kind = CodeGeneratorKind::from_class(db, literal, specialization); + let mut subclass_overrides_superclass_declaration = false; let mut has_dynamic_superclass = false; let mut has_typeddict_in_mro = false; @@ -176,14 +176,12 @@ fn check_class_declaration<'db>( continue; } - let Some(superclass_type_as_callable) = superclass_type - .try_upcast_to_callable(db) - .map(|callables| callables.into_type(db)) - else { + let Some(superclass_type_as_callable) = superclass_type.try_upcast_to_callable(db) else { continue; }; - if type_on_subclass_instance.is_assignable_to(db, superclass_type_as_callable) { + if type_on_subclass_instance.is_assignable_to(db, superclass_type_as_callable.into_type(db)) + { continue; } From 408895e8d4be00bbb833ba0bb437621e3d73cad5 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Thu, 27 Nov 2025 11:19:29 +0000 Subject: [PATCH 07/13] use a standalone query to check if it's a function definition --- crates/ty/docs/rules.md | 177 +++++++++++------- .../resources/mdtest/call/union.md | 2 +- crates/ty_python_semantic/src/types/liskov.rs | 56 +++++- 3 files changed, 152 insertions(+), 83 deletions(-) diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 3a77ed7946979..5227756920efb 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -39,7 +39,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -63,7 +63,7 @@ Calling a non-callable object will raise a `TypeError` at runtime. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -95,7 +95,7 @@ f(int) # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -126,7 +126,7 @@ a = 1 Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -158,7 +158,7 @@ class C(A, B): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -190,7 +190,7 @@ class B(A): ... Default level: error · Preview (since 1.0.0) · Related issues · -View source +View source @@ -218,7 +218,7 @@ type B = A Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -245,7 +245,7 @@ class B(A, A): ... Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -357,7 +357,7 @@ def test(): -> "Literal[5]": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -387,7 +387,7 @@ class C(A, B): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -413,7 +413,7 @@ t[3] # IndexError: tuple index out of range Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -502,7 +502,7 @@ an atypical memory layout. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -529,7 +529,7 @@ func("foo") # error: [invalid-argument-type] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -557,7 +557,7 @@ a: int = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -591,7 +591,7 @@ C.instance_var = 3 # error: Cannot assign to instance variable Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -627,7 +627,7 @@ asyncio.run(main()) Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -651,7 +651,7 @@ class A(42): ... # error: [invalid-base] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -678,7 +678,7 @@ with 1: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -707,7 +707,7 @@ a: str Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -751,7 +751,7 @@ except ZeroDivisionError: Default level: error · Added in 0.0.1-alpha.28 · Related issues · -View source +View source @@ -793,7 +793,7 @@ class D(A): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -826,7 +826,7 @@ class C[U](Generic[T]): ... Default level: error · Added in 0.0.1-alpha.17 · Related issues · -View source +View source @@ -865,7 +865,7 @@ carol = Person(name="Carol", age=25) # typo! Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -900,7 +900,7 @@ def f(t: TypeVar("U")): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -934,7 +934,7 @@ class B(metaclass=f): ... Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1041,7 +1041,7 @@ Correct use of `@override` is enforced by ty's `invalid-explicit-override` rule. Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -1073,7 +1073,7 @@ TypeError: can only inherit from a NamedTuple type and Generic Default level: error · Preview (since 1.0.0) · Related issues · -View source +View source @@ -1103,7 +1103,7 @@ Baz = NewType("Baz", int | str) # error: invalid base for `typing.NewType` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1153,7 +1153,7 @@ def foo(x: int) -> int: ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1179,7 +1179,7 @@ def f(a: int = ''): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1210,7 +1210,7 @@ P2 = ParamSpec("S2") # error: ParamSpec name must match the variable it's assig Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1244,7 +1244,7 @@ TypeError: Protocols can only inherit from other protocols, got Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1293,7 +1293,7 @@ def g(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1318,7 +1318,7 @@ def func() -> int: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1376,7 +1376,7 @@ TODO #14889 Default level: error · Added in 0.0.1-alpha.6 · Related issues · -View source +View source @@ -1403,7 +1403,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -1450,7 +1450,7 @@ Bar[int] # error: too few arguments Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1480,7 +1480,7 @@ TYPE_CHECKING = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1510,7 +1510,7 @@ b: Annotated[int] # `Annotated` expects at least two arguments Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1544,7 +1544,7 @@ f(10) # Error Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1578,7 +1578,7 @@ class C: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1613,7 +1613,7 @@ T = TypeVar('T', bound=str) # valid bound TypeVar Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1638,7 +1638,7 @@ func() # TypeError: func() missing 1 required positional argument: 'x' Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1671,7 +1671,7 @@ alice["age"] # KeyError Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1700,7 +1700,7 @@ func("string") # error: [no-matching-overload] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1724,7 +1724,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1744,13 +1744,46 @@ for i in 34: # TypeError: 'int' object is not iterable pass ``` +## `override-of-final-method` + + +Default level: error · +Added in 0.0.1-alpha.29 · +Related issues · +View source + + + +**What it does** + +Checks for methods on subclasses that override superclass methods decorated with `@final`. + +**Why is this bad?** + +Decorating a method with `@final` declares to the type checker that it should not be +overridden on any subclass. + +**Example** + + +```python +from typing import final + +class A: + @final + def foo(self): ... + +class B(A): + def foo(self): ... # Error raised here +``` + ## `parameter-already-assigned` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1777,7 +1810,7 @@ f(1, x=2) # Error raised here Default level: error · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -1835,7 +1868,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1865,7 +1898,7 @@ static_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known tr Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1894,7 +1927,7 @@ class B(A): ... # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1921,7 +1954,7 @@ f("foo") # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1949,7 +1982,7 @@ def _(x: int): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1995,7 +2028,7 @@ class A: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2022,7 +2055,7 @@ f(x=1, y=2) # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2050,7 +2083,7 @@ A().foo # AttributeError: 'A' object has no attribute 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2075,7 +2108,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2100,7 +2133,7 @@ print(x) # NameError: name 'x' is not defined Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2137,7 +2170,7 @@ b1 < b2 < b1 # exception raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2165,7 +2198,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2190,7 +2223,7 @@ l[1:10:0] # ValueError: slice step cannot be zero Default level: warn · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -2231,7 +2264,7 @@ class SubProto(BaseProto, Protocol): Default level: warn · Added in 0.0.1-alpha.16 · Related issues · -View source +View source @@ -2319,7 +2352,7 @@ a = 20 / 0 # type: ignore Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2347,7 +2380,7 @@ A.c # AttributeError: type object 'A' has no attribute 'c' Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2379,7 +2412,7 @@ A()[0] # TypeError: 'A' object is not subscriptable Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2411,7 +2444,7 @@ from module import a # ImportError: cannot import name 'a' from 'module' Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2438,7 +2471,7 @@ cast(int, f()) # Redundant Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2462,7 +2495,7 @@ reveal_type(1) # NameError: name 'reveal_type' is not defined Default level: warn · Added in 0.0.1-alpha.15 · Related issues · -View source +View source @@ -2520,7 +2553,7 @@ def g(): Default level: warn · Added in 0.0.1-alpha.7 · Related issues · -View source +View source @@ -2559,7 +2592,7 @@ class D(C): ... # error: [unsupported-base] Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2622,7 +2655,7 @@ def foo(x: int | str) -> int | str: Default level: ignore · Preview (since 0.0.1-alpha.1) · Related issues · -View source +View source @@ -2646,7 +2679,7 @@ Dividing by zero raises a `ZeroDivisionError` at runtime. Default level: ignore · Added in 0.0.1-alpha.1 · Related issues · -View source +View source diff --git a/crates/ty_python_semantic/resources/mdtest/call/union.md b/crates/ty_python_semantic/resources/mdtest/call/union.md index 45ccf78ed34c8..4f374ac754673 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/union.md +++ b/crates/ty_python_semantic/resources/mdtest/call/union.md @@ -277,6 +277,6 @@ def _(flag: bool): x = f({"x": 1}) reveal_type(x) # revealed: int - # error: [invalid-argument-type] "Argument to function `f` is incorrect: Expected `T`, found `dict[Unknown | str, Unknown | int] & dict[str, int]`" + # error: [invalid-argument-type] "Argument to function `f` is incorrect: Expected `T`, found `dict[str, int] & dict[Unknown | str, Unknown | int]`" f({"y": 1}) ``` diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index c37721283112f..f418a576583d2 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -6,8 +6,9 @@ use ruff_db::diagnostic::Annotation; use rustc_hash::FxHashSet; use crate::{ + Db, place::Place, - semantic_index::{place_table, use_def_map}, + semantic_index::{place_table, scope::ScopeId, symbol::ScopedSymbolId, use_def_map}, types::{ ClassBase, ClassLiteral, ClassType, KnownClass, Type, class::CodeGeneratorKind, @@ -41,6 +42,46 @@ fn check_class_declaration<'db>( class: ClassType<'db>, member: &MemberWithDefinition<'db>, ) { + /// Salsa-tracked query to check whether a symbol in a superclas scope is a function definition. + /// + /// We need to know this for compatibility with pyright and mypy, neither of which emit an error + /// on `C.f` here: + /// + /// ```python + /// from typing import final + /// + /// class A: + /// @final + /// def f(self) -> None: ... + /// + /// class B: + /// f = A.f + /// + /// class C(B): + /// def f(self) -> None: ... # no error here + /// ``` + /// + /// This is a Salsa-tracked query because it has to look at the AST node for the definition, + /// which might be in a different Python module. If this weren't a tracked query, we could + /// introduce cross-module dependencies and over-invalidation. + #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] + fn is_function_definition<'db>( + db: &'db dyn Db, + scope: ScopeId<'db>, + symbol: ScopedSymbolId, + ) -> bool { + let Some(binding) = use_def_map(db, scope) + .end_of_scope_symbol_bindings(symbol) + .next() + else { + return false; + }; + let Some(definition) = binding.binding.definition() else { + return false; + }; + definition.kind(db).is_function_def() + } + let db = context.db(); let MemberWithDefinition { member, definition } = member; @@ -131,15 +172,10 @@ fn check_class_declaration<'db>( _ => return None, }; - if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) { - use_def_map(db, superclass_scope) - .end_of_scope_symbol_bindings(superclass_symbol_id) - .next()? - .binding - .definition()? - .kind(db) - .is_function_def() - .then_some((superclass, underlying_function)) + if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) + && is_function_definition(db, superclass_scope, superclass_symbol_id) + { + Some((superclass, underlying_function)) } else { None } From 5cc8e66cd4f2953cc11be1d1008d1ef95b98c030 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Thu, 27 Nov 2025 12:58:57 +0000 Subject: [PATCH 08/13] check if rules are enabled before doing expensive analysis --- crates/ty_python_semantic/src/types/liskov.rs | 115 +++++++++++++----- 1 file changed, 86 insertions(+), 29 deletions(-) diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index f418a576583d2..e96fba19e4cb0 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -2,11 +2,13 @@ //! //! [Liskov Substitution Principle]: https://en.wikipedia.org/wiki/Liskov_substitution_principle +use bitflags::bitflags; use ruff_db::diagnostic::Annotation; use rustc_hash::FxHashSet; use crate::{ Db, + lint::LintId, place::Place, semantic_index::{place_table, scope::ScopeId, symbol::ScopedSymbolId, use_def_map}, types::{ @@ -14,8 +16,8 @@ use crate::{ class::CodeGeneratorKind, context::InferContext, diagnostic::{ - INVALID_EXPLICIT_OVERRIDE, report_invalid_method_override, - report_overridden_final_method, + INVALID_EXPLICIT_OVERRIDE, INVALID_METHOD_OVERRIDE, OVERRIDE_OF_FINAL_METHOD, + report_invalid_method_override, report_overridden_final_method, }, function::{FunctionDecorators, KnownFunction}, ide_support::{MemberWithDefinition, all_declarations_and_bindings}, @@ -24,7 +26,8 @@ use crate::{ pub(super) fn check_class<'db>(context: &InferContext<'db, '_>, class: ClassLiteral<'db>) { let db = context.db(); - if class.is_known(db, KnownClass::Object) { + let configuration = OverrideRulesConfig::from(context); + if configuration.no_rules_enabled() { return; } @@ -33,12 +36,13 @@ pub(super) fn check_class<'db>(context: &InferContext<'db, '_>, class: ClassLite all_declarations_and_bindings(db, class.body_scope(db)).collect(); for member in own_class_members { - check_class_declaration(context, class_specialized, &member); + check_class_declaration(context, configuration, class_specialized, &member); } } fn check_class_declaration<'db>( context: &InferContext<'db, '_>, + configuration: OverrideRulesConfig, class: ClassType<'db>, member: &MemberWithDefinition<'db>, ) { @@ -155,31 +159,35 @@ fn check_class_declaration<'db>( break; }; - overridden_final_method = overridden_final_method.or_else(|| { - let superclass_symbol_id = superclass_symbol_id?; - - // TODO: `@final` should be more like a type qualifier: - // we should also recognise `@final`-decorated methods that don't end up - // as being function- or property-types (because they're wrapped by other - // decorators that transform the type into something else). - let underlying_function = match superclass - .own_class_member(db, None, &member.name) - .ignore_possibly_undefined()? - { - Type::BoundMethod(method) => method.function(db), - Type::FunctionLiteral(function) => function, - Type::PropertyInstance(property) => property.getter(db)?.as_function_literal()?, - _ => return None, - }; - - if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) - && is_function_definition(db, superclass_scope, superclass_symbol_id) - { - Some((superclass, underlying_function)) - } else { - None - } - }); + if configuration.check_final_method_overridden() { + overridden_final_method = overridden_final_method.or_else(|| { + let superclass_symbol_id = superclass_symbol_id?; + + // TODO: `@final` should be more like a type qualifier: + // we should also recognise `@final`-decorated methods that don't end up + // as being function- or property-types (because they're wrapped by other + // decorators that transform the type into something else). + let underlying_function = match superclass + .own_class_member(db, None, &member.name) + .ignore_possibly_undefined()? + { + Type::BoundMethod(method) => method.function(db), + Type::FunctionLiteral(function) => function, + Type::PropertyInstance(property) => { + property.getter(db)?.as_function_literal()? + } + _ => return None, + }; + + if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) + && is_function_definition(db, superclass_scope, superclass_symbol_id) + { + Some((superclass, underlying_function)) + } else { + None + } + }); + } // ********************************************************** // Everything below this point in the loop @@ -192,6 +200,10 @@ fn check_class_declaration<'db>( continue; } + if !configuration.check_method_liskov_violations() { + continue; + } + // TODO: Check Liskov on non-methods too let Type::FunctionLiteral(subclass_function) = member.ty else { continue; @@ -308,3 +320,48 @@ pub(super) enum MethodKind<'db> { #[default] NotSynthesized, } + +bitflags! { + /// Bitflags representing which override-related rules have been enabled. + #[derive(Default, Debug, Copy, Clone)] + struct OverrideRulesConfig: u8 { + const LISKOV_METHODS = 1 << 0; + const EXPLICIT_OVERRIDE = 1 << 1; + const FINAL_METHOD_OVERRIDDEN = 1 << 2; + } +} + +impl From<&InferContext<'_, '_>> for OverrideRulesConfig { + fn from(value: &InferContext<'_, '_>) -> Self { + let db = value.db(); + let rule_selection = db.rule_selection(value.file()); + + let mut config = OverrideRulesConfig::empty(); + + if rule_selection.is_enabled(LintId::of(&INVALID_METHOD_OVERRIDE)) { + config |= OverrideRulesConfig::LISKOV_METHODS; + } + if rule_selection.is_enabled(LintId::of(&INVALID_EXPLICIT_OVERRIDE)) { + config |= OverrideRulesConfig::EXPLICIT_OVERRIDE; + } + if rule_selection.is_enabled(LintId::of(&OVERRIDE_OF_FINAL_METHOD)) { + config |= OverrideRulesConfig::FINAL_METHOD_OVERRIDDEN; + } + + config + } +} + +impl OverrideRulesConfig { + const fn no_rules_enabled(self) -> bool { + self.is_empty() + } + + const fn check_method_liskov_violations(self) -> bool { + self.contains(OverrideRulesConfig::LISKOV_METHODS) + } + + const fn check_final_method_overridden(self) -> bool { + self.contains(OverrideRulesConfig::FINAL_METHOD_OVERRIDDEN) + } +} From ce8d67e17b1d85df1cd590b1c6dfd96650a88f0c Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 28 Nov 2025 12:37:55 +0000 Subject: [PATCH 09/13] Update crates/ty_python_semantic/resources/mdtest/final.md Co-authored-by: Micha Reiser --- crates/ty_python_semantic/resources/mdtest/final.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index d1b3c8b9040a9..7d8b53a535ed6 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -42,27 +42,35 @@ def lossy_decorator(fn: Callable) -> Callable: ... class Parent: @final def foo(self): ... + @final @property def my_property1(self) -> int: ... + @property @final def my_property2(self) -> int: ... + @final @classmethod def class_method1(cls) -> int: ... + @staticmethod @final def static_method1() -> int: ... + @final @classmethod def class_method2(cls) -> int: ... + @staticmethod @final def static_method2() -> int: ... + @lossy_decorator @final def decorated_1(self): ... + @final @lossy_decorator def decorated_2(self): ... From 6d40d0e9d61f2275560df8c95fc9c88760a3876d Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 28 Nov 2025 13:24:51 +0000 Subject: [PATCH 10/13] make recommendation clearer and add autofix --- .../resources/mdtest/final.md | 10 +- ..._a_me\342\200\246_(338615109711a91b).snap" | 604 +++++++++++------- ..._case\342\200\246_(2389d52c5ecfa2bd).snap" | 7 + ...`@fin\342\200\246_(9863b583f4c651c5).snap" | 20 + ...ods_d\342\200\246_(861757f48340ed92).snap" | 105 +++ .../src/types/diagnostic.rs | 52 +- .../ty_python_semantic/src/types/function.rs | 56 +- .../src/types/infer/builder.rs | 4 +- crates/ty_python_semantic/src/types/liskov.rs | 3 +- 9 files changed, 587 insertions(+), 274 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index 7d8b53a535ed6..b9c00ce6666a5 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -55,13 +55,13 @@ class Parent: @classmethod def class_method1(cls) -> int: ... - @staticmethod + @classmethod @final - def static_method1() -> int: ... + def class_method2(cls) -> int: ... @final - @classmethod - def class_method2(cls) -> int: ... + @staticmethod + def static_method1() -> int: ... @staticmethod @final @@ -70,7 +70,7 @@ class Parent: @lossy_decorator @final def decorated_1(self): ... - + @final @lossy_decorator def decorated_2(self): ... diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" index e3a2545fad885..d8816c44a30d0 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" @@ -12,114 +12,122 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md ## mdtest_snippet.pyi ``` - 1 | from typing_extensions import final, Callable, TypeVar - 2 | - 3 | def lossy_decorator(fn: Callable) -> Callable: ... - 4 | - 5 | class Parent: - 6 | @final - 7 | def foo(self): ... - 8 | @final - 9 | @property -10 | def my_property1(self) -> int: ... -11 | @property -12 | @final -13 | def my_property2(self) -> int: ... -14 | @final -15 | @classmethod -16 | def class_method1(cls) -> int: ... -17 | @staticmethod -18 | @final -19 | def static_method1() -> int: ... -20 | @final -21 | @classmethod -22 | def class_method2(cls) -> int: ... -23 | @staticmethod -24 | @final -25 | def static_method2() -> int: ... -26 | @lossy_decorator -27 | @final -28 | def decorated_1(self): ... -29 | @final -30 | @lossy_decorator -31 | def decorated_2(self): ... -32 | -33 | class Child(Parent): -34 | def foo(self): ... # error: [override-of-final-method] -35 | @property -36 | def my_property1(self) -> int: ... # error: [override-of-final-method] -37 | @property -38 | def my_property2(self) -> int: ... # error: [override-of-final-method] -39 | @classmethod -40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] -41 | @staticmethod -42 | def static_method1() -> int: ... # error: [override-of-final-method] -43 | @classmethod -44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] -45 | @staticmethod -46 | def static_method2() -> int: ... # error: [override-of-final-method] -47 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] -48 | @lossy_decorator -49 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] -50 | -51 | class OtherChild(Parent): ... -52 | -53 | class Grandchild(OtherChild): -54 | @staticmethod -55 | # TODO: we should emit a Liskov violation here too -56 | # error: [override-of-final-method] -57 | def foo(): ... -58 | @property -59 | # TODO: we should emit a Liskov violation here too -60 | # error: [override-of-final-method] -61 | def my_property1(self) -> str: ... -62 | # TODO: we should emit a Liskov violation here too -63 | # error: [override-of-final-method] -64 | class_method1 = None -65 | -66 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: -67 | -68 | T = TypeVar("T") -69 | -70 | def identity(x: T) -> T: ... -71 | -72 | class Foo: -73 | @final -74 | @identity -75 | @identity -76 | @identity -77 | @identity -78 | @identity -79 | @identity -80 | @identity -81 | @identity -82 | @identity -83 | @identity -84 | @identity -85 | @identity -86 | @identity -87 | @identity -88 | @identity -89 | @identity -90 | @identity -91 | @identity -92 | def bar(self): ... -93 | -94 | class Baz(Foo): -95 | def bar(self): ... # error: [override-of-final-method] + 1 | from typing_extensions import final, Callable, TypeVar + 2 | + 3 | def lossy_decorator(fn: Callable) -> Callable: ... + 4 | + 5 | class Parent: + 6 | @final + 7 | def foo(self): ... + 8 | + 9 | @final + 10 | @property + 11 | def my_property1(self) -> int: ... + 12 | + 13 | @property + 14 | @final + 15 | def my_property2(self) -> int: ... + 16 | + 17 | @final + 18 | @classmethod + 19 | def class_method1(cls) -> int: ... + 20 | + 21 | @classmethod + 22 | @final + 23 | def class_method2(cls) -> int: ... + 24 | + 25 | @final + 26 | @staticmethod + 27 | def static_method1() -> int: ... + 28 | + 29 | @staticmethod + 30 | @final + 31 | def static_method2() -> int: ... + 32 | + 33 | @lossy_decorator + 34 | @final + 35 | def decorated_1(self): ... + 36 | + 37 | @final + 38 | @lossy_decorator + 39 | def decorated_2(self): ... + 40 | + 41 | class Child(Parent): + 42 | def foo(self): ... # error: [override-of-final-method] + 43 | @property + 44 | def my_property1(self) -> int: ... # error: [override-of-final-method] + 45 | @property + 46 | def my_property2(self) -> int: ... # error: [override-of-final-method] + 47 | @classmethod + 48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] + 49 | @staticmethod + 50 | def static_method1() -> int: ... # error: [override-of-final-method] + 51 | @classmethod + 52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + 53 | @staticmethod + 54 | def static_method2() -> int: ... # error: [override-of-final-method] + 55 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] + 56 | @lossy_decorator + 57 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] + 58 | + 59 | class OtherChild(Parent): ... + 60 | + 61 | class Grandchild(OtherChild): + 62 | @staticmethod + 63 | # TODO: we should emit a Liskov violation here too + 64 | # error: [override-of-final-method] + 65 | def foo(): ... + 66 | @property + 67 | # TODO: we should emit a Liskov violation here too + 68 | # error: [override-of-final-method] + 69 | def my_property1(self) -> str: ... + 70 | # TODO: we should emit a Liskov violation here too + 71 | # error: [override-of-final-method] + 72 | class_method1 = None + 73 | + 74 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: + 75 | + 76 | T = TypeVar("T") + 77 | + 78 | def identity(x: T) -> T: ... + 79 | + 80 | class Foo: + 81 | @final + 82 | @identity + 83 | @identity + 84 | @identity + 85 | @identity + 86 | @identity + 87 | @identity + 88 | @identity + 89 | @identity + 90 | @identity + 91 | @identity + 92 | @identity + 93 | @identity + 94 | @identity + 95 | @identity + 96 | @identity + 97 | @identity + 98 | @identity + 99 | @identity +100 | def bar(self): ... +101 | +102 | class Baz(Foo): +103 | def bar(self): ... # error: [override-of-final-method] ``` # Diagnostics ``` error[override-of-final-method]: Cannot override `Parent.foo` - --> src/mdtest_snippet.pyi:34:9 + --> src/mdtest_snippet.pyi:42:9 | -33 | class Child(Parent): -34 | def foo(self): ... # error: [override-of-final-method] +41 | class Child(Parent): +42 | def foo(self): ... # error: [override-of-final-method] | ^^^ Override of `foo` from superclass `Parent` -35 | @property -36 | def my_property1(self) -> int: ... # error: [override-of-final-method] +43 | @property +44 | 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 @@ -129,188 +137,261 @@ info: `Parent.foo` is decorated with `@final`, forbidding overrides | ------ 7 | def foo(self): ... | --- `Parent.foo` defined here -8 | @final -9 | @property +8 | +9 | @final | +help: Remove the override of `foo` info: rule `override-of-final-method` is enabled by default +39 | def decorated_2(self): ... +40 | +41 | class Child(Parent): + - def foo(self): ... # error: [override-of-final-method] +42 + # error: [override-of-final-method] +43 | @property +44 | def my_property1(self) -> int: ... # error: [override-of-final-method] +45 | @property +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:36:9 + --> src/mdtest_snippet.pyi:44:9 | -34 | def foo(self): ... # error: [override-of-final-method] -35 | @property -36 | def my_property1(self) -> int: ... # error: [override-of-final-method] +42 | def foo(self): ... # error: [override-of-final-method] +43 | @property +44 | def my_property1(self) -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` -37 | @property -38 | def my_property2(self) -> int: ... # error: [override-of-final-method] +45 | @property +46 | def my_property2(self) -> int: ... # error: [override-of-final-method] | info: `Parent.my_property1` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:8:5 + --> src/mdtest_snippet.pyi:9:5 | - 6 | @final 7 | def foo(self): ... - 8 | @final + 8 | + 9 | @final | ------ - 9 | @property -10 | def my_property1(self) -> int: ... +10 | @property +11 | def my_property1(self) -> int: ... | ------------ `Parent.my_property1` defined here -11 | @property -12 | @final +12 | +13 | @property | +help: Remove the override of `my_property1` info: rule `override-of-final-method` is enabled by default +40 | +41 | class Child(Parent): +42 | def foo(self): ... # error: [override-of-final-method] + - @property + - def my_property1(self) -> int: ... # error: [override-of-final-method] +43 + # error: [override-of-final-method] +44 | @property +45 | def my_property2(self) -> int: ... # error: [override-of-final-method] +46 | @classmethod +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:38:9 + --> src/mdtest_snippet.pyi:46:9 | -36 | def my_property1(self) -> int: ... # error: [override-of-final-method] -37 | @property -38 | def my_property2(self) -> int: ... # error: [override-of-final-method] +44 | def my_property1(self) -> int: ... # error: [override-of-final-method] +45 | @property +46 | def my_property2(self) -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^ Override of `my_property2` from superclass `Parent` -39 | @classmethod -40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +47 | @classmethod +48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] | info: `Parent.my_property2` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:12:5 + --> src/mdtest_snippet.pyi:14:5 | -10 | def my_property1(self) -> int: ... -11 | @property -12 | @final +13 | @property +14 | @final | ------ -13 | def my_property2(self) -> int: ... +15 | def my_property2(self) -> int: ... | ------------ `Parent.my_property2` defined here -14 | @final -15 | @classmethod +16 | +17 | @final | +help: Remove the override of `my_property2` info: rule `override-of-final-method` is enabled by default +42 | def foo(self): ... # error: [override-of-final-method] +43 | @property +44 | def my_property1(self) -> int: ... # error: [override-of-final-method] + - @property + - def my_property2(self) -> int: ... # error: [override-of-final-method] +45 + # error: [override-of-final-method] +46 | @classmethod +47 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +48 | @staticmethod +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:40:9 + --> src/mdtest_snippet.pyi:48:9 | -38 | def my_property2(self) -> int: ... # error: [override-of-final-method] -39 | @classmethod -40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +46 | def my_property2(self) -> int: ... # error: [override-of-final-method] +47 | @classmethod +48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` -41 | @staticmethod -42 | def static_method1() -> int: ... # error: [override-of-final-method] +49 | @staticmethod +50 | def static_method1() -> int: ... # error: [override-of-final-method] | info: `Parent.class_method1` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:14:5 + --> src/mdtest_snippet.pyi:17:5 | -12 | @final -13 | def my_property2(self) -> int: ... -14 | @final +15 | def my_property2(self) -> int: ... +16 | +17 | @final | ------ -15 | @classmethod -16 | def class_method1(cls) -> int: ... +18 | @classmethod +19 | def class_method1(cls) -> int: ... | ------------- `Parent.class_method1` defined here -17 | @staticmethod -18 | @final +20 | +21 | @classmethod | +help: Remove the override of `class_method1` info: rule `override-of-final-method` is enabled by default +44 | def my_property1(self) -> int: ... # error: [override-of-final-method] +45 | @property +46 | def my_property2(self) -> int: ... # error: [override-of-final-method] + - @classmethod + - def class_method1(cls) -> int: ... # error: [override-of-final-method] +47 + # error: [override-of-final-method] +48 | @staticmethod +49 | def static_method1() -> int: ... # error: [override-of-final-method] +50 | @classmethod +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:42:9 + --> src/mdtest_snippet.pyi:50:9 | -40 | def class_method1(cls) -> int: ... # error: [override-of-final-method] -41 | @staticmethod -42 | def static_method1() -> int: ... # error: [override-of-final-method] +48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +49 | @staticmethod +50 | def static_method1() -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^^^ Override of `static_method1` from superclass `Parent` -43 | @classmethod -44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +51 | @classmethod +52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] | info: `Parent.static_method1` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:18:5 + --> src/mdtest_snippet.pyi:25:5 | -16 | def class_method1(cls) -> int: ... -17 | @staticmethod -18 | @final +23 | def class_method2(cls) -> int: ... +24 | +25 | @final | ------ -19 | def static_method1() -> int: ... +26 | @staticmethod +27 | def static_method1() -> int: ... | -------------- `Parent.static_method1` defined here -20 | @final -21 | @classmethod +28 | +29 | @staticmethod | +help: Remove the override of `static_method1` info: rule `override-of-final-method` is enabled by default +46 | def my_property2(self) -> int: ... # error: [override-of-final-method] +47 | @classmethod +48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] + - @staticmethod + - def static_method1() -> int: ... # error: [override-of-final-method] +49 + # error: [override-of-final-method] +50 | @classmethod +51 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +52 | @staticmethod +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:44:9 + --> src/mdtest_snippet.pyi:52:9 | -42 | def static_method1() -> int: ... # error: [override-of-final-method] -43 | @classmethod -44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +50 | def static_method1() -> int: ... # error: [override-of-final-method] +51 | @classmethod +52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^^ Override of `class_method2` from superclass `Parent` -45 | @staticmethod -46 | def static_method2() -> int: ... # error: [override-of-final-method] +53 | @staticmethod +54 | def static_method2() -> int: ... # error: [override-of-final-method] | info: `Parent.class_method2` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:20:5 + --> src/mdtest_snippet.pyi:22:5 | -18 | @final -19 | def static_method1() -> int: ... -20 | @final - | ------ 21 | @classmethod -22 | def class_method2(cls) -> int: ... +22 | @final + | ------ +23 | def class_method2(cls) -> int: ... | ------------- `Parent.class_method2` defined here -23 | @staticmethod -24 | @final +24 | +25 | @final | +help: Remove the override of `class_method2` info: rule `override-of-final-method` is enabled by default +48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +49 | @staticmethod +50 | def static_method1() -> int: ... # error: [override-of-final-method] + - @classmethod + - def class_method2(cls) -> int: ... # error: [override-of-final-method] +51 + # error: [override-of-final-method] +52 | @staticmethod +53 | def static_method2() -> int: ... # error: [override-of-final-method] +54 | def decorated_1(self): ... # TODO: should emit [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:46:9 + --> src/mdtest_snippet.pyi:54:9 | -44 | def class_method2(cls) -> int: ... # error: [override-of-final-method] -45 | @staticmethod -46 | def static_method2() -> int: ... # error: [override-of-final-method] +52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +53 | @staticmethod +54 | def static_method2() -> int: ... # error: [override-of-final-method] | ^^^^^^^^^^^^^^ Override of `static_method2` from superclass `Parent` -47 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] -48 | @lossy_decorator +55 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +56 | @lossy_decorator | info: `Parent.static_method2` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:24:5 + --> src/mdtest_snippet.pyi:30:5 | -22 | def class_method2(cls) -> int: ... -23 | @staticmethod -24 | @final +29 | @staticmethod +30 | @final | ------ -25 | def static_method2() -> int: ... +31 | def static_method2() -> int: ... | -------------- `Parent.static_method2` defined here -26 | @lossy_decorator -27 | @final +32 | +33 | @lossy_decorator | +help: Remove the override of `static_method2` info: rule `override-of-final-method` is enabled by default +50 | def static_method1() -> int: ... # error: [override-of-final-method] +51 | @classmethod +52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + - @staticmethod + - def static_method2() -> int: ... # error: [override-of-final-method] +53 + # error: [override-of-final-method] +54 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +55 | @lossy_decorator +56 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] +note: This is an unsafe fix and may change runtime behavior ``` ``` error[override-of-final-method]: Cannot override `Parent.foo` - --> src/mdtest_snippet.pyi:57:9 + --> src/mdtest_snippet.pyi:65:9 | -55 | # TODO: we should emit a Liskov violation here too -56 | # error: [override-of-final-method] -57 | def foo(): ... +63 | # TODO: we should emit a Liskov violation here too +64 | # error: [override-of-final-method] +65 | def foo(): ... | ^^^ Override of `foo` from superclass `Parent` -58 | @property -59 | # TODO: we should emit a Liskov violation here too +66 | @property +67 | # TODO: we should emit a Liskov violation here too | info: `Parent.foo` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:6:5 @@ -320,95 +401,138 @@ info: `Parent.foo` is decorated with `@final`, forbidding overrides | ------ 7 | def foo(self): ... | --- `Parent.foo` defined here -8 | @final -9 | @property +8 | +9 | @final | +help: Remove the override of `foo` info: rule `override-of-final-method` is enabled by default +59 | class OtherChild(Parent): ... +60 | +61 | class Grandchild(OtherChild): + - @staticmethod + - # TODO: we should emit a Liskov violation here too + - # error: [override-of-final-method] + - def foo(): ... +62 + +63 | @property +64 | # TODO: we should emit a Liskov violation here too +65 | # 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:61:9 + --> src/mdtest_snippet.pyi:69:9 | -59 | # TODO: we should emit a Liskov violation here too -60 | # error: [override-of-final-method] -61 | def my_property1(self) -> str: ... +67 | # TODO: we should emit a Liskov violation here too +68 | # error: [override-of-final-method] +69 | def my_property1(self) -> str: ... | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` -62 | # TODO: we should emit a Liskov violation here too -63 | # error: [override-of-final-method] +70 | # TODO: we should emit a Liskov violation here too +71 | # error: [override-of-final-method] | info: `Parent.my_property1` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:8:5 + --> src/mdtest_snippet.pyi:9:5 | - 6 | @final 7 | def foo(self): ... - 8 | @final + 8 | + 9 | @final | ------ - 9 | @property -10 | def my_property1(self) -> int: ... +10 | @property +11 | def my_property1(self) -> int: ... | ------------ `Parent.my_property1` defined here -11 | @property -12 | @final +12 | +13 | @property | +help: Remove the override of `my_property1` info: rule `override-of-final-method` is enabled by default +63 | # TODO: we should emit a Liskov violation here too +64 | # error: [override-of-final-method] +65 | def foo(): ... + - @property + - # TODO: we should emit a Liskov violation here too + - # error: [override-of-final-method] + - def my_property1(self) -> str: ... +66 + +67 | # TODO: we should emit a Liskov violation here too +68 | # error: [override-of-final-method] +69 | 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:64:5 + --> src/mdtest_snippet.pyi:72:5 | -62 | # TODO: we should emit a Liskov violation here too -63 | # error: [override-of-final-method] -64 | class_method1 = None +70 | # TODO: we should emit a Liskov violation here too +71 | # error: [override-of-final-method] +72 | class_method1 = None | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` -65 | -66 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: +73 | +74 | # 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:14:5 + --> src/mdtest_snippet.pyi:17:5 | -12 | @final -13 | def my_property2(self) -> int: ... -14 | @final +15 | def my_property2(self) -> int: ... +16 | +17 | @final | ------ -15 | @classmethod -16 | def class_method1(cls) -> int: ... +18 | @classmethod +19 | def class_method1(cls) -> int: ... | ------------- `Parent.class_method1` defined here -17 | @staticmethod -18 | @final +20 | +21 | @classmethod | +help: Remove the override of `class_method1` info: rule `override-of-final-method` is enabled by default +69 | def my_property1(self) -> str: ... +70 | # TODO: we should emit a Liskov violation here too +71 | # error: [override-of-final-method] + - class_method1 = None +72 + +73 | +74 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: +75 | +note: This is an unsafe fix and may change runtime behavior ``` ``` error[override-of-final-method]: Cannot override `Foo.bar` - --> src/mdtest_snippet.pyi:95:9 - | -94 | class Baz(Foo): -95 | def bar(self): ... # error: [override-of-final-method] - | ^^^ Override of `bar` from superclass `Foo` - | + --> src/mdtest_snippet.pyi:103:9 + | +102 | class Baz(Foo): +103 | def bar(self): ... # error: [override-of-final-method] + | ^^^ Override of `bar` from superclass `Foo` + | info: `Foo.bar` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.pyi:73:5 - | -72 | class Foo: -73 | @final - | ------ -74 | @identity -75 | @identity - | - ::: src/mdtest_snippet.pyi:92:9 - | -90 | @identity -91 | @identity -92 | def bar(self): ... - | --- `Foo.bar` defined here -93 | -94 | class Baz(Foo): - | + --> src/mdtest_snippet.pyi:81:5 + | + 80 | class Foo: + 81 | @final + | ------ + 82 | @identity + 83 | @identity + | + ::: src/mdtest_snippet.pyi:100:9 + | + 98 | @identity + 99 | @identity +100 | def bar(self): ... + | --- `Foo.bar` defined here +101 | +102 | class Baz(Foo): + | +help: Remove the override of `bar` info: rule `override-of-final-method` is enabled by default +100 | def bar(self): ... +101 | +102 | class Baz(Foo): + - def bar(self): ... # error: [override-of-final-method] +103 + # 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\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" index 4ab0b1aa3e6ad..fd02a5e92af66 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" @@ -47,6 +47,13 @@ info: `module1.Foo.f` is decorated with `@final`, forbidding overrides 5 | def f(self): ... | - `module1.Foo.f` defined here | +help: Remove the override of `f` info: rule `override-of-final-method` is enabled by default +1 | import module1 +2 | +3 | class Foo(module1.Foo): + - def f(self): ... # error: [override-of-final-method] +4 + # 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\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" index aa69ded0b6f9c..75a5f9c7768ca 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" @@ -52,7 +52,18 @@ info: `A.f` is decorated with `@final`, forbidding overrides 6 | 7 | class B(A): | +help: Remove the override of `f` info: rule `override-of-final-method` is enabled by default +5 | def f(self): ... +6 | +7 | class B(A): + - @final + - def f(self): ... # error: [override-of-final-method] +8 + # error: [override-of-final-method] +9 | +10 | class C(B): +11 | @final +note: This is an unsafe fix and may change runtime behavior ``` @@ -76,6 +87,15 @@ info: `B.f` is decorated with `@final`, forbidding overrides 10 | 11 | class C(B): | +help: Remove the override of `f` info: rule `override-of-final-method` is enabled by default +9 | def f(self): ... # error: [override-of-final-method] +10 | +11 | class C(B): + - @final + - # we only emit one error here, not two + - def f(self): ... # error: [override-of-final-method] +12 + # 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\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" index 033b734eb29b8..4098892196d7c 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" @@ -154,7 +154,21 @@ info: `Good.bar` is decorated with `@final`, forbidding overrides 7 | @overload 8 | def bar(self, x: int) -> int: ... | +help: Remove all overloads for `bar` info: rule `override-of-final-method` is enabled by default +13 | def baz(self, x: int) -> int: ... +14 | +15 | class ChildOfGood(Good): + - @overload + - def bar(self, x: str) -> str: ... + - @overload + - def bar(self, x: int) -> int: ... # error: [override-of-final-method] +16 + +17 + # error: [override-of-final-method] +18 | @overload +19 | def baz(self, x: str) -> str: ... +20 | @overload +note: This is an unsafe fix and may change runtime behavior ``` @@ -182,7 +196,21 @@ info: `Good.baz` is decorated with `@final`, forbidding overrides 12 | @overload 13 | def baz(self, x: int) -> int: ... | +help: Remove all overloads for `baz` info: rule `override-of-final-method` is enabled by default +17 | def bar(self, x: str) -> str: ... +18 | @overload +19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + - @overload + - def baz(self, x: str) -> str: ... + - @overload + - def baz(self, x: int) -> int: ... # error: [override-of-final-method] +20 + +21 + # error: [override-of-final-method] +22 | +23 | class Bad: +24 | @overload +note: This is an unsafe fix and may change runtime behavior ``` @@ -247,7 +275,21 @@ info: `Bad.bar` is decorated with `@final`, forbidding overrides 28 | @overload 29 | @final | +help: Remove all overloads for `bar` info: rule `override-of-final-method` is enabled by default +37 | def baz(self, x: int) -> int: ... +38 | +39 | class ChildOfBad(Bad): + - @overload + - def bar(self, x: str) -> str: ... + - @overload + - def bar(self, x: int) -> int: ... # error: [override-of-final-method] +40 + +41 + # error: [override-of-final-method] +42 | @overload +43 | def baz(self, x: str) -> str: ... +44 | @overload +note: This is an unsafe fix and may change runtime behavior ``` @@ -270,7 +312,18 @@ info: `Bad.baz` is decorated with `@final`, forbidding overrides 34 | @final 35 | @overload | +help: Remove all overloads for `baz` info: rule `override-of-final-method` is enabled by default +41 | def bar(self, x: str) -> str: ... +42 | @overload +43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + - @overload + - def baz(self, x: str) -> str: ... + - @overload + - def baz(self, x: int) -> int: ... # error: [override-of-final-method] +44 + +45 + # error: [override-of-final-method] +note: This is an unsafe fix and may change runtime behavior ``` @@ -295,7 +348,25 @@ info: `Good.f` is decorated with `@final`, forbidding overrides | - `Good.f` defined here 10 | return x | +help: Remove all overloads and the implementation for `f` info: rule `override-of-final-method` is enabled by default +10 | return x +11 | +12 | class ChildOfGood(Good): + - @overload + - def f(self, x: str) -> str: ... + - @overload + - def f(self, x: int) -> int: ... +13 + +14 + +15 | # error: [override-of-final-method] + - def f(self, x: int | str) -> int | str: + - return x +16 + +17 | +18 | class Bad: +19 | @overload +note: This is an unsafe fix and may change runtime behavior ``` @@ -383,7 +454,17 @@ info: `Bad.f` is decorated with `@final`, forbidding overrides | - `Bad.f` defined here 29 | return x | +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 ``` @@ -407,7 +488,16 @@ info: `Bad.g` is decorated with `@final`, forbidding overrides | - `Bad.g` defined here 38 | return x | +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 ``` @@ -430,7 +520,15 @@ info: `Bad.h` is decorated with `@final`, forbidding overrides | - `Bad.h` defined here 47 | return x | +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 ``` @@ -452,6 +550,13 @@ info: `Bad.i` is decorated with `@final`, forbidding overrides | - `Bad.i` defined here 56 | return x | +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 5d5e83db8031c..2abd4e8ce412c 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -17,7 +17,7 @@ use crate::types::call::CallError; use crate::types::class::{ CodeGeneratorKind, DisjointBase, DisjointBaseKind, Field, MethodDecorator, }; -use crate::types::function::{FunctionType, KnownFunction}; +use crate::types::function::{FunctionType, KnownFunction, OverloadLiteral}; use crate::types::liskov::MethodKind; use crate::types::string_annotation::{ BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION, @@ -3800,6 +3800,7 @@ pub(super) fn report_overridden_final_method<'db>( context: &InferContext<'db, '_>, member: &str, subclass_definition: Definition<'db>, + subclass_type: Type<'db>, superclass: ClassType<'db>, subclass: ClassType<'db>, superclass_method: FunctionType<'db>, @@ -3836,7 +3837,7 @@ pub(super) fn report_overridden_final_method<'db>( ); let superclass_function_literal = if superclass_method.file(db).is_stub(db) { - superclass_method.first_overload(db) + superclass_method.first_overload_or_implementation(db) } else { superclass_method.literal(db).last_definition(db) }; @@ -3856,6 +3857,53 @@ 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, + }; + + if let Some(function) = underlying_function { + let overload_deletion = |overload: &OverloadLiteral<'db>| { + Edit::range_deletion(overload.node(db, context.file(), context.module()).range()) + }; + + 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), + )); + } + ([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), + )); + } + ([], Some(implementation)) => { + diagnostic.help(format_args!("Remove the override of `{member}`")); + diagnostic.set_fix(Fix::unsafe_edit(overload_deletion(&implementation))); + } + ([], None) => { + // Should be impossible to get here: how would we even infer a function as a function + // if it has 0 overloads and no implementation? + unreachable!( + "A function should always have an implementation and/or >=1 overloads" + ); + } + } + } 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(), + ))); + } } /// This function receives an unresolved `from foo import bar` import, diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 5232fc5d84924..3b31532a37283 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -278,7 +278,7 @@ impl<'db> OverloadLiteral<'db> { || is_implicit_classmethod(self.name(db)) } - fn node<'ast>( + pub(crate) fn node<'ast>( self, db: &dyn Db, file: File, @@ -619,32 +619,40 @@ impl<'db> FunctionLiteral<'db> { self.last_definition(db).spans(db) } - #[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size, cycle_initial=overloads_and_implementation_cycle_initial)] fn overloads_and_implementation( self, db: &'db dyn Db, - ) -> (Box<[OverloadLiteral<'db>]>, Option>) { - let self_overload = self.last_definition(db); - let mut current = self_overload; - let mut overloads = vec![]; - - while let Some(previous) = current.previous_overload(db) { - let overload = previous.last_definition(db); - overloads.push(overload); - current = overload; - } + ) -> (&'db [OverloadLiteral<'db>], Option>) { + #[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size, cycle_initial=overloads_and_implementation_cycle_initial)] + fn overloads_and_implementation_inner<'db>( + db: &'db dyn Db, + function: FunctionLiteral<'db>, + ) -> (Box<[OverloadLiteral<'db>]>, Option>) { + let self_overload = function.last_definition(db); + let mut current = self_overload; + let mut overloads = vec![]; + + while let Some(previous) = current.previous_overload(db) { + let overload = previous.last_definition(db); + overloads.push(overload); + current = overload; + } - // Overloads are inserted in reverse order, from bottom to top. - overloads.reverse(); + // Overloads are inserted in reverse order, from bottom to top. + overloads.reverse(); - let implementation = if self_overload.is_overload(db) { - overloads.push(self_overload); - None - } else { - Some(self_overload) - }; + let implementation = if self_overload.is_overload(db) { + overloads.push(self_overload); + None + } else { + Some(self_overload) + }; + + (overloads.into_boxed_slice(), implementation) + } - (overloads.into_boxed_slice(), implementation) + let (overloads, implementation) = overloads_and_implementation_inner(db, self); + (overloads.as_ref(), *implementation) } fn iter_overloads_and_implementation( @@ -652,7 +660,7 @@ impl<'db> FunctionLiteral<'db> { db: &'db dyn Db, ) -> impl Iterator> + 'db { let (implementation, overloads) = self.overloads_and_implementation(db); - overloads.iter().chain(implementation).copied() + overloads.into_iter().chain(implementation.iter().copied()) } /// Typed externally-visible signature for this function. @@ -927,7 +935,7 @@ impl<'db> FunctionType<'db> { pub(crate) fn overloads_and_implementation( self, db: &'db dyn Db, - ) -> &'db (Box<[OverloadLiteral<'db>]>, Option>) { + ) -> (&'db [OverloadLiteral<'db>], Option>) { self.literal(db).overloads_and_implementation(db) } @@ -940,7 +948,7 @@ impl<'db> FunctionType<'db> { self.literal(db).iter_overloads_and_implementation(db) } - pub(crate) fn first_overload(self, db: &'db dyn Db) -> OverloadLiteral<'db> { + pub(crate) fn first_overload_or_implementation(self, db: &'db dyn Db) -> OverloadLiteral<'db> { self.iter_overloads_and_implementation(db) .next() .expect("A function must have at least one overload/implementation") diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index b8005da421cfa..dffba11af9537 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -1044,7 +1044,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } // Check that the overloaded function has at least two overloads - if let [single_overload] = overloads.as_ref() { + if let [single_overload] = overloads { let function_node = function.node(self.db(), self.file(), self.module()); if let Some(builder) = self .context @@ -1164,7 +1164,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { (FunctionDecorators::OVERRIDE, "override"), ] { if let Some(implementation) = implementation { - for overload in overloads.as_ref() { + for overload in overloads { if !overload.has_known_decorator(self.db(), decorator) { continue; } diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index e96fba19e4cb0..a15481fc0b9bd 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -276,7 +276,7 @@ fn check_class_declaration<'db>( && function.has_known_decorator(db, FunctionDecorators::OVERRIDE) { let function_literal = if context.in_stub() { - function.first_overload(db) + function.first_overload_or_implementation(db) } else { function.literal(db).last_definition(db) }; @@ -307,6 +307,7 @@ fn check_class_declaration<'db>( context, &member.name, *definition, + type_on_subclass_instance, superclass, class, superclass_method, From cb520d50ebd7b4e25d12d39f5e15b043ffd36b5f Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 28 Nov 2025 14:12:21 +0000 Subject: [PATCH 11/13] Add tests for possibly-undefined cases --- .../resources/mdtest/final.md | 91 ++++- ...fined\342\200\246_(fc7b496fd1986deb).snap" | 352 ++++++++++++++++++ .../src/types/diagnostic.rs | 27 +- crates/ty_python_semantic/src/types/liskov.rs | 71 ++-- 4 files changed, 503 insertions(+), 38 deletions(-) create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index b9c00ce6666a5..b4120d64a8424 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -292,20 +292,31 @@ class ChildOfBad(Bad): ## Edge case: the function is decorated with `@final` but originally defined elsewhere -As of 2025-11-26, pyrefly emits a diagnostic on this, but mypy and pyright do not: +As of 2025-11-26, pyrefly emits a diagnostic on this, but mypy and pyright do not. For mypy and +pyright to emit a diagnostic, the superclass definition decorated with `@final` must be a literal +function definition: an assignment definition where the right-hand side of the assignment is a +`@final-decorated` function is not sufficient for them to consider the superclass definition as +being `@final`. + +For now, we choose to follow mypy's and pyright's behaviour here, in order to maximise compatibility +with other type checkers. We may decide to change this in the future, however, as it would simplify +our implementation. Mypy's and pyright's behaviour here is also arguably inconsistent with their +treatment of other type qualifiers such as `Final`. As discussed in +, both type checkers view the `Final` +type qualifier as travelling *across* scopes. ```py from typing import final class A: @final - def method(self): ... + def method(self) -> None: ... class B: method = A.method class C(B): - def method(self): ... # no diagnostic + def method(self) -> None: ... # no diagnostic here (see prose discussion above) ``` ## Constructor methods are also checked @@ -378,3 +389,77 @@ class D(B): # error: [subclass-of-final-class] # TODO: we should emit a diagnostic here def method(self): ... ``` + +## An `@final` method is overridden by an implicit instance attribute + +```py +from typing import final, Any + +class Parent: + @final + def method(self) -> None: ... + +class Child(Parent): + def __init__(self) -> None: + self.method: Any = 42 # TODO: we should emit `[override-of-final-method]` here +``` + +## A possibly-undefined `@final` method is overridden + + + +```py +from typing import final + +def coinflip() -> bool: + return False + +class A: + if coinflip(): + @final + def method1(self) -> None: ... + else: + def method1(self) -> None: ... + + if coinflip(): + def method2(self) -> None: ... + else: + @final + def method2(self) -> None: ... + + if coinflip(): + @final + def method3(self) -> None: ... + else: + @final + def method3(self) -> None: ... + + if coinflip(): + def method4(self) -> None: ... + elif coinflip(): + @final + def method4(self) -> None: ... + else: + def method4(self) -> None: ... + +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] + +# 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 + + if coinflip(): + 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] +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" new file mode 100644 index 0000000000000..96146a5c4781e --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" @@ -0,0 +1,352 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - A possibly-undefined `@final` method is overridden +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | from typing import final + 2 | + 3 | def coinflip() -> bool: + 4 | return False + 5 | + 6 | class A: + 7 | if coinflip(): + 8 | @final + 9 | def method1(self) -> None: ... +10 | else: +11 | def method1(self) -> None: ... +12 | +13 | if coinflip(): +14 | def method2(self) -> None: ... +15 | else: +16 | @final +17 | def method2(self) -> None: ... +18 | +19 | if coinflip(): +20 | @final +21 | def method3(self) -> None: ... +22 | else: +23 | @final +24 | def method3(self) -> None: ... +25 | +26 | if coinflip(): +27 | def method4(self) -> None: ... +28 | elif coinflip(): +29 | @final +30 | def method4(self) -> None: ... +31 | else: +32 | def method4(self) -> None: ... +33 | +34 | class B(A): +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: ... # error: [override-of-final-method] +52 | def method3(self) -> None: ... # error: [override-of-final-method] +53 | def method4(self) -> None: ... # error: [override-of-final-method] +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `A.method1` + --> src/mdtest_snippet.py:35:9 + | +34 | class B(A): +35 | def method1(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Override of `method1` from superclass `A` +36 | def method2(self) -> None: ... # error: [override-of-final-method] +37 | def method3(self) -> None: ... # error: [override-of-final-method] + | +info: `A.method1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:8:9 + | + 6 | class A: + 7 | if coinflip(): + 8 | @final + | ------ + 9 | def method1(self) -> None: ... + | ------- `A.method1` defined here +10 | else: +11 | def method1(self) -> None: ... + | +help: Remove the override of `method1` +info: rule `override-of-final-method` is enabled by default +32 | def method4(self) -> None: ... +33 | +34 | class B(A): + - def method1(self) -> None: ... # error: [override-of-final-method] +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] +note: This is an unsafe fix and may change runtime behavior + +``` + +``` +error[override-of-final-method]: Cannot override `A.method2` + --> src/mdtest_snippet.py:36:9 + | +34 | class B(A): +35 | def method1(self) -> None: ... # error: [override-of-final-method] +36 | def method2(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Override of `method2` 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 + | +14 | def method2(self) -> None: ... +15 | else: +16 | @final + | ------ +17 | def method2(self) -> None: ... + | ------- `A.method2` defined here +18 | +19 | if coinflip(): + | +help: Remove the override of `method2` +info: rule `override-of-final-method` is enabled by default +33 | +34 | class B(A): +35 | def method1(self) -> None: ... # error: [override-of-final-method] + - 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 | +note: This is an unsafe fix and may change runtime behavior + +``` + +``` +error[override-of-final-method]: Cannot override `A.method3` + --> src/mdtest_snippet.py:37:9 + | +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] + | ^^^^^^^ Override of `method3` from superclass `A` +38 | def method4(self) -> None: ... # error: [override-of-final-method] + | +info: `A.method3` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:20:9 + | +19 | if coinflip(): +20 | @final + | ------ +21 | def method3(self) -> None: ... + | ------- `A.method3` defined here +22 | else: +23 | @final + | +help: Remove the override of `method3` +info: rule `override-of-final-method` is enabled by default +34 | class B(A): +35 | def method1(self) -> None: ... # error: [override-of-final-method] +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... +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 + | +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] + | ^^^^^^^ Override of `method4` from superclass `A` +39 | +40 | # Possible overrides of possibly `@final` methods... + | +info: `A.method4` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:29:9 + | +27 | def method4(self) -> None: ... +28 | elif coinflip(): +29 | @final + | ------ +30 | def method4(self) -> None: ... + | ------- `A.method4` defined here +31 | else: +32 | def method4(self) -> None: ... + | +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 + | +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] + | ^^^^^^^ Override of `method1` from superclass `A` +47 | else: +48 | pass + | +info: `A.method1` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:8:9 + | + 6 | class A: + 7 | if coinflip(): + 8 | @final + | ------ + 9 | def method1(self) -> None: ... + | ------- `A.method1` defined here +10 | else: +11 | def method1(self) -> None: ... + | +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.method2` + --> src/mdtest_snippet.py:51:13 + | +50 | if coinflip(): +51 | def method2(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Override of `method2` from superclass `A` +52 | def method3(self) -> None: ... # error: [override-of-final-method] +53 | def method4(self) -> None: ... # error: [override-of-final-method] + | +info: `A.method2` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:16:9 + | +14 | def method2(self) -> None: ... +15 | else: +16 | @final + | ------ +17 | def method2(self) -> None: ... + | ------- `A.method2` defined here +18 | +19 | if coinflip(): + | +help: Remove the override of `method2` +info: rule `override-of-final-method` is enabled by default +48 | pass +49 | +50 | if coinflip(): + - def method2(self) -> None: ... # error: [override-of-final-method] +51 + # error: [override-of-final-method] +52 | def method3(self) -> None: ... # error: [override-of-final-method] +53 | 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.method3` + --> src/mdtest_snippet.py:52:13 + | +50 | if coinflip(): +51 | def method2(self) -> None: ... # error: [override-of-final-method] +52 | def method3(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Override of `method3` from superclass `A` +53 | def method4(self) -> None: ... # error: [override-of-final-method] + | +info: `A.method3` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:20:9 + | +19 | if coinflip(): +20 | @final + | ------ +21 | def method3(self) -> None: ... + | ------- `A.method3` defined here +22 | else: +23 | @final + | +help: Remove the override of `method3` +info: rule `override-of-final-method` is enabled by default +49 | +50 | if coinflip(): +51 | def method2(self) -> None: ... # error: [override-of-final-method] + - def method3(self) -> None: ... # error: [override-of-final-method] +52 + # error: [override-of-final-method] +53 | 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:53:13 + | +51 | def method2(self) -> None: ... # error: [override-of-final-method] +52 | def method3(self) -> None: ... # error: [override-of-final-method] +53 | def method4(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Override of `method4` from superclass `A` + | +info: `A.method4` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:29:9 + | +27 | def method4(self) -> None: ... +28 | elif coinflip(): +29 | @final + | ------ +30 | def method4(self) -> None: ... + | ------- `A.method4` defined here +31 | else: +32 | def method4(self) -> None: ... + | +help: Remove the override of `method4` +info: rule `override-of-final-method` is enabled by default +50 | if coinflip(): +51 | def method2(self) -> None: ... # error: [override-of-final-method] +52 | def method3(self) -> None: ... # error: [override-of-final-method] + - def method4(self) -> None: ... # error: [override-of-final-method] +53 + # 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 2abd4e8ce412c..eab2fd753d991 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -17,7 +17,7 @@ use crate::types::call::CallError; use crate::types::class::{ CodeGeneratorKind, DisjointBase, DisjointBaseKind, Field, MethodDecorator, }; -use crate::types::function::{FunctionType, KnownFunction, OverloadLiteral}; +use crate::types::function::{FunctionDecorators, FunctionType, KnownFunction, OverloadLiteral}; use crate::types::liskov::MethodKind; use crate::types::string_annotation::{ BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION, @@ -3803,7 +3803,7 @@ pub(super) fn report_overridden_final_method<'db>( subclass_type: Type<'db>, superclass: ClassType<'db>, subclass: ClassType<'db>, - superclass_method: FunctionType<'db>, + superclass_method_defs: &[FunctionType<'db>], ) { let db = context.db(); @@ -3836,17 +3836,26 @@ pub(super) fn report_overridden_final_method<'db>( ), ); - let superclass_function_literal = if superclass_method.file(db).is_stub(db) { - superclass_method.first_overload_or_implementation(db) + let first_final_superclass_definition = superclass_method_defs + .iter() + .find(|function| function.has_known_decorator(db, FunctionDecorators::FINAL)) + .expect( + "At least one function definition in the superclass should be decorated with `@final`", + ); + + let superclass_function_literal = if first_final_superclass_definition.file(db).is_stub(db) { + first_final_superclass_definition.first_overload_or_implementation(db) } else { - superclass_method.literal(db).last_definition(db) + first_final_superclass_definition + .literal(db) + .last_definition(db) }; sub.annotate( - Annotation::secondary(Span::from( - superclass_function_literal - .focus_range(db, &parsed_module(db, superclass_method.file(db)).load(db)), - )) + Annotation::secondary(Span::from(superclass_function_literal.focus_range( + db, + &parsed_module(db, first_final_superclass_definition.file(db)).load(db), + ))) .message(format_args!("`{superclass_name}.{member}` defined here")), ); diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index a15481fc0b9bd..79b79889fc01e 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -19,7 +19,7 @@ use crate::{ INVALID_EXPLICIT_OVERRIDE, INVALID_METHOD_OVERRIDE, OVERRIDE_OF_FINAL_METHOD, report_invalid_method_override, report_overridden_final_method, }, - function::{FunctionDecorators, KnownFunction}, + function::{FunctionDecorators, FunctionType, KnownFunction}, ide_support::{MemberWithDefinition, all_declarations_and_bindings}, }, }; @@ -46,7 +46,8 @@ fn check_class_declaration<'db>( class: ClassType<'db>, member: &MemberWithDefinition<'db>, ) { - /// Salsa-tracked query to check whether a symbol in a superclas scope is a function definition. + /// Salsa-tracked query to check whether any of the definitions of a symbol + /// in a superclass scope are function definitions. /// /// We need to know this for compatibility with pyright and mypy, neither of which emit an error /// on `C.f` here: @@ -74,16 +75,37 @@ fn check_class_declaration<'db>( scope: ScopeId<'db>, symbol: ScopedSymbolId, ) -> bool { - let Some(binding) = use_def_map(db, scope) + use_def_map(db, scope) .end_of_scope_symbol_bindings(symbol) - .next() - else { - return false; - }; - let Some(definition) = binding.binding.definition() else { - return false; - }; - definition.kind(db).is_function_def() + .filter_map(|binding| binding.binding.definition()) + .any(|definition| definition.kind(db).is_function_def()) + } + + fn extract_underlying_functions<'db>( + db: &'db dyn Db, + ty: Type<'db>, + ) -> Option; 1]>> { + match ty { + Type::FunctionLiteral(function) => Some(smallvec::smallvec_inline![function]), + Type::BoundMethod(method) => Some(smallvec::smallvec_inline![method.function(db)]), + Type::PropertyInstance(property) => { + extract_underlying_functions(db, property.getter(db)?) + } + Type::Union(union) => { + let mut functions = smallvec::smallvec![]; + for member in union.elements(db) { + if let Some(mut member_functions) = extract_underlying_functions(db, *member) { + functions.append(&mut member_functions); + } + } + if functions.is_empty() { + None + } else { + Some(functions) + } + } + _ => None, + } } let db = context.db(); @@ -167,22 +189,19 @@ fn check_class_declaration<'db>( // we should also recognise `@final`-decorated methods that don't end up // as being function- or property-types (because they're wrapped by other // decorators that transform the type into something else). - let underlying_function = match superclass - .own_class_member(db, None, &member.name) - .ignore_possibly_undefined()? - { - Type::BoundMethod(method) => method.function(db), - Type::FunctionLiteral(function) => function, - Type::PropertyInstance(property) => { - property.getter(db)?.as_function_literal()? - } - _ => return None, - }; - - if underlying_function.has_known_decorator(db, FunctionDecorators::FINAL) + let underlying_functions = extract_underlying_functions( + db, + superclass + .own_class_member(db, None, &member.name) + .ignore_possibly_undefined()?, + )?; + + if underlying_functions + .iter() + .any(|function| function.has_known_decorator(db, FunctionDecorators::FINAL)) && is_function_definition(db, superclass_scope, superclass_symbol_id) { - Some((superclass, underlying_function)) + Some((superclass, underlying_functions)) } else { None } @@ -310,7 +329,7 @@ fn check_class_declaration<'db>( type_on_subclass_instance, superclass, class, - superclass_method, + &superclass_method, ); } } From 33915c1541c12d4243f71ef6a4ead93123403dc6 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 28 Nov 2025 14:38:48 +0000 Subject: [PATCH 12/13] Avoid repetition in the primary diagnostic message --- .../resources/mdtest/final.md | 17 +- ...fined\342\200\246_(fc7b496fd1986deb).snap" | 16 +- ..._a_me\342\200\246_(338615109711a91b).snap" | 431 +++++++++--------- ..._case\342\200\246_(2389d52c5ecfa2bd).snap" | 2 +- ...`@fin\342\200\246_(9863b583f4c651c5).snap" | 4 +- ...ods_d\342\200\246_(861757f48340ed92).snap" | 271 +++++------ .../src/types/diagnostic.rs | 2 +- .../ty_python_semantic/src/types/function.rs | 8 +- 8 files changed, 390 insertions(+), 361 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index b4120d64a8424..fa8c660e53990 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -76,20 +76,31 @@ class Parent: def decorated_2(self): ... class Child(Parent): - def foo(self): ... # error: [override-of-final-method] + # explicitly test the concise diagnostic message, + # which is different to the verbose diagnostic summary message: + # + # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`" + def foo(self): ... @property def my_property1(self) -> int: ... # error: [override-of-final-method] + @property def my_property2(self) -> int: ... # error: [override-of-final-method] + @classmethod def class_method1(cls) -> int: ... # error: [override-of-final-method] + @staticmethod def static_method1() -> int: ... # error: [override-of-final-method] + @classmethod def class_method2(cls) -> int: ... # error: [override-of-final-method] + @staticmethod def static_method2() -> int: ... # error: [override-of-final-method] + def decorated_1(self): ... # TODO: should emit [override-of-final-method] + @lossy_decorator def decorated_2(self): ... # TODO: should emit [override-of-final-method] @@ -181,6 +192,7 @@ class Good: def bar(self, x: str) -> str: ... @overload def bar(self, x: int) -> int: ... + @final @overload def baz(self, x: str) -> str: ... @@ -192,6 +204,7 @@ class ChildOfGood(Good): 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 @@ -204,6 +217,7 @@ class Bad: @final # error: [invalid-overload] def bar(self, x: int) -> int: ... + @overload def baz(self, x: str) -> str: ... @final @@ -216,6 +230,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 diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" index 96146a5c4781e..4b1c501542119 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" @@ -75,7 +75,7 @@ error[override-of-final-method]: Cannot override `A.method1` | 34 | class B(A): 35 | def method1(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Override of `method1` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 36 | def method2(self) -> None: ... # error: [override-of-final-method] 37 | def method3(self) -> None: ... # error: [override-of-final-method] | @@ -112,7 +112,7 @@ error[override-of-final-method]: Cannot override `A.method2` 34 | class B(A): 35 | def method1(self) -> None: ... # error: [override-of-final-method] 36 | def method2(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Override of `method2` from superclass `A` + | ^^^^^^^ 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] | @@ -149,7 +149,7 @@ error[override-of-final-method]: Cannot override `A.method3` 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] - | ^^^^^^^ Override of `method3` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 38 | def method4(self) -> None: ... # error: [override-of-final-method] | info: `A.method3` is decorated with `@final`, forbidding overrides @@ -184,7 +184,7 @@ error[override-of-final-method]: Cannot override `A.method4` 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] - | ^^^^^^^ Override of `method4` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 39 | 40 | # Possible overrides of possibly `@final` methods... | @@ -221,7 +221,7 @@ error[override-of-final-method]: Cannot override `A.method1` 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] - | ^^^^^^^ Override of `method1` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 47 | else: 48 | pass | @@ -257,7 +257,7 @@ error[override-of-final-method]: Cannot override `A.method2` | 50 | if coinflip(): 51 | def method2(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Override of `method2` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 52 | def method3(self) -> None: ... # error: [override-of-final-method] 53 | def method4(self) -> None: ... # error: [override-of-final-method] | @@ -293,7 +293,7 @@ error[override-of-final-method]: Cannot override `A.method3` 50 | if coinflip(): 51 | def method2(self) -> None: ... # error: [override-of-final-method] 52 | def method3(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Override of `method3` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` 53 | def method4(self) -> None: ... # error: [override-of-final-method] | info: `A.method3` is decorated with `@final`, forbidding overrides @@ -326,7 +326,7 @@ error[override-of-final-method]: Cannot override `A.method4` 51 | def method2(self) -> None: ... # error: [override-of-final-method] 52 | def method3(self) -> None: ... # error: [override-of-final-method] 53 | def method4(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Override of `method4` from superclass `A` + | ^^^^^^^ Overrides a definition from superclass `A` | info: `A.method4` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.py:29:9 diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" index d8816c44a30d0..ad51738ec59eb 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Cannot_override_a_me\342\200\246_(338615109711a91b).snap" @@ -53,57 +53,57 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md 39 | def decorated_2(self): ... 40 | 41 | class Child(Parent): - 42 | def foo(self): ... # error: [override-of-final-method] - 43 | @property - 44 | def my_property1(self) -> int: ... # error: [override-of-final-method] - 45 | @property - 46 | def my_property2(self) -> int: ... # error: [override-of-final-method] - 47 | @classmethod - 48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] - 49 | @staticmethod - 50 | def static_method1() -> int: ... # error: [override-of-final-method] - 51 | @classmethod - 52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] - 53 | @staticmethod - 54 | def static_method2() -> int: ... # error: [override-of-final-method] - 55 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] - 56 | @lossy_decorator - 57 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] + 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] 58 | - 59 | class OtherChild(Parent): ... - 60 | - 61 | class Grandchild(OtherChild): + 59 | @classmethod + 60 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + 61 | 62 | @staticmethod - 63 | # TODO: we should emit a Liskov violation here too - 64 | # error: [override-of-final-method] - 65 | def foo(): ... - 66 | @property - 67 | # TODO: we should emit a Liskov violation here too - 68 | # error: [override-of-final-method] - 69 | def my_property1(self) -> str: ... - 70 | # TODO: we should emit a Liskov violation here too - 71 | # error: [override-of-final-method] - 72 | class_method1 = None - 73 | - 74 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: - 75 | - 76 | T = TypeVar("T") - 77 | - 78 | def identity(x: T) -> T: ... - 79 | - 80 | class Foo: - 81 | @final - 82 | @identity - 83 | @identity - 84 | @identity - 85 | @identity - 86 | @identity - 87 | @identity - 88 | @identity - 89 | @identity - 90 | @identity - 91 | @identity - 92 | @identity + 63 | def static_method2() -> int: ... # error: [override-of-final-method] + 64 | + 65 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] + 66 | + 67 | @lossy_decorator + 68 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] + 69 | + 70 | class OtherChild(Parent): ... + 71 | + 72 | class Grandchild(OtherChild): + 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 @@ -111,23 +111,35 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md 97 | @identity 98 | @identity 99 | @identity -100 | def bar(self): ... -101 | -102 | class Baz(Foo): -103 | def bar(self): ... # error: [override-of-final-method] +100 | @identity +101 | @identity +102 | @identity +103 | @identity +104 | @identity +105 | @identity +106 | @identity +107 | @identity +108 | @identity +109 | @identity +110 | @identity +111 | def bar(self): ... +112 | +113 | class Baz(Foo): +114 | def bar(self): ... # error: [override-of-final-method] ``` # Diagnostics ``` error[override-of-final-method]: Cannot override `Parent.foo` - --> src/mdtest_snippet.pyi:42:9 + --> src/mdtest_snippet.pyi:46:9 | -41 | class Child(Parent): -42 | def foo(self): ... # error: [override-of-final-method] - | ^^^ Override of `foo` from superclass `Parent` -43 | @property -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] +44 | # +45 | # error: [override-of-final-method] "Cannot override final member `foo` from superclass `Parent`" +46 | def foo(self): ... + | ^^^ Overrides a definition from superclass `Parent` +47 | @property +48 | 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 @@ -142,28 +154,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 -39 | def decorated_2(self): ... -40 | -41 | class Child(Parent): - - def foo(self): ... # error: [override-of-final-method] -42 + # error: [override-of-final-method] -43 | @property -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] -45 | @property +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`" + - def foo(self): ... +46 + +47 | @property +48 | def my_property1(self) -> int: ... # error: [override-of-final-method] +49 | 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:44:9 + --> src/mdtest_snippet.pyi:48:9 | -42 | def foo(self): ... # error: [override-of-final-method] -43 | @property -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` -45 | @property -46 | def my_property2(self) -> int: ... # error: [override-of-final-method] +46 | def foo(self): ... +47 | @property +48 | def my_property1(self) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^ Overrides a definition from superclass `Parent` +49 | +50 | @property | info: `Parent.my_property1` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:9:5 @@ -180,29 +192,28 @@ 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 -40 | -41 | class Child(Parent): -42 | def foo(self): ... # error: [override-of-final-method] +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] -43 + # error: [override-of-final-method] -44 | @property -45 | def my_property2(self) -> int: ... # error: [override-of-final-method] -46 | @classmethod +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:46:9 + --> src/mdtest_snippet.pyi:51:9 | -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] -45 | @property -46 | def my_property2(self) -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^ Override of `my_property2` from superclass `Parent` -47 | @classmethod -48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +50 | @property +51 | def my_property2(self) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^ Overrides a definition from superclass `Parent` +52 | +53 | @classmethod | info: `Parent.my_property2` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:14:5 @@ -217,29 +228,28 @@ info: `Parent.my_property2` is decorated with `@final`, forbidding overrides | help: Remove the override of `my_property2` info: rule `override-of-final-method` is enabled by default -42 | def foo(self): ... # error: [override-of-final-method] -43 | @property -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] +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] -45 + # error: [override-of-final-method] -46 | @classmethod -47 | def class_method1(cls) -> int: ... # error: [override-of-final-method] -48 | @staticmethod +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:48:9 + --> src/mdtest_snippet.pyi:54:9 | -46 | def my_property2(self) -> int: ... # error: [override-of-final-method] -47 | @classmethod -48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` -49 | @staticmethod -50 | def static_method1() -> int: ... # error: [override-of-final-method] +53 | @classmethod +54 | def class_method1(cls) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^ Overrides a definition from superclass `Parent` +55 | +56 | @staticmethod | info: `Parent.class_method1` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:17:5 @@ -256,29 +266,28 @@ info: `Parent.class_method1` is decorated with `@final`, forbidding overrides | help: Remove the override of `class_method1` info: rule `override-of-final-method` is enabled by default -44 | def my_property1(self) -> int: ... # error: [override-of-final-method] -45 | @property -46 | def my_property2(self) -> int: ... # error: [override-of-final-method] +50 | @property +51 | def my_property2(self) -> int: ... # error: [override-of-final-method] +52 | - @classmethod - def class_method1(cls) -> int: ... # error: [override-of-final-method] -47 + # error: [override-of-final-method] -48 | @staticmethod -49 | def static_method1() -> int: ... # error: [override-of-final-method] -50 | @classmethod +53 + # error: [override-of-final-method] +54 | +55 | @staticmethod +56 | 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:50:9 + --> src/mdtest_snippet.pyi:57:9 | -48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] -49 | @staticmethod -50 | def static_method1() -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^^^ Override of `static_method1` from superclass `Parent` -51 | @classmethod -52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +56 | @staticmethod +57 | def static_method1() -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^^ Overrides a definition from superclass `Parent` +58 | +59 | @classmethod | info: `Parent.static_method1` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:25:5 @@ -295,29 +304,28 @@ info: `Parent.static_method1` is decorated with `@final`, forbidding overrides | help: Remove the override of `static_method1` info: rule `override-of-final-method` is enabled by default -46 | def my_property2(self) -> int: ... # error: [override-of-final-method] -47 | @classmethod -48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +53 | @classmethod +54 | def class_method1(cls) -> int: ... # error: [override-of-final-method] +55 | - @staticmethod - def static_method1() -> int: ... # error: [override-of-final-method] -49 + # error: [override-of-final-method] -50 | @classmethod -51 | def class_method2(cls) -> int: ... # error: [override-of-final-method] -52 | @staticmethod +56 + # error: [override-of-final-method] +57 | +58 | @classmethod +59 | 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:52:9 + --> src/mdtest_snippet.pyi:60:9 | -50 | def static_method1() -> int: ... # error: [override-of-final-method] -51 | @classmethod -52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^^ Override of `class_method2` from superclass `Parent` -53 | @staticmethod -54 | def static_method2() -> int: ... # error: [override-of-final-method] +59 | @classmethod +60 | def class_method2(cls) -> int: ... # error: [override-of-final-method] + | ^^^^^^^^^^^^^ Overrides a definition from superclass `Parent` +61 | +62 | @staticmethod | info: `Parent.class_method2` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:22:5 @@ -332,29 +340,28 @@ info: `Parent.class_method2` is decorated with `@final`, forbidding overrides | help: Remove the override of `class_method2` info: rule `override-of-final-method` is enabled by default -48 | def class_method1(cls) -> int: ... # error: [override-of-final-method] -49 | @staticmethod -50 | def static_method1() -> int: ... # error: [override-of-final-method] +56 | @staticmethod +57 | def static_method1() -> int: ... # error: [override-of-final-method] +58 | - @classmethod - def class_method2(cls) -> int: ... # error: [override-of-final-method] -51 + # error: [override-of-final-method] -52 | @staticmethod -53 | def static_method2() -> int: ... # error: [override-of-final-method] -54 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +59 + # error: [override-of-final-method] +60 | +61 | @staticmethod +62 | 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:54:9 + --> src/mdtest_snippet.pyi:63:9 | -52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] -53 | @staticmethod -54 | def static_method2() -> int: ... # error: [override-of-final-method] - | ^^^^^^^^^^^^^^ Override of `static_method2` from superclass `Parent` -55 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] -56 | @lossy_decorator +62 | @staticmethod +63 | 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] | info: `Parent.static_method2` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:30:5 @@ -369,29 +376,29 @@ info: `Parent.static_method2` is decorated with `@final`, forbidding overrides | help: Remove the override of `static_method2` info: rule `override-of-final-method` is enabled by default -50 | def static_method1() -> int: ... # error: [override-of-final-method] -51 | @classmethod -52 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +59 | @classmethod +60 | def class_method2(cls) -> int: ... # error: [override-of-final-method] +61 | - @staticmethod - def static_method2() -> int: ... # error: [override-of-final-method] -53 + # error: [override-of-final-method] -54 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] -55 | @lossy_decorator -56 | def decorated_2(self): ... # TODO: should emit [override-of-final-method] +62 + # error: [override-of-final-method] +63 | +64 | def decorated_1(self): ... # TODO: should emit [override-of-final-method] +65 | note: This is an unsafe fix and may change runtime behavior ``` ``` error[override-of-final-method]: Cannot override `Parent.foo` - --> src/mdtest_snippet.pyi:65:9 + --> src/mdtest_snippet.pyi:76:9 | -63 | # TODO: we should emit a Liskov violation here too -64 | # error: [override-of-final-method] -65 | def foo(): ... - | ^^^ Override of `foo` from superclass `Parent` -66 | @property -67 | # TODO: we should emit a Liskov violation here too +74 | # TODO: we should emit a Liskov violation here too +75 | # error: [override-of-final-method] +76 | def foo(): ... + | ^^^ Overrides a definition from superclass `Parent` +77 | @property +78 | # TODO: we should emit a Liskov violation here too | info: `Parent.foo` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:6:5 @@ -406,31 +413,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 -59 | class OtherChild(Parent): ... -60 | -61 | class Grandchild(OtherChild): +70 | class OtherChild(Parent): ... +71 | +72 | class Grandchild(OtherChild): - @staticmethod - # TODO: we should emit a Liskov violation here too - # error: [override-of-final-method] - def foo(): ... -62 + -63 | @property -64 | # TODO: we should emit a Liskov violation here too -65 | # error: [override-of-final-method] +73 + +74 | @property +75 | # TODO: we should emit a Liskov violation here too +76 | # 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:69:9 + --> src/mdtest_snippet.pyi:80:9 | -67 | # TODO: we should emit a Liskov violation here too -68 | # error: [override-of-final-method] -69 | def my_property1(self) -> str: ... - | ^^^^^^^^^^^^ Override of `my_property1` from superclass `Parent` -70 | # TODO: we should emit a Liskov violation here too -71 | # error: [override-of-final-method] +78 | # TODO: we should emit a Liskov violation here too +79 | # error: [override-of-final-method] +80 | 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] | info: `Parent.my_property1` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:9:5 @@ -447,31 +454,31 @@ 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 -63 | # TODO: we should emit a Liskov violation here too -64 | # error: [override-of-final-method] -65 | def foo(): ... +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: ... -66 + -67 | # TODO: we should emit a Liskov violation here too -68 | # error: [override-of-final-method] -69 | class_method1 = None +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:72:5 + --> src/mdtest_snippet.pyi:83:5 | -70 | # TODO: we should emit a Liskov violation here too -71 | # error: [override-of-final-method] -72 | class_method1 = None - | ^^^^^^^^^^^^^ Override of `class_method1` from superclass `Parent` -73 | -74 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: +81 | # TODO: we should emit a Liskov violation here too +82 | # error: [override-of-final-method] +83 | 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: | info: `Parent.class_method1` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.pyi:17:5 @@ -488,51 +495,51 @@ info: `Parent.class_method1` is decorated with `@final`, forbidding overrides | help: Remove the override of `class_method1` info: rule `override-of-final-method` is enabled by default -69 | def my_property1(self) -> str: ... -70 | # TODO: we should emit a Liskov violation here too -71 | # 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] - class_method1 = None -72 + -73 | -74 | # Diagnostic edge case: `final` is very far away from the method definition in the source code: -75 | +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:103:9 + --> src/mdtest_snippet.pyi:114:9 | -102 | class Baz(Foo): -103 | def bar(self): ... # error: [override-of-final-method] - | ^^^ Override of `bar` from superclass `Foo` +113 | class Baz(Foo): +114 | 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:81:5 + --> src/mdtest_snippet.pyi:92:5 | - 80 | class Foo: - 81 | @final + 91 | class Foo: + 92 | @final | ------ - 82 | @identity - 83 | @identity + 93 | @identity + 94 | @identity | - ::: src/mdtest_snippet.pyi:100:9 + ::: src/mdtest_snippet.pyi:111:9 | - 98 | @identity - 99 | @identity -100 | def bar(self): ... +109 | @identity +110 | @identity +111 | def bar(self): ... | --- `Foo.bar` defined here -101 | -102 | class Baz(Foo): +112 | +113 | class Baz(Foo): | help: Remove the override of `bar` info: rule `override-of-final-method` is enabled by default -100 | def bar(self): ... -101 | -102 | class Baz(Foo): +111 | def bar(self): ... +112 | +113 | class Baz(Foo): - def bar(self): ... # error: [override-of-final-method] -103 + # error: [override-of-final-method] +114 + # 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\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" index fd02a5e92af66..5e7f11ca02f92 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Diagnostic_edge_case\342\200\246_(2389d52c5ecfa2bd).snap" @@ -36,7 +36,7 @@ error[override-of-final-method]: Cannot override `module1.Foo.f` | 3 | class Foo(module1.Foo): 4 | def f(self): ... # error: [override-of-final-method] - | ^ Override of `f` from superclass `module1.Foo` + | ^ Overrides a definition from superclass `module1.Foo` | info: `module1.Foo.f` is decorated with `@final`, forbidding overrides --> src/module1.py:4:5 diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" index 75a5f9c7768ca..342b7ec8b6815 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Only_the_first_`@fin\342\200\246_(9863b583f4c651c5).snap" @@ -37,7 +37,7 @@ error[override-of-final-method]: Cannot override `A.f` 7 | class B(A): 8 | @final 9 | def f(self): ... # error: [override-of-final-method] - | ^ Override of `f` from superclass `A` + | ^ Overrides a definition from superclass `A` 10 | 11 | class C(B): | @@ -74,7 +74,7 @@ error[override-of-final-method]: Cannot override `B.f` 12 | @final 13 | # we only emit one error here, not two 14 | def f(self): ... # error: [override-of-final-method] - | ^ Override of `f` from superclass `B` + | ^ Overrides a definition from superclass `B` | info: `B.f` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.py:8:5 diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" index 4098892196d7c..38ec75b003e7a 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_Overloaded_methods_d\342\200\246_(861757f48340ed92).snap" @@ -20,45 +20,49 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md 6 | def bar(self, x: str) -> str: ... 7 | @overload 8 | def bar(self, x: int) -> int: ... - 9 | @final -10 | @overload -11 | def baz(self, x: str) -> str: ... -12 | @overload -13 | def baz(self, x: int) -> int: ... -14 | -15 | class ChildOfGood(Good): -16 | @overload -17 | def bar(self, x: str) -> str: ... -18 | @overload -19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] -20 | @overload -21 | def baz(self, x: str) -> str: ... + 9 | +10 | @final +11 | @overload +12 | def baz(self, x: str) -> str: ... +13 | @overload +14 | def baz(self, x: int) -> int: ... +15 | +16 | class ChildOfGood(Good): +17 | @overload +18 | def bar(self, x: str) -> str: ... +19 | @overload +20 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +21 | 22 | @overload -23 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] -24 | -25 | class Bad: -26 | @overload -27 | def bar(self, x: str) -> str: ... +23 | def baz(self, x: str) -> str: ... +24 | @overload +25 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] +26 | +27 | class Bad: 28 | @overload -29 | @final -30 | # error: [invalid-overload] -31 | def bar(self, x: int) -> int: ... -32 | @overload -33 | def baz(self, x: str) -> str: ... -34 | @final +29 | def bar(self, x: str) -> str: ... +30 | @overload +31 | @final +32 | # error: [invalid-overload] +33 | def bar(self, x: int) -> int: ... +34 | 35 | @overload -36 | # error: [invalid-overload] -37 | def baz(self, x: int) -> int: ... -38 | -39 | class ChildOfBad(Bad): -40 | @overload -41 | def bar(self, x: str) -> str: ... -42 | @overload -43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] -44 | @overload -45 | def baz(self, x: str) -> str: ... -46 | @overload -47 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] +36 | def baz(self, x: str) -> str: ... +37 | @final +38 | @overload +39 | # error: [invalid-overload] +40 | def baz(self, x: int) -> int: ... +41 | +42 | class ChildOfBad(Bad): +43 | @overload +44 | def bar(self, x: str) -> str: ... +45 | @overload +46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +47 | +48 | @overload +49 | def baz(self, x: str) -> str: ... +50 | @overload +51 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] ``` ## main.py @@ -133,14 +137,14 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md ``` error[override-of-final-method]: Cannot override `Good.bar` - --> src/stub.pyi:19:9 + --> src/stub.pyi:20:9 | -17 | def bar(self, x: str) -> str: ... -18 | @overload -19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] - | ^^^ Override of `bar` from superclass `Good` -20 | @overload -21 | def baz(self, x: str) -> str: ... +18 | def bar(self, x: str) -> str: ... +19 | @overload +20 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Overrides a definition from superclass `Good` +21 | +22 | @overload | info: `Good.bar` is decorated with `@final`, forbidding overrides --> src/stub.pyi:5:5 @@ -156,79 +160,79 @@ info: `Good.bar` is decorated with `@final`, forbidding overrides | help: Remove all overloads for `bar` info: rule `override-of-final-method` is enabled by default -13 | def baz(self, x: int) -> int: ... -14 | -15 | class ChildOfGood(Good): +14 | def baz(self, x: int) -> int: ... +15 | +16 | class ChildOfGood(Good): - @overload - def bar(self, x: str) -> str: ... - @overload - def bar(self, x: int) -> int: ... # error: [override-of-final-method] -16 + -17 + # error: [override-of-final-method] -18 | @overload -19 | def baz(self, x: str) -> str: ... +17 + +18 + # error: [override-of-final-method] +19 | 20 | @overload +21 | def baz(self, x: str) -> str: ... note: This is an unsafe fix and may change runtime behavior ``` ``` error[override-of-final-method]: Cannot override `Good.baz` - --> src/stub.pyi:23:9 + --> src/stub.pyi:25:9 | -21 | def baz(self, x: str) -> str: ... -22 | @overload -23 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] - | ^^^ Override of `baz` from superclass `Good` -24 | -25 | class Bad: +23 | def baz(self, x: str) -> str: ... +24 | @overload +25 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Overrides a definition from superclass `Good` +26 | +27 | class Bad: | info: `Good.baz` is decorated with `@final`, forbidding overrides - --> src/stub.pyi:9:5 + --> src/stub.pyi:10:5 | - 7 | @overload 8 | def bar(self, x: int) -> int: ... - 9 | @final + 9 | +10 | @final | ------ -10 | @overload -11 | def baz(self, x: str) -> str: ... +11 | @overload +12 | def baz(self, x: str) -> str: ... | --- `Good.baz` defined here -12 | @overload -13 | def baz(self, x: int) -> int: ... +13 | @overload +14 | def baz(self, x: int) -> int: ... | help: Remove all overloads for `baz` info: rule `override-of-final-method` is enabled by default -17 | def bar(self, x: str) -> str: ... -18 | @overload -19 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +19 | @overload +20 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +21 | - @overload - def baz(self, x: str) -> str: ... - @overload - def baz(self, x: int) -> int: ... # error: [override-of-final-method] -20 + -21 + # error: [override-of-final-method] -22 | -23 | class Bad: -24 | @overload +22 + +23 + # error: [override-of-final-method] +24 | +25 | class Bad: +26 | @overload note: This is an unsafe fix and may change runtime behavior ``` ``` error[invalid-overload]: `@final` decorator should be applied only to the first overload - --> src/stub.pyi:27:9 + --> src/stub.pyi:29:9 | -25 | class Bad: -26 | @overload -27 | def bar(self, x: str) -> str: ... - | --- First overload defined here +27 | class Bad: 28 | @overload -29 | @final -30 | # error: [invalid-overload] -31 | def bar(self, x: int) -> int: ... +29 | def bar(self, x: str) -> str: ... + | --- First overload defined here +30 | @overload +31 | @final +32 | # error: [invalid-overload] +33 | def bar(self, x: int) -> int: ... | ^^^ -32 | @overload -33 | def baz(self, x: str) -> str: ... +34 | +35 | @overload | info: rule `invalid-overload` is enabled by default @@ -236,19 +240,18 @@ info: rule `invalid-overload` is enabled by default ``` error[invalid-overload]: `@final` decorator should be applied only to the first overload - --> src/stub.pyi:33:9 + --> src/stub.pyi:36:9 | -31 | def bar(self, x: int) -> int: ... -32 | @overload -33 | def baz(self, x: str) -> str: ... - | --- First overload defined here -34 | @final 35 | @overload -36 | # error: [invalid-overload] -37 | def baz(self, x: int) -> int: ... +36 | def baz(self, x: str) -> str: ... + | --- First overload defined here +37 | @final +38 | @overload +39 | # error: [invalid-overload] +40 | def baz(self, x: int) -> int: ... | ^^^ -38 | -39 | class ChildOfBad(Bad): +41 | +42 | class ChildOfBad(Bad): | info: rule `invalid-overload` is enabled by default @@ -256,73 +259,73 @@ info: rule `invalid-overload` is enabled by default ``` error[override-of-final-method]: Cannot override `Bad.bar` - --> src/stub.pyi:43:9 + --> src/stub.pyi:46:9 | -41 | def bar(self, x: str) -> str: ... -42 | @overload -43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] - | ^^^ Override of `bar` from superclass `Bad` -44 | @overload -45 | def baz(self, x: str) -> str: ... +44 | def bar(self, x: str) -> str: ... +45 | @overload +46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Overrides a definition from superclass `Bad` +47 | +48 | @overload | info: `Bad.bar` is decorated with `@final`, forbidding overrides - --> src/stub.pyi:27:9 + --> src/stub.pyi:29:9 | -25 | class Bad: -26 | @overload -27 | def bar(self, x: str) -> str: ... - | --- `Bad.bar` defined here +27 | class Bad: 28 | @overload -29 | @final +29 | def bar(self, x: str) -> str: ... + | --- `Bad.bar` defined here +30 | @overload +31 | @final | help: Remove all overloads for `bar` info: rule `override-of-final-method` is enabled by default -37 | def baz(self, x: int) -> int: ... -38 | -39 | class ChildOfBad(Bad): +40 | def baz(self, x: int) -> int: ... +41 | +42 | class ChildOfBad(Bad): - @overload - def bar(self, x: str) -> str: ... - @overload - def bar(self, x: int) -> int: ... # error: [override-of-final-method] -40 + -41 + # error: [override-of-final-method] -42 | @overload -43 | def baz(self, x: str) -> str: ... -44 | @overload +43 | +44 + # error: [override-of-final-method] +45 + +46 | @overload +47 | def baz(self, x: str) -> str: ... +48 | @overload note: This is an unsafe fix and may change runtime behavior ``` ``` error[override-of-final-method]: Cannot override `Bad.baz` - --> src/stub.pyi:47:9 + --> src/stub.pyi:51:9 | -45 | def baz(self, x: str) -> str: ... -46 | @overload -47 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] - | ^^^ Override of `baz` from superclass `Bad` +49 | def baz(self, x: str) -> str: ... +50 | @overload +51 | def baz(self, x: int) -> int: ... # error: [override-of-final-method] + | ^^^ Overrides a definition from superclass `Bad` | info: `Bad.baz` is decorated with `@final`, forbidding overrides - --> src/stub.pyi:33:9 + --> src/stub.pyi:36:9 | -31 | def bar(self, x: int) -> int: ... -32 | @overload -33 | def baz(self, x: str) -> str: ... - | --- `Bad.baz` defined here -34 | @final 35 | @overload +36 | def baz(self, x: str) -> str: ... + | --- `Bad.baz` defined here +37 | @final +38 | @overload | help: Remove all overloads for `baz` info: rule `override-of-final-method` is enabled by default -41 | def bar(self, x: str) -> str: ... -42 | @overload -43 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +45 | @overload +46 | def bar(self, x: int) -> int: ... # error: [override-of-final-method] +47 | - @overload - def baz(self, x: str) -> str: ... - @overload - def baz(self, x: int) -> int: ... # error: [override-of-final-method] -44 + -45 + # error: [override-of-final-method] +48 + +49 + # error: [override-of-final-method] note: This is an unsafe fix and may change runtime behavior ``` @@ -334,7 +337,7 @@ error[override-of-final-method]: Cannot override `Good.f` 16 | def f(self, x: int) -> int: ... 17 | # error: [override-of-final-method] 18 | def f(self, x: int | str) -> int | str: - | ^ Override of `f` from superclass `Good` + | ^ Overrides a definition from superclass `Good` 19 | return x | info: `Good.f` is decorated with `@final`, forbidding overrides @@ -441,7 +444,7 @@ error[override-of-final-method]: Cannot override `Bad.f` 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] - | ^ Override of `f` from superclass `Bad` + | ^ Overrides a definition from superclass `Bad` 61 | g = None # error: [override-of-final-method] 62 | h = None # error: [override-of-final-method] | @@ -475,7 +478,7 @@ error[override-of-final-method]: Cannot override `Bad.g` 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] - | ^ Override of `g` from superclass `Bad` + | ^ Overrides a definition from superclass `Bad` 62 | h = None # error: [override-of-final-method] 63 | i = None # error: [override-of-final-method] | @@ -508,7 +511,7 @@ error[override-of-final-method]: Cannot override `Bad.h` 60 | f = None # error: [override-of-final-method] 61 | g = None # error: [override-of-final-method] 62 | h = None # error: [override-of-final-method] - | ^ Override of `h` from superclass `Bad` + | ^ Overrides a definition from superclass `Bad` 63 | i = None # error: [override-of-final-method] | info: `Bad.h` is decorated with `@final`, forbidding overrides @@ -539,7 +542,7 @@ error[override-of-final-method]: Cannot override `Bad.i` 61 | g = None # error: [override-of-final-method] 62 | h = None # error: [override-of-final-method] 63 | i = None # error: [override-of-final-method] - | ^ Override of `i` from superclass `Bad` + | ^ Overrides a definition from superclass `Bad` | info: `Bad.i` is decorated with `@final`, forbidding overrides --> src/main.py:55:9 diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index eab2fd753d991..726cf7296d77a 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -3823,7 +3823,7 @@ pub(super) fn report_overridden_final_method<'db>( let mut diagnostic = builder.into_diagnostic(format_args!("Cannot override `{superclass_name}.{member}`")); diagnostic.set_primary_message(format_args!( - "Override of `{member}` from superclass `{superclass_name}`" + "Overrides a definition from superclass `{superclass_name}`" )); diagnostic.set_concise_message(format_args!( "Cannot override final member `{member}` from superclass `{superclass_name}`" diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 3b31532a37283..7c07a86af159d 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -623,7 +623,11 @@ impl<'db> FunctionLiteral<'db> { self, db: &'db dyn Db, ) -> (&'db [OverloadLiteral<'db>], Option>) { - #[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size, cycle_initial=overloads_and_implementation_cycle_initial)] + #[salsa::tracked( + returns(ref), + heap_size=ruff_memory_usage::heap_size, + cycle_initial=overloads_and_implementation_cycle_initial + )] fn overloads_and_implementation_inner<'db>( db: &'db dyn Db, function: FunctionLiteral<'db>, @@ -816,7 +820,7 @@ impl<'db> FunctionType<'db> { } /// Returns the AST node for this function. - pub(crate) fn node<'ast>( + pub(super) fn node<'ast>( self, db: &dyn Db, file: File, From f8eb0d27abb1104ce1d93eb8bb8f7fdc650a9063 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 28 Nov 2025 14:48:33 +0000 Subject: [PATCH 13/13] todo a couple more things --- .../resources/mdtest/final.md | 6 +- ...fined\342\200\246_(fc7b496fd1986deb).snap" | 80 ++++++------------- 2 files changed, 29 insertions(+), 57 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index fa8c660e53990..1ccb647e4dd51 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -474,7 +474,11 @@ class C(A): pass if coinflip(): - def method2(self) -> None: ... # error: [override-of-final-method] + def method2(self) -> None: ... # TODO: should emit [override-of-final-method] + else: + def method2(self) -> None: ... # TODO: should emit [override-of-final-method] + + if coinflip(): def method3(self) -> None: ... # error: [override-of-final-method] def method4(self) -> None: ... # error: [override-of-final-method] ``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" index 4b1c501542119..41b39fef34a33 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi\342\200\246_-_A_possibly-undefined\342\200\246_(fc7b496fd1986deb).snap" @@ -62,9 +62,13 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md 48 | pass 49 | 50 | if coinflip(): -51 | def method2(self) -> None: ... # error: [override-of-final-method] -52 | def method3(self) -> None: ... # error: [override-of-final-method] -53 | def method4(self) -> None: ... # error: [override-of-final-method] +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] ``` # Diagnostics @@ -251,50 +255,14 @@ note: This is an unsafe fix and may change runtime behavior ``` -``` -error[override-of-final-method]: Cannot override `A.method2` - --> src/mdtest_snippet.py:51:13 - | -50 | if coinflip(): -51 | def method2(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Overrides a definition from superclass `A` -52 | def method3(self) -> None: ... # error: [override-of-final-method] -53 | def method4(self) -> None: ... # error: [override-of-final-method] - | -info: `A.method2` is decorated with `@final`, forbidding overrides - --> src/mdtest_snippet.py:16:9 - | -14 | def method2(self) -> None: ... -15 | else: -16 | @final - | ------ -17 | def method2(self) -> None: ... - | ------- `A.method2` defined here -18 | -19 | if coinflip(): - | -help: Remove the override of `method2` -info: rule `override-of-final-method` is enabled by default -48 | pass -49 | -50 | if coinflip(): - - def method2(self) -> None: ... # error: [override-of-final-method] -51 + # error: [override-of-final-method] -52 | def method3(self) -> None: ... # error: [override-of-final-method] -53 | 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.method3` - --> src/mdtest_snippet.py:52:13 + --> src/mdtest_snippet.py:56:13 | -50 | if coinflip(): -51 | def method2(self) -> None: ... # error: [override-of-final-method] -52 | def method3(self) -> None: ... # error: [override-of-final-method] +55 | if coinflip(): +56 | def method3(self) -> None: ... # error: [override-of-final-method] | ^^^^^^^ Overrides a definition from superclass `A` -53 | def method4(self) -> None: ... # error: [override-of-final-method] +57 | def method4(self) -> None: ... # error: [override-of-final-method] | info: `A.method3` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.py:20:9 @@ -309,23 +277,23 @@ 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 -49 | -50 | if coinflip(): -51 | def method2(self) -> None: ... # error: [override-of-final-method] +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] -52 + # error: [override-of-final-method] -53 | def method4(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:53:13 + --> src/mdtest_snippet.py:57:13 | -51 | def method2(self) -> None: ... # error: [override-of-final-method] -52 | def method3(self) -> None: ... # error: [override-of-final-method] -53 | def method4(self) -> None: ... # error: [override-of-final-method] +55 | if coinflip(): +56 | def method3(self) -> None: ... # error: [override-of-final-method] +57 | def method4(self) -> None: ... # error: [override-of-final-method] | ^^^^^^^ Overrides a definition from superclass `A` | info: `A.method4` is decorated with `@final`, forbidding overrides @@ -342,11 +310,11 @@ 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 -50 | if coinflip(): -51 | def method2(self) -> None: ... # error: [override-of-final-method] -52 | def method3(self) -> None: ... # error: [override-of-final-method] +54 | +55 | if coinflip(): +56 | def method3(self) -> None: ... # error: [override-of-final-method] - def method4(self) -> None: ... # error: [override-of-final-method] -53 + # error: [override-of-final-method] +57 + # error: [override-of-final-method] note: This is an unsafe fix and may change runtime behavior ```