[ty] Extend Liskov checks to also cover classmethods and staticmethods (#21598)

## Summary

Building on https://github.com/astral-sh/ruff/pull/21436.

There's nothing conceptually more complicated about this, it just
requires its own set of tests and its own subdiagnostic hint.

I also uncovered another inconsistency between mypy/pyright/pyrefly,
which is fun. In this case, I suggest we go with pyright's behaviour.

## Test Plan

mdtests/snapshots
This commit is contained in:
Alex Waygood
2025-11-24 23:14:06 +00:00
committed by GitHub
parent bfd65c4215
commit adf095e889
7 changed files with 386 additions and 86 deletions

View File

@@ -523,3 +523,63 @@ class Baz(NamedTuple):
class Spam(Baz):
def _asdict(self) -> tuple[int, ...]: ... # error: [invalid-method-override]
```
## Staticmethods and classmethods
Methods decorated with `@staticmethod` or `@classmethod` are checked in much the same way as other
methods.
<!-- snapshot-diagnostics -->
```pyi
class Parent:
def instance_method(self, x: int) -> int: ...
@classmethod
def class_method(cls, x: int) -> int: ...
@staticmethod
def static_method(x: int) -> int: ...
class BadChild1(Parent):
@staticmethod
def instance_method(self, x: int) -> int: ... # error: [invalid-method-override]
# TODO: we should emit `invalid-method-override` here.
# Although the method has the same signature as `Parent.class_method`
# when accessed on instances, it does not have the same signature as
# `Parent.class_method` when accessed on the class object itself
def class_method(cls, x: int) -> int: ...
def static_method(x: int) -> int: ... # error: [invalid-method-override]
class BadChild2(Parent):
# TODO: we should emit `invalid-method-override` here.
# Although the method has the same signature as `Parent.class_method`
# when accessed on instances, it does not have the same signature as
# `Parent.class_method` when accessed on the class object itself.
#
# Note that whereas `BadChild1.class_method` is reported as a Liskov violation by
# mypy, pyright and pyrefly, pyright is the only one of those three to report a
# Liskov violation on this method as of 2025-11-23.
@classmethod
def instance_method(self, x: int) -> int: ...
@staticmethod
def class_method(cls, x: int) -> int: ... # error: [invalid-method-override]
@classmethod
def static_method(x: int) -> int: ... # error: [invalid-method-override]
class BadChild3(Parent):
@classmethod
def class_method(cls, x: bool) -> object: ... # error: [invalid-method-override]
@staticmethod
def static_method(x: bool) -> object: ... # error: [invalid-method-override]
class GoodChild1(Parent):
@classmethod
def class_method(cls, x: int) -> int: ...
@staticmethod
def static_method(x: int) -> int: ...
class GoodChild2(Parent):
@classmethod
def class_method(cls, x: object) -> bool: ...
@staticmethod
def static_method(x: object) -> bool: ...
```

View File

@@ -0,0 +1,220 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: liskov.md - The Liskov Substitution Principle - Staticmethods and classmethods
mdtest path: crates/ty_python_semantic/resources/mdtest/liskov.md
---
# Python source files
## mdtest_snippet.pyi
```
1 | class Parent:
2 | def instance_method(self, x: int) -> int: ...
3 | @classmethod
4 | def class_method(cls, x: int) -> int: ...
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
7 |
8 | class BadChild1(Parent):
9 | @staticmethod
10 | def instance_method(self, x: int) -> int: ... # error: [invalid-method-override]
11 | # TODO: we should emit `invalid-method-override` here.
12 | # Although the method has the same signature as `Parent.class_method`
13 | # when accessed on instances, it does not have the same signature as
14 | # `Parent.class_method` when accessed on the class object itself
15 | def class_method(cls, x: int) -> int: ...
16 | def static_method(x: int) -> int: ... # error: [invalid-method-override]
17 |
18 | class BadChild2(Parent):
19 | # TODO: we should emit `invalid-method-override` here.
20 | # Although the method has the same signature as `Parent.class_method`
21 | # when accessed on instances, it does not have the same signature as
22 | # `Parent.class_method` when accessed on the class object itself.
23 | #
24 | # Note that whereas `BadChild1.class_method` is reported as a Liskov violation by
25 | # mypy, pyright and pyrefly, pyright is the only one of those three to report a
26 | # Liskov violation on this method as of 2025-11-23.
27 | @classmethod
28 | def instance_method(self, x: int) -> int: ...
29 | @staticmethod
30 | def class_method(cls, x: int) -> int: ... # error: [invalid-method-override]
31 | @classmethod
32 | def static_method(x: int) -> int: ... # error: [invalid-method-override]
33 |
34 | class BadChild3(Parent):
35 | @classmethod
36 | def class_method(cls, x: bool) -> object: ... # error: [invalid-method-override]
37 | @staticmethod
38 | def static_method(x: bool) -> object: ... # error: [invalid-method-override]
39 |
40 | class GoodChild1(Parent):
41 | @classmethod
42 | def class_method(cls, x: int) -> int: ...
43 | @staticmethod
44 | def static_method(x: int) -> int: ...
45 |
46 | class GoodChild2(Parent):
47 | @classmethod
48 | def class_method(cls, x: object) -> bool: ...
49 | @staticmethod
50 | def static_method(x: object) -> bool: ...
```
# Diagnostics
```
error[invalid-method-override]: Invalid override of method `instance_method`
--> src/mdtest_snippet.pyi:10:9
|
8 | class BadChild1(Parent):
9 | @staticmethod
10 | def instance_method(self, x: int) -> int: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.instance_method`
11 | # TODO: we should emit `invalid-method-override` here.
12 | # Although the method has the same signature as `Parent.class_method`
|
::: src/mdtest_snippet.pyi:2:9
|
1 | class Parent:
2 | def instance_method(self, x: int) -> int: ...
| ------------------------------------ `Parent.instance_method` defined here
3 | @classmethod
4 | def class_method(cls, x: int) -> int: ...
|
info: `BadChild1.instance_method` is a staticmethod but `Parent.instance_method` is an instance method
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```
```
error[invalid-method-override]: Invalid override of method `static_method`
--> src/mdtest_snippet.pyi:16:9
|
14 | # `Parent.class_method` when accessed on the class object itself
15 | def class_method(cls, x: int) -> int: ...
16 | def static_method(x: int) -> int: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.static_method`
17 |
18 | class BadChild2(Parent):
|
::: src/mdtest_snippet.pyi:6:9
|
4 | def class_method(cls, x: int) -> int: ...
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
| ---------------------------- `Parent.static_method` defined here
7 |
8 | class BadChild1(Parent):
|
info: `BadChild1.static_method` is an instance method but `Parent.static_method` is a staticmethod
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```
```
error[invalid-method-override]: Invalid override of method `class_method`
--> src/mdtest_snippet.pyi:30:9
|
28 | def instance_method(self, x: int) -> int: ...
29 | @staticmethod
30 | def class_method(cls, x: int) -> int: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.class_method`
31 | @classmethod
32 | def static_method(x: int) -> int: ... # error: [invalid-method-override]
|
::: src/mdtest_snippet.pyi:4:9
|
2 | def instance_method(self, x: int) -> int: ...
3 | @classmethod
4 | def class_method(cls, x: int) -> int: ...
| -------------------------------- `Parent.class_method` defined here
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
|
info: `BadChild2.class_method` is a staticmethod but `Parent.class_method` is a classmethod
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```
```
error[invalid-method-override]: Invalid override of method `static_method`
--> src/mdtest_snippet.pyi:32:9
|
30 | def class_method(cls, x: int) -> int: ... # error: [invalid-method-override]
31 | @classmethod
32 | def static_method(x: int) -> int: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.static_method`
33 |
34 | class BadChild3(Parent):
|
::: src/mdtest_snippet.pyi:6:9
|
4 | def class_method(cls, x: int) -> int: ...
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
| ---------------------------- `Parent.static_method` defined here
7 |
8 | class BadChild1(Parent):
|
info: `BadChild2.static_method` is a classmethod but `Parent.static_method` is a staticmethod
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```
```
error[invalid-method-override]: Invalid override of method `class_method`
--> src/mdtest_snippet.pyi:36:9
|
34 | class BadChild3(Parent):
35 | @classmethod
36 | def class_method(cls, x: bool) -> object: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.class_method`
37 | @staticmethod
38 | def static_method(x: bool) -> object: ... # error: [invalid-method-override]
|
::: src/mdtest_snippet.pyi:4:9
|
2 | def instance_method(self, x: int) -> int: ...
3 | @classmethod
4 | def class_method(cls, x: int) -> int: ...
| -------------------------------- `Parent.class_method` defined here
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
|
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```
```
error[invalid-method-override]: Invalid override of method `static_method`
--> src/mdtest_snippet.pyi:38:9
|
36 | def class_method(cls, x: bool) -> object: ... # error: [invalid-method-override]
37 | @staticmethod
38 | def static_method(x: bool) -> object: ... # error: [invalid-method-override]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.static_method`
39 |
40 | class GoodChild1(Parent):
|
::: src/mdtest_snippet.pyi:6:9
|
4 | def class_method(cls, x: int) -> int: ...
5 | @staticmethod
6 | def static_method(x: int) -> int: ...
| ---------------------------- `Parent.static_method` defined here
7 |
8 | class BadChild1(Parent):
|
info: This violates the Liskov Substitution Principle
info: rule `invalid-method-override` is enabled by default
```

View File

@@ -1107,13 +1107,6 @@ impl<'db> Type<'db> {
}
}
pub(crate) const fn as_bound_method(self) -> Option<BoundMethodType<'db>> {
match self {
Type::BoundMethod(bound_method) => Some(bound_method),
_ => None,
}
}
#[track_caller]
pub(crate) const fn expect_class_literal(self) -> ClassLiteral<'db> {
self.as_class_literal()

View File

@@ -1264,6 +1264,14 @@ impl MethodDecorator {
(false, false) => Ok(Self::None),
}
}
pub(crate) const fn description(self) -> &'static str {
match self {
MethodDecorator::None => "an instance method",
MethodDecorator::ClassMethod => "a classmethod",
MethodDecorator::StaticMethod => "a staticmethod",
}
}
}
/// Kind-specific metadata for different types of fields

View File

@@ -8,13 +8,13 @@ use super::{
use crate::diagnostic::did_you_mean;
use crate::diagnostic::format_enumeration;
use crate::lint::{Level, LintRegistryBuilder, LintStatus};
use crate::place::Place;
use crate::semantic_index::definition::{Definition, DefinitionKind};
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::KnownInstanceType;
use crate::types::call::CallError;
use crate::types::class::{DisjointBase, DisjointBaseKind, Field};
use crate::types::class::{DisjointBase, DisjointBaseKind, Field, MethodDecorator};
use crate::types::function::{FunctionType, KnownFunction};
use crate::types::liskov::{MethodKind, SynthesizedMethodKind};
use crate::types::string_annotation::{
@@ -27,6 +27,7 @@ use crate::types::{
ProtocolInstanceType, SpecialFormType, SubclassOfInner, Type, TypeContext, binding_type,
infer_isolated_expression, protocol_class::ProtocolClass,
};
use crate::types::{KnownInstanceType, MemberLookupPolicy};
use crate::{Db, DisplaySettings, FxIndexMap, Module, ModuleName, Program, declare_lint};
use itertools::Itertools;
use ruff_db::{
@@ -3519,6 +3520,27 @@ pub(super) fn report_invalid_method_override<'db>(
"Definition is incompatible with `{overridden_method}`"
));
let class_member = |cls: ClassType<'db>| {
cls.class_member(db, member, MemberLookupPolicy::default())
.place
};
if let Place::Defined(Type::FunctionLiteral(subclass_function), _, _) = class_member(subclass)
&& let Place::Defined(Type::FunctionLiteral(superclass_function), _, _) =
class_member(superclass)
&& let Ok(superclass_function_kind) =
MethodDecorator::try_from_fn_type(db, superclass_function)
&& let Ok(subclass_function_kind) = MethodDecorator::try_from_fn_type(db, subclass_function)
&& superclass_function_kind != subclass_function_kind
{
diagnostic.info(format_args!(
"`{class_name}.{member}` is {subclass_function_kind} \
but `{overridden_method}` is {superclass_function_kind}",
superclass_function_kind = superclass_function_kind.description(),
subclass_function_kind = subclass_function_kind.description(),
));
}
diagnostic.info("This violates the Liskov Substitution Principle");
if !subclass_definition_kind.is_function_def()
@@ -3545,9 +3567,11 @@ pub(super) fn report_invalid_method_override<'db>(
.full_range(db, &parsed_module(db, superclass_scope.file(db)).load(db)),
);
let superclass_function_span = superclass_type
.as_bound_method()
.and_then(|method| signature_span(method.function(db)));
let superclass_function_span = match superclass_type {
Type::FunctionLiteral(function) => signature_span(function),
Type::BoundMethod(method) => signature_span(method.function(db)),
_ => None,
};
let superclass_definition_kind = definition.kind(db);

View File

@@ -49,11 +49,6 @@ fn check_class_declaration<'db>(
return;
};
// TODO: classmethods and staticmethods
if function.is_classmethod(db) || function.is_staticmethod(db) {
return;
}
// Constructor methods are not checked for Liskov compliance
if matches!(
&*member.name,