From 0171985c2796e8f3cbc45498f4b2713c8c88dcb6 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Tue, 24 Dec 2024 00:26:31 +0000 Subject: [PATCH 1/6] [red-knot] Report classes inheriting from bases with incompatible `__slots__` --- .../resources/mdtest/slots.md | 174 ++++++++++++++++++ .../src/types/diagnostic.rs | 66 +++++++ .../src/types/infer.rs | 110 +++++++---- .../red_knot_python_semantic/src/types/mro.rs | 43 +++++ 4 files changed, 356 insertions(+), 37 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/slots.md diff --git a/crates/red_knot_python_semantic/resources/mdtest/slots.md b/crates/red_knot_python_semantic/resources/mdtest/slots.md new file mode 100644 index 0000000000000..9f426e816df8d --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/slots.md @@ -0,0 +1,174 @@ +# `__slots__` + +## Not specified and empty + +```py +class A: ... + +class B: + __slots__ = () + +class C: + __slots__ = ("lorem", "ipsum") + +class AB(A, B): ... # fine +class AC(A, C): ... # fine +class BC(B, C): ... # fine + +class ABC(A, B, C): ... # fine +``` + +## Incompatible tuples + +```py +class A: + __slots__ = ("a", "b") + +class B: + __slots__ = ("c", "d") + +class C( + A, # error: [incompatible-slots] + B, # error: [incompatible-slots] +): ... +``` + +## Same value + +```py +class A: + __slots__ = ("a", "b") + +class B: + __slots__ = ("a", "b") + +class C( + A, # error: [incompatible-slots] + B, # error: [incompatible-slots] +): ... +``` + +## Strings + +```py +class A: + __slots__ = "abc" + +class B: + __slots__ = ("abc") + +class AB( + A, # error: [incompatible-slots] + B, # error: [incompatible-slots] +): ... +``` + +## Invalid + +TODO: Emit diagnostics + +```py +class NonString1: + __slots__ = 42 + +class NonString2: + __slots__ = b"ar" + +class NonIdentifier1: + __slots__ = "42" + +class NonIdentifier2: + __slots__ = ("lorem", "42") + +class NonIdentifier3: + __slots__ = (e for e in ("lorem", "42")) +``` + +## Inheritance + +```py +class A: + __slots__ = ("a", "b") + +class B(A): ... + +class C: + __slots__ = ("c", "d") + +class D(C): ... + +class E( + B, # error: [incompatible-slots] + D, # error: [incompatible-slots] +): ... +``` + +## False negatives + +### Possibly unbound + +```py +def _(flag: bool): + class A: + if flag: + __slots__ = ("a", "b") + + class B: + __slots__ = ("c", "d") + + # Might or might not be fine at runtime + class C(A, B): ... +``` + +### Bound but with different types + +```py +def _(flag: bool): + class A: + if flag: + __slots__ = ("a", "b") + else: + __slots__ = () + + class B: + __slots__ = ("c", "d") + + # Might or might not be fine at runtime + class C(A, B): ... +``` + +### Non-tuples + +```py +class A: + __slots__ = ["a", "b"] # This is treated as "dynamic" + +class B: + __slots__ = ("c", "d") + +# False negative: [incompatible-slots] +class C(A, B): ... +``` + +### Post-hoc modifications + +```py +class A: + __slots__ = () + __slots__ += ("a", "b") + +reveal_type(A.__slots__) # revealed: @Todo(Support for more binary expressions) + +class B: + __slots__ = ("c", "d") + +# False negative: [incompatible-slots] +class C(A, B): ... +``` + +### Built-ins with implicit layouts + +```py +# False negative: [incompatible-slots] +class A(int, str): ... +``` diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 4d4fb738e4ce7..56b19fe7d6499 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -27,6 +27,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&CYCLIC_CLASS_DEFINITION); registry.register_lint(&DIVISION_BY_ZERO); registry.register_lint(&DUPLICATE_BASE); + registry.register_lint(&INCOMPATIBLE_SLOTS); registry.register_lint(&INCONSISTENT_MRO); registry.register_lint(&INDEX_OUT_OF_BOUNDS); registry.register_lint(&INVALID_ASSIGNMENT); @@ -148,6 +149,63 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for classes whose bases define incompatible `__slots__`. + /// + /// ## Why is this bad? + /// Inheriting from bases with incompatible `__slots__`s + /// will lead to a `TypeError` at runtime. + /// + /// Classes with no or empty `__slots__` is always compatible: + /// + /// ```python + /// class A: ... + /// class B: + /// __slots__ = () + /// class C: + /// __slots__ = ("a", "b") + /// + /// # fine + /// class D(A, B, C): ... + /// ``` + /// + /// Class with non-empty `__slots__` cannot participate in multiple inheritance: + /// + /// ```python + /// class A: + /// __slots__ = ("a", "b") + /// + /// class B: + /// __slots__ = ("a", "b") # Even if the values are the same + /// + /// # TypeError: multiple bases have instance lay-out conflict + /// class C(A, B): ... + /// ``` + /// + /// ## Known problems + /// Dynamic (not tuple or string literal) `__slots__` are not checked. + /// Additionally, classes inheriting from built-in classes with implicit layouts + /// like `str` or `int` are also not checked. + /// + /// ```pycon + /// >>> hasattr(int, "__slots__") + /// False + /// >>> hasattr(str, "__slots__") + /// False + /// >>> class A(int, str): ... + /// Traceback (most recent call last): + /// File "", line 1, in + /// class A(int, str): ... + /// TypeError: multiple bases have instance lay-out conflict + /// ``` + pub(crate) static INCOMPATIBLE_SLOTS = { + summary: "detects class definitions whose MRO has conflicting `__slots__`", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// TODO #14889 pub(crate) static INCONSISTENT_MRO = { @@ -813,3 +871,11 @@ pub(crate) fn report_invalid_exception_cause(context: &InferContext, node: &ast: ), ); } + +pub(crate) fn report_base_with_incompatible_slots(context: &InferContext, node: &ast::Expr) { + context.report_lint( + &INCOMPATIBLE_SLOTS, + node.into(), + format_args!("Class base has incompatible `__slots__`"), + ); +} diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 4547246b43209..0cf7c8e4e2012 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -51,14 +51,15 @@ use crate::semantic_index::SemanticIndex; use crate::stdlib::builtins_module_scope; use crate::types::class_base::ClassBase; use crate::types::diagnostic::{ - report_invalid_assignment, report_unresolved_module, TypeCheckDiagnostics, CALL_NON_CALLABLE, - CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, - CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_BASE, - INVALID_CONTEXT_MANAGER, INVALID_DECLARATION, INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, - INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, - UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR, + report_base_with_incompatible_slots, report_invalid_assignment, report_unresolved_module, + TypeCheckDiagnostics, CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, + CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, + DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_BASE, INVALID_CONTEXT_MANAGER, INVALID_DECLARATION, + INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_VARIABLE_CONSTRAINTS, + POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, + UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR, }; -use crate::types::mro::MroErrorKind; +use crate::types::mro::{MroErrorKind, SlotsKind}; use crate::types::unpacker::{UnpackResult, Unpacker}; use crate::types::{ bindings_ty, builtins_symbol, declarations_ty, global_symbol, symbol, todo_type, @@ -585,40 +586,75 @@ impl<'db> TypeInferenceBuilder<'db> { } // (3) Check that the class's MRO is resolvable - if let Err(mro_error) = class.try_mro(self.db()).as_ref() { - match mro_error.reason() { - MroErrorKind::DuplicateBases(duplicates) => { - let base_nodes = class_node.bases(); - for (index, duplicate) in duplicates { - self.context.report_lint( - &DUPLICATE_BASE, - (&base_nodes[*index]).into(), - format_args!("Duplicate base class `{}`", duplicate.name(self.db())), - ); + match class.try_mro(self.db()).as_ref() { + Err(mro_error) => { + match mro_error.reason() { + MroErrorKind::DuplicateBases(duplicates) => { + let base_nodes = class_node.bases(); + for (index, duplicate) in duplicates { + self.context.report_lint( + &DUPLICATE_BASE, + (&base_nodes[*index]).into(), + format_args!("Duplicate base class `{}`", duplicate.name(self.db())), + ); + } } + MroErrorKind::InvalidBases(bases) => { + let base_nodes = class_node.bases(); + for (index, base_ty) in bases { + self.context.report_lint( + &INVALID_BASE, + (&base_nodes[*index]).into(), + format_args!( + "Invalid class base with type `{}` (all bases must be a class, `Any`, `Unknown` or `Todo`)", + base_ty.display(self.db()) + ), + ); + } + } + MroErrorKind::UnresolvableMro { bases_list } => self.context.report_lint( + &INCONSISTENT_MRO, + class_node.into(), + format_args!( + "Cannot create a consistent method resolution order (MRO) for class `{}` with bases list `[{}]`", + class.name(self.db()), + bases_list.iter().map(|base| base.display(self.db())).join(", ") + ), + ) } - MroErrorKind::InvalidBases(bases) => { - let base_nodes = class_node.bases(); - for (index, base_ty) in bases { - self.context.report_lint( - &INVALID_BASE, - (&base_nodes[*index]).into(), - format_args!( - "Invalid class base with type `{}` (all bases must be a class, `Any`, `Unknown` or `Todo`)", - base_ty.display(self.db()) - ), - ); + } + Ok(_) => { + let mut first_non_empty = None; + let mut has_incompatible = false; + + for (index, base) in class.explicit_bases(self.db()).iter().enumerate() { + let Some(ClassLiteralType { class: base }) = base.into_class_literal() + else { + continue; + }; + + let slots_kind = SlotsKind::from(self.db(), base); + let base_node = &class_node.bases()[index]; + + if !matches!(slots_kind, SlotsKind::NotEmpty) { + continue; + } + + if first_non_empty.is_none() { + first_non_empty = Some(index); + continue; } + + has_incompatible = true; + report_base_with_incompatible_slots(&self.context, base_node); + } + + if has_incompatible { + if let Some(index) = first_non_empty { + let base_node = &class_node.bases()[index]; + report_base_with_incompatible_slots(&self.context, base_node); + }; } - MroErrorKind::UnresolvableMro { bases_list } => self.context.report_lint( - &INCONSISTENT_MRO, - class_node.into(), - format_args!( - "Cannot create a consistent method resolution order (MRO) for class `{}` with bases list `[{}]`", - class.name(self.db()), - bases_list.iter().map(|base| base.display(self.db())).join(", ") - ), - ) } } diff --git a/crates/red_knot_python_semantic/src/types/mro.rs b/crates/red_knot_python_semantic/src/types/mro.rs index 2f447f1582d3d..72eb62b6f843d 100644 --- a/crates/red_knot_python_semantic/src/types/mro.rs +++ b/crates/red_knot_python_semantic/src/types/mro.rs @@ -3,6 +3,7 @@ use std::ops::Deref; use rustc_hash::FxHashSet; +use crate::symbol::{Boundness, Symbol}; use crate::types::class_base::ClassBase; use crate::types::{Class, KnownClass, Type}; use crate::Db; @@ -328,3 +329,45 @@ fn c3_merge(mut sequences: Vec>) -> Option { } } } + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) enum SlotsKind { + /// `__slots__` is not found in the class nor any of its bases. + NotSpecified, + /// `__slots__` is defined but empty: `__slots__ = ()`. + Empty, + /// `__slots__` is defined and is not empty: `__slots__ = ("a", "b")`. + NotEmpty, + /// `__slots__` is defined but its value is dynamic: + /// * `__slots__ = tuple(a for a in b)` + /// * `__slots__ = ["a", "b"]` + Dynamic, +} + +impl SlotsKind { + pub(super) fn from(db: &dyn Db, base: Class) -> Self { + let Symbol::Type(slots_ty, bound) = base.class_member(db, "__slots__") else { + return Self::NotSpecified; + }; + + if matches!(bound, Boundness::PossiblyUnbound) { + return Self::Dynamic; + }; + + match slots_ty { + // __slots__ = ("a", "b") + Type::Tuple(tuple) => { + if tuple.elements(db).is_empty() { + Self::Empty + } else { + Self::NotEmpty + } + } + + // __slots__ = "abc" # Same as `("abc",)` + Type::StringLiteral(_) => Self::NotEmpty, + + _ => Self::Dynamic, + } + } +} From ce40ebe8b1dacd4b9b088766b20d8c4a60cc8623 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Tue, 24 Dec 2024 00:34:16 +0000 Subject: [PATCH 2/6] Formatting --- crates/red_knot_python_semantic/resources/mdtest/slots.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/slots.md b/crates/red_knot_python_semantic/resources/mdtest/slots.md index 9f426e816df8d..f53137eeb77f8 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/slots.md +++ b/crates/red_knot_python_semantic/resources/mdtest/slots.md @@ -14,7 +14,6 @@ class C: class AB(A, B): ... # fine class AC(A, C): ... # fine class BC(B, C): ... # fine - class ABC(A, B, C): ... # fine ``` @@ -55,7 +54,7 @@ class A: __slots__ = "abc" class B: - __slots__ = ("abc") + __slots__ = ("abc",) class AB( A, # error: [incompatible-slots] @@ -96,7 +95,6 @@ class C: __slots__ = ("c", "d") class D(C): ... - class E( B, # error: [incompatible-slots] D, # error: [incompatible-slots] From eb388e0612fc30951ad90c2856a75a3a3034e8ba Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Wed, 25 Dec 2024 00:27:28 +0000 Subject: [PATCH 3/6] Per review --- .../resources/mdtest/slots.md | 13 +++++++ .../src/types/diagnostic.rs | 5 +-- .../src/types/infer.rs | 35 ++++++++++++++----- .../red_knot_python_semantic/src/types/mro.rs | 2 +- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/slots.md b/crates/red_knot_python_semantic/resources/mdtest/slots.md index f53137eeb77f8..6cf84ce658796 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/slots.md +++ b/crates/red_knot_python_semantic/resources/mdtest/slots.md @@ -101,6 +101,19 @@ class E( ): ... ``` +## Single solid base + +```py +class A: + __slots__ = ("a", "b") + +class B(A): ... +class C(A): ... + +class D(B, A): ... # fine +class E(B, C, A): ... # fine +``` + ## False negatives ### Possibly unbound diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 56b19fe7d6499..65f85c5f61880 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -157,7 +157,7 @@ declare_lint! { /// Inheriting from bases with incompatible `__slots__`s /// will lead to a `TypeError` at runtime. /// - /// Classes with no or empty `__slots__` is always compatible: + /// Classes with no or empty `__slots__` are always compatible: /// /// ```python /// class A: ... @@ -170,7 +170,8 @@ declare_lint! { /// class D(A, B, C): ... /// ``` /// - /// Class with non-empty `__slots__` cannot participate in multiple inheritance: + /// Multiple inheritance from more than one different class + /// defining non-empty `__slots__` is not allowed: /// /// ```python /// class A: diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 0cf7c8e4e2012..56f97ba835055 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -624,8 +624,9 @@ impl<'db> TypeInferenceBuilder<'db> { } } Ok(_) => { - let mut first_non_empty = None; - let mut has_incompatible = false; + let mut first_with_solid_base = None; + let mut common_solid_base = None; + let mut found_second = false; for (index, base) in class.explicit_bases(self.db()).iter().enumerate() { let Some(ClassLiteralType { class: base }) = base.into_class_literal() @@ -633,24 +634,40 @@ impl<'db> TypeInferenceBuilder<'db> { continue; }; - let slots_kind = SlotsKind::from(self.db(), base); + let solid_base = base.iter_mro(self.db()).find_map(|current| { + let ClassBase::Class(current) = current else { + return None; + }; + + match SlotsKind::from(self.db(), current) { + SlotsKind::NotEmpty => Some(current), + SlotsKind::NotSpecified | SlotsKind::Empty => None, + SlotsKind::Dynamic => None, + } + }); + let base_node = &class_node.bases()[index]; - if !matches!(slots_kind, SlotsKind::NotEmpty) { + if solid_base.is_none() { + continue; + } + + if first_with_solid_base.is_none() { + first_with_solid_base = Some(index); + common_solid_base = solid_base; continue; } - if first_non_empty.is_none() { - first_non_empty = Some(index); + if solid_base == common_solid_base { continue; } - has_incompatible = true; + found_second = true; report_base_with_incompatible_slots(&self.context, base_node); } - if has_incompatible { - if let Some(index) = first_non_empty { + if found_second { + if let Some(index) = first_with_solid_base { let base_node = &class_node.bases()[index]; report_base_with_incompatible_slots(&self.context, base_node); }; diff --git a/crates/red_knot_python_semantic/src/types/mro.rs b/crates/red_knot_python_semantic/src/types/mro.rs index 72eb62b6f843d..be4d539f5aa4c 100644 --- a/crates/red_knot_python_semantic/src/types/mro.rs +++ b/crates/red_knot_python_semantic/src/types/mro.rs @@ -346,7 +346,7 @@ pub(super) enum SlotsKind { impl SlotsKind { pub(super) fn from(db: &dyn Db, base: Class) -> Self { - let Symbol::Type(slots_ty, bound) = base.class_member(db, "__slots__") else { + let Symbol::Type(slots_ty, bound) = base.own_class_member(db, "__slots__") else { return Self::NotSpecified; }; From 0d42f07c8880bcd733cdfefa97041fc5021216ec Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Wed, 25 Dec 2024 00:35:56 +0000 Subject: [PATCH 4/6] Formatting --- crates/red_knot_python_semantic/resources/mdtest/slots.md | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/slots.md b/crates/red_knot_python_semantic/resources/mdtest/slots.md index 6cf84ce658796..4065c5d2f5861 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/slots.md +++ b/crates/red_knot_python_semantic/resources/mdtest/slots.md @@ -109,7 +109,6 @@ class A: class B(A): ... class C(A): ... - class D(B, A): ... # fine class E(B, C, A): ... # fine ``` From 5248b2a467457e93039db5f5f14ee656942bd7f3 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Thu, 26 Dec 2024 23:59:03 +0000 Subject: [PATCH 5/6] Per review --- crates/red_knot_python_semantic/src/types/mro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/src/types/mro.rs b/crates/red_knot_python_semantic/src/types/mro.rs index be4d539f5aa4c..a77bb86a859b0 100644 --- a/crates/red_knot_python_semantic/src/types/mro.rs +++ b/crates/red_knot_python_semantic/src/types/mro.rs @@ -332,7 +332,7 @@ fn c3_merge(mut sequences: Vec>) -> Option { #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub(super) enum SlotsKind { - /// `__slots__` is not found in the class nor any of its bases. + /// `__slots__` is not found in the class. NotSpecified, /// `__slots__` is defined but empty: `__slots__ = ()`. Empty, From e7e0d221418d4548055a19b63282cee38ae05bb1 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 27 Dec 2024 11:39:05 +0000 Subject: [PATCH 6/6] improve encapsulation --- crates/red_knot_python_semantic/src/types.rs | 1 + .../src/types/infer.rs | 67 ++---------- .../red_knot_python_semantic/src/types/mro.rs | 43 -------- .../src/types/slots.rs | 103 ++++++++++++++++++ 4 files changed, 113 insertions(+), 101 deletions(-) create mode 100644 crates/red_knot_python_semantic/src/types/slots.rs diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 4514e136dfa77..547ce40c703ac 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -46,6 +46,7 @@ mod infer; mod mro; mod narrow; mod signatures; +mod slots; mod string_annotation; mod unpacker; diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 56f97ba835055..0f7a83d69eb61 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -51,15 +51,14 @@ use crate::semantic_index::SemanticIndex; use crate::stdlib::builtins_module_scope; use crate::types::class_base::ClassBase; use crate::types::diagnostic::{ - report_base_with_incompatible_slots, report_invalid_assignment, report_unresolved_module, - TypeCheckDiagnostics, CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, - CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, - DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_BASE, INVALID_CONTEXT_MANAGER, INVALID_DECLARATION, - INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_VARIABLE_CONSTRAINTS, - POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, - UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR, + report_invalid_assignment, report_unresolved_module, TypeCheckDiagnostics, CALL_NON_CALLABLE, + CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, + CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_BASE, + INVALID_CONTEXT_MANAGER, INVALID_DECLARATION, INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, + INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, + UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR, }; -use crate::types::mro::{MroErrorKind, SlotsKind}; +use crate::types::mro::MroErrorKind; use crate::types::unpacker::{UnpackResult, Unpacker}; use crate::types::{ bindings_ty, builtins_symbol, declarations_ty, global_symbol, symbol, todo_type, @@ -80,6 +79,7 @@ use super::diagnostic::{ report_possibly_unresolved_reference, report_slice_step_size_zero, report_unresolved_reference, SUBCLASS_OF_FINAL_CLASS, }; +use super::slots::check_class_slots; use super::string_annotation::{ parse_string_annotation, BYTE_STRING_TYPE_ANNOTATION, FSTRING_TYPE_ANNOTATION, }; @@ -623,56 +623,7 @@ impl<'db> TypeInferenceBuilder<'db> { ) } } - Ok(_) => { - let mut first_with_solid_base = None; - let mut common_solid_base = None; - let mut found_second = false; - - for (index, base) in class.explicit_bases(self.db()).iter().enumerate() { - let Some(ClassLiteralType { class: base }) = base.into_class_literal() - else { - continue; - }; - - let solid_base = base.iter_mro(self.db()).find_map(|current| { - let ClassBase::Class(current) = current else { - return None; - }; - - match SlotsKind::from(self.db(), current) { - SlotsKind::NotEmpty => Some(current), - SlotsKind::NotSpecified | SlotsKind::Empty => None, - SlotsKind::Dynamic => None, - } - }); - - let base_node = &class_node.bases()[index]; - - if solid_base.is_none() { - continue; - } - - if first_with_solid_base.is_none() { - first_with_solid_base = Some(index); - common_solid_base = solid_base; - continue; - } - - if solid_base == common_solid_base { - continue; - } - - found_second = true; - report_base_with_incompatible_slots(&self.context, base_node); - } - - if found_second { - if let Some(index) = first_with_solid_base { - let base_node = &class_node.bases()[index]; - report_base_with_incompatible_slots(&self.context, base_node); - }; - } - } + Ok(_) => check_class_slots(&self.context, class, class_node) } // (4) Check that the class's metaclass can be determined without error. diff --git a/crates/red_knot_python_semantic/src/types/mro.rs b/crates/red_knot_python_semantic/src/types/mro.rs index a77bb86a859b0..2f447f1582d3d 100644 --- a/crates/red_knot_python_semantic/src/types/mro.rs +++ b/crates/red_knot_python_semantic/src/types/mro.rs @@ -3,7 +3,6 @@ use std::ops::Deref; use rustc_hash::FxHashSet; -use crate::symbol::{Boundness, Symbol}; use crate::types::class_base::ClassBase; use crate::types::{Class, KnownClass, Type}; use crate::Db; @@ -329,45 +328,3 @@ fn c3_merge(mut sequences: Vec>) -> Option { } } } - -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub(super) enum SlotsKind { - /// `__slots__` is not found in the class. - NotSpecified, - /// `__slots__` is defined but empty: `__slots__ = ()`. - Empty, - /// `__slots__` is defined and is not empty: `__slots__ = ("a", "b")`. - NotEmpty, - /// `__slots__` is defined but its value is dynamic: - /// * `__slots__ = tuple(a for a in b)` - /// * `__slots__ = ["a", "b"]` - Dynamic, -} - -impl SlotsKind { - pub(super) fn from(db: &dyn Db, base: Class) -> Self { - let Symbol::Type(slots_ty, bound) = base.own_class_member(db, "__slots__") else { - return Self::NotSpecified; - }; - - if matches!(bound, Boundness::PossiblyUnbound) { - return Self::Dynamic; - }; - - match slots_ty { - // __slots__ = ("a", "b") - Type::Tuple(tuple) => { - if tuple.elements(db).is_empty() { - Self::Empty - } else { - Self::NotEmpty - } - } - - // __slots__ = "abc" # Same as `("abc",)` - Type::StringLiteral(_) => Self::NotEmpty, - - _ => Self::Dynamic, - } - } -} diff --git a/crates/red_knot_python_semantic/src/types/slots.rs b/crates/red_knot_python_semantic/src/types/slots.rs new file mode 100644 index 0000000000000..fe6ce48b777a7 --- /dev/null +++ b/crates/red_knot_python_semantic/src/types/slots.rs @@ -0,0 +1,103 @@ +use ruff_python_ast as ast; + +use crate::db::Db; +use crate::symbol::{Boundness, Symbol}; +use crate::types::class_base::ClassBase; +use crate::types::diagnostic::report_base_with_incompatible_slots; +use crate::types::{Class, ClassLiteralType, Type}; + +use super::InferContext; + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +enum SlotsKind { + /// `__slots__` is not found in the class. + NotSpecified, + /// `__slots__` is defined but empty: `__slots__ = ()`. + Empty, + /// `__slots__` is defined and is not empty: `__slots__ = ("a", "b")`. + NotEmpty, + /// `__slots__` is defined but its value is dynamic: + /// * `__slots__ = tuple(a for a in b)` + /// * `__slots__ = ["a", "b"]` + Dynamic, +} + +impl SlotsKind { + fn from(db: &dyn Db, base: Class) -> Self { + let Symbol::Type(slots_ty, bound) = base.own_class_member(db, "__slots__") else { + return Self::NotSpecified; + }; + + if matches!(bound, Boundness::PossiblyUnbound) { + return Self::Dynamic; + }; + + match slots_ty { + // __slots__ = ("a", "b") + Type::Tuple(tuple) => { + if tuple.elements(db).is_empty() { + Self::Empty + } else { + Self::NotEmpty + } + } + + // __slots__ = "abc" # Same as `("abc",)` + Type::StringLiteral(_) => Self::NotEmpty, + + _ => Self::Dynamic, + } + } +} + +pub(super) fn check_class_slots(context: &InferContext, class: Class, node: &ast::StmtClassDef) { + let db = context.db(); + + let mut first_with_solid_base = None; + let mut common_solid_base = None; + let mut found_second = false; + + for (index, base) in class.explicit_bases(db).iter().enumerate() { + let Type::ClassLiteral(ClassLiteralType { class: base }) = base else { + continue; + }; + + let solid_base = base.iter_mro(db).find_map(|current| { + let ClassBase::Class(current) = current else { + return None; + }; + + match SlotsKind::from(db, current) { + SlotsKind::NotEmpty => Some(current), + SlotsKind::NotSpecified | SlotsKind::Empty => None, + SlotsKind::Dynamic => None, + } + }); + + if solid_base.is_none() { + continue; + } + + let base_node = &node.bases()[index]; + + if first_with_solid_base.is_none() { + first_with_solid_base = Some(index); + common_solid_base = solid_base; + continue; + } + + if solid_base == common_solid_base { + continue; + } + + found_second = true; + report_base_with_incompatible_slots(context, base_node); + } + + if found_second { + if let Some(index) = first_with_solid_base { + let base_node = &node.bases()[index]; + report_base_with_incompatible_slots(context, base_node); + }; + } +}