Issue OpenCyphal#99 proposal

This is a draft PR to review a proposed change to the public APIs for pydsdl.

This is non-breaking change that adds a new public method and datatype

## New Method

read_files - a file-oriented entry point to the front end. This takes a
list of target DSDL files allowing the user to maintain an explicit list
instead of depending on globular filesystem discovery.

## New Datatype

DsdlFile – We define and publish a new abstract data type that encapsulates
all the information collected about and from a given dsdl file. This allows
a backend to associate datatypes with files for the purposes of managing
dependencies (e.g. this would allow the creation of .d files by a back end).

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "test/public_regulated_data_types"]
+	path = test/public_regulated_data_types
+	url = https://github.com/OpenCyphal/public_regulated_data_types.git

pydsdl/__init__.py

@@ -25,8 +25,10 @@
 _sys.path = [str(_Path(__file__).parent / "third_party")] + _sys.path
 
 # Never import anything that is not available here - API stability guarantees are only provided for the exposed items.
+from ._dsdl import PrintOutputHandler as PrintOutputHandler
+from ._dsdl import DsdlFile as DsdlFile
 from ._namespace import read_namespace as read_namespace
-from ._namespace import PrintOutputHandler as PrintOutputHandler
+from ._namespace import read_files as read_files
 
 # Error model.
 from ._error import FrontendError as FrontendError

pydsdl/_data_type_builder.py

@@ -2,16 +2,12 @@
 # This software is distributed under the terms of the MIT License.
 # Author: Pavel Kirienko <[email protected]>
 
-from typing import Optional, Callable, Iterable
 import logging
 from pathlib import Path
-from . import _serializable
-from . import _expression
-from . import _error
-from . import _dsdl_definition
-from . import _parser
-from . import _data_schema_builder
-from . import _port_id_ranges
+from typing import Callable, Iterable, Optional
+
+from . import _data_schema_builder, _error, _expression, _parser, _port_id_ranges, _serializable
+from ._dsdl import DefinitionVisitor, DsdlFileBuildable
 
 
 class AssertionCheckFailureError(_error.InvalidDefinitionError):
@@ -42,21 +38,25 @@ class MissingSerializationModeError(_error.InvalidDefinitionError):
 
 
 class DataTypeBuilder(_parser.StatementStreamProcessor):
+
+    # pylint: disable=too-many-arguments
     def __init__(
         self,
-        definition: _dsdl_definition.DSDLDefinition,
-        lookup_definitions: Iterable[_dsdl_definition.DSDLDefinition],
+        definition: DsdlFileBuildable,
+        lookup_definitions: Iterable[DsdlFileBuildable],
+        definition_visitors: Iterable[DefinitionVisitor],
         print_output_handler: Callable[[int, str], None],
         allow_unregulated_fixed_port_id: bool,
     ):
         self._definition = definition
         self._lookup_definitions = list(lookup_definitions)
+        self._definition_visitors = definition_visitors
         self._print_output_handler = print_output_handler
         self._allow_unregulated_fixed_port_id = allow_unregulated_fixed_port_id
         self._element_callback = None  # type: Optional[Callable[[str], None]]
 
-        assert isinstance(self._definition, _dsdl_definition.DSDLDefinition)
-        assert all(map(lambda x: isinstance(x, _dsdl_definition.DSDLDefinition), lookup_definitions))
+        assert isinstance(self._definition, DsdlFileBuildable)
+        assert all(map(lambda x: isinstance(x, DsdlFileBuildable), lookup_definitions))
         assert callable(self._print_output_handler)
         assert isinstance(self._allow_unregulated_fixed_port_id, bool)
 
@@ -198,6 +198,7 @@ def resolve_versioned_data_type(self, name: str, version: _serializable.Version)
         del name
         found = list(filter(lambda d: d.full_name == full_name and d.version == version, self._lookup_definitions))
         if not found:
+
             # Play Sherlock to help the user with mistakes like https://forum.opencyphal.org/t/904/2
             requested_ns = full_name.split(_serializable.CompositeType.NAME_COMPONENT_SEPARATOR)[0]
             lookup_nss = set(x.root_namespace for x in self._lookup_definitions)
@@ -221,15 +222,20 @@ def resolve_versioned_data_type(self, name: str, version: _serializable.Version)
             raise _error.InternalError("Conflicting definitions: %r" % found)
 
         target_definition = found[0]
-        assert isinstance(target_definition, _dsdl_definition.DSDLDefinition)
+        for visitor in self._definition_visitors:
+            visitor.on_discover_lookup_dependent_file(self._definition, target_definition)
+
+        assert isinstance(target_definition, DsdlFileBuildable)
         assert target_definition.full_name == full_name
         assert target_definition.version == version
         # Recursion is cool.
-        return target_definition.read(
+        dt = target_definition.read(
             lookup_definitions=self._lookup_definitions,
+            definition_visitors=self._definition_visitors,
             print_output_handler=self._print_output_handler,
             allow_unregulated_fixed_port_id=self._allow_unregulated_fixed_port_id,
         )
+        return dt
 
     def _queue_attribute(self, element_callback: Callable[[str], None]) -> None:
         self._flush_attribute("")
@@ -266,7 +272,9 @@ def _on_assert_directive(self, line_number: int, value: Optional[_expression.Any
         elif value is None:
             raise InvalidDirectiveError("Assert directive requires an expression")
         else:
-            raise InvalidDirectiveError("The assertion check expression must yield a boolean, not %s" % value.TYPE_NAME)
+            raise InvalidDirectiveError(
+                "The assertion check expression must yield a boolean, not %s" % value.TYPE_NAME
+            )
 
     def _on_extent_directive(self, line_number: int, value: Optional[_expression.Any]) -> None:
         if self._structs[-1].serialization_mode is not None:
@@ -300,7 +308,9 @@ def _on_union_directive(self, _ln: int, value: Optional[_expression.Any]) -> Non
         if self._structs[-1].union:
             raise InvalidDirectiveError("Duplicated union directive")
         if self._structs[-1].attributes:
-            raise InvalidDirectiveError("The union directive must be placed before the first " "attribute definition")
+            raise InvalidDirectiveError(
+                "The union directive must be placed before the first " "attribute definition"
+            )
         self._structs[-1].make_union()
 
     def _on_deprecated_directive(self, _ln: int, value: Optional[_expression.Any]) -> None:

pydsdl/_dsdl.py

@@ -0,0 +1,194 @@
+# Copyright (C) OpenCyphal Development Team  <opencyphal.org>
+# Copyright Amazon.com Inc. or its affiliates.
+# SPDX-License-Identifier: MIT
+
+from abc import ABC, abstractmethod
+from pathlib import Path
+from typing import Callable, Iterable, List, Optional, Set, Tuple, TypeVar, Union
+
+from ._serializable import CompositeType, Version
+
+PrintOutputHandler = Callable[[Path, int, str], None]
+"""Invoked when the frontend encounters a print directive or needs to output a generic diagnostic."""
+
+
+class DsdlFile(ABC):
+    """
+    Interface for DSDL files. This interface is used by the parser to abstract DSDL type details inferred from the
+    filesystem.
+    """
+
+    @property
+    @abstractmethod
+    def composite_type(self) -> Optional[CompositeType]:
+        """The composite type that was read from the DSDL file or None if the type has not been parsed yet."""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def full_name(self) -> str:
+        """The full name, e.g., uavcan.node.Heartbeat"""
+        raise NotImplementedError()
+
+    @property
+    def name_components(self) -> List[str]:
+        """Components of the full name as a list, e.g., ['uavcan', 'node', 'Heartbeat']"""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def short_name(self) -> str:
+        """The last component of the full name, e.g., Heartbeat of uavcan.node.Heartbeat"""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def full_namespace(self) -> str:
+        """The full name without the short name, e.g., uavcan.node for uavcan.node.Heartbeat"""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def root_namespace(self) -> str:
+        """The first component of the full name, e.g., uavcan of uavcan.node.Heartbeat"""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def text(self) -> str:
+        """The source text in its raw unprocessed form (with comments, formatting intact, and everything)"""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def version(self) -> Version:
+        """
+        The version of the DSDL definition.
+        """
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def fixed_port_id(self) -> Optional[int]:
+        """Either the fixed port ID as integer, or None if not defined for this type."""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def has_fixed_port_id(self) -> bool:
+        """
+        If the type has a fixed port ID defined, this method returns True. Equivalent to ``fixed_port_id is not None``.
+        """
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def file_path(self) -> Path:
+        """The path to the DSDL file on the filesystem."""
+        raise NotImplementedError()
+
+    @property
+    @abstractmethod
+    def root_namespace_path(self) -> Path:
+        """
+        The path to the root namespace directory on the filesystem.
+        """
+        raise NotImplementedError()
+
+
+class DefinitionVisitor(ABC):
+    """
+    An interface that allows visitors to index dependent types of a target DSDL file. This allows visitors to
+    build a closure of dependent types while parsing a set of target DSDL files.
+    """
+
+    @abstractmethod
+    def on_discover_lookup_dependent_file(self, target_dsdl_file: DsdlFile, dependent_type: DsdlFile) -> None:
+        """
+        Called by the parser after if finds a dependent type but before it parses a file in a lookup namespace.
+        :param DsdlFile target_dsdl_file: The target DSDL file that has dependencies the parser is searching for.
+        :param DsdlFile dependent_type: The dependency of target_dsdl_file file the parser is about to parse.
+        :raises DependentFileError: If the dependent file is not allowed by the visitor.
+        """
+        raise NotImplementedError()
+
+
+class DsdlFileBuildable(DsdlFile):
+    """
+    A DSDL file that can construct a composite type from its contents.
+    """
+
+    @abstractmethod
+    def read(
+        self,
+        lookup_definitions: Iterable["DsdlFileBuildable"],
+        definition_visitors: Iterable[DefinitionVisitor],
+        print_output_handler: Callable[[int, str], None],
+        allow_unregulated_fixed_port_id: bool,
+    ) -> CompositeType:
+        """
+        Reads the data type definition and returns its high-level data type representation.
+        The output should be cached; all following invocations should read from this cache.
+        Caching is very important, because it is expected that the same definition may be referred to multiple
+        times (e.g., for composition or when accessing external constants). Re-processing a definition every time
+        it is accessed would be a huge waste of time.
+        Note, however, that this may lead to unexpected complications if one is attempting to re-read a definition
+        with different inputs (e.g., different lookup paths) expecting to get a different result: caching would
+        get in the way. That issue is easy to avoid by creating a new instance of the object.
+        :param lookup_definitions:              List of definitions available for referring to.
+        :param definition_visitors:             Visitors to notify about discovered dependencies.
+        :param print_output_handler:            Used for @print and for diagnostics: (line_number, text) -> None.
+        :param allow_unregulated_fixed_port_id: Do not complain about fixed unregulated port IDs.
+        :return: The data type representation.
+        """
+        raise NotImplementedError()
+
+
+SortedFileT = TypeVar("SortedFileT", DsdlFile, DsdlFileBuildable)
+SortedFileList = List[SortedFileT]
+"""A list of DSDL files sorted by name, newest version first."""
+
+FileSortKey: Callable[[SortedFileT], Tuple[str, int, int]] = lambda d: (
+    d.full_name,
+    -d.version.major,
+    -d.version.minor,
+)
+
+
+def file_sort(file_list: Iterable[SortedFileT]) -> SortedFileList:
+    """
+    Sorts a list of DSDL files lexicographically by name, newest version first.
+    """
+    return list(sorted(file_list, key=FileSortKey))
+
+
+UniformCollectionT = TypeVar("UniformCollectionT", Iterable, Set, List)
+
+
+def is_uniform_or_raise(collection: UniformCollectionT) -> UniformCollectionT:
+    """
+    Raises an error if the collection is not uniform.
+    """
+    first = type(next(iter(collection)))
+    if not all(isinstance(x, first) for x in collection):
+        raise TypeError(f"Not all items in collection were of type {str(first)}.")
+    return collection
+
+
+PathListT = TypeVar(
+    "PathListT", List[Path], Set[Path], List[str], Set[str], Union[List[Path], List[str]], Union[Set[Path], Set[str]]
+)
+
+
+def normalize_paths_argument(
+    namespaces_or_namespace: Union[None, Path, str, Iterable[Union[Path, str]]],
+    type_cast: Callable[[Iterable], PathListT],
+) -> PathListT:
+    """
+    Normalizes the input argument to a list of paths.
+    """
+    if namespaces_or_namespace is None:
+        return type_cast([])
+    if isinstance(namespaces_or_namespace, (Path, str)):
+        return type_cast([Path(namespaces_or_namespace)])
+    return is_uniform_or_raise(type_cast(namespaces_or_namespace))