[ty] Implement typing.final for methods (#21646)
Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
@@ -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<KnownClass> {
|
||||
let (class_literal, _) = self.class_literal(db);
|
||||
class_literal.known(db)
|
||||
|
||||
@@ -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::{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,
|
||||
@@ -101,6 +101,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);
|
||||
@@ -1614,6 +1615,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.
|
||||
@@ -3625,7 +3653,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}")
|
||||
@@ -3768,6 +3796,125 @@ 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>,
|
||||
subclass_type: Type<'db>,
|
||||
superclass: ClassType<'db>,
|
||||
subclass: ClassType<'db>,
|
||||
superclass_method_defs: &[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!(
|
||||
"Overrides a definition 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 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 {
|
||||
first_final_superclass_definition
|
||||
.literal(db)
|
||||
.last_definition(db)
|
||||
};
|
||||
|
||||
sub.annotate(
|
||||
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")),
|
||||
);
|
||||
|
||||
if let Some(decorator_span) =
|
||||
superclass_function_literal.find_known_decorator_span(db, KnownFunction::Final)
|
||||
{
|
||||
sub.annotate(Annotation::secondary(decorator_span));
|
||||
}
|
||||
|
||||
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,
|
||||
/// where `foo` can be resolved to a module but that module does not
|
||||
/// have a `bar` member or submodule.
|
||||
|
||||
@@ -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>(
|
||||
pub(crate) 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<Span> {
|
||||
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<Span> {
|
||||
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(
|
||||
@@ -584,32 +619,44 @@ 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<OverloadLiteral<'db>>) {
|
||||
let self_overload = self.last_definition(db);
|
||||
let mut current = self_overload;
|
||||
let mut overloads = vec![];
|
||||
) -> (&'db [OverloadLiteral<'db>], Option<OverloadLiteral<'db>>) {
|
||||
#[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<OverloadLiteral<'db>>) {
|
||||
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;
|
||||
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();
|
||||
|
||||
let implementation = if self_overload.is_overload(db) {
|
||||
overloads.push(self_overload);
|
||||
None
|
||||
} else {
|
||||
Some(self_overload)
|
||||
};
|
||||
|
||||
(overloads.into_boxed_slice(), implementation)
|
||||
}
|
||||
|
||||
// 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)
|
||||
};
|
||||
|
||||
(overloads.into_boxed_slice(), implementation)
|
||||
let (overloads, implementation) = overloads_and_implementation_inner(db, self);
|
||||
(overloads.as_ref(), *implementation)
|
||||
}
|
||||
|
||||
fn iter_overloads_and_implementation(
|
||||
@@ -617,7 +664,7 @@ impl<'db> FunctionLiteral<'db> {
|
||||
db: &'db dyn Db,
|
||||
) -> impl Iterator<Item = OverloadLiteral<'db>> + '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.
|
||||
@@ -773,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,
|
||||
@@ -892,7 +939,7 @@ impl<'db> FunctionType<'db> {
|
||||
pub(crate) fn overloads_and_implementation(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
) -> &'db (Box<[OverloadLiteral<'db>]>, Option<OverloadLiteral<'db>>) {
|
||||
) -> (&'db [OverloadLiteral<'db>], Option<OverloadLiteral<'db>>) {
|
||||
self.literal(db).overloads_and_implementation(db)
|
||||
}
|
||||
|
||||
@@ -905,6 +952,12 @@ impl<'db> FunctionType<'db> {
|
||||
self.literal(db).iter_overloads_and_implementation(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")
|
||||
}
|
||||
|
||||
/// Typed externally-visible signature for this function.
|
||||
///
|
||||
/// This is the signature as seen by external callers, possibly modified by decorators and/or
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -2,26 +2,32 @@
|
||||
//!
|
||||
//! [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,
|
||||
semantic_index::{place_table, scope::ScopeId, symbol::ScopedSymbolId, use_def_map},
|
||||
types::{
|
||||
ClassBase, ClassLiteral, ClassType, KnownClass, Type,
|
||||
class::CodeGeneratorKind,
|
||||
context::InferContext,
|
||||
definition_expression_type,
|
||||
diagnostic::{INVALID_EXPLICIT_OVERRIDE, report_invalid_method_override},
|
||||
function::{FunctionDecorators, KnownFunction},
|
||||
diagnostic::{
|
||||
INVALID_EXPLICIT_OVERRIDE, INVALID_METHOD_OVERRIDE, OVERRIDE_OF_FINAL_METHOD,
|
||||
report_invalid_method_override, report_overridden_final_method,
|
||||
},
|
||||
function::{FunctionDecorators, FunctionType, KnownFunction},
|
||||
ide_support::{MemberWithDefinition, all_declarations_and_bindings},
|
||||
},
|
||||
};
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -30,56 +36,100 @@ 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>,
|
||||
) {
|
||||
/// 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:
|
||||
///
|
||||
/// ```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 {
|
||||
use_def_map(db, scope)
|
||||
.end_of_scope_symbol_bindings(symbol)
|
||||
.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<smallvec::SmallVec<[FunctionType<'db>; 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();
|
||||
|
||||
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;
|
||||
};
|
||||
|
||||
// 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 {
|
||||
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;
|
||||
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 +146,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 +173,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,14 +181,74 @@ 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 {
|
||||
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_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_functions))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// **********************************************************
|
||||
// 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 {
|
||||
continue;
|
||||
}
|
||||
|
||||
if !configuration.check_method_liskov_violations() {
|
||||
continue;
|
||||
}
|
||||
|
||||
// TODO: Check Liskov on non-methods too
|
||||
let Type::FunctionLiteral(subclass_function) = member.ty else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if type_on_subclass_instance.is_assignable_to(db, superclass_type_as_callable) {
|
||||
// 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) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if type_on_subclass_instance.is_assignable_to(db, superclass_type_as_callable.into_type(db))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -149,7 +257,7 @@ fn check_class_declaration<'db>(
|
||||
&member.name,
|
||||
class,
|
||||
*definition,
|
||||
function,
|
||||
subclass_function,
|
||||
superclass,
|
||||
superclass_type,
|
||||
method_kind,
|
||||
@@ -187,13 +295,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_or_implementation(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 +308,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 +320,18 @@ fn check_class_declaration<'db>(
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
if let Some((superclass, superclass_method)) = overridden_final_method {
|
||||
report_overridden_final_method(
|
||||
context,
|
||||
&member.name,
|
||||
*definition,
|
||||
type_on_subclass_instance,
|
||||
superclass,
|
||||
class,
|
||||
&superclass_method,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
@@ -229,3 +340,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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user