From 81c97e9e9437ae68e23c047e4c6fb2abcf0c80d6 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 25 Nov 2025 18:42:40 +0000 Subject: [PATCH] [ty] Implement `typing.override` (#21627) ## Summary Part of https://github.com/astral-sh/ty/issues/155. This implements the basic check (`@override`-decorated methods should override things!), but not the strict check specified in https://typing.python.org/en/latest/spec/class-compat.html#strict-enforcement-per-project, which should be a separate error code. ## Test Plan mdtests and snapshots --------- Co-authored-by: Carl Meyer --- crates/ty/docs/rules.md | 180 ++++++---- .../resources/mdtest/override.md | 314 ++++++++++++++++++ ...override`_-_Basics_(b7c220f8171f11f0).snap | 293 ++++++++++++++++ .../src/types/diagnostic.rs | 53 ++- .../ty_python_semantic/src/types/function.rs | 2 +- crates/ty_python_semantic/src/types/liskov.rs | 154 ++++++--- .../e2e__commands__debug_command.snap | 1 + ty.schema.json | 10 + 8 files changed, 886 insertions(+), 121 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/override.md create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index fcbab4402a..a6b571c402 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 · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -217,7 +217,7 @@ class B(A, A): ... Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -329,7 +329,7 @@ def test(): -> "Literal[5]": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -359,7 +359,7 @@ class C(A, B): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -385,7 +385,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 @@ -474,7 +474,7 @@ an atypical memory layout. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -501,7 +501,7 @@ func("foo") # error: [invalid-argument-type] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -529,7 +529,7 @@ a: int = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -563,7 +563,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 @@ -599,7 +599,7 @@ asyncio.run(main()) Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -623,7 +623,7 @@ class A(42): ... # error: [invalid-base] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -650,7 +650,7 @@ with 1: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -679,7 +679,7 @@ a: str Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -717,13 +717,55 @@ except ZeroDivisionError: This rule corresponds to Ruff's [`except-with-non-exception-classes` (`B030`)](https://docs.astral.sh/ruff/rules/except-with-non-exception-classes) +## `invalid-explicit-override` + + +Default level: error · +Added in 0.0.1-alpha.28 · +Related issues · +View source + + + +**What it does** + +Checks for methods that are decorated with `@override` but do not override any method in a superclass. + +**Why is this bad?** + +Decorating a method with `@override` declares to the type checker that the intention is that it should +override a method from a superclass. + +**Example** + + +```python +from typing import override + +class A: + @override + def foo(self): ... # Error raised here + +class B(A): + @override + def ffooo(self): ... # Error raised here + +class C: + @override + def __repr__(self): ... # fine: overrides `object.__repr__` + +class D(A): + @override + def foo(self): ... # fine: overrides `A.foo` +``` + ## `invalid-generic-class` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -756,7 +798,7 @@ class C[U](Generic[T]): ... Default level: error · Added in 0.0.1-alpha.17 · Related issues · -View source +View source @@ -795,7 +837,7 @@ carol = Person(name="Carol", age=25) # typo! Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -830,7 +872,7 @@ def f(t: TypeVar("U")): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -864,7 +906,7 @@ class B(metaclass=f): ... Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -950,7 +992,7 @@ and `__ne__` methods accept `object` as their second argument. Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -982,7 +1024,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 @@ -1012,7 +1054,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 @@ -1062,7 +1104,7 @@ def foo(x: int) -> int: ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1088,7 +1130,7 @@ def f(a: int = ''): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1119,7 +1161,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 @@ -1153,7 +1195,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 @@ -1202,7 +1244,7 @@ def g(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1227,7 +1269,7 @@ def func() -> int: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1285,7 +1327,7 @@ TODO #14889 Default level: error · Added in 0.0.1-alpha.6 · Related issues · -View source +View source @@ -1312,7 +1354,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1342,7 +1384,7 @@ TYPE_CHECKING = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1372,7 +1414,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 @@ -1406,7 +1448,7 @@ f(10) # Error Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1440,7 +1482,7 @@ class C: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1475,7 +1517,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 @@ -1500,7 +1542,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 @@ -1533,7 +1575,7 @@ alice["age"] # KeyError Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1562,7 +1604,7 @@ func("string") # error: [no-matching-overload] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1586,7 +1628,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 @@ -1612,7 +1654,7 @@ for i in 34: # TypeError: 'int' object is not iterable Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1639,7 +1681,7 @@ f(1, x=2) # Error raised here Default level: error · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -1697,7 +1739,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1727,7 +1769,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 @@ -1756,7 +1798,7 @@ class B(A): ... # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1783,7 +1825,7 @@ f("foo") # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1811,7 +1853,7 @@ def _(x: int): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1857,7 +1899,7 @@ class A: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1884,7 +1926,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 @@ -1912,7 +1954,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 @@ -1937,7 +1979,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1962,7 +2004,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 @@ -1999,7 +2041,7 @@ b1 < b2 < b1 # exception raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2027,7 +2069,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 @@ -2052,7 +2094,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 @@ -2093,7 +2135,7 @@ class SubProto(BaseProto, Protocol): Default level: warn · Added in 0.0.1-alpha.16 · Related issues · -View source +View source @@ -2181,7 +2223,7 @@ a = 20 / 0 # type: ignore Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2209,7 +2251,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 @@ -2241,7 +2283,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 @@ -2273,7 +2315,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 @@ -2300,7 +2342,7 @@ cast(int, f()) # Redundant Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2324,7 +2366,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 @@ -2382,7 +2424,7 @@ def g(): Default level: warn · Added in 0.0.1-alpha.7 · Related issues · -View source +View source @@ -2421,7 +2463,7 @@ class D(C): ... # error: [unsupported-base] Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2484,7 +2526,7 @@ def foo(x: int | str) -> int | str: Default level: ignore · Preview (since 0.0.1-alpha.1) · Related issues · -View source +View source @@ -2508,7 +2550,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/override.md b/crates/ty_python_semantic/resources/mdtest/override.md new file mode 100644 index 0000000000..c1ab23d790 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/override.md @@ -0,0 +1,314 @@ +# `typing.override` + +## Basics + +Decorating a method with `typing.override` decorator is an explicit indication to a type checker +that the method is intended to override a method on a superclass. If the decorated method does not +in fact override anything, a type checker should report a diagnostic on that method. + + + +```pyi +from typing_extensions import override, Callable, TypeVar + +def lossy_decorator(fn: Callable) -> Callable: ... + +class A: + @override + def __repr__(self): ... # fine: overrides `object.__repr__` + +class Parent: + def foo(self): ... + @property + def my_property1(self) -> int: ... + @property + def my_property2(self) -> int: ... + baz = None + @classmethod + def class_method1(cls) -> int: ... + @staticmethod + def static_method1() -> int: ... + @classmethod + def class_method2(cls) -> int: ... + @staticmethod + def static_method2() -> int: ... + @lossy_decorator + def decorated_1(self): ... + @lossy_decorator + def decorated_2(self): ... + @lossy_decorator + def decorated_3(self): ... + +class Child(Parent): + @override + def foo(self): ... # fine: overrides `Parent.foo` + @property + @override + def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + @override + @property + def my_property2(self) -> int: ... # fine: overrides `Parent.my_property2` + @override + def baz(self): ... # fine: overrides `Parent.baz` + @classmethod + @override + def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + @staticmethod + @override + def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + @override + @classmethod + def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + @override + @staticmethod + def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + @override + def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + @override + @lossy_decorator + def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + @lossy_decorator + @override + def decorated_3(self): ... # fine: overrides `Parent.decorated_3` + +class OtherChild(Parent): ... + +class Grandchild(OtherChild): + @override + def foo(self): ... # fine: overrides `Parent.foo` + @override + @property + def bar(self) -> int: ... # fine: overrides `Parent.bar` + @override + def baz(self): ... # fine: overrides `Parent.baz` + @classmethod + @override + def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + @staticmethod + @override + def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + @override + @classmethod + def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + @override + @staticmethod + def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + @override + def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + @override + @lossy_decorator + def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + @lossy_decorator + @override + def decorated_3(self): ... # fine: overrides `Parent.decorated_3` + +class Invalid: + @override + def ___reprrr__(self): ... # error: [invalid-explicit-override] + @override + @classmethod + def foo(self): ... # error: [invalid-explicit-override] + @classmethod + @override + def bar(self): ... # error: [invalid-explicit-override] + @staticmethod + @override + def baz(): ... # error: [invalid-explicit-override] + @override + @staticmethod + def eggs(): ... # error: [invalid-explicit-override] + @property + @override + def bad_property1(self) -> int: ... # TODO: should emit `invalid-explicit-override` here + @override + @property + def bad_property2(self) -> int: ... # TODO: should emit `invalid-explicit-override` here + @lossy_decorator + @override + def lossy(self): ... # TODO: should emit `invalid-explicit-override` here + @override + @lossy_decorator + def lossy2(self): ... # TODO: should emit `invalid-explicit-override` here + +# TODO: all overrides in this class should cause us to emit *Liskov* violations, +# but not `@override` violations +class LiskovViolatingButNotOverrideViolating(Parent): + @override + @property + def foo(self) -> int: ... + @override + def my_property1(self) -> int: ... + @staticmethod + @override + def class_method1() -> int: ... + @classmethod + @override + def static_method1(cls) -> int: ... + +# Diagnostic edge case: `override` is very far away from the method definition in the source code: + +T = TypeVar("T") + +def identity(x: T) -> T: ... + +class Foo: + @override + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + @identity + def bar(self): ... # error: [invalid-explicit-override] +``` + +## Overloads + +The typing spec states that for an overloaded method, `@override` should only be applied to the +implementation function. However, we nonetheless respect the decorator in this situation, even +though we also emit `invalid-overload` on these methods. + +```py +from typing_extensions import override, overload + +class Spam: + @overload + def foo(self, x: str) -> str: ... + @overload + def foo(self, x: int) -> int: ... + @override + def foo(self, x: str | int) -> str | int: # error: [invalid-explicit-override] + return x + + @overload + @override + def bar(self, x: str) -> str: ... + @overload + @override + def bar(self, x: int) -> int: ... + @override + # error: [invalid-overload] "`@override` decorator should be applied only to the overload implementation" + # error: [invalid-overload] "`@override` decorator should be applied only to the overload implementation" + # error: [invalid-explicit-override] + def bar(self, x: str | int) -> str | int: + return x + + @overload + @override + def baz(self, x: str) -> str: ... + @overload + def baz(self, x: int) -> int: ... + # error: [invalid-overload] "`@override` decorator should be applied only to the overload implementation" + # error: [invalid-explicit-override] + def baz(self, x: str | int) -> str | int: + return x +``` + +In a stub file, `@override` should always be applied to the first overload. Even if it isn't, we +always emit `invalid-explicit-override` diagnostics on the first overload. + +`module.pyi`: + +```pyi +from typing_extensions import override, overload + +class Spam: + @overload + def foo(self, x: str) -> str: ... # error: [invalid-explicit-override] + @overload + @override + # error: [invalid-overload] "`@override` decorator should be applied only to the first overload" + def foo(self, x: int) -> int: ... + + @overload + @override + def bar(self, x: str) -> str: ... # error: [invalid-explicit-override] + @overload + @override + # error: [invalid-overload] "`@override` decorator should be applied only to the first overload" + def bar(self, x: int) -> int: ... + + @overload + @override + def baz(self, x: str) -> str: ... # error: [invalid-explicit-override] + @overload + def baz(self, x: int) -> int: ... +``` + +## Classes inheriting from `Any` + +```py +from typing_extensions import Any, override +from does_not_exist import SomethingUnknown # error: [unresolved-import] + +class Parent1(Any): ... +class Parent2(SomethingUnknown): ... + +class Child1(Parent1): + @override + def bar(self): ... # fine + +class Child2(Parent2): + @override + def bar(self): ... # fine +``` + +## Override of a synthesized method + +```pyi +from typing_extensions import NamedTuple, TypedDict, override, Any, Self +from dataclasses import dataclass + +@dataclass(order=True) +class ParentDataclass: + x: int + +class Child(ParentDataclass): + @override + def __lt__(self, other: ParentDataclass) -> bool: ... # fine + +class MyNamedTuple(NamedTuple): + x: int + + @override + # TODO: this raises an exception at runtime (which we should emit a diagnostic for). + # It shouldn't be an `invalid-explicit-override` diagnostic, however. + def _asdict(self, /) -> dict[str, Any]: ... + +class MyNamedTupleParent(NamedTuple): + x: int + +class MyNamedTupleChild(MyNamedTupleParent): + @override + def _asdict(self, /) -> dict[str, Any]: ... # fine + +class MyTypedDict(TypedDict): + x: int + + @override + # TODO: it's invalid to define a method on a `TypedDict` class, + # so we should emit a diagnostic here. + # It shouldn't be an `invalid-explicit-override` diagnostic, however. + def copy(self) -> Self: ... + +class Grandparent(Any): ... + +class Parent(Grandparent, NamedTuple): # error: [invalid-named-tuple] + x: int + +class Child(Parent): + @override + def foo(self): ... # fine because `Any` is in the MRO +``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap new file mode 100644 index 0000000000..6e0cf0c01e --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap @@ -0,0 +1,293 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: override.md - `typing.override` - Basics +mdtest path: crates/ty_python_semantic/resources/mdtest/override.md +--- + +# Python source files + +## mdtest_snippet.pyi + +``` + 1 | from typing_extensions import override, Callable, TypeVar + 2 | + 3 | def lossy_decorator(fn: Callable) -> Callable: ... + 4 | + 5 | class A: + 6 | @override + 7 | def __repr__(self): ... # fine: overrides `object.__repr__` + 8 | + 9 | class Parent: + 10 | def foo(self): ... + 11 | @property + 12 | def my_property1(self) -> int: ... + 13 | @property + 14 | def my_property2(self) -> int: ... + 15 | baz = None + 16 | @classmethod + 17 | def class_method1(cls) -> int: ... + 18 | @staticmethod + 19 | def static_method1() -> int: ... + 20 | @classmethod + 21 | def class_method2(cls) -> int: ... + 22 | @staticmethod + 23 | def static_method2() -> int: ... + 24 | @lossy_decorator + 25 | def decorated_1(self): ... + 26 | @lossy_decorator + 27 | def decorated_2(self): ... + 28 | @lossy_decorator + 29 | def decorated_3(self): ... + 30 | + 31 | class Child(Parent): + 32 | @override + 33 | def foo(self): ... # fine: overrides `Parent.foo` + 34 | @property + 35 | @override + 36 | def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + 37 | @override + 38 | @property + 39 | def my_property2(self) -> int: ... # fine: overrides `Parent.my_property2` + 40 | @override + 41 | def baz(self): ... # fine: overrides `Parent.baz` + 42 | @classmethod + 43 | @override + 44 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + 45 | @staticmethod + 46 | @override + 47 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + 48 | @override + 49 | @classmethod + 50 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + 51 | @override + 52 | @staticmethod + 53 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + 54 | @override + 55 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + 56 | @override + 57 | @lossy_decorator + 58 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + 59 | @lossy_decorator + 60 | @override + 61 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` + 62 | + 63 | class OtherChild(Parent): ... + 64 | + 65 | class Grandchild(OtherChild): + 66 | @override + 67 | def foo(self): ... # fine: overrides `Parent.foo` + 68 | @override + 69 | @property + 70 | def bar(self) -> int: ... # fine: overrides `Parent.bar` + 71 | @override + 72 | def baz(self): ... # fine: overrides `Parent.baz` + 73 | @classmethod + 74 | @override + 75 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + 76 | @staticmethod + 77 | @override + 78 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + 79 | @override + 80 | @classmethod + 81 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + 82 | @override + 83 | @staticmethod + 84 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + 85 | @override + 86 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + 87 | @override + 88 | @lossy_decorator + 89 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + 90 | @lossy_decorator + 91 | @override + 92 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` + 93 | + 94 | class Invalid: + 95 | @override + 96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] + 97 | @override + 98 | @classmethod + 99 | def foo(self): ... # error: [invalid-explicit-override] +100 | @classmethod +101 | @override +102 | def bar(self): ... # error: [invalid-explicit-override] +103 | @staticmethod +104 | @override +105 | def baz(): ... # error: [invalid-explicit-override] +106 | @override +107 | @staticmethod +108 | def eggs(): ... # error: [invalid-explicit-override] +109 | @property +110 | @override +111 | def bad_property1(self) -> int: ... # TODO: should emit `invalid-explicit-override` here +112 | @override +113 | @property +114 | def bad_property2(self) -> int: ... # TODO: should emit `invalid-explicit-override` here +115 | @lossy_decorator +116 | @override +117 | def lossy(self): ... # TODO: should emit `invalid-explicit-override` here +118 | @override +119 | @lossy_decorator +120 | def lossy2(self): ... # TODO: should emit `invalid-explicit-override` here +121 | +122 | # TODO: all overrides in this class should cause us to emit *Liskov* violations, +123 | # but not `@override` violations +124 | class LiskovViolatingButNotOverrideViolating(Parent): +125 | @override +126 | @property +127 | def foo(self) -> int: ... +128 | @override +129 | def my_property1(self) -> int: ... +130 | @staticmethod +131 | @override +132 | def class_method1() -> int: ... +133 | @classmethod +134 | @override +135 | def static_method1(cls) -> int: ... +136 | +137 | # Diagnostic edge case: `override` is very far away from the method definition in the source code: +138 | +139 | T = TypeVar("T") +140 | +141 | def identity(x: T) -> T: ... +142 | +143 | class Foo: +144 | @override +145 | @identity +146 | @identity +147 | @identity +148 | @identity +149 | @identity +150 | @identity +151 | @identity +152 | @identity +153 | @identity +154 | @identity +155 | @identity +156 | @identity +157 | @identity +158 | @identity +159 | @identity +160 | @identity +161 | @identity +162 | @identity +163 | def bar(self): ... # error: [invalid-explicit-override] +``` + +# Diagnostics + +``` +error[invalid-explicit-override]: Method `___reprrr__` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:95:5 + | +94 | class Invalid: +95 | @override + | --------- +96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] + | ^^^^^^^^^^^ +97 | @override +98 | @classmethod + | +info: No `___reprrr__` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `foo` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:97:5 + | + 95 | @override + 96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] + 97 | @override + | --------- + 98 | @classmethod + 99 | def foo(self): ... # error: [invalid-explicit-override] + | ^^^ +100 | @classmethod +101 | @override + | +info: No `foo` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `bar` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:101:5 + | + 99 | def foo(self): ... # error: [invalid-explicit-override] +100 | @classmethod +101 | @override + | --------- +102 | def bar(self): ... # error: [invalid-explicit-override] + | ^^^ +103 | @staticmethod +104 | @override + | +info: No `bar` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `baz` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:104:5 + | +102 | def bar(self): ... # error: [invalid-explicit-override] +103 | @staticmethod +104 | @override + | --------- +105 | def baz(): ... # error: [invalid-explicit-override] + | ^^^ +106 | @override +107 | @staticmethod + | +info: No `baz` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `eggs` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:106:5 + | +104 | @override +105 | def baz(): ... # error: [invalid-explicit-override] +106 | @override + | --------- +107 | @staticmethod +108 | def eggs(): ... # error: [invalid-explicit-override] + | ^^^^ +109 | @property +110 | @override + | +info: No `eggs` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `bar` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:163:9 + | +161 | @identity +162 | @identity +163 | def bar(self): ... # error: [invalid-explicit-override] + | ^^^ + | + ::: src/mdtest_snippet.pyi:144:5 + | +143 | class Foo: +144 | @override + | --------- +145 | @identity +146 | @identity + | +info: No `bar` definitions were found on any superclasses of `Foo` +info: rule `invalid-explicit-override` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 0ab5928e2c..82db60c7b9 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -14,9 +14,11 @@ use crate::semantic_index::place::{PlaceTable, ScopedPlaceId}; use crate::semantic_index::{global_scope, place_table, use_def_map}; use crate::suppression::FileSuppressionId; use crate::types::call::CallError; -use crate::types::class::{DisjointBase, DisjointBaseKind, Field, MethodDecorator}; +use crate::types::class::{ + CodeGeneratorKind, DisjointBase, DisjointBaseKind, Field, MethodDecorator, +}; use crate::types::function::{FunctionType, KnownFunction}; -use crate::types::liskov::{MethodKind, SynthesizedMethodKind}; +use crate::types::liskov::MethodKind; use crate::types::string_annotation::{ BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION, IMPLICIT_CONCATENATED_STRING_TYPE_ANNOTATION, INVALID_SYNTAX_IN_FORWARD_ANNOTATION, @@ -114,6 +116,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&UNRESOLVED_GLOBAL); registry.register_lint(&MISSING_TYPED_DICT_KEY); registry.register_lint(&INVALID_METHOD_OVERRIDE); + registry.register_lint(&INVALID_EXPLICIT_OVERRIDE); // String annotations registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION); @@ -1545,6 +1548,42 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for methods that are decorated with `@override` but do not override any method in a superclass. + /// + /// ## Why is this bad? + /// Decorating a method with `@override` declares to the type checker that the intention is that it should + /// override a method from a superclass. + /// + /// ## Example + /// + /// ```python + /// from typing import override + /// + /// class A: + /// @override + /// def foo(self): ... # Error raised here + /// + /// class B(A): + /// @override + /// def ffooo(self): ... # Error raised here + /// + /// class C: + /// @override + /// def __repr__(self): ... # fine: overrides `object.__repr__` + /// + /// class D(A): + /// @override + /// def foo(self): ... # fine: overrides `A.foo` + /// ``` + pub(crate) static INVALID_EXPLICIT_OVERRIDE = { + summary: "detects methods that are decorated with `@override` but do not override any method in a superclass", + status: LintStatus::stable("0.0.1-alpha.28"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for `assert_type()` and `assert_never()` calls where the actual type @@ -3592,20 +3631,20 @@ pub(super) fn report_invalid_method_override<'db>( } } } - MethodKind::Synthesized(synthesized_kind) => { + MethodKind::Synthesized(class_kind) => { let make_sub = |message: fmt::Arguments| SubDiagnostic::new(SubDiagnosticSeverity::Info, message); - let mut sub = match synthesized_kind { - SynthesizedMethodKind::Dataclass => make_sub(format_args!( + let mut sub = match class_kind { + CodeGeneratorKind::DataclassLike(_) => make_sub(format_args!( "`{overridden_method}` is a generated method created because \ `{superclass_name}` is a dataclass" )), - SynthesizedMethodKind::NamedTuple => make_sub(format_args!( + CodeGeneratorKind::NamedTuple => make_sub(format_args!( "`{overridden_method}` is a generated method created because \ `{superclass_name}` inherits from `typing.NamedTuple`" )), - SynthesizedMethodKind::TypedDict => make_sub(format_args!( + CodeGeneratorKind::TypedDict => make_sub(format_args!( "`{overridden_method}` is a generated method created because \ `{superclass_name}` is a `TypedDict`" )), diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 80e0d802d9..222d1cfb77 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(super) fn node<'ast>( self, db: &dyn Db, file: File, diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index 2547e391d3..75b9f11a01 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -2,6 +2,7 @@ //! //! [Liskov Substitution Principle]: https://en.wikipedia.org/wiki/Liskov_substitution_principle +use ruff_db::diagnostic::Annotation; use rustc_hash::FxHashSet; use crate::{ @@ -11,7 +12,9 @@ use crate::{ ClassBase, ClassLiteral, ClassType, KnownClass, Type, class::CodeGeneratorKind, context::InferContext, - diagnostic::report_invalid_method_override, + definition_expression_type, + diagnostic::{INVALID_EXPLICIT_OVERRIDE, report_invalid_method_override}, + function::{FunctionDecorators, KnownFunction}, ide_support::{MemberWithDefinition, all_declarations_and_bindings}, }, }; @@ -57,12 +60,12 @@ fn check_class_declaration<'db>( 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!( - CodeGeneratorKind::from_class(db, class.class_literal(db).0, None), - Some(CodeGeneratorKind::DataclassLike(_)) - ) + && matches!(class_kind, Some(CodeGeneratorKind::DataclassLike(_))) { return; } @@ -73,11 +76,28 @@ fn check_class_declaration<'db>( return; }; - for superclass in class.iter_mro(db).skip(1).filter_map(ClassBase::into_class) { - let superclass_symbol_table = - place_table(db, superclass.class_literal(db).0.body_scope(db)); + let mut subclass_overrides_superclass_declaration = false; + let mut has_dynamic_superclass = false; + let mut has_typeddict_in_mro = false; + let mut liskov_diagnostic_emitted = false; - let mut method_kind = MethodKind::NotSynthesized; + for class_base in class.iter_mro(db).skip(1) { + let superclass = match class_base { + ClassBase::Protocol | ClassBase::Generic => continue, + ClassBase::Dynamic(_) => { + has_dynamic_superclass = true; + continue; + } + ClassBase::TypedDict => { + has_typeddict_in_mro = true; + continue; + } + ClassBase::Class(class) => class, + }; + + let (superclass_literal, superclass_specialization) = superclass.class_literal(db); + let superclass_symbol_table = place_table(db, superclass_literal.body_scope(db)); + 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) { @@ -85,39 +105,31 @@ fn check_class_declaration<'db>( continue; } } else { - let (superclass_literal, superclass_specialization) = superclass.class_literal(db); if superclass_literal .own_synthesized_member(db, superclass_specialization, None, &member.name) .is_none() { continue; } - let class_kind = - CodeGeneratorKind::from_class(db, superclass_literal, superclass_specialization); + method_kind = + CodeGeneratorKind::from_class(db, superclass_literal, superclass_specialization) + .map(MethodKind::Synthesized) + .unwrap_or_default(); + } - method_kind = match class_kind { - Some(CodeGeneratorKind::NamedTuple) => { - MethodKind::Synthesized(SynthesizedMethodKind::NamedTuple) - } - Some(CodeGeneratorKind::DataclassLike(_)) => { - MethodKind::Synthesized(SynthesizedMethodKind::Dataclass) - } - // It's invalid to define a method on a `TypedDict` (and this should be - // reported elsewhere), but it's valid to override other things on a - // `TypedDict`, so this case isn't relevant right now but may become - // so when we expand Liskov checking in the future - Some(CodeGeneratorKind::TypedDict) => { - MethodKind::Synthesized(SynthesizedMethodKind::TypedDict) - } - None => MethodKind::NotSynthesized, - }; + 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 else { - // If not defined on any superclass, nothing to check + // If not defined on any superclass, no point in continuing to walk up the MRO break; }; @@ -143,23 +155,77 @@ fn check_class_declaration<'db>( method_kind, ); - // Only one diagnostic should be emitted per each invalid override, - // even if it overrides multiple superclasses incorrectly! - // It's possible `report_invalid_method_override` didn't emit a diagnostic because there's a - // suppression comment, but that too should cause us to exit early here. - break; + liskov_diagnostic_emitted = true; + } + + if !subclass_overrides_superclass_declaration && !has_dynamic_superclass { + if has_typeddict_in_mro { + if !KnownClass::TypedDictFallback + .to_instance(db) + .member(db, &member.name) + .place + .is_undefined() + { + subclass_overrides_superclass_declaration = true; + } + } else if class_kind == Some(CodeGeneratorKind::NamedTuple) { + if !KnownClass::NamedTupleFallback + .to_instance(db) + .member(db, &member.name) + .place + .is_undefined() + { + subclass_overrides_superclass_declaration = true; + } + } + } + + if !subclass_overrides_superclass_declaration + && !has_dynamic_superclass + && definition.kind(db).is_function_def() + && let Type::FunctionLiteral(function) = member.ty + && 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") + } else { + function.literal(db).last_definition(db) + }; + if let Some(builder) = context.report_lint( + &INVALID_EXPLICIT_OVERRIDE, + function_literal.focus_range(db, context.module()), + ) { + let mut diagnostic = builder.into_diagnostic(format_args!( + "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)) + }) + { + diagnostic.annotate(Annotation::secondary(context.span(decorator))); + } + diagnostic.info(format_args!( + "No `{member}` definitions were found on any superclasses of `{class}`", + member = &member.name, + class = class.name(db) + )); + } } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum MethodKind { - Synthesized(SynthesizedMethodKind), +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub(super) enum MethodKind<'db> { + Synthesized(CodeGeneratorKind<'db>), + #[default] NotSynthesized, } - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum SynthesizedMethodKind { - NamedTuple, - Dataclass, - TypedDict, -} 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 58c809b4f4..e2480cae4e 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 @@ -53,6 +53,7 @@ Settings: Settings { "invalid-context-manager": Error (Default), "invalid-declaration": Error (Default), "invalid-exception-caught": Error (Default), + "invalid-explicit-override": Error (Default), "invalid-generic-class": Error (Default), "invalid-ignore-comment": Warning (Default), "invalid-key": Error (Default), diff --git a/ty.schema.json b/ty.schema.json index f97e394da5..fda46afe4b 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -563,6 +563,16 @@ } ] }, + "invalid-explicit-override": { + "title": "detects methods that are decorated with `@override` but do not override any method in a superclass", + "description": "## What it does\nChecks for methods that are decorated with `@override` but do not override any method in a superclass.\n\n## Why is this bad?\nDecorating a method with `@override` declares to the type checker that the intention is that it should\noverride a method from a superclass.\n\n## Example\n\n```python\nfrom typing import override\n\nclass A:\n @override\n def foo(self): ... # Error raised here\n\nclass B(A):\n @override\n def ffooo(self): ... # Error raised here\n\nclass C:\n @override\n def __repr__(self): ... # fine: overrides `object.__repr__`\n\nclass D(A):\n @override\n def foo(self): ... # fine: overrides `A.foo`\n```", + "default": "error", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "invalid-generic-class": { "title": "detects invalid generic classes", "description": "## What it does\nChecks for the creation of invalid generic classes\n\n## Why is this bad?\nThere are several requirements that you must follow when defining a generic class.\n\n## Examples\n```python\nfrom typing import Generic, TypeVar\n\nT = TypeVar(\"T\") # okay\n\n# error: class uses both PEP-695 syntax and legacy syntax\nclass C[U](Generic[T]): ...\n```\n\n## References\n- [Typing spec: Generics](https://typing.python.org/en/latest/spec/generics.html#introduction)",