[red-knot] Report classes inheriting from bases with incompatible `__…

…slots__`

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): ...
+```

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 "<python-input-0>", line 1, in <module>
+    ///     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__`"),
+    );
+}

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(", ")
-                        ),
-                    )
                 }
             }