Refactor the aggregation check logic to also use it for deprecation consistency checking

Author: pavel-kirienko

pydsdl/_serializable/_array.py

@@ -7,7 +7,7 @@
 import typing
 import warnings
 from .._bit_length_set import BitLengthSet
-from ._serializable import SerializableType, TypeParameterError, AggregationError
+from ._serializable import SerializableType, TypeParameterError, AggregationFailure
 from ._primitive import UnsignedIntegerType, PrimitiveType
 
 
@@ -22,17 +22,16 @@ def __init__(self, element_type: SerializableType, capacity: int):
         self._capacity = int(capacity)
         if self._capacity < 1:
             raise InvalidNumberOfElementsError("Array capacity cannot be less than 1")
-        if not self.element_type.is_valid_aggregate(self):
-            raise AggregationError("Specified element type of %r is not valid" % str(self))
 
     @property
     def deprecated(self) -> bool:
         return self.element_type.deprecated
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
-        # At the moment, arrays of arrays are not allowed, but this is ensured by the grammar definition.
-        # This restriction will be lifted eventually. All that needs to be done is to update the grammar only.
-        return True
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
+        af = self.element_type.check_aggregation(self)
+        if af is not None:
+            return AggregationFailure(self, aggregate, "Element type of %r is not valid: %s" % (str(self), af.message))
+        return super().check_aggregation(aggregate)
 
     @property
     def element_type(self) -> SerializableType:

pydsdl/_serializable/_composite.py

@@ -10,7 +10,8 @@
 from .. import _expression
 from .. import _port_id_ranges
 from .._bit_length_set import BitLengthSet
-from ._serializable import SerializableType, TypeParameterError, AggregationError
+from .._error import InvalidDefinitionError
+from ._serializable import SerializableType, TypeParameterError, AggregationFailure
 from ._attribute import Attribute, Field, PaddingField, Constant
 from ._name import check_name, InvalidNameError
 from ._primitive import PrimitiveType, UnsignedIntegerType
@@ -39,7 +40,7 @@ class MalformedUnionError(TypeParameterError):
     pass
 
 
-class DeprecatedDependencyError(AggregationError):
+class AggregationError(InvalidDefinitionError):
     pass
 
 
@@ -123,22 +124,13 @@ def __init__(  # pylint: disable=too-many-arguments
                 if not (0 <= port_id <= _port_id_ranges.MAX_SUBJECT_ID):
                     raise InvalidFixedPortIDError("Fixed subject ID %r is not valid" % port_id)
 
-        # Consistent deprecation check.
-        # A non-deprecated type cannot be dependent on deprecated types.
-        # A deprecated type can be dependent on anything.
-        if not self.deprecated:
-            for a in self._attributes:
-                if a.data_type.deprecated:
-                    raise DeprecatedDependencyError(
-                        "A type cannot depend on deprecated types unless it is also deprecated."
-                    )
-
         # Aggregation check. For example:
         #   - Types like utf8 and byte cannot be used outside of arrays.
         #   - A non-deprecated type cannot depend on a deprecated type.
         for a in self._attributes:
-            if not a.data_type.is_valid_aggregate(self):
-                raise AggregationError("Type of %r is not a valid field type for %s" % (str(a), self))
+            af = a.data_type.check_aggregation(self)
+            if af is not None:
+                raise AggregationError("Type of %r is not a valid field type for %s: %s" % (str(a), self, af.message))
 
     @property
     def full_name(self) -> str:
@@ -196,12 +188,8 @@ def bit_length_set(self) -> BitLengthSet:
         """
         raise NotImplementedError
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
-        if self.deprecated:  # Deprecation consistency check: non-deprecated cannot depend on a deprecated type.
-            if isinstance(aggregate, CompositeType) and not aggregate.deprecated:
-                return False
-
-        return True
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
+        return super().check_aggregation(aggregate)
 
     @property
     def deprecated(self) -> bool:
@@ -615,8 +603,11 @@ def iterate_fields_with_offsets(
         base_offset = base_offset + self.delimiter_header_type.bit_length_set
         return self.inner_type.iterate_fields_with_offsets(base_offset)
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
-        return self.inner_type.is_valid_aggregate(aggregate)
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
+        af = self.inner_type.check_aggregation(aggregate)
+        if af is not None:
+            return af
+        return super().check_aggregation(aggregate)
 
     def __repr__(self) -> str:
         return "%s(inner=%r, extent=%r)" % (self.__class__.__name__, self.inner_type, self.extent)

pydsdl/_serializable/_primitive.py

@@ -10,7 +10,7 @@
 import typing
 import fractions
 from .._bit_length_set import BitLengthSet
-from ._serializable import SerializableType, TypeParameterError
+from ._serializable import SerializableType, TypeParameterError, AggregationFailure
 
 
 ValueRange = typing.NamedTuple("ValueRange", [("min", fractions.Fraction), ("max", fractions.Fraction)])
@@ -56,8 +56,8 @@ def deprecated(self) -> bool:
         """Primitive types cannot be deprecated."""
         return False
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
-        return True
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
+        return super().check_aggregation(aggregate)
 
     @property
     def bit_length(self) -> int:
@@ -174,10 +174,12 @@ class ByteType(UnsignedIntegerType):
     def __init__(self) -> None:
         super().__init__(bit_length=PrimitiveType.BITS_IN_BYTE, cast_mode=PrimitiveType.CastMode.TRUNCATED)
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
         from ._array import ArrayType
 
-        return isinstance(aggregate, ArrayType)
+        if not isinstance(aggregate, ArrayType):
+            return AggregationFailure(self, aggregate, "The byte type can only be used as an array element type")
+        return super().check_aggregation(aggregate)
 
     def __str__(self) -> str:
         return "byte"
@@ -187,10 +189,14 @@ class UTF8Type(UnsignedIntegerType):
     def __init__(self) -> None:
         super().__init__(bit_length=8, cast_mode=PrimitiveType.CastMode.TRUNCATED)
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
         from ._array import VariableLengthArrayType
 
-        return isinstance(aggregate, VariableLengthArrayType)
+        if not isinstance(aggregate, VariableLengthArrayType):
+            return AggregationFailure(
+                self, aggregate, "The utf8 type can only be used as a variable-length array element type"
+            )
+        return super().check_aggregation(aggregate)
 
     def __str__(self) -> str:
         return "utf8"

pydsdl/_serializable/_serializable.py

@@ -3,6 +3,7 @@
 # Author: Pavel Kirienko <[email protected]>
 
 import abc
+from typing import Optional, NamedTuple
 from .. import _expression
 from .. import _error
 from .._bit_length_set import BitLengthSet
@@ -12,8 +13,18 @@ class TypeParameterError(_error.InvalidDefinitionError):
     pass
 
 
-class AggregationError(_error.InvalidDefinitionError):
-    pass
+AggregationFailure = NamedTuple(
+    "AggregationFailure",
+    [
+        ("inner", "SerializableType"),
+        ("outer", "SerializableType"),
+        ("message", str),
+    ],
+)
+"""
+This is returned by :meth:`SerializableType.check_aggregation()` to indicate the reason of the failure.
+Eventually this will need to be replaced with a dataclass (sadly no dataclasses in Python 3.6).
+"""
 
 
 class SerializableType(_expression.Any):
@@ -69,14 +80,20 @@ def deprecated(self) -> bool:
         """
         raise NotImplementedError
 
-    @abc.abstractmethod
-    def is_valid_aggregate(self, aggregate: "SerializableType") -> bool:
+    def check_aggregation(self, aggregate: "SerializableType") -> Optional[AggregationFailure]:
         """
-        Returns true iff the argument is a suitable aggregate type for this element type.
+        Returns None iff the argument is a suitable aggregate type for this element type;
+        otherwise, returns an instance of :class:`AggregationFailure` describing the reason of the failure.
         This is needed to detect incorrect aggregations, such as padding fields in unions,
-        or standalone utf8 or byte, etc.
+        or standalone utf8 or byte, or a deprecated type nested into a non-deprecated type, etc.
         """
-        raise NotImplementedError
+        if self.deprecated and not aggregate.deprecated:
+            return AggregationFailure(
+                self,
+                aggregate,
+                "A non-deprecated type %s cannot depend on a deprecated type %s" % (aggregate, self),
+            )
+        return None
 
     def _attribute(self, name: _expression.String) -> _expression.Any:
         if name.native_value == "_bit_length_":  # Experimental non-standard extension

pydsdl/_serializable/_void.py

@@ -2,8 +2,9 @@
 # This software is distributed under the terms of the MIT License.
 # Author: Pavel Kirienko <[email protected]>
 
+import typing
 from .._bit_length_set import BitLengthSet
-from ._serializable import SerializableType
+from ._serializable import SerializableType, AggregationFailure
 from ._primitive import InvalidBitLengthError
 
 
@@ -29,11 +30,12 @@ def deprecated(self) -> bool:
         """Void types cannot be deprecated."""
         return False
 
-    def is_valid_aggregate(self, aggregate: SerializableType) -> bool:
+    def check_aggregation(self, aggregate: "SerializableType") -> typing.Optional[AggregationFailure]:
         from ._composite import StructureType, CompositeType
 
-        # Unions are not allowed to contain void types. Only StructureType is allowed to do so.
-        return isinstance(aggregate, CompositeType) and isinstance(aggregate.inner_type, StructureType)
+        if not isinstance(aggregate, CompositeType) or not isinstance(aggregate.inner_type, StructureType):
+            return AggregationFailure(self, aggregate, "Void types can only be aggregated into structures")
+        return super().check_aggregation(aggregate)
 
     @property
     def bit_length(self) -> int:

pydsdl/_test.py

@@ -1696,7 +1696,7 @@ def _unittest_inconsistent_deprecation(wrkspc: Workspace) -> None:
         ],
     )
 
-    with raises(_error.InvalidDefinitionError, match="(?i).*depend.*deprecated.*"):
+    with raises(_error.InvalidDefinitionError, match="(?i).*depend.*deprecated.*") as exc_info:
         parse_definition(
             wrkspc.parse_new(
                 "ns/C.1.0.dsdl",
@@ -1709,8 +1709,9 @@ def _unittest_inconsistent_deprecation(wrkspc: Workspace) -> None:
             ),
             [wrkspc.parse_new("ns/X.1.0.dsdl", "@deprecated\n@sealed")],
         )
+    print(exc_info.value.text)
 
-    with raises(_error.InvalidDefinitionError, match="(?i).*depend.*deprecated.*"):
+    with raises(_error.InvalidDefinitionError, match="(?i).*depend.*deprecated.*") as exc_info:
         parse_definition(
             wrkspc.parse_new(
                 "ns/C.1.0.dsdl",
@@ -1723,6 +1724,7 @@ def _unittest_inconsistent_deprecation(wrkspc: Workspace) -> None:
             ),
             [wrkspc.parse_new("ns/X.1.0.dsdl", "@deprecated\n@sealed")],
         )
+    print(exc_info.value.text)
 
     parse_definition(
         wrkspc.parse_new(