Add a new declare-non-slot error code (#9564)

---------

Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>

Author: 3 people

doc/data/messages/d/declare-non-slot/bad.py

@@ -0,0 +1,5 @@
+class Student:
+    __slots__ = ("name",)
+
+    name: str
+    surname: str  # [declare-non-slot]

doc/data/messages/d/declare-non-slot/good.py

@@ -0,0 +1,5 @@
+class Student:
+    __slots__ = ("name", "surname")
+
+    name: str
+    surname: str

doc/user_guide/checkers/features.rst

@@ -282,6 +282,9 @@ Classes checker Messages
   Used when a method has an attribute different the "self" as first argument.
   This is considered as an error since this is a so common convention that you
   shouldn't break it!
+:declare-non-slot (E0245): *No such name %r in __slots__*
+  Raised when a type annotation on a class is absent from the list of names in
+  __slots__, and __slots__ does not contain a __dict__ entry.
 :unexpected-special-method-signature (E0302): *The special method %r expects %s param(s), %d %s given*
   Emitted when a special method was defined with an invalid number of
   parameters. If it has too few or too many, it might not work at all.

doc/user_guide/messages/messages_overview.rst

@@ -69,6 +69,7 @@ All messages in the error category:
    error/catching-non-exception
    error/class-variable-slots-conflict
    error/continue-in-finally
+   error/declare-non-slot
    error/dict-iter-missing-items
    error/duplicate-argument-name
    error/duplicate-bases

doc/whatsnew/fragments/9499.feature

@@ -0,0 +1,3 @@
+Add new `declare-non-slot` error which reports when a class has a `__slots__` member and a type hint on the class is not present in `__slots__`.
+
+Refs #9499

pylint/checkers/classes/class_checker.py

@@ -702,6 +702,12 @@ def _has_same_layout_slots(
         "Used when a class tries to extend an inherited Enum class. "
         "Doing so will raise a TypeError at runtime.",
     ),
+    "E0245": (
+        "No such name %r in __slots__",
+        "declare-non-slot",
+        "Raised when a type annotation on a class is absent from the list of names in __slots__, "
+        "and __slots__ does not contain a __dict__ entry.",
+    ),
     "R0202": (
         "Consider using a decorator instead of calling classmethod",
         "no-classmethod-decorator",
@@ -870,6 +876,7 @@ def _dummy_rgx(self) -> Pattern[str]:
         "invalid-enum-extension",
         "subclassed-final-class",
         "implicit-flag-alias",
+        "declare-non-slot",
     )
     def visit_classdef(self, node: nodes.ClassDef) -> None:
         """Init visit variable _accessed."""
@@ -878,6 +885,50 @@ def visit_classdef(self, node: nodes.ClassDef) -> None:
         self._check_proper_bases(node)
         self._check_typing_final(node)
         self._check_consistent_mro(node)
+        self._check_declare_non_slot(node)
+
+    def _check_declare_non_slot(self, node: nodes.ClassDef) -> None:
+        if not self._has_valid_slots(node):
+            return
+
+        slot_names = self._get_classdef_slots_names(node)
+
+        # Stop if empty __slots__ in the class body, this likely indicates that
+        # this class takes part in multiple inheritance with other slotted classes.
+        if not slot_names:
+            return
+
+        # Stop if we find __dict__, since this means attributes can be set
+        # dynamically
+        if "__dict__" in slot_names:
+            return
+
+        for base in node.bases:
+            ancestor = safe_infer(base)
+            if not isinstance(ancestor, nodes.ClassDef):
+                continue
+            # if any base doesn't have __slots__, attributes can be set dynamically, so stop
+            if not self._has_valid_slots(ancestor):
+                return
+            for slot_name in self._get_classdef_slots_names(ancestor):
+                if slot_name == "__dict__":
+                    return
+                slot_names.append(slot_name)
+
+        # Every class in bases has __slots__, our __slots__ is non-empty and there is no __dict__
+
+        for child in node.body:
+            if isinstance(child, nodes.AnnAssign):
+                if child.value is not None:
+                    continue
+                if isinstance(child.target, nodes.AssignName):
+                    if child.target.name not in slot_names:
+                        self.add_message(
+                            "declare-non-slot",
+                            args=child.target.name,
+                            node=child.target,
+                            confidence=INFERENCE,
+                        )
 
     def _check_consistent_mro(self, node: nodes.ClassDef) -> None:
         """Detect that a class has a consistent mro or duplicate bases."""
@@ -1482,6 +1533,24 @@ def _check_functools_or_not(self, decorator: nodes.Attribute) -> bool:
 
         return "functools" in dict(import_node.names)
 
+    def _has_valid_slots(self, node: nodes.ClassDef) -> bool:
+        if "__slots__" not in node.locals:
+            return False
+
+        for slots in node.ilookup("__slots__"):
+            # check if __slots__ is a valid type
+            if isinstance(slots, util.UninferableBase):
+                return False
+            if not is_iterable(slots) and not is_comprehension(slots):
+                return False
+            if isinstance(slots, nodes.Const):
+                return False
+            if not hasattr(slots, "itered"):
+                # we can't obtain the values, maybe a .deque?
+                return False
+
+        return True
+
     def _check_slots(self, node: nodes.ClassDef) -> None:
         if "__slots__" not in node.locals:
             return
@@ -1515,13 +1584,19 @@ def _check_slots(self, node: nodes.ClassDef) -> None:
                     continue
             self._check_redefined_slots(node, slots, values)
 
-    def _check_redefined_slots(
-        self,
-        node: nodes.ClassDef,
-        slots_node: nodes.NodeNG,
-        slots_list: list[nodes.NodeNG],
-    ) -> None:
-        """Check if `node` redefines a slot which is defined in an ancestor class."""
+    def _get_classdef_slots_names(self, node: nodes.ClassDef) -> list[str]:
+
+        slots_names = []
+        for slots in node.ilookup("__slots__"):
+            if isinstance(slots, nodes.Dict):
+                values = [item[0] for item in slots.items]
+            else:
+                values = slots.itered()
+            slots_names.extend(self._get_slots_names(values))
+
+        return slots_names
+
+    def _get_slots_names(self, slots_list: list[nodes.NodeNG]) -> list[str]:
         slots_names: list[str] = []
         for slot in slots_list:
             if isinstance(slot, nodes.Const):
@@ -1531,6 +1606,16 @@ def _check_redefined_slots(
                 inferred_slot_value = getattr(inferred_slot, "value", None)
                 if isinstance(inferred_slot_value, str):
                     slots_names.append(inferred_slot_value)
+        return slots_names
+
+    def _check_redefined_slots(
+        self,
+        node: nodes.ClassDef,
+        slots_node: nodes.NodeNG,
+        slots_list: list[nodes.NodeNG],
+    ) -> None:
+        """Check if `node` redefines a slot which is defined in an ancestor class."""
+        slots_names: list[str] = self._get_slots_names(slots_list)
 
         # Slots of all parent classes
         ancestors_slots_names = {

tests/functional/r/regression_02/regression_5479.py

@@ -1,7 +1,7 @@
 """Test for a regression on slots and annotated assignments.
 Reported in https://github.com/pylint-dev/pylint/issues/5479
 """
-# pylint: disable=too-few-public-methods, unused-private-member, missing-class-docstring, missing-function-docstring
+# pylint: disable=too-few-public-methods, unused-private-member, missing-class-docstring, missing-function-docstring, declare-non-slot
 
 from __future__ import annotations
 

tests/functional/s/slots_checks.py

@@ -128,3 +128,62 @@ class Parent:
 
 class ChildNotAffectedByValueInSlot(Parent):
     __slots__ = ('first', )
+
+
+class ClassTypeHintNotInSlotsWithoutDict:
+    __slots__ = ("a", "b")
+
+    a: int
+    b: str
+    c: bool # [declare-non-slot]
+
+
+class ClassTypeHintNotInSlotsWithDict:
+    __slots__ = ("a", "b", "__dict__")
+
+    a: int
+    b: str
+    c: bool
+
+
+class BaseNoSlots:
+    pass
+
+
+class DerivedWithSlots(BaseNoSlots):
+    __slots__ = ("age",)
+
+    price: int
+
+
+class BaseWithSlots:
+    __slots__ = ("a", "b",)
+
+
+class DerivedWithMoreSlots(BaseWithSlots):
+    __slots__ = ("c",)
+
+    # Is in base __slots__
+    a: int
+
+    # Not in any base __slots__
+    d: int # [declare-non-slot]
+    e: str= "AnnAssign.value is not None"
+
+
+class BaseWithSlotsDict:
+    __slots__ = ("__dict__", )
+
+class DerivedTypeHintNotInSlots(BaseWithSlotsDict):
+    __slots__ = ("other", )
+
+    a: int
+    def __init__(self) -> None:
+        super().__init__()
+        self.a = 42
+
+
+class ClassWithEmptySlotsAndAnnotation:
+    __slots__ = ()
+
+    a: int

tests/functional/s/slots_checks.txt

@@ -14,3 +14,5 @@ invalid-slots:81:0:81:16:TwelfthBad:Invalid __slots__ object:UNDEFINED
 class-variable-slots-conflict:114:17:114:24:ValueInSlotConflict:Value 'first' in slots conflicts with class variable:UNDEFINED
 class-variable-slots-conflict:114:45:114:53:ValueInSlotConflict:Value 'fourth' in slots conflicts with class variable:UNDEFINED
 class-variable-slots-conflict:114:36:114:43:ValueInSlotConflict:Value 'third' in slots conflicts with class variable:UNDEFINED
+declare-non-slot:138:4:138:5:ClassTypeHintNotInSlotsWithoutDict:No such name 'c' in __slots__:INFERENCE
+declare-non-slot:170:4:170:5:DerivedWithMoreSlots:No such name 'd' in __slots__:INFERENCE