diff --git a/crates/ruff_benchmark/benches/ty_walltime.rs b/crates/ruff_benchmark/benches/ty_walltime.rs index 9abd6c7c83bc8..6c8e89de0c7a6 100644 --- a/crates/ruff_benchmark/benches/ty_walltime.rs +++ b/crates/ruff_benchmark/benches/ty_walltime.rs @@ -223,7 +223,7 @@ static STATIC_FRAME: Benchmark = Benchmark::new( max_dep_date: "2025-08-09", python_version: PythonVersion::PY311, }, - 900, + 950, ); #[track_caller] diff --git a/crates/ty_python_semantic/resources/mdtest/liskov.md b/crates/ty_python_semantic/resources/mdtest/liskov.md index 24a1c3e2c21eb..a20cb1dd8c835 100644 --- a/crates/ty_python_semantic/resources/mdtest/liskov.md +++ b/crates/ty_python_semantic/resources/mdtest/liskov.md @@ -583,3 +583,54 @@ class GoodChild2(Parent): @staticmethod def static_method(x: object) -> bool: ... ``` + +## Method incompatibly overridden by a non-method + + + +```pyi +from typing import ClassVar, Final, Callable, Any + +class Parent: + def method(self): ... + +class Child1(Parent): + method = None # error: [invalid-method-override] + +class Child2(Parent): + method: ClassVar = None # error: [invalid-method-override] + +class Child3(Parent): + method: None = None # error: [invalid-method-override] + +class Child4(Parent): + # error: [invalid-assignment] "Object of type `None` is not assignable to `(...) -> Unknown`" + # error: [invalid-method-override] + method: Callable = None + +class Child5(Parent): + method: Callable | None = None # error: [invalid-method-override] + +class Child6(Parent): + method: Final = None # error: [invalid-method-override] + +class Child7(Parent): + method: None # error: [invalid-method-override] + +class Child8(Parent): + # A `Callable` type is insufficient for the type to be assignable to + # the superclass definition: the definition on the superclass is a + # *function-like* callable, which can be used as a method descriptor, + # and which has `types.FunctionType` attributes such as `__name__`, + # `__kwdefaults__`, etc. + method: Callable # error: [invalid-method-override] + +class Child9(Parent): + method: Any + +class Child10(Parent): + # fine: although the local-narrowed type is `None`, + # the type as accessed on the class object or on instances of the class + # will be `Any`, which is assignable to the declared type on the superclass + method: Any = None +``` diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index 1e1ac07aabdc0..9aa8ef5b07707 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -2668,7 +2668,7 @@ violates the Liskov principle (this also matches the behaviour of other type che from typing import Iterable class Foo(Iterable[int]): - __iter__ = None + __iter__ = None # error: [invalid-method-override] static_assert(is_subtype_of(Foo, Iterable[int])) diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/liskov.md_-_The_Liskov_Substitut\342\200\246_-_Method_incompatibly_\342\200\246_(d78f218877d1ccf3).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/liskov.md_-_The_Liskov_Substitut\342\200\246_-_Method_incompatibly_\342\200\246_(d78f218877d1ccf3).snap" new file mode 100644 index 0000000000000..45755094f4070 --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/liskov.md_-_The_Liskov_Substitut\342\200\246_-_Method_incompatibly_\342\200\246_(d78f218877d1ccf3).snap" @@ -0,0 +1,270 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: liskov.md - The Liskov Substitution Principle - Method incompatibly overridden by a non-method +mdtest path: crates/ty_python_semantic/resources/mdtest/liskov.md +--- + +# Python source files + +## mdtest_snippet.pyi + +``` + 1 | from typing import ClassVar, Final, Callable, Any + 2 | + 3 | class Parent: + 4 | def method(self): ... + 5 | + 6 | class Child1(Parent): + 7 | method = None # error: [invalid-method-override] + 8 | + 9 | class Child2(Parent): +10 | method: ClassVar = None # error: [invalid-method-override] +11 | +12 | class Child3(Parent): +13 | method: None = None # error: [invalid-method-override] +14 | +15 | class Child4(Parent): +16 | # error: [invalid-assignment] "Object of type `None` is not assignable to `(...) -> Unknown`" +17 | # error: [invalid-method-override] +18 | method: Callable = None +19 | +20 | class Child5(Parent): +21 | method: Callable | None = None # error: [invalid-method-override] +22 | +23 | class Child6(Parent): +24 | method: Final = None # error: [invalid-method-override] +25 | +26 | class Child7(Parent): +27 | method: None # error: [invalid-method-override] +28 | +29 | class Child8(Parent): +30 | # A `Callable` type is insufficient for the type to be assignable to +31 | # the superclass definition: the definition on the superclass is a +32 | # *function-like* callable, which can be used as a method descriptor, +33 | # and which has `types.FunctionType` attributes such as `__name__`, +34 | # `__kwdefaults__`, etc. +35 | method: Callable # error: [invalid-method-override] +36 | +37 | class Child9(Parent): +38 | method: Any +39 | +40 | class Child10(Parent): +41 | # fine: although the local-narrowed type is `None`, +42 | # the type as accessed on the class object or on instances of the class +43 | # will be `Any`, which is assignable to the declared type on the superclass +44 | method: Any = None +``` + +# Diagnostics + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:4:9 + | +3 | class Parent: +4 | def method(self): ... + | ------------ `Parent.method` defined here +5 | +6 | class Child1(Parent): +7 | method = None # error: [invalid-method-override] + | ^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +8 | +9 | class Child2(Parent): + | +info: `Child1.method` has type `None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:10:5 + | + 9 | class Child2(Parent): +10 | method: ClassVar = None # error: [invalid-method-override] + | ^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +11 | +12 | class Child3(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child2.method` has type `Unknown | None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:13:5 + | +12 | class Child3(Parent): +13 | method: None = None # error: [invalid-method-override] + | ^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +14 | +15 | class Child4(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child3.method` has type `None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:18:5 + | +16 | # error: [invalid-assignment] "Object of type `None` is not assignable to `(...) -> Unknown`" +17 | # error: [invalid-method-override] +18 | method: Callable = None + | ^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +19 | +20 | class Child5(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child4.method` has type `(...) -> Unknown` +info: `(...) -> Unknown` is a callable type with the correct signature, but objects with this type cannot necessarily be used as method descriptors +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-assignment]: Object of type `None` is not assignable to `(...) -> Unknown` + --> src/mdtest_snippet.pyi:18:13 + | +16 | # error: [invalid-assignment] "Object of type `None` is not assignable to `(...) -> Unknown`" +17 | # error: [invalid-method-override] +18 | method: Callable = None + | -------- ^^^^ Incompatible value of type `None` + | | + | Declared type +19 | +20 | class Child5(Parent): + | +info: rule `invalid-assignment` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:21:5 + | +20 | class Child5(Parent): +21 | method: Callable | None = None # error: [invalid-method-override] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +22 | +23 | class Child6(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child5.method` has type `((...) -> Unknown) | None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:24:5 + | +23 | class Child6(Parent): +24 | method: Final = None # error: [invalid-method-override] + | ^^^^^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +25 | +26 | class Child7(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child6.method` has type `None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:27:5 + | +26 | class Child7(Parent): +27 | method: None # error: [invalid-method-override] + | ^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +28 | +29 | class Child8(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child7.method` has type `None`, which is not callable +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` + +``` +error[invalid-method-override]: Invalid override of method `method` + --> src/mdtest_snippet.pyi:35:5 + | +33 | # and which has `types.FunctionType` attributes such as `__name__`, +34 | # `__kwdefaults__`, etc. +35 | method: Callable # error: [invalid-method-override] + | ^^^^^^^^^^^^^^^^ Definition is incompatible with `Parent.method` +36 | +37 | class Child9(Parent): + | + ::: src/mdtest_snippet.pyi:4:9 + | + 3 | class Parent: + 4 | def method(self): ... + | ------------ `Parent.method` defined here + 5 | + 6 | class Child1(Parent): + | +info: `Child8.method` has type `(...) -> Unknown` +info: `(...) -> Unknown` is a callable type with the correct signature, but objects with this type cannot necessarily be used as method descriptors +info: This violates the Liskov Substitution Principle +info: rule `invalid-method-override` is enabled by default + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/type_compendium/tuple.md b/crates/ty_python_semantic/resources/mdtest/type_compendium/tuple.md index e323d25a17bea..bf3e81923a451 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_compendium/tuple.md +++ b/crates/ty_python_semantic/resources/mdtest/type_compendium/tuple.md @@ -37,7 +37,7 @@ reveal_type(().__class__()) # revealed: tuple[()] reveal_type((1, 2).__class__((1, 2))) # revealed: tuple[Literal[1], Literal[2]] class LiskovUncompliantIterable(Iterable[int]): - # TODO we should emit an error here about the Liskov violation + # error: [invalid-method-override] __iter__ = None def f(x: Iterable[int], y: list[str], z: Never, aa: list[Never], bb: LiskovUncompliantIterable): diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 03acaa53357f9..36ba3c7ca2a43 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs @@ -737,6 +737,13 @@ impl DefinitionKind<'_> { matches!(self, DefinitionKind::Assignment(_)) } + pub(crate) const fn as_function_def(&self) -> Option<&AstNodeRef> { + match self { + DefinitionKind::Function(function) => Some(function), + _ => None, + } + } + pub(crate) const fn is_function_def(&self) -> bool { matches!(self, DefinitionKind::Function(_)) } diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 050a55fb93844..59a5a97a59a77 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -27,7 +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::types::{CallableType, KnownInstanceType, MemberLookupPolicy}; use crate::{Db, DisplaySettings, FxIndexMap, Module, ModuleName, Program, declare_lint}; use itertools::Itertools; use ruff_db::{ @@ -3464,14 +3464,14 @@ pub(super) fn report_invalid_method_override<'db>( member: &str, subclass: ClassType<'db>, subclass_definition: Definition<'db>, - subclass_function: FunctionType<'db>, + subclass_type: Type<'db>, superclass: ClassType<'db>, superclass_type: Type<'db>, superclass_method_kind: MethodKind, ) { let db = context.db(); - let signature_span = |function: FunctionType<'db>| { + let signature_span_inner = |function: FunctionType<'db>| { function .literal(db) .last_definition(db) @@ -3479,23 +3479,31 @@ pub(super) fn report_invalid_method_override<'db>( .map(|spans| spans.signature) }; + let signature_span = |ty| match ty { + Type::FunctionLiteral(function) => signature_span_inner(function), + Type::BoundMethod(method) => signature_span_inner(method.function(db)), + _ => None, + }; + let subclass_definition_kind = subclass_definition.kind(db); - let subclass_definition_signature_span = signature_span(subclass_function); + let subclass_definition_signature_span = signature_span(subclass_type); // If the function was originally defined elsewhere and simply assigned // in the body of the class here, we cannot use the range associated with the `FunctionType` - let diagnostic_range = if subclass_definition_kind.is_function_def() { - subclass_definition_signature_span - .as_ref() - .and_then(Span::range) - .unwrap_or_else(|| { - subclass_function - .node(db, context.file(), context.module()) - .range - }) - } else { - subclass_definition.full_range(db, context.module()).range() - }; + let diagnostic_range = subclass_definition_kind + .as_function_def() + .map(|node| { + let function = node.node(context.module()); + TextRange::new( + function.name.start(), + function + .returns + .as_deref() + .map(Ranged::end) + .unwrap_or(function.parameters.end()), + ) + }) + .unwrap_or_else(|| subclass_definition.full_range(db, context.module()).range()); let class_name = subclass.name(db); let superclass_name = superclass.name(db); @@ -3539,6 +3547,33 @@ pub(super) fn report_invalid_method_override<'db>( superclass_function_kind = superclass_function_kind.description(), subclass_function_kind = subclass_function_kind.description(), )); + } else if !matches!( + subclass_type, + Type::FunctionLiteral(_) | Type::BoundMethod(_) + ) { + if let Some(callables) = subclass_type.try_upcast_to_callable(db) { + diagnostic.info(format_args!( + "`{class_name}.{member}` has type `{}`", + subclass_type.display(db) + )); + if let Some(callable) = callables.exactly_one() + && !callable.is_function_like(db) + && let Some(superclass_callable) = superclass_type.try_upcast_to_callable(db) + && Type::Callable(CallableType::new(db, callable.signatures(db), true)) + .is_assignable_to(db, superclass_callable.into_type(db)) + { + diagnostic.info(format_args!( + "`{}` is a callable type with the correct signature, \ + but objects with this type cannot necessarily be used as method descriptors", + subclass_type.display(db) + )); + } + } else { + diagnostic.info(format_args!( + "`{class_name}.{member}` has type `{}`, which is not callable", + subclass_type.display(db) + )); + } } diagnostic.info("This violates the Liskov Substitution Principle"); @@ -3568,8 +3603,8 @@ pub(super) fn report_invalid_method_override<'db>( ); let superclass_function_span = match superclass_type { - Type::FunctionLiteral(function) => signature_span(function), - Type::BoundMethod(method) => signature_span(method.function(db)), + Type::FunctionLiteral(function) => signature_span_inner(function), + Type::BoundMethod(method) => signature_span_inner(method.function(db)), _ => None, }; diff --git a/crates/ty_python_semantic/src/types/liskov.rs b/crates/ty_python_semantic/src/types/liskov.rs index 2547e391d3ed1..4904d9ff9998a 100644 --- a/crates/ty_python_semantic/src/types/liskov.rs +++ b/crates/ty_python_semantic/src/types/liskov.rs @@ -8,7 +8,7 @@ use crate::{ place::Place, semantic_index::place_table, types::{ - ClassBase, ClassLiteral, ClassType, KnownClass, Type, + ClassBase, ClassLiteral, ClassType, Type, class::CodeGeneratorKind, context::InferContext, diagnostic::report_invalid_method_override, @@ -18,10 +18,6 @@ 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) { - return; - } - let class_specialized = class.identity_specialization(db); let own_class_members: FxHashSet<_> = all_declarations_and_bindings(db, class.body_scope(db)).collect(); @@ -40,11 +36,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; }; @@ -121,14 +112,17 @@ fn check_class_declaration<'db>( break; }; - let Some(superclass_type_as_callable) = superclass_type - .try_upcast_to_callable(db) - .map(|callables| callables.into_type(db)) - else { - continue; + let superclass_type_as_callable = match superclass_type { + Type::FunctionLiteral(function) => function.into_callable_type(db), + Type::BoundMethod(method) => method.into_callable_type(db), + Type::Callable(callable) => callable, + // TODO: check subclass overrides even if the definition on the superclass is not a method + _ => continue, }; - if type_on_subclass_instance.is_assignable_to(db, superclass_type_as_callable) { + if type_on_subclass_instance + .is_assignable_to(db, Type::Callable(superclass_type_as_callable)) + { continue; } @@ -137,7 +131,7 @@ fn check_class_declaration<'db>( &member.name, class, *definition, - function, + type_on_subclass_instance, superclass, superclass_type, method_kind,