Add validation of hinted sub controllers

Author: GDYendell

src/fastcs/controllers/base_controller.py

@@ -54,22 +54,19 @@ def __init__(
         # Internal state that should not be accessed directly by base classes
         self.__attributes: dict[str, Attribute] = {}
         self.__sub_controllers: dict[str, BaseController] = {}
-        self.__hinted_attributes = self._validate_attribute_type_hints()
+
+        self.__hinted_attributes: dict[str, HintedAttribute] = {}
+        self.__hinted_sub_controllers: dict[str, type[BaseController]] = {}
+        self._find_type_hints()
 
         self._bind_attrs()
 
         ios = ios or []
         self._attribute_ref_io_map = {io.ref_type: io for io in ios}
         self._validate_io(ios)
 
-    def _validate_attribute_type_hints(self) -> dict[str, HintedAttribute]:
-        """Validate `Attribute` type hints for introspection
-
-        Returns:
-            A dictionary of `HintedAttribute` from type hints on this controller class
-
-        """
-        hinted_attributes = {}
+    def _find_type_hints(self):
+        """Find `Attribute` and `Controller` type hints for introspection validation"""
         for name, hint in get_type_hints(type(self)).items():
             if isinstance(hint, _GenericAlias):  # e.g. AttrR[int]
                 args = get_args(hint)
@@ -88,9 +85,12 @@ def _validate_attribute_type_hints(self) -> dict[str, HintedAttribute]:
                             f"Invalid type hint for attribute {name}: {hint}"
                         )
 
-                hinted_attributes[name] = HintedAttribute(attr_type=hint, dtype=dtype)
+                self.__hinted_attributes[name] = HintedAttribute(
+                    attr_type=hint, dtype=dtype
+                )
 
-        return hinted_attributes
+            elif isinstance(hint, type) and issubclass(hint, BaseController):
+                self.__hinted_sub_controllers[name] = hint
 
     def _bind_attrs(self) -> None:
         """Search for Attributes and Methods to bind them to this instance.
@@ -156,16 +156,19 @@ async def initialise(self):
 
     def post_initialise(self):
         """Hook to call after all attributes added, before serving the application"""
-        self._validate_hinted_attributes()
+        self._validate_type_hints()
         self._connect_attribute_ios()
 
-    def _validate_hinted_attributes(self):
-        """Validate all `Attribute` type-hints were introspected"""
+    def _validate_type_hints(self):
+        """Validate all `Attribute` and `Controller` type-hints were introspected"""
         for name in self.__hinted_attributes:
             self._validate_hinted_attribute(name)
 
+        for name in self.__hinted_sub_controllers:
+            self._validate_hinted_controller(name)
+
         for subcontroller in self.sub_controllers.values():
-            subcontroller._validate_hinted_attributes()  # noqa: SLF001
+            subcontroller._validate_type_hints()  # noqa: SLF001
 
     def _validate_hinted_attribute(self, name: str):
         """Check that an `Attribute` with the given name exists on the controller"""
@@ -183,6 +186,22 @@ def _validate_hinted_attribute(self, name: str):
                 attribute=attr,
             )
 
+    def _validate_hinted_controller(self, name: str):
+        """Check that a sub controller with the given name exists on the controller"""
+        controller = getattr(self, name, None)
+        if controller is None or not isinstance(controller, BaseController):
+            raise RuntimeError(
+                f"Controller `{self.__class__.__name__}` failed to introspect "
+                f"hinted controller `{name}` during initialisation"
+            )
+        else:
+            logger.debug(
+                "Validated hinted sub controller",
+                name=name,
+                controller=self,
+                sub_controller=controller,
+            )
+
     def _connect_attribute_ios(self) -> None:
         """Connect ``Attribute`` callbacks to ``AttributeIO``s"""
         for attr in self.__attributes.values():
@@ -263,6 +282,15 @@ def add_sub_controller(self, name: str, sub_controller: BaseController):
                 f"Controller {self} has existing sub controller {name}: "
                 f"{self.__sub_controllers[name]}"
             )
+        elif name in self.__hinted_sub_controllers:
+            hint = self.__hinted_sub_controllers[name]
+            if not isinstance(sub_controller, hint):
+                raise RuntimeError(
+                    f"Controller '{self.__class__.__name__}' introspection of "
+                    f"hinted sub controller '{name}' does not match defined type. "
+                    f"Expected '{hint.__name__}' got "
+                    f"'{sub_controller.__class__.__name__}'."
+                )
         elif name in self.__attributes:
             raise ValueError(
                 f"Cannot add sub controller {sub_controller}. "

tests/test_controllers.py

@@ -148,7 +148,7 @@ def test_controller_vector_iter():
         assert sub_controllers[index] == child
 
 
-def test_controller_introspection_hint_validation():
+def test_attribute_hint_validation():
     class HintedController(Controller):
         read_write_int: AttrRW[int]
 
@@ -162,15 +162,15 @@ class HintedController(Controller):
 
     with pytest.raises(RuntimeError, match="failed to introspect hinted attribute"):
         controller.read_write_int = 5  # type: ignore
-        controller._validate_hinted_attributes()
+        controller._validate_type_hints()
 
     with pytest.raises(RuntimeError, match="failed to introspect hinted attribute"):
-        controller._validate_hinted_attributes()
+        controller._validate_type_hints()
 
     controller.add_attribute("read_write_int", AttrRW(Int()))
 
 
-def test_controller_introspection_hint_validation_enum():
+def test_enum_attribute_hint_validation():
     class GoodEnum(enum.IntEnum):
         VAL = 0
 
@@ -186,3 +186,20 @@ class HintedController(Controller):
         controller.add_attribute("enum", AttrRW(Enum(BadEnum)))
 
     controller.add_attribute("enum", AttrRW(Enum(GoodEnum)))
+
+
+@pytest.mark.asyncio
+async def test_sub_controller_hint_validation():
+    class HintedController(Controller):
+        child: SomeSubController
+
+    controller = HintedController()
+
+    with pytest.raises(RuntimeError, match="failed to introspect hinted controller"):
+        controller._validate_type_hints()
+
+    with pytest.raises(RuntimeError, match="does not match defined type"):
+        controller.add_sub_controller("child", Controller())
+
+    controller.add_sub_controller("child", SomeSubController())
+    controller._validate_type_hints()