From 503bf0f09e952186383dbd36cbed7b2d5ffb9811 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Fri, 29 Mar 2024 22:49:32 -0700 Subject: [PATCH 01/11] Adding support for file-based reads and externalized type dependencies This is non-breaking change that adds a new public 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. Furthermore this method returns a set which is the transitive closure of types depended on by the target list of types. This allows consumers to track dependencies and compiler back ends to generate .d files and otherwise support incremental builds. The new method may increase performance for systems with large pools of messages when generating code for a small sub-set as it only considers the target and dependencies of the target when parsing dsdl files. --- .devcontainer/devcontainer.json | 24 + .github/workflows/test-and-release.yml | 3 +- .readthedocs.yml | 7 + conftest.py | 71 +++ docs/pages/pydsdl.rst | 5 +- docs/requirements.txt | 4 +- noxfile.py | 63 ++- pydsdl/__init__.py | 5 +- pydsdl/_bit_length_set/_symbolic_test.py | 20 +- pydsdl/_data_type_builder.py | 36 +- pydsdl/_dsdl.py | 259 +++++++++ pydsdl/_dsdl_definition.py | 102 ++-- pydsdl/_error.py | 7 +- pydsdl/_namespace.py | 643 +++++++++++++++++------ pydsdl/_namespace_reader.py | 390 ++++++++++++++ pydsdl/_parser.py | 6 +- pydsdl/_serializable/_composite.py | 135 +++-- pydsdl/_test.py | 12 +- setup.cfg | 2 +- test/test_public_types.py | 47 ++ 20 files changed, 1549 insertions(+), 292 deletions(-) create mode 100644 .devcontainer/devcontainer.json create mode 100644 conftest.py create mode 100644 pydsdl/_dsdl.py create mode 100644 pydsdl/_namespace_reader.py create mode 100644 test/test_public_types.py diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..fec6c63 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,24 @@ +{ + "name": "Python dev environment", + "image": "ghcr.io/opencyphal/toxic:tx22.4.2", + "workspaceFolder": "/workspace", + "workspaceMount": "source=${localWorkspaceFolder},target=/workspace,type=bind,consistency=delegated", + "mounts": [ + "source=root-vscode-server,target=/root/.vscode-server/extensions,type=volume", + "source=pydsdl-tox,target=/workspace/.nox,type=volume" + ], + "customizations": { + "vscode": { + "extensions": [ + "uavcan.dsdl", + "wholroyd.jinja", + "streetsidesoftware.code-spell-checker", + "ms-python.python", + "ms-python.mypy-type-checker", + "ms-python.black-formatter", + "ms-python.pylint" + ] + } + }, + "postCreateCommand": "git clone --depth 1 git@github.com:OpenCyphal/public_regulated_data_types.git .dsdl-test && nox -e test-3.12" +} diff --git a/.github/workflows/test-and-release.yml b/.github/workflows/test-and-release.yml index 9efebdd..cc3a1cb 100644 --- a/.github/workflows/test-and-release.yml +++ b/.github/workflows/test-and-release.yml @@ -13,7 +13,7 @@ jobs: fail-fast: false matrix: os: [ ubuntu-latest, macos-latest ] - python: [ '3.7', '3.8', '3.9', '3.10', '3.11' ] + python: [ '3.8', '3.9', '3.10', '3.11', '3.12' ] include: - os: windows-2019 python: '3.10' @@ -60,7 +60,6 @@ jobs: echo "${{ runner.os }} not supported" exit 1 fi - python -c "import pydsdl; pydsdl.read_namespace('.dsdl-test/uavcan', [])" shell: bash pydsdl-release: diff --git a/.readthedocs.yml b/.readthedocs.yml index 97fb527..3cebaef 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -2,6 +2,13 @@ version: 2 +build: + os: ubuntu-22.04 + tools: + python: "3.12" + apt_packages: + - graphviz + sphinx: configuration: docs/conf.py fail_on_warning: true diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..d73f2d7 --- /dev/null +++ b/conftest.py @@ -0,0 +1,71 @@ +# +# Copyright (C) OpenCyphal Development Team +# Copyright Amazon.com Inc. or its affiliates. +# SPDX-License-Identifier: MIT +# +""" +Configuration for pytest tests including fixtures and hooks. +""" + +import tempfile +from pathlib import Path +from typing import Any, Optional + +import pytest + + +# +-------------------------------------------------------------------------------------------------------------------+ +# | TEST FIXTURES +# +-------------------------------------------------------------------------------------------------------------------+ +class TemporaryDsdlContext: + """ + Powers the temp_dsdl_factory test fixture. + """ + def __init__(self) -> None: + self._base_dir: Optional[Any] = None + + def new_file(self, file_path: Path, text: Optional[str] = None) -> Path: + if file_path.is_absolute(): + raise ValueError(f"{file_path} is an absolute path. The test fixture requires relative paths to work.") + file = self.base_dir / file_path + file.parent.mkdir(parents=True, exist_ok=True) + if text is not None: + file.write_text(text) + return file + + @property + def base_dir(self) -> Path: + if self._base_dir is None: + self._base_dir = tempfile.TemporaryDirectory() + return Path(self._base_dir.name).resolve() + + def _test_path_finalizer(self) -> None: + """ + Finalizer to clean up any temporary directories created during the test. + """ + if self._base_dir is not None: + self._base_dir.cleanup() + del self._base_dir + self._base_dir = None + +@pytest.fixture(scope="function") +def temp_dsdl_factory(request: pytest.FixtureRequest) -> Any: # pylint: disable=unused-argument + """ + Fixture for pydsdl tests that have to create files as part of the test. This object stays in-scope for a given + test method and does not requires a context manager in the test itself. + + Call `new_file(path)` to create a new file path in the fixture's temporary directory. This will create all + uncreated parent directories but will _not_ create the file unless text is provided: `new_file(path, "hello")` + """ + f = TemporaryDsdlContext() + request.addfinalizer(f._test_path_finalizer) # pylint: disable=protected-access + return f + + + +@pytest.fixture +def public_types() -> Path: + """ + Path to the public regulated data types directory used for tests. + """ + return Path(".dsdl-test") / "uavcan" diff --git a/docs/pages/pydsdl.rst b/docs/pages/pydsdl.rst index 4e73ac0..9a9a7c4 100644 --- a/docs/pages/pydsdl.rst +++ b/docs/pages/pydsdl.rst @@ -12,10 +12,11 @@ You can find a practical usage example in the Nunavut code generation library th :local: -The main function -+++++++++++++++++ +The main functions +++++++++++++++++++ .. autofunction:: pydsdl.read_namespace +.. autofunction:: pydsdl.read_files Type model diff --git a/docs/requirements.txt b/docs/requirements.txt index 9dc2a91..3532611 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,3 +1,3 @@ -sphinx == 4.4.0 -sphinx_rtd_theme == 1.0.0 +sphinx >= 5.0 +sphinx_rtd_theme >= 1.0.0 sphinx-computron >= 0.2, < 2.0 diff --git a/noxfile.py b/noxfile.py index 6939d8e..b74b4c1 100644 --- a/noxfile.py +++ b/noxfile.py @@ -10,7 +10,7 @@ import nox -PYTHONS = ["3.8", "3.9", "3.10", "3.11"] +PYTHONS = ["3.8", "3.9", "3.10", "3.11", "3.12"] """The newest supported Python shall be listed LAST.""" nox.options.error_on_external_run = True @@ -33,7 +33,6 @@ def clean(session): "*.log", "*.tmp", ".nox", - ".dsdl-test", ] for w in wildcards: for f in Path.cwd().glob(w): @@ -49,9 +48,9 @@ def test(session): session.log("Using the newest supported Python: %s", is_latest_python(session)) session.install("-e", ".") session.install( - "pytest ~= 7.3", - "pytest-randomly ~= 3.12", - "coverage ~= 7.2", + "pytest ~= 8.1", + "pytest-randomly ~= 3.15", + "coverage ~= 7.5", ) session.run("coverage", "run", "-m", "pytest") session.run("coverage", "report", "--fail-under=95") @@ -61,7 +60,7 @@ def test(session): session.log(f"OPEN IN WEB BROWSER: file://{report_file}") -@nox.session(python=["3.7"]) +@nox.session(python=["3.8"]) def test_eol(session): """This is a minimal test session for those old Pythons that have EOLed.""" session.install("-e", ".") @@ -83,30 +82,33 @@ def pristine(session): @nox.session(python=PYTHONS, reuse_venv=True) def lint(session): - session.log("Using the newest supported Python: %s", is_latest_python(session)) - session.install( - "mypy ~= 1.2.0", - "pylint ~= 2.17.2", - ) - session.run( - "mypy", - "--strict", - f"--config-file={ROOT_DIR / 'setup.cfg'}", - "pydsdl", - env={ - "MYPYPATH": str(THIRD_PARTY_DIR), - }, - ) - session.run( - "pylint", - str(ROOT_DIR / "pydsdl"), - env={ - "PYTHONPATH": str(THIRD_PARTY_DIR), - }, - ) + if is_oldest_python(session): + # we run mypy and pylint only on the oldest Python version to ensure maximum compatibility + session.install( + "mypy ~= 1.10", + "types-parsimonious", + "pylint ~= 3.2", + ) + session.run( + "mypy", + "--strict", + f"--config-file={ROOT_DIR / 'setup.cfg'}", + "pydsdl", + env={ + "MYPYPATH": str(THIRD_PARTY_DIR), + }, + ) + session.run( + "pylint", + str(ROOT_DIR / "pydsdl"), + env={ + "PYTHONPATH": str(THIRD_PARTY_DIR), + }, + ) if is_latest_python(session): - session.install("black ~= 23.3") - session.run("black", "--check", ".") + # we run black only on the newest Python version to ensure that the code is formatted with the latest version + session.install("black ~= 24.4") + session.run("black", "--check", f"{ROOT_DIR / 'pydsdl'}") @nox.session(reuse_venv=True) @@ -121,3 +123,6 @@ def docs(session): def is_latest_python(session) -> bool: return PYTHONS[-1] in session.run("python", "-V", silent=True) + +def is_oldest_python(session) -> bool: + return PYTHONS[0] in session.run("python", "-V", silent=True) diff --git a/pydsdl/__init__.py b/pydsdl/__init__.py index d07f1f2..aea7d00 100644 --- a/pydsdl/__init__.py +++ b/pydsdl/__init__.py @@ -7,7 +7,7 @@ import sys as _sys from pathlib import Path as _Path -__version__ = "1.20.1" +__version__ = "1.21.0" __version_info__ = tuple(map(int, __version__.split(".")[:3])) __license__ = "MIT" __author__ = "OpenCyphal" @@ -25,8 +25,9 @@ _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 ._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 diff --git a/pydsdl/_bit_length_set/_symbolic_test.py b/pydsdl/_bit_length_set/_symbolic_test.py index db262fa..1c414d7 100644 --- a/pydsdl/_bit_length_set/_symbolic_test.py +++ b/pydsdl/_bit_length_set/_symbolic_test.py @@ -2,7 +2,6 @@ # This software is distributed under the terms of the MIT License. # Author: Pavel Kirienko -import typing import random import itertools from ._symbolic import NullaryOperator, validate_numerically @@ -140,7 +139,7 @@ def _unittest_repetition() -> None: ) assert op.min == 7 * 3 assert op.max == 17 * 3 - assert set(op.expand()) == set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 3))) # type: ignore + assert set(op.expand()) == set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 3))) assert set(op.expand()) == {21, 25, 29, 31, 33, 35, 39, 41, 45, 51} assert set(op.modulo(7)) == {0, 1, 2, 3, 4, 5, 6} assert set(op.modulo(8)) == {1, 3, 5, 7} @@ -149,7 +148,7 @@ def _unittest_repetition() -> None: for _ in range(1): child = NullaryOperator(random.randint(0, 100) for _ in range(random.randint(1, 10))) k = random.randint(0, 10) - ref = set(map(sum, itertools.combinations_with_replacement(child.expand(), k))) # type: ignore + ref = set(map(sum, itertools.combinations_with_replacement(child.expand(), k))) op = RepetitionOperator(child, k) assert set(op.expand()) == ref @@ -157,7 +156,7 @@ def _unittest_repetition() -> None: assert op.max == max(child.expand()) * k div = random.randint(1, 64) - assert set(op.modulo(div)) == {typing.cast(int, x) % div for x in ref} + assert set(op.modulo(div)) == {x % div for x in ref} validate_numerically(op) @@ -173,9 +172,9 @@ def _unittest_range_repetition() -> None: assert op.max == 17 * 3 assert set(op.expand()) == ( {0} - | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 1))) # type: ignore - | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 2))) # type: ignore - | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 3))) # type: ignore + | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 1))) + | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 2))) + | set(map(sum, itertools.combinations_with_replacement([7, 11, 17], 3))) ) assert set(op.expand()) == {0, 7, 11, 14, 17, 18, 21, 22, 24, 25, 28, 29, 31, 33, 34, 35, 39, 41, 45, 51} assert set(op.modulo(7)) == {0, 1, 2, 3, 4, 5, 6} @@ -197,10 +196,7 @@ def _unittest_range_repetition() -> None: k_max = random.randint(0, 10) ref = set( itertools.chain( - *( - map(sum, itertools.combinations_with_replacement(child.expand(), k)) # type: ignore - for k in range(k_max + 1) - ) + *(map(sum, itertools.combinations_with_replacement(child.expand(), k)) for k in range(k_max + 1)) ) ) op = RangeRepetitionOperator(child, k_max) @@ -210,7 +206,7 @@ def _unittest_range_repetition() -> None: assert op.max == max(child.expand()) * k_max div = random.randint(1, 64) - assert set(op.modulo(div)) == {typing.cast(int, x) % div for x in ref} + assert set(op.modulo(div)) == {x % div for x in ref} validate_numerically(op) diff --git a/pydsdl/_data_type_builder.py b/pydsdl/_data_type_builder.py index 4572da3..63b3ba4 100644 --- a/pydsdl/_data_type_builder.py +++ b/pydsdl/_data_type_builder.py @@ -2,16 +2,12 @@ # This software is distributed under the terms of the MIT License. # Author: Pavel Kirienko -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) @@ -65,7 +65,7 @@ def __init__( def finalize(self) -> _serializable.CompositeType: if len(self._structs) == 1: # Structure type - (builder,) = self._structs # type: _data_schema_builder.DataSchemaBuilder, + (builder,) = self._structs out = self._make_composite( builder=builder, name=self._definition.full_name, @@ -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(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("") diff --git a/pydsdl/_dsdl.py b/pydsdl/_dsdl.py new file mode 100644 index 0000000..4e2f83b --- /dev/null +++ b/pydsdl/_dsdl.py @@ -0,0 +1,259 @@ +# Copyright (C) OpenCyphal Development Team +# Copyright Amazon.com Inc. or its affiliates. +# SPDX-License-Identifier: MIT + +from abc import ABC, abstractmethod +from pathlib import Path +from typing import Any, 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. Where properties are duplicated between the composite type and this file the composite type is to be + considered canonical. The properties directly on this class are inferred from the dsdl file path before the + composite type has been parsed. + """ + + @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 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() + + +DefinitionVisitor = Callable[[DsdlFile, DsdlFileBuildable], None] +""" +Called by the parser after if finds a dependent type but before it parses a file in a lookup namespace. +:param DsdlFile argument 0: The target DSDL file that has dependencies the parser is searching for. +:param DsdlFile argument 1: The dependency of target_dsdl_file file the parser is about to parse. +""" + +SortedFileT = TypeVar("SortedFileT", DsdlFile, DsdlFileBuildable, CompositeType) +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[SortedFileT]: + """ + Sorts a list of DSDL files lexicographically by name, newest version first. + """ + return list(sorted(file_list, key=FileSortKey)) + + +def normalize_paths_argument_to_list( + namespaces_or_namespace: Union[None, Path, str, Iterable[Union[Path, str]]], +) -> List[Path]: + """ + Normalizes the input argument to a list of paths. + """ + if namespaces_or_namespace is None: + return [] + if isinstance(namespaces_or_namespace, (Path, str)): + return [Path(namespaces_or_namespace)] + + def _convert(arg: Any) -> Path: + if not isinstance(arg, (str, Path)): + raise TypeError(f"Invalid type: {type(arg)}") + return Path(arg) if isinstance(arg, str) else arg + + return [_convert(arg) for arg in namespaces_or_namespace] + + +def normalize_paths_argument_to_set( + namespaces_or_namespace: Union[None, Path, str, Iterable[Union[Path, str]]], +) -> Set[Path]: + """ + Normalizes the input argument to a set of paths. + """ + return set(normalize_paths_argument_to_list(namespaces_or_namespace)) + + +# +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ + + +def _unittest_dsdl_normalize_paths_argument_to_list() -> None: + + from pytest import raises as assert_raises + + # Test with None argument + result = normalize_paths_argument_to_list(None) + assert result == [] + + # Test with single string argument + result = normalize_paths_argument_to_list("path/to/namespace") + assert result == [Path("path/to/namespace")] + + # Test with single Path argument + result = normalize_paths_argument_to_list(Path("path/to/namespace")) + assert result == [Path("path/to/namespace")] + + # Test with list of strings argument + result = normalize_paths_argument_to_list(["path/to/namespace1", "path/to/namespace2"]) + assert result == [Path("path/to/namespace1"), Path("path/to/namespace2")] + + # Test with list of Path arguments + result = normalize_paths_argument_to_list([Path("path/to/namespace1"), Path("path/to/namespace2")]) + assert result == [Path("path/to/namespace1"), Path("path/to/namespace2")] + + # Test with mixed list of strings and Path arguments + result = normalize_paths_argument_to_list(["path/to/namespace1", Path("path/to/namespace2")]) + assert result == [Path("path/to/namespace1"), Path("path/to/namespace2")] + + # Test with invalid argument type + with assert_raises(TypeError): + normalize_paths_argument_to_list(42) # type: ignore + + # Test with invalid argument type + with assert_raises(TypeError): + normalize_paths_argument_to_list([42]) # type: ignore + + +def _unittest_dsdl_normalize_paths_argument_to_set() -> None: + + from pytest import raises as assert_raises + + # Test with None argument + result = normalize_paths_argument_to_set(None) + assert result == set() + + # Test with single string argument + result = normalize_paths_argument_to_set("path/to/namespace") + assert result == {Path("path/to/namespace")} + + # Test with single Path argument + result = normalize_paths_argument_to_set(Path("path/to/namespace")) + assert result == {Path("path/to/namespace")} + + # Test with list of strings argument + result = normalize_paths_argument_to_set(["path/to/namespace1", "path/to/namespace2"]) + assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} + + # Test with list of Path arguments + result = normalize_paths_argument_to_set([Path("path/to/namespace1"), Path("path/to/namespace2")]) + assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} + + # Test with mixed list of strings and Path arguments + result = normalize_paths_argument_to_set(["path/to/namespace1", Path("path/to/namespace2")]) + assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} + + # Test with invalid argument type + with assert_raises(TypeError): + normalize_paths_argument_to_set(42) # type: ignore + + # Test with invalid argument type + with assert_raises(TypeError): + normalize_paths_argument_to_set([42]) # type: ignore diff --git a/pydsdl/_dsdl_definition.py b/pydsdl/_dsdl_definition.py index a8114da..f8f7dad 100644 --- a/pydsdl/_dsdl_definition.py +++ b/pydsdl/_dsdl_definition.py @@ -2,14 +2,16 @@ # This software is distributed under the terms of the MIT License. # Author: Pavel Kirienko -import time -from typing import Iterable, Callable, Optional, List import logging +import time from pathlib import Path -from ._error import FrontendError, InvalidDefinitionError, InternalError -from ._serializable import CompositeType, Version -from . import _parser +from typing import Callable, Iterable, List, Optional +from . import _parser +from ._data_type_builder import DataTypeBuilder +from ._dsdl import DefinitionVisitor, DsdlFileBuildable +from ._error import FrontendError, InternalError, InvalidDefinitionError +from ._serializable import CompositeType, Version _logger = logging.getLogger(__name__) @@ -23,7 +25,7 @@ def __init__(self, text: str, path: Path): super().__init__(text=text, path=Path(path)) -class DSDLDefinition: +class DSDLDefinition(DsdlFileBuildable): """ A DSDL type definition source abstracts the filesystem level details away, presenting a higher-level interface that operates solely on the level of type names, namespaces, fixed identifiers, and so on. @@ -36,15 +38,18 @@ def __init__(self, file_path: Path, root_namespace_path: Path): del file_path self._root_namespace_path = Path(root_namespace_path) del root_namespace_path - with open(self._file_path) as f: - self._text = str(f.read()) + self._text: Optional[str] = None # Checking the sanity of the root directory path - can't contain separators if CompositeType.NAME_COMPONENT_SEPARATOR in self._root_namespace_path.name: raise FileNameFormatError("Invalid namespace name", path=self._root_namespace_path) # Determining the relative path within the root namespace directory - relative_path = self._root_namespace_path.name / self._file_path.relative_to(self._root_namespace_path) + try: + relative_path = self._root_namespace_path.name / self._file_path.relative_to(self._root_namespace_path) + except ValueError: + # the file is not under the same root path so we'll have to make an assumption that the + relative_path = Path(self._root_namespace_path.name) / self._file_path.name # Parsing the basename, e.g., 434.GetTransportStatistics.0.1.dsdl basename_components = relative_path.name.split(".")[:-1] @@ -86,31 +91,24 @@ def __init__(self, file_path: Path, root_namespace_path: Path): self._cached_type: Optional[CompositeType] = None + # +-----------------------------------------------------------------------+ + # | DsdlFileBuildable :: INTERFACE | + # +-----------------------------------------------------------------------+ def read( self, - lookup_definitions: Iterable["DSDLDefinition"], + 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 is cached; all following invocations will read from the 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 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. - """ log_prefix = "%s.%d.%d" % (self.full_name, self.version.major, self.version.minor) if self._cached_type is not None: _logger.debug("%s: Cache hit", log_prefix) return self._cached_type + if not self._file_path.exists(): + raise InvalidDefinitionError("Attempt to read DSDL file that doesn't exist.", self._file_path) + started_at = time.monotonic() # Remove the target definition from the lookup list in order to prevent @@ -124,17 +122,17 @@ def read( ", ".join(set(sorted(map(lambda x: x.root_namespace, lookup_definitions)))), ) try: - builder = _data_type_builder.DataTypeBuilder( + builder = DataTypeBuilder( definition=self, lookup_definitions=lookup_definitions, + definition_visitors=definition_visitors, print_output_handler=print_output_handler, allow_unregulated_fixed_port_id=allow_unregulated_fixed_port_id, ) - with open(self.file_path) as f: - _parser.parse(f.read(), builder) - self._cached_type = builder.finalize() + _parser.parse(self.text, builder) + self._cached_type = builder.finalize() _logger.info( "%s: Processed in %.0f ms; category: %s, fixed port ID: %s", log_prefix, @@ -151,34 +149,38 @@ def read( except Exception as ex: # pragma: no cover raise InternalError(culprit=ex, path=self.file_path) from ex + # +-----------------------------------------------------------------------+ + # | DsdlFile :: INTERFACE | + # +-----------------------------------------------------------------------+ + @property + def composite_type(self) -> Optional[CompositeType]: + return self._cached_type + @property def full_name(self) -> str: - """The full name, e.g., uavcan.node.Heartbeat""" return self._name @property def name_components(self) -> List[str]: - """Components of the full name as a list, e.g., ['uavcan', 'node', 'Heartbeat']""" return self._name.split(CompositeType.NAME_COMPONENT_SEPARATOR) @property def short_name(self) -> str: - """The last component of the full name, e.g., Heartbeat of uavcan.node.Heartbeat""" return self.name_components[-1] @property def full_namespace(self) -> str: - """The full name without the short name, e.g., uavcan.node for uavcan.node.Heartbeat""" return str(CompositeType.NAME_COMPONENT_SEPARATOR.join(self.name_components[:-1])) @property def root_namespace(self) -> str: - """The first component of the full name, e.g., uavcan of uavcan.node.Heartbeat""" return self.name_components[0] @property def text(self) -> str: - """The source text in its raw unprocessed form (with comments, formatting intact, and everything)""" + if self._text is None: + with open(self._file_path) as f: + self._text = str(f.read()) return self._text @property @@ -187,7 +189,6 @@ def version(self) -> Version: @property def fixed_port_id(self) -> Optional[int]: - """Either the fixed port ID as integer, or None if not defined for this type.""" return self._fixed_port_id @property @@ -202,6 +203,12 @@ def file_path(self) -> Path: def root_namespace_path(self) -> Path: return self._root_namespace_path + # +-----------------------------------------------------------------------+ + # | Python :: SPECIAL FUNCTIONS | + # +-----------------------------------------------------------------------+ + def __hash__(self) -> int: + return hash((self.full_name, self.version)) + def __eq__(self, other: object) -> bool: """ Two definitions will compare equal if they share the same name AND version number. @@ -222,6 +229,25 @@ def __str__(self) -> str: __repr__ = __str__ -# Moved this import here to break recursive dependency. -# Maybe I have messed up the architecture? Should think about it later. -from . import _data_type_builder # pylint: disable=wrong-import-position +# +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ + + +def _unittest_dsdl_definition_read_non_existant() -> None: + from pytest import raises as expect_raises + + target = Path("root", "ns", "Target.1.1.dsdl") + target_definition = DSDLDefinition(target, target.parent) + + def print_output(line_number: int, text: str) -> None: # pragma: no cover + pass + + with expect_raises(InvalidDefinitionError): + target_definition.read([], [], print_output, True) + + +def _unittest_dsdl_definition_read_text(temp_dsdl_factory) -> None: # type: ignore + target_root = Path("root", "ns") + target_file_path = Path(target_root / "Target.1.1.dsdl") + dsdl_file = temp_dsdl_factory.new_file(target_root / target_file_path, "@sealed") + target_definition = DSDLDefinition(dsdl_file, target_root) + assert "@sealed" == target_definition.text diff --git a/pydsdl/_error.py b/pydsdl/_error.py index d301765..283486f 100644 --- a/pydsdl/_error.py +++ b/pydsdl/_error.py @@ -108,6 +108,9 @@ class InvalidDefinitionError(FrontendError): """ +# +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ + + def _unittest_error() -> None: try: raise FrontendError("Hello world!") @@ -124,8 +127,8 @@ def _unittest_error() -> None: try: raise FrontendError("Hello world!", path=Path("path/to/file.dsdl")) except Exception as ex: - assert str(ex) == "path/to/file.dsdl: Hello world!" assert repr(ex) == "FrontendError: 'path/to/file.dsdl: Hello world!'" + assert str(ex) == "path/to/file.dsdl: Hello world!" def _unittest_internal_error_github_reporting() -> None: @@ -150,7 +153,7 @@ def _unittest_internal_error_github_reporting() -> None: print(ex) assert ex.path == Path("FILE_PATH") assert ex.line == 42 - # We have to ignore the last couple of characters because Python before 3.7 reprs Exceptions like this: + # We have to ignore the last couple of characters because Python before 3.7 repr's Exceptions like this: # Exception('ERROR TEXT',) # But newer Pythons do it like this: # Exception('ERROR TEXT') diff --git a/pydsdl/_namespace.py b/pydsdl/_namespace.py index 8e9501e..dd77c43 100644 --- a/pydsdl/_namespace.py +++ b/pydsdl/_namespace.py @@ -4,13 +4,19 @@ # pylint: disable=logging-not-lazy -from typing import Iterable, Callable, DefaultDict, List, Optional, Union, Set, Dict -import logging import collections +import logging +from itertools import product, repeat from pathlib import Path -from . import _serializable -from . import _dsdl_definition -from . import _error +from typing import Callable, DefaultDict, Dict, Iterable, List, Optional, Set, Tuple, Union + +from . import _dsdl_definition, _error, _serializable +from ._dsdl import DsdlFileBuildable, PrintOutputHandler, SortedFileList +from ._dsdl import file_sort as dsdl_file_sort +from ._dsdl import normalize_paths_argument_to_list, normalize_paths_argument_to_set +from ._namespace_reader import DsdlDefinitions, read_definitions + +_logger = logging.getLogger(__name__) class RootNamespaceNameCollisionError(_error.InvalidDefinitionError): @@ -69,8 +75,13 @@ class SealingConsistencyError(_error.InvalidDefinitionError): """ -PrintOutputHandler = Callable[[Path, int, str], None] -"""Invoked when the frontend encounters a print directive or needs to output a generic diagnostic.""" +class DsdlPathInferenceError(_error.InvalidDefinitionError): + """ + Raised when the namespace, type, fixed port ID, or version cannot be inferred from a file path. + """ + + +# +--[PUBLIC API]-----------------------------------------------------------------------------------------------------+ def read_namespace( @@ -81,7 +92,7 @@ def read_namespace( allow_root_namespace_name_collision: bool = True, ) -> List[_serializable.CompositeType]: """ - This function is the main entry point of the library. + This function is a main entry point for the library. It reads all DSDL definitions from the specified root namespace directory and produces the annotated AST. :param root_namespace_directory: The path of the root namespace directory that will be read. @@ -108,48 +119,26 @@ def read_namespace( the same root namespace name multiple times in the lookup dirs. This will enable defining a namespace partially and let other entities define new messages or new sub-namespaces in the same root namespace. - :return: A list of :class:`pydsdl.CompositeType` sorted lexicographically by full data type name, - then by major version (newest version first), then by minor version (newest version first). - The ordering guarantee allows the caller to always find the newest version simply by picking - the first matching occurrence. + :return: A list of :class:`pydsdl.CompositeType` found under the `root_namespace_directory` and sorted + lexicographically by full data type name, then by major version (newest version first), then by minor + version (newest version first). The ordering guarantee allows the caller to always find the newest version + simply by picking the first matching occurrence. :raises: :class:`pydsdl.FrontendError`, :class:`MemoryError`, :class:`SystemError`, :class:`OSError` if directories do not exist or inaccessible, :class:`ValueError`/:class:`TypeError` if the arguments are invalid. """ - # Add the own root namespace to the set of lookup directories, sort lexicographically, remove duplicates. - # We'd like this to be an iterable list of strings but we handle the common practice of passing in a single path. - if lookup_directories is None: - lookup_directories_path_list: List[Path] = [] - elif isinstance(lookup_directories, (str, bytes, Path)): - lookup_directories_path_list = [Path(lookup_directories)] - else: - lookup_directories_path_list = list(map(Path, lookup_directories)) - - for a in lookup_directories_path_list: - if not isinstance(a, (str, Path)): - raise TypeError("Lookup directories shall be an iterable of paths. Found in list: " + type(a).__name__) - _logger.debug(_LOG_LIST_ITEM_PREFIX + str(a)) - # Normalize paths and remove duplicates. Resolve symlinks to avoid ambiguities. root_namespace_directory = Path(root_namespace_directory).resolve() - lookup_directories_path_list.append(root_namespace_directory) - lookup_directories_path_list = list(sorted({x.resolve() for x in lookup_directories_path_list})) - _logger.debug("Lookup directories are listed below:") - for a in lookup_directories_path_list: - _logger.debug(_LOG_LIST_ITEM_PREFIX + str(a)) - - # Check for common usage errors and warn the user if anything looks suspicious. - _ensure_no_common_usage_errors(root_namespace_directory, lookup_directories_path_list, _logger.warning) - # Check the namespaces. - _ensure_no_nested_root_namespaces(lookup_directories_path_list) - - if not allow_root_namespace_name_collision: - _ensure_no_namespace_name_collisions(lookup_directories_path_list) + lookup_directories_path_list = _construct_lookup_directories_path_list( + [root_namespace_directory], + normalize_paths_argument_to_list(lookup_directories), + allow_root_namespace_name_collision, + ) # Construct DSDL definitions from the target and the lookup dirs. - target_dsdl_definitions = _construct_dsdl_definitions_from_namespace(root_namespace_directory) + target_dsdl_definitions = _construct_dsdl_definitions_from_namespaces([root_namespace_directory]) if not target_dsdl_definitions: _logger.info("The namespace at %s is empty", root_namespace_directory) return [] @@ -157,9 +146,124 @@ def read_namespace( for x in target_dsdl_definitions: _logger.debug(_LOG_LIST_ITEM_PREFIX + str(x)) - lookup_dsdl_definitions = [] # type: List[_dsdl_definition.DSDLDefinition] - for ld in lookup_directories_path_list: - lookup_dsdl_definitions += _construct_dsdl_definitions_from_namespace(ld) + return _complete_read_function( + target_dsdl_definitions, lookup_directories_path_list, print_output_handler, allow_unregulated_fixed_port_id + ).direct + + +# pylint: disable=too-many-arguments +def read_files( + dsdl_files: Union[None, Path, str, Iterable[Union[Path, str]]], + root_namespace_directories_or_names: Union[None, Path, str, Iterable[Union[Path, str]]], + lookup_directories: Union[None, Path, str, Iterable[Union[Path, str]]] = None, + print_output_handler: Optional[PrintOutputHandler] = None, + allow_unregulated_fixed_port_id: bool = False, +) -> Tuple[List[_serializable.CompositeType], List[_serializable.CompositeType]]: + """ + This function is a main entry point for the library. + It reads all DSDL definitions from the specified ``dsdl_files`` and produces the annotated AST for these types and + the transitive closure of the types they depend on. + + :param dsdl_files: A list of paths to dsdl files to parse. + + :param root_namespace_directories_or_names: This can be a set of names of root namespaces or relative paths to + root namespaces. All ``dsdl_files`` provided must be under one of these roots. For example, given: + + .. code-block:: python + + dsdl_files = [ + Path("workspace/project/types/animals/felines/Tabby.1.0.dsdl"), + Path("workspace/project/types/animals/canines/Boxer.1.0.dsdl"), + Path("workspace/project/types/plants/trees/DouglasFir.1.0.dsdl") + ] + + + then this argument must be one of: + + .. code-block:: python + + root_namespace_directories_or_names = ["animals", "plants"] + + root_namespace_directories_or_names = [ + Path("workspace/project/types/animals"), + Path("workspace/project/types/plants") + ] + + + :param lookup_directories: List of other namespace directories containing data type definitions that are + referred to from the target dsdl files. For example, if you are reading vendor-specific types, + the list of lookup directories should always include a path to the standard root namespace ``uavcan``, + otherwise the types defined in the vendor-specific namespace won't be able to use data types from the + standard namespace. + + :param print_output_handler: If provided, this callable will be invoked when a ``@print`` directive + is encountered or when the frontend needs to emit a diagnostic; + the arguments are: path, line number (1-based), text. + If not provided, no output will be produced except for the standard Python logging subsystem + (but ``@print`` expressions will be evaluated anyway, and a failed evaluation will be a fatal error). + + :param allow_unregulated_fixed_port_id: Do not reject unregulated fixed port identifiers. + As demanded by the specification, the frontend rejects unregulated fixed port ID by default. + This is a dangerous feature that must not be used unless you understand the risks. + Please read https://opencyphal.org/guide. + + :return: A Tuple of lists of :class:`pydsdl.CompositeType`. The first index in the Tuple are the types parsed from + the ``dsdl_files`` argument. The second index are types that the target ``dsdl_files`` utilizes. + A note for using these values to describe build dependencies: each :class:`pydsdl.CompositeType` has two + fields that provide links back to the filesystem where the dsdl files were located when parsing the type; + ``source_file_path`` and ``source_file_path_to_root``. + + :raises: :class:`pydsdl.FrontendError`, :class:`MemoryError`, :class:`SystemError`, + :class:`OSError` if directories do not exist or inaccessible, + :class:`ValueError`/:class:`TypeError` if the arguments are invalid. + """ + # Normalize paths and remove duplicates. Resolve symlinks to avoid ambiguities. + target_dsdl_definitions = _construct_dsdl_definitions_from_files( + normalize_paths_argument_to_list(dsdl_files), + normalize_paths_argument_to_set(root_namespace_directories_or_names), + ) + if len(target_dsdl_definitions) == 0: + _logger.info("No DSDL files found in the specified directories") + return ([], []) + + if _logger.isEnabledFor(logging.DEBUG): # pragma: no cover + _logger.debug("Target DSDL definitions are listed below:") + + for x in target_dsdl_definitions: + _logger.debug(_LOG_LIST_ITEM_PREFIX + str(x.file_path)) + + root_namespaces = {f.root_namespace_path.resolve() for f in target_dsdl_definitions} + lookup_directories_path_list = _construct_lookup_directories_path_list( + root_namespaces, + normalize_paths_argument_to_list(lookup_directories), + True, + ) + + definitions = _complete_read_function( + target_dsdl_definitions, lookup_directories_path_list, print_output_handler, allow_unregulated_fixed_port_id + ) + + return (definitions.direct, definitions.transitive) + + +# +--[INTERNAL API::PUBLIC API HELPERS]-------------------------------------------------------------------------------+ +# These are functions called by the public API before the actual processing begins. + +DSDL_FILE_SUFFIX = ".dsdl" +DSDL_FILE_GLOB = f"*{DSDL_FILE_SUFFIX}" +DSDL_FILE_SUFFIX_LEGACY = ".uavcan" +DSDL_FILE_GLOB_LEGACY = f"*{DSDL_FILE_SUFFIX_LEGACY}" +_LOG_LIST_ITEM_PREFIX = " " * 4 + + +def _complete_read_function( + target_dsdl_definitions: SortedFileList[DsdlFileBuildable], + lookup_directories_path_list: List[Path], + print_output_handler: Optional[PrintOutputHandler], + allow_unregulated_fixed_port_id: bool, +) -> DsdlDefinitions: + + lookup_dsdl_definitions = _construct_dsdl_definitions_from_namespaces(lookup_directories_path_list) # Check for collisions against the lookup definitions also. _ensure_no_collisions(target_dsdl_definitions, lookup_dsdl_definitions) @@ -177,8 +281,9 @@ def read_namespace( ", ".join(set(sorted(map(lambda t: t.root_namespace, lookup_dsdl_definitions)))), ) - # Read the constructed definitions. - types = _read_namespace_definitions( + # This is the biggie. All the rest of the wranging is just to get to this point. This will take the + # most time and memory. + definitions = read_definitions( target_dsdl_definitions, lookup_dsdl_definitions, print_output_handler, allow_unregulated_fixed_port_id ) @@ -188,62 +293,107 @@ def read_namespace( # directories may contain issues and mistakes that are outside of the control of the user (e.g., # they could be managed by a third party) -- the user shouldn't be affected by mistakes committed # by the third party. - _ensure_no_fixed_port_id_collisions(types) - _ensure_minor_version_compatibility(types) + _ensure_no_fixed_port_id_collisions(definitions.direct) + _ensure_minor_version_compatibility(definitions.transitive + definitions.direct) - return types + return definitions -DSDL_FILE_GLOB = "*.dsdl" -DSDL_FILE_GLOB_LEGACY = "*.uavcan" -_LOG_LIST_ITEM_PREFIX = " " * 4 +def _construct_lookup_directories_path_list( + root_namespace_directories: Iterable[Path], + lookup_directories_path_list: List[Path], + allow_root_namespace_name_collision: bool, +) -> List[Path]: + """ + Intermediate transformation and validation of inputs into a list of lookup directories as paths. -_logger = logging.getLogger(__name__) + :param root_namespace_directory: The path of the root namespace directory that will be read. + For example, ``dsdl/uavcan`` to read the ``uavcan`` namespace. + + :param lookup_directories: List of other namespace directories containing data type definitions that are + referred to from the target root namespace. For example, if you are reading a vendor-specific namespace, + the list of lookup directories should always include a path to the standard root namespace ``uavcan``, + otherwise the types defined in the vendor-specific namespace won't be able to use data types from the + standard namespace. + :param allow_root_namespace_name_collision: Allow using the source root namespace name in the look up dirs or + the same root namespace name multiple times in the lookup dirs. This will enable defining a namespace + partially and let other entities define new messages or new sub-namespaces in the same root namespace. -def _read_namespace_definitions( - target_definitions: List[_dsdl_definition.DSDLDefinition], - lookup_definitions: List[_dsdl_definition.DSDLDefinition], - print_output_handler: Optional[PrintOutputHandler] = None, - allow_unregulated_fixed_port_id: bool = False, -) -> List[_serializable.CompositeType]: - """ - Construct type descriptors from the specified target definitions. - Allow the target definitions to use the lookup definitions within themselves. - :param target_definitions: Which definitions to read. - :param lookup_definitions: Which definitions can be used by the processed definitions. - :return: A list of types. + :return: A list of lookup directories as paths. + + :raises: :class:`pydsdl.FrontendError`, :class:`MemoryError`, :class:`SystemError`, + :class:`OSError` if directories do not exist or inaccessible, + :class:`ValueError`/:class:`TypeError` if the arguments are invalid. """ + # Add the own root namespace to the set of lookup directories, sort lexicographically, remove duplicates. + # We'd like this to be an iterable list of strings but we handle the common practice of passing in a single path. - def make_print_handler(definition: _dsdl_definition.DSDLDefinition) -> Callable[[int, str], None]: - def handler(line_number: int, text: str) -> None: - if print_output_handler: # pragma: no branch - assert isinstance(line_number, int) and isinstance(text, str) - assert line_number > 0, "Line numbers must be one-based" - print_output_handler(definition.file_path, line_number, text) + # Normalize paths and remove duplicates. Resolve symlinks to avoid ambiguities. + lookup_directories_path_list.extend(root_namespace_directories) + lookup_directories_path_list = list(sorted({x.resolve() for x in lookup_directories_path_list})) + _logger.debug("Lookup directories are listed below:") + for a in lookup_directories_path_list: + _logger.debug(_LOG_LIST_ITEM_PREFIX + str(a)) - return handler + # Check for common usage errors and warn the user if anything looks suspicious. + _ensure_no_common_usage_errors(root_namespace_directories, lookup_directories_path_list, _logger.warning) - types = [] # type: List[_serializable.CompositeType] - for tdd in target_definitions: - try: - dt = tdd.read(lookup_definitions, make_print_handler(tdd), allow_unregulated_fixed_port_id) - except _error.FrontendError as ex: # pragma: no cover - ex.set_error_location_if_unknown(path=tdd.file_path) - raise ex - except (MemoryError, SystemError): # pragma: no cover - raise - except Exception as ex: # pragma: no cover - raise _error.InternalError(culprit=ex, path=tdd.file_path) from ex - else: - types.append(dt) + # Check the namespaces and ensure that there are no name collisions. + _ensure_no_namespace_name_collisions_or_nested_root_namespaces( + lookup_directories_path_list, allow_root_namespace_name_collision + ) - return types + return lookup_directories_path_list + + +def _construct_dsdl_definitions_from_files( + dsdl_files: List[Path], + valid_roots: Set[Path], +) -> SortedFileList[DsdlFileBuildable]: + """ """ + output = set() # type: Set[DsdlFileBuildable] + for fp in dsdl_files: + root_namespace_path = _infer_path_to_root(fp, valid_roots) + if fp.suffix == DSDL_FILE_SUFFIX_LEGACY: + _logger.warning( + "File uses deprecated extension %r, please rename to use %r: %s", + DSDL_FILE_SUFFIX_LEGACY, + DSDL_FILE_SUFFIX, + fp, + ) + output.add(_dsdl_definition.DSDLDefinition(fp, root_namespace_path)) + + return dsdl_file_sort(output) + + +def _construct_dsdl_definitions_from_namespaces( + root_namespace_paths: List[Path], +) -> SortedFileList[DsdlFileBuildable]: + """ + Accepts a directory path, returns a sorted list of abstract DSDL file representations. Those can be read later. + The definitions are sorted by name lexicographically, then by major version (greatest version first), + then by minor version (same ordering as the major version). + """ + source_file_paths: Set[Tuple[Path, Path]] = set() # index of all file paths already found + for root_namespace_path in root_namespace_paths: + for p in root_namespace_path.rglob(DSDL_FILE_GLOB): + source_file_paths.add((p, root_namespace_path)) + for p in root_namespace_path.rglob(DSDL_FILE_GLOB_LEGACY): + source_file_paths.add((p, root_namespace_path)) + _logger.warning( + "File uses deprecated extension %r, please rename to use %r: %s", + DSDL_FILE_GLOB_LEGACY, + DSDL_FILE_GLOB, + p, + ) + + return dsdl_file_sort([_dsdl_definition.DSDLDefinition(*p) for p in source_file_paths]) def _ensure_no_collisions( - target_definitions: List[_dsdl_definition.DSDLDefinition], - lookup_definitions: List[_dsdl_definition.DSDLDefinition], + target_definitions: List[DsdlFileBuildable], + lookup_definitions: List[DsdlFileBuildable], ) -> None: for tg in target_definitions: tg_full_namespace_period = tg.full_namespace.lower() + "." @@ -375,7 +525,7 @@ def _ensure_minor_version_compatibility_pairwise( def _ensure_no_common_usage_errors( - root_namespace_directory: Path, lookup_directories: Iterable[Path], reporter: Callable[[str], None] + root_namespace_directories: Iterable[Path], lookup_directories: Iterable[Path], reporter: Callable[[str], None] ) -> None: suspicious_base_names = [ "public_regulated_data_types", @@ -391,7 +541,7 @@ def is_valid_name(s: str) -> bool: return True # resolve() will also normalize the case in case-insensitive filesystems. - all_paths = {root_namespace_directory.resolve()} | {x.resolve() for x in lookup_directories} + all_paths = {y.resolve() for y in root_namespace_directories} | {x.resolve() for x in lookup_directories} for p in all_paths: try: candidates = [x for x in p.iterdir() if x.is_dir() and is_valid_name(x.name)] @@ -408,59 +558,91 @@ def is_valid_name(s: str) -> bool: reporter(report) -def _ensure_no_nested_root_namespaces(directories: Iterable[Path]) -> None: - dirs = {x.resolve() for x in directories} # normalize the case in case-insensitive filesystems - for a in dirs: - for b in dirs: - if a.samefile(b): - continue +def _ensure_no_namespace_name_collisions_or_nested_root_namespaces( + directories: Iterable[Path], allow_name_collisions: bool +) -> None: + directories = {x.resolve() for x in directories} # normalize the case in case-insensitive filesystems + + def check_each(path_tuple_with_result: Tuple[Tuple[Path, Path], List[int]]) -> bool: + path_tuple = path_tuple_with_result[0] + if not path_tuple[0].samefile(path_tuple[1]): + if not allow_name_collisions and path_tuple[0].name.lower() == path_tuple[1].name.lower(): + return True try: - a.relative_to(b) + path_tuple[0].relative_to(path_tuple[1]) except ValueError: pass else: - raise NestedRootNamespaceError( - "The following namespace is nested inside this one, which is not permitted: %s" % a, path=b - ) - - -def _ensure_no_namespace_name_collisions(directories: Iterable[Path]) -> None: - directories = {x.resolve() for x in directories} # normalize the case in case-insensitive filesystems - for a in directories: - for b in directories: - if a.samefile(b): - continue - if a.name.lower() == b.name.lower(): - _logger.info("Collision: %r [%r] == %r [%r]", a, a.name, b, b.name) - raise RootNamespaceNameCollisionError("The name of this namespace conflicts with %s" % b, path=a) + path_tuple_with_result[1][0] = 1 + return True + return False + + # zip a list[1] of int 0 so we can assign a failure type. 0 is name collision and 1 is nested root namespace + # further cartesian checks can be added here using this pattern + + # next/filter returns the first failure or None if no failures + check_result = next(filter(check_each, zip(product(directories, directories), repeat([0]))), None) + + if check_result: + path_tuple = check_result[0] + failure_type = check_result[1][0] + if failure_type == 0: + raise RootNamespaceNameCollisionError( + "The following namespaces have the same name: %s" % path_tuple[0], path=path_tuple[1] + ) + else: + raise NestedRootNamespaceError( + "The following namespace is nested inside this one, which is not permitted: %s" % path_tuple[0], + path=path_tuple[1], + ) -def _construct_dsdl_definitions_from_namespace(root_namespace_path: Path) -> List[_dsdl_definition.DSDLDefinition]: +def _infer_path_to_root(dsdl_path: Path, valid_dsdl_roots_or_path_to_root: Set[Path]) -> Path: """ - Accepts a directory path, returns a sorted list of abstract DSDL file representations. Those can be read later. - The definitions are sorted by name lexicographically, then by major version (greatest version first), - then by minor version (same ordering as the major version). + Infer the path to the namespace root of a DSDL file path. + :param dsdl_path: The path to the alleged DSDL file. + :param valid_dsdl_roots_or_path_to_root: The set of valid root names or paths under which the type must reside. + :return The path to the root namespace directory. + :raises DsdlPathInferenceError: If the namespace root cannot be inferred from the provided information. """ - source_file_paths: Set[Path] = set() - for p in root_namespace_path.rglob(DSDL_FILE_GLOB): - source_file_paths.add(p) - for p in root_namespace_path.rglob(DSDL_FILE_GLOB_LEGACY): - source_file_paths.add(p) - _logger.warning( - "File uses deprecated extension %r, please rename to use %r: %s", DSDL_FILE_GLOB_LEGACY, DSDL_FILE_GLOB, p + if valid_dsdl_roots_or_path_to_root is None: + raise _error.InternalError("valid_dsdl_roots_or_path_to_root was None") + + if dsdl_path.is_absolute() and len(valid_dsdl_roots_or_path_to_root) == 0: + raise DsdlPathInferenceError( + f"dsdl_path ({dsdl_path}) is absolute and the provided valid root names are empty. The DSDL root of " + "an absolute path cannot be inferred without this information.", ) - output = [] # type: List[_dsdl_definition.DSDLDefinition] - for fp in sorted(source_file_paths): - dsdl_def = _dsdl_definition.DSDLDefinition(fp, root_namespace_path) - output.append(dsdl_def) + if len(valid_dsdl_roots_or_path_to_root) == 0: + # if we have no valid roots we can only infer the root of the path. We require the path to be relative + # to avoid accidental inferences given that dsdl file trees starting from a filesystem root are rare. + return Path(dsdl_path.parts[0]) + + # The strongest inference is when the path is relative to a known root. + for path_to_root in valid_dsdl_roots_or_path_to_root: + try: + _ = dsdl_path.relative_to(path_to_root) + except ValueError: + continue + return path_to_root + + # A weaker, but valid inference is when the path is a child of a known root folder name. + root_parts = {x.parts[-1] for x in valid_dsdl_roots_or_path_to_root if len(x.parts) == 1} + parts = list(dsdl_path.parent.parts) + for i, part in list(enumerate(parts)): + if part in root_parts: + return Path().joinpath(*parts[: i + 1]) + # +1 to include the root folder + raise DsdlPathInferenceError(f"No valid root found in path {str(dsdl_path)}") - # Lexicographically by name, newest version first. - return list(sorted(output, key=lambda d: (d.full_name, -d.version.major, -d.version.minor))) + +# +--[ UNIT TESTS ]---------------------------------------------------------------------------------------------------+ def _unittest_dsdl_definition_constructor() -> None: import tempfile + from ._dsdl_definition import FileNameFormatError with tempfile.TemporaryDirectory() as directory: @@ -472,9 +654,9 @@ def _unittest_dsdl_definition_constructor() -> None: (root / "nested/2.Asd.21.32.dsdl").write_text("# TEST B") (root / "nested/Foo.32.43.dsdl").write_text("# TEST C") - dsdl_defs = _construct_dsdl_definitions_from_namespace(root) + dsdl_defs = _construct_dsdl_definitions_from_namespaces([root]) print(dsdl_defs) - lut = {x.full_name: x for x in dsdl_defs} # type: Dict[str, _dsdl_definition.DSDLDefinition] + lut = {x.full_name: x for x in dsdl_defs} # type: Dict[str, DsdlFileBuildable] assert len(lut) == 3 assert str(lut["foo.Qwerty"]) == repr(lut["foo.Qwerty"]) @@ -528,7 +710,7 @@ def _unittest_dsdl_definition_constructor() -> None: (root / "nested/Malformed.MAJOR.MINOR.dsdl").touch() try: - _construct_dsdl_definitions_from_namespace(root) + _construct_dsdl_definitions_from_namespaces([root]) except FileNameFormatError as ex: print(ex) (root / "nested/Malformed.MAJOR.MINOR.dsdl").unlink() @@ -537,7 +719,7 @@ def _unittest_dsdl_definition_constructor() -> None: (root / "nested/NOT_A_NUMBER.Malformed.1.0.dsdl").touch() try: - _construct_dsdl_definitions_from_namespace(root) + _construct_dsdl_definitions_from_namespaces([root]) except FileNameFormatError as ex: print(ex) (root / "nested/NOT_A_NUMBER.Malformed.1.0.dsdl").unlink() @@ -546,26 +728,26 @@ def _unittest_dsdl_definition_constructor() -> None: (root / "nested/Malformed.dsdl").touch() try: - _construct_dsdl_definitions_from_namespace(root) + _construct_dsdl_definitions_from_namespaces([root]) except FileNameFormatError as ex: print(ex) (root / "nested/Malformed.dsdl").unlink() else: # pragma: no cover assert False - _construct_dsdl_definitions_from_namespace(root) # making sure all errors are cleared + _construct_dsdl_definitions_from_namespaces([root]) # making sure all errors are cleared (root / "nested/super.bad").mkdir() (root / "nested/super.bad/Unreachable.1.0.dsdl").touch() try: - _construct_dsdl_definitions_from_namespace(root) + _construct_dsdl_definitions_from_namespaces([root]) except FileNameFormatError as ex: print(ex) else: # pragma: no cover assert False try: - _construct_dsdl_definitions_from_namespace(root / "nested/super.bad") + _construct_dsdl_definitions_from_namespaces([root / "nested/super.bad"]) except FileNameFormatError as ex: print(ex) else: # pragma: no cover @@ -582,9 +764,9 @@ def _unittest_dsdl_definition_constructor_legacy() -> None: root = di / "foo" root.mkdir() (root / "123.Qwerty.123.234.uavcan").write_text("# TEST A") - dsdl_defs = _construct_dsdl_definitions_from_namespace(root) + dsdl_defs = _construct_dsdl_definitions_from_namespaces([root]) print(dsdl_defs) - lut = {x.full_name: x for x in dsdl_defs} # type: Dict[str, _dsdl_definition.DSDLDefinition] + lut = {x.full_name: x for x in dsdl_defs} # type: Dict[str, DsdlFileBuildable] assert len(lut) == 1 t = lut["foo.Qwerty"] assert t.file_path == root / "123.Qwerty.123.234.uavcan" @@ -609,33 +791,34 @@ def _unittest_common_usage_errors() -> None: reports = [] # type: List[str] - _ensure_no_common_usage_errors(root_ns_dir, [], reports.append) + _ensure_no_common_usage_errors([root_ns_dir], [], reports.append) assert not reports - _ensure_no_common_usage_errors(root_ns_dir, [di / "baz"], reports.append) + _ensure_no_common_usage_errors([root_ns_dir], [di / "baz"], reports.append) assert not reports dir_dsdl = root_ns_dir / "dsdl" dir_dsdl.mkdir() - _ensure_no_common_usage_errors(dir_dsdl, [di / "baz"], reports.append) + _ensure_no_common_usage_errors([dir_dsdl], [di / "baz"], reports.append) assert not reports # Because empty. dir_dsdl_vscode = dir_dsdl / ".vscode" dir_dsdl_vscode.mkdir() - _ensure_no_common_usage_errors(dir_dsdl, [di / "baz"], reports.append) + _ensure_no_common_usage_errors([dir_dsdl], [di / "baz"], reports.append) assert not reports # Because the name is not valid. dir_dsdl_uavcan = dir_dsdl / "uavcan" dir_dsdl_uavcan.mkdir() - _ensure_no_common_usage_errors(dir_dsdl, [di / "baz"], reports.append) + _ensure_no_common_usage_errors([dir_dsdl], [di / "baz"], reports.append) (rep,) = reports reports.clear() assert str(dir_dsdl_uavcan.resolve()).lower() in rep.lower() def _unittest_nested_roots() -> None: - from pytest import raises import tempfile + from pytest import raises + with tempfile.TemporaryDirectory() as directory: di = Path(directory) (di / "a").mkdir() @@ -643,13 +826,13 @@ def _unittest_nested_roots() -> None: (di / "a/b").mkdir() (di / "a/c").mkdir() (di / "aa/b").mkdir() - _ensure_no_nested_root_namespaces([]) - _ensure_no_nested_root_namespaces([di / "a"]) - _ensure_no_nested_root_namespaces([di / "a/b", di / "a/c"]) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([], True) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([di / "a"], True) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([di / "a/b", di / "a/c"], True) with raises(NestedRootNamespaceError): - _ensure_no_nested_root_namespaces([di / "a/b", di / "a"]) - _ensure_no_nested_root_namespaces([di / "aa/b", di / "a"]) - _ensure_no_nested_root_namespaces([di / "a/b", di / "aa"]) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([di / "a/b", di / "a"], True) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([di / "aa/b", di / "a"], True) + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([di / "a/b", di / "aa"], True) def _unittest_issue_71() -> None: # https://github.com/OpenCyphal/pydsdl/issues/71 @@ -663,3 +846,165 @@ def _unittest_issue_71() -> None: # https://github.com/OpenCyphal/pydsdl/issues (real / "Msg.0.1.dsdl").write_text("@sealed") assert len(read_namespace(real, [real, link])) == 1 assert len(read_namespace(link, [real, link])) == 1 + + +def _unittest_type_from_path_inference() -> None: + from pytest import raises as expect_raises + + # To determine the namespace do + + dsdl_file = Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl") + path_to_root = _infer_path_to_root(dsdl_file, {Path("/repo/uavcan")}) + namespace_parts = dsdl_file.parent.relative_to(path_to_root.parent).parts + + assert path_to_root == Path("/repo/uavcan") + assert namespace_parts == ("uavcan", "foo", "bar") + + # The simplest inference made is when relative dsdl paths are provided with no additional information. In this + # case the method assumes that the relative path is the correct and complete namespace of the type: + + # relative path + root = _infer_path_to_root(Path("uavcan/foo/bar/435.baz.1.0.dsdl"), set()) + assert root == Path("uavcan") + + # The root namespace is not inferred in an absolute path without additional data: + + with expect_raises(DsdlPathInferenceError): + _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), set()) + + with expect_raises(_error.InternalError): + _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), None) # type: ignore + + # If an absolute path is provided along with a path-to-root "hint" then the former must be relative to the + # latter: + + # dsdl file path is not contained within the root path + with expect_raises(DsdlPathInferenceError): + _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("/not-a-repo")}) + + root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("/repo/uavcan")}) + assert root == Path("/repo/uavcan") + + # The priority is given to paths that are relative to the root when both simple root names and paths are provided: + root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo"), Path("/repo/uavcan")}) + assert root == Path("/repo/uavcan") + + root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo"), Path("repo/uavcan")}) + assert root == Path("repo/uavcan") + + # Finally, the method will infer the root namespace from simple folder names if no additional information is + # provided: + + valid_roots = {Path("uavcan"), Path("cyphal")} + + # absolute dsdl path using valid roots + root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots) + assert root == Path("/repo/uavcan") + + # relative dsdl path using valid roots + root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots) + assert root == Path("repo/uavcan") + + # absolute dsdl path using valid roots but an invalid file path + with expect_raises(DsdlPathInferenceError): + _ = _infer_path_to_root(Path("/repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots) + + # relative dsdl path using valid roots but an invalid file path + with expect_raises(DsdlPathInferenceError): + _ = _infer_path_to_root(Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots) + + # relative dsdl path with invalid root fragments + invalid_root_fragments = {Path("cyphal", "acme")} + with expect_raises(DsdlPathInferenceError): + _ = _infer_path_to_root(Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), invalid_root_fragments) + + # In this example, foo/bar might look like a valid root path but it is not relative to repo/uavcan/foo/bar and is + # not considered after relative path inference has failed because it is not a simple root name. + root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo/bar"), Path("foo")}) + assert root == Path("repo/uavcan/foo") + + # when foo/bar is placed within the proper, relative path it is considered as a valid root and is preferred over + # the simple root name "foo": + root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("repo/uavcan/foo/bar"), Path("foo")}) + assert root == Path("repo/uavcan/foo/bar") + + +def _unittest_type_read_files_example(temp_dsdl_factory) -> None: # type: ignore + + # let's test the comments for the read function + dsdl_files = [ + Path("workspace/project/types/animals/felines/Tabby.1.0.uavcan"), # keep .uavcan to cover the warning + Path("workspace/project/types/animals/canines/Boxer.1.0.dsdl"), + Path("workspace/project/types/plants/trees/DouglasFir.1.0.dsdl"), + ] + + dsdl_files_abs = [] + root_namespace_paths = set() + for dsdl_file in dsdl_files: + dsdl_files_abs.append(temp_dsdl_factory.new_file(dsdl_file, "@sealed")) + root_namespace_paths.add(temp_dsdl_factory.base_dir / dsdl_file.parent.parent) + root_namespace_directories_or_names_simple = ["animals", "plants"] + + direct, transitive = read_files(dsdl_files_abs, root_namespace_directories_or_names_simple) + + assert len(direct) == len(dsdl_files) + assert len(transitive) == 0 + + for direct_type in direct: + assert direct_type.root_namespace in root_namespace_directories_or_names_simple + assert direct_type.source_file_path_to_root in root_namespace_paths + + direct, _ = read_files(dsdl_files_abs, root_namespace_paths) + + assert len(direct) == len(dsdl_files) + + for direct_type in direct: + assert direct_type.root_namespace in root_namespace_directories_or_names_simple + assert direct_type.source_file_path_to_root in root_namespace_paths + + +def _unittest_targets_found_in_lookup_namespaces(temp_dsdl_factory) -> None: # type: ignore + + # call read_files with a list of dsdl files which are also located in the provided lookup namespaces + + plant_1_0 = Path("types/plants/Plant.1.0.dsdl") + tree_1_0 = Path("types/plants/trees/Tree.1.0.dsdl") + douglas_fir_1_0 = Path("types/plants/trees/DouglasFir.1.0.dsdl") + + plant_file = temp_dsdl_factory.new_file(plant_1_0, "@sealed\n") + test_files = [ + temp_dsdl_factory.new_file(tree_1_0, "@sealed\nplants.Plant.1.0 plt\n"), + temp_dsdl_factory.new_file(douglas_fir_1_0, "@sealed\nplants.trees.Tree.1.0 tree\n"), + ] + lookup_dirs = [plant_file.parent] + + direct, transitive = read_files(test_files, lookup_dirs) + + assert len(direct) == len(test_files) + assert len(transitive) == 1 + + +def _unittest_read_files_empty_args() -> None: + direct, transitive = read_files([], []) + + assert len(direct) == 0 + assert len(transitive) == 0 + + +def _unittest_ensure_no_collisions(temp_dsdl_factory) -> None: # type: ignore + from pytest import raises as expect_raises + + # gratuitous coverage of the collision check where other tests don't cover some edge cases + _ensure_no_namespace_name_collisions_or_nested_root_namespaces([], False) + + with expect_raises(DataTypeNameCollisionError): + _ensure_no_collisions( + [_dsdl_definition.DSDLDefinition(Path("a/b.1.0.dsdl"), Path("a"))], + [_dsdl_definition.DSDLDefinition(Path("a/B.1.0.dsdl"), Path("a"))], + ) + + with expect_raises(DataTypeNameCollisionError): + _ensure_no_collisions( + [_dsdl_definition.DSDLDefinition(Path("a/b/c.1.0.dsdl"), Path("a"))], + [_dsdl_definition.DSDLDefinition(Path("a/b.1.0.dsdl"), Path("a"))], + ) diff --git a/pydsdl/_namespace_reader.py b/pydsdl/_namespace_reader.py new file mode 100644 index 0000000..63180de --- /dev/null +++ b/pydsdl/_namespace_reader.py @@ -0,0 +1,390 @@ +# Copyright (C) OpenCyphal Development Team +# Copyright Amazon.com Inc. or its affiliates. +# SPDX-License-Identifier: MIT + + +import functools +import logging +from pathlib import Path +from typing import Dict, List, NamedTuple, Optional, Set, cast + +from ._dsdl import DsdlFile, DsdlFileBuildable, PrintOutputHandler, SortedFileList +from ._dsdl import file_sort as dsdl_file_sort +from ._error import FrontendError, InternalError +from ._serializable._composite import CompositeType + + +# pylint: disable=too-many-arguments +def _read_definitions( + target_definitions: SortedFileList[DsdlFileBuildable], + lookup_definitions: SortedFileList[DsdlFileBuildable], + print_output_handler: Optional[PrintOutputHandler], + allow_unregulated_fixed_port_id: bool, + direct: Set[CompositeType], + transitive: Set[CompositeType], + file_pool: Dict[Path, DsdlFileBuildable], + level: int, +) -> None: + """ + Don't look at me! I'm hideous! + (recursive method with a lot of arguments. See read_definitions for documentation) + """ + + _pending_definitions: Set[DsdlFileBuildable] = set() + + def callback(_: DsdlFile, dependent_type: DsdlFileBuildable) -> None: + if dependent_type.file_path not in file_pool: + _pending_definitions.add(dependent_type) + + def print_handler(file: Path, line: int, message: str) -> None: + if print_output_handler is not None: + print_output_handler(file, line, message) + + for target_definition in target_definitions: + + if not isinstance(target_definition, DsdlFileBuildable): + raise TypeError("Expected DsdlFileBuildable, got: " + type(target_definition).__name__) + + target_definition = file_pool.setdefault(target_definition.file_path, target_definition) + # make sure we are working with the same object for a given file path + + if target_definition.composite_type is not None and ( + target_definition.composite_type in direct or target_definition.composite_type in transitive + ): + logging.debug("Skipping target file %s because it has already been processed", target_definition.file_path) + if level == 0 and target_definition.composite_type in transitive: + # promote to direct + transitive.remove(target_definition.composite_type) + direct.add(target_definition.composite_type) + continue + + try: + new_composite_type = target_definition.read( + lookup_definitions, + [callback], + functools.partial(print_handler, target_definition.file_path), + allow_unregulated_fixed_port_id, + ) + except FrontendError as ex: # pragma: no cover + ex.set_error_location_if_unknown(path=target_definition.file_path) + raise ex + except Exception as ex: # pragma: no cover + raise InternalError(culprit=ex, path=target_definition.file_path) from ex + + if level == 0: + + direct.add(new_composite_type) + try: + transitive.remove(new_composite_type) + except KeyError: + pass + else: + transitive.add(new_composite_type) + + if len(_pending_definitions) > 0: + _read_definitions( + dsdl_file_sort(_pending_definitions), + lookup_definitions, + print_output_handler, + allow_unregulated_fixed_port_id, + direct, + transitive, + file_pool, + level + 1, + ) + _pending_definitions.clear() + + +# +---[FILE: PUBLIC]--------------------------------------------------------------------------------------------------+ + +DsdlDefinitions = NamedTuple( + "DsdlDefinitions", [("direct", SortedFileList[CompositeType]), ("transitive", SortedFileList[CompositeType])] +) +""" +Common DSDL definition set including the direct dependencies requested and the transitive dependencies found. The former +and latter sets will be disjoint. +""" + + +def read_definitions( + target_definitions: SortedFileList[DsdlFileBuildable], + lookup_definitions: SortedFileList[DsdlFileBuildable], + print_output_handler: Optional[PrintOutputHandler], + allow_unregulated_fixed_port_id: bool, +) -> DsdlDefinitions: + """ + Given a set of DSDL files, this method reads the text and invokes the parser for each and for any files found in the + lookup set where these are used by the target set. + + :param target_definitions: List of definitions to read. + :param lookup_definitions: List of definitions available for referring to. + :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. + :raises InvalidDefinitionError: If a dependency is missing. + :raises InternalError: If an unexpected error occurs. + """ + _direct: Set[CompositeType] = set() + _transitive: Set[CompositeType] = set() + _file_pool: Dict[Path, DsdlFileBuildable] = {} + _read_definitions( + target_definitions, + lookup_definitions, + print_output_handler, + allow_unregulated_fixed_port_id, + _direct, + _transitive, + _file_pool, + 0, + ) + return DsdlDefinitions( + dsdl_file_sort(_direct), + dsdl_file_sort(_transitive), + ) + + +# +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ + + +def _unittest_namespace_reader_read_definitions(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + target = temp_dsdl_factory.new_file(Path("root", "ns", "Target.1.1.dsdl"), "@sealed") + target_definitions = [cast(DsdlFileBuildable, _dsdl_definition.DSDLDefinition(target, target.parent))] + lookup_definitions: List[DsdlFileBuildable] = [] + + read_definitions(target_definitions, lookup_definitions, None, True) + + +def _unittest_namespace_reader_read_definitions_multiple(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "Target.1.1.dsdl"), "@sealed\nns.Aisle.1.0 paper_goods\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "Target.2.0.dsdl"), "@sealed\nns.Aisle.2.0 paper_goods\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "Walmart.2.4.dsdl"), "@sealed\nns.Aisle.1.0 paper_goods\n"), + ] + aisles = [ + temp_dsdl_factory.new_file(Path("root", "ns", "Aisle.1.0.dsdl"), "@sealed"), + temp_dsdl_factory.new_file(Path("root", "ns", "Aisle.2.0.dsdl"), "@sealed"), + temp_dsdl_factory.new_file(Path("root", "ns", "Aisle.3.0.dsdl"), "@sealed"), + ] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(a, a.parent) for a in aisles], + None, + True, + ) + + assert len(definitions.direct) == 3 + assert len(definitions.transitive) == 2 + + +def _unittest_namespace_reader_read_definitions_multiple_no_load(temp_dsdl_factory) -> None: # type: ignore + """ + Ensure that the loader does not load files that are not in the transitive closure of the target files. + """ + from . import _dsdl_definition + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "Adams.1.0.dsdl"), "@sealed\nns.Tacoma.1.0 volcano\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "Hood.1.0.dsdl"), "@sealed\nns.Rainer.1.0 volcano\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "StHelens.2.1.dsdl"), "@sealed\nns.Baker.1.0 volcano\n"), + ] + dependencies = [ + temp_dsdl_factory.new_file(Path("root", "ns", "Tacoma.1.0.dsdl"), "@sealed"), + temp_dsdl_factory.new_file(Path("root", "ns", "Rainer.1.0.dsdl"), "@sealed"), + temp_dsdl_factory.new_file(Path("root", "ns", "Baker.1.0.dsdl"), "@sealed"), + Path( + "root", "ns", "Shasta.1.0.dsdl" + ), # since this isn't in the transitive closure of target dependencies it will + # never be read thus it will not be an error that it does not exist. + ] + + target_definitions = [cast(DsdlFileBuildable, _dsdl_definition.DSDLDefinition(t, t.parent)) for t in targets] + lookup_definitions = [cast(DsdlFileBuildable, _dsdl_definition.DSDLDefinition(a, a.parent)) for a in dependencies] + _ = read_definitions( + target_definitions, + lookup_definitions, + None, + True, + ) + + # make sure Shasta.1.0 was never accessed but Tacoma 1.0 was + last_item = lookup_definitions[-1] + assert isinstance(last_item, _dsdl_definition.DSDLDefinition) + assert last_item._text is None # pylint: disable=protected-access + assert lookup_definitions[0].composite_type is not None + + # Make sure text is cached. + assert lookup_definitions[0].text == lookup_definitions[0].text + + +def _unittest_namespace_reader_read_definitions_promotion(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + user_1_0 = temp_dsdl_factory.new_file(Path("root", "ns", "User.1.0.dsdl"), "@sealed\n") + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "User.2.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + user_1_0, + ] + lookups = [user_1_0] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(l, l.parent) for l in lookups], + None, + True, + ) + + assert len(definitions.direct) == 2 + assert len(definitions.transitive) == 0 + + +def _unittest_namespace_reader_read_definitions_no_demote(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + user_1_0 = temp_dsdl_factory.new_file(Path("root", "ns", "User.1.0.dsdl"), "@sealed\n") + targets = [ + user_1_0, + temp_dsdl_factory.new_file(Path("root", "ns", "User.2.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + ] + lookups = [user_1_0] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(l, l.parent) for l in lookups], + None, + True, + ) + + assert len(definitions.direct) == 2 + assert len(definitions.transitive) == 0 + + +def _unittest_namespace_reader_read_definitions_no_promote(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "User.2.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "User.3.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + ] + lookups = [temp_dsdl_factory.new_file(Path("root", "ns", "User.1.0.dsdl"), "@sealed\n")] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(l, l.parent) for l in lookups], + None, + True, + ) + + assert len(definitions.direct) == 2 + assert len(definitions.transitive) == 1 + + +def _unittest_namespace_reader_read_definitions_twice(temp_dsdl_factory) -> None: # type: ignore + from . import _dsdl_definition + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "User.2.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "User.2.0.dsdl"), "@sealed\nns.User.1.0 old_guy\n"), + ] + lookups = [temp_dsdl_factory.new_file(Path("root", "ns", "User.1.0.dsdl"), "@sealed\n")] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(l, l.parent) for l in lookups], + None, + True, + ) + + assert len(definitions.direct) == 1 + assert len(definitions.transitive) == 1 + + +def _unittest_namespace_reader_read_definitions_missing_dependency(temp_dsdl_factory) -> None: # type: ignore + """ + Verify that an error is raised when a dependency is missing. + """ + from pytest import raises as assert_raises + + from . import _dsdl_definition + from ._data_type_builder import UndefinedDataTypeError + + with assert_raises(UndefinedDataTypeError): + read_definitions( + [ + _dsdl_definition.DSDLDefinition( + f := temp_dsdl_factory.new_file( + Path("root", "ns", "Cat.1.0.dsdl"), "@sealed\nns.Birman.1.0 fluffy\n" + ), + f.parent, + ) + ], + [], + None, + True, + ) + + +def _unittest_namespace_reader_read_definitions_target_in_lookup(temp_dsdl_factory) -> None: # type: ignore + """ + Ensure the direct and transitive sets are disjoint. + """ + from . import _dsdl_definition + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "Ontario.1.0.dsdl"), "@sealed\nns.NewBrunswick.1.0 place\n"), + temp_dsdl_factory.new_file(Path("root", "ns", "NewBrunswick.1.0.dsdl"), "@sealed"), + ] + lookup = [ + temp_dsdl_factory.new_file(Path("root", "ns", "NewBrunswick.1.0.dsdl"), "@sealed"), + ] + + definitions = read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [_dsdl_definition.DSDLDefinition(l, l.parent) for l in lookup], + None, + True, + ) + + assert len(definitions.direct) == 2 + assert len(definitions.transitive) == 0 + + +def _unittest_namespace_reader_read_defs_target_dont_allow_unregulated(temp_dsdl_factory) -> None: # type: ignore + """ + Ensure that an error is raised when an invalid, fixed port ID is used without an override. + """ + from pytest import raises as assert_raises + + from . import _dsdl_definition + from ._data_type_builder import UnregulatedFixedPortIDError + + targets = [ + temp_dsdl_factory.new_file(Path("root", "ns", "845.Lice.1.0.dsdl"), "@sealed\n"), + ] + + with assert_raises(UnregulatedFixedPortIDError): + read_definitions( + [_dsdl_definition.DSDLDefinition(t, t.parent) for t in targets], + [], + None, + False, + ) + + +def _unittest_namespace_reader_type_error() -> None: + + from pytest import raises as assert_raises + + from . import _dsdl_definition + + with assert_raises(TypeError): + read_definitions( + [""], # type: ignore + [], + None, + True, + ) diff --git a/pydsdl/_parser.py b/pydsdl/_parser.py index b73a533..4eda81a 100644 --- a/pydsdl/_parser.py +++ b/pydsdl/_parser.py @@ -8,7 +8,7 @@ import functools import fractions from pathlib import Path -from typing import List, Tuple +from typing import cast, List, Tuple import parsimonious from parsimonious.nodes import Node as _Node from . import _error @@ -263,11 +263,11 @@ def visit_type_version_specifier(self, _n: _Node, children: _Children) -> _seria return _serializable.Version(major=major.as_native_integer(), minor=minor.as_native_integer()) def visit_type_primitive_truncated(self, _n: _Node, children: _Children) -> _serializable.PrimitiveType: - _kw, _sp, cons = children # type: _Node, _Node, _PrimitiveTypeConstructor + _kw, _sp, cons = cast(Tuple[_Node, _Node, _PrimitiveTypeConstructor], children) return cons(_serializable.PrimitiveType.CastMode.TRUNCATED) def visit_type_primitive_saturated(self, _n: _Node, children: _Children) -> _serializable.PrimitiveType: - _, cons = children # type: _Node, _PrimitiveTypeConstructor + _, cons = cast(Tuple[_Node, _PrimitiveTypeConstructor], children) return cons(_serializable.PrimitiveType.CastMode.SATURATED) def visit_type_primitive_name_boolean(self, _n: _Node, _c: _Children) -> _PrimitiveTypeConstructor: diff --git a/pydsdl/_serializable/_composite.py b/pydsdl/_serializable/_composite.py index 6ed71da..1153cc7 100644 --- a/pydsdl/_serializable/_composite.py +++ b/pydsdl/_serializable/_composite.py @@ -5,17 +5,15 @@ import abc import math import typing -import itertools from pathlib import Path -from .. import _expression -from .. import _port_id_ranges + +from .. import _expression, _port_id_ranges from .._bit_length_set import BitLengthSet +from ._attribute import Attribute, Constant, Field, PaddingField +from ._name import InvalidNameError, check_name +from ._primitive import PrimitiveType, UnsignedIntegerType from ._serializable import SerializableType, TypeParameterError -from ._attribute import Attribute, Field, PaddingField, Constant -from ._name import check_name, InvalidNameError from ._void import VoidType -from ._primitive import PrimitiveType, UnsignedIntegerType - Version = typing.NamedTuple("Version", [("major", int), ("minor", int)]) @@ -55,7 +53,7 @@ class CompositeType(SerializableType): MAX_VERSION_NUMBER = 255 NAME_COMPONENT_SEPARATOR = "." - def __init__( # pylint: disable=too-many-arguments + def __init__( # pylint: disable=too-many-arguments, too-many-locals self, name: str, version: Version, @@ -97,9 +95,26 @@ def __init__( # pylint: disable=too-many-arguments "Name is too long: %r is longer than %d characters" % (self._name, self.MAX_NAME_LENGTH) ) - for component in self._name.split(self.NAME_COMPONENT_SEPARATOR): + self._name_components = self._name.split(self.NAME_COMPONENT_SEPARATOR) + for component in self._name_components: check_name(component) + def search_up_for_root(path: Path, namespace_components: typing.List[str]) -> Path: + if namespace_components[-1] != path.stem: + raise InvalidNameError( + f"{path.stem} != {namespace_components[-1]}. Source file directory structure " + f"is not consistent with the type's namespace ({self._name_components}, " + f"{self._source_file_path})" + ) + if len(namespace_components) == 1: + return path + return search_up_for_root(path.parent, namespace_components[:-1]) + + self._path_to_root_namespace = search_up_for_root( + self._source_file_path.parent, + (self.namespace_components if not self._has_parent_service else self.namespace_components[:-1]), + ) + # Version check version_valid = ( (0 <= self._version.major <= self.MAX_VERSION_NUMBER) @@ -148,7 +163,12 @@ def full_name(self) -> str: @property def name_components(self) -> typing.List[str]: """Components of the full name as a list, e.g., ``['uavcan', 'node', 'Heartbeat']``.""" - return self._name.split(CompositeType.NAME_COMPONENT_SEPARATOR) + return self._name_components + + @property + def namespace_components(self) -> typing.List[str]: + """Components of the namspace as a list, e.g., ``['uavcan', 'node']``.""" + return self._name_components[:-1] @property def short_name(self) -> str: @@ -163,7 +183,7 @@ def doc(self) -> str: @property def full_namespace(self) -> str: """The full name without the short name, e.g., ``uavcan.node`` for ``uavcan.node.Heartbeat``.""" - return str(CompositeType.NAME_COMPONENT_SEPARATOR.join(self.name_components[:-1])) + return str(CompositeType.NAME_COMPONENT_SEPARATOR.join(self.namespace_components)) @property def root_namespace(self) -> str: @@ -239,10 +259,30 @@ def has_fixed_port_id(self) -> bool: @property def source_file_path(self) -> Path: """ - For synthesized types such as service request/response sections, this property is defined as an empty string. + The path to the dsdl file from which this type was read. + For synthesized types such as service request/response sections, this property is the path to the service type + since request and response types are defined within the service type's dsdl file. """ return self._source_file_path + @property + def source_file_path_to_root(self) -> Path: + """ + The path to the folder that is the root namespace folder for the `source_file_path` this type was read from. + The `source_file_path` will always be relative to the `source_file_path_to_root` but not all types that share + the same `root_namespace` will have the same path to their root folder since types may be contributed to a + root namespace from several different file trees. For example: + + ``` + path0 = "workspace_0/project_a/types/animal/feline/Tabby.1.0.dsdl" + path1 = "workspace_1/project_b/types/animal/canine/Boxer.1.0.dsdl" + ``` + + In these examples path0 and path1 will produce composite types with `animal` as the root namespace but both + with have different `source_file_path_to_root` paths. + """ + return self._path_to_root_namespace + @property def alignment_requirement(self) -> int: # This is more general than required by the Specification, but it is done this way in case if we decided @@ -681,19 +721,25 @@ def iterate_fields_with_offsets( raise TypeError("Service types do not have serializable fields. Use either request or response.") +# +--[UNIT TESTS]-----------------------------------------------------------------------------------------------------+ + + def _unittest_composite_types() -> None: # pylint: disable=too-many-statements + from typing import Optional + from pytest import raises - from ._primitive import SignedIntegerType, FloatType + from ._array import FixedLengthArrayType, VariableLengthArrayType + from ._primitive import FloatType, SignedIntegerType - def try_name(name: str) -> CompositeType: + def try_name(name: str, file_path: Optional[Path] = None) -> CompositeType: return StructureType( name=name, version=Version(0, 1), attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=file_path or Path(*name.split(".")), has_parent_service=False, ) @@ -721,6 +767,9 @@ def try_name(name: str) -> CompositeType: with raises(InvalidNameError, match="(?i).*cannot contain.*"): try_name("namespace.n-s.T") + with raises(InvalidNameError, match=".*Source file directory structure is not consistent.*"): + try_name("a.Foo", Path("foo/bar/b/Foo.0.1.dsdl")) + assert try_name("root.nested.T").full_name == "root.nested.T" assert try_name("root.nested.T").full_namespace == "root.nested" assert try_name("root.nested.T").root_namespace == "root" @@ -733,7 +782,7 @@ def try_name(name: str) -> CompositeType: attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("a", "A"), has_parent_service=False, ) @@ -748,7 +797,7 @@ def try_name(name: str) -> CompositeType: ], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("a", "A"), has_parent_service=False, ) @@ -762,7 +811,7 @@ def try_name(name: str) -> CompositeType: ], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("uavcan", "node", "Heartbeat"), has_parent_service=False, ) assert u["a"].name == "a" @@ -788,7 +837,7 @@ def try_name(name: str) -> CompositeType: ], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("a", "A"), has_parent_service=False, ) assert s["a"].name == "a" @@ -847,7 +896,7 @@ def try_union_fields(field_types: typing.List[SerializableType]) -> UnionType: attributes=atr, deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("a") / "A", has_parent_service=False, ) @@ -891,7 +940,7 @@ def try_union_fields(field_types: typing.List[SerializableType]) -> UnionType: # The reference values for the following test are explained in the array tests above tu8 = UnsignedIntegerType(8, cast_mode=PrimitiveType.CastMode.TRUNCATED) small = VariableLengthArrayType(tu8, 2) - outer = FixedLengthArrayType(small, 2) # unpadded bit length values: {4, 12, 20, 28, 36} + outer = FixedLengthArrayType(small, 2) # un-padded bit length values: {4, 12, 20, 28, 36} # Above plus one bit to each, plus 16-bit for the unsigned integer field assert try_union_fields( @@ -912,7 +961,7 @@ def try_struct_fields(field_types: typing.List[SerializableType]) -> StructureTy attributes=atr, deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("a") / "A", has_parent_service=False, ) @@ -944,9 +993,12 @@ def try_struct_fields(field_types: typing.List[SerializableType]) -> StructureTy def _unittest_field_iterators() -> None: # pylint: disable=too-many-locals + import itertools + from pytest import raises - from ._primitive import BooleanType, FloatType + from ._array import FixedLengthArrayType, VariableLengthArrayType + from ._primitive import BooleanType, FloatType saturated = PrimitiveType.CastMode.SATURATED _seq_no = 0 @@ -960,7 +1012,7 @@ def make_type(meta: typing.Type[CompositeType], attributes: typing.Iterable[Attr attributes=attributes, deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("fake_root") / "ns" / f"Type{str(_seq_no)}", has_parent_service=False, ) @@ -1228,7 +1280,7 @@ def validate_iterator( attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("ns", "S_1_0.dsdl"), has_parent_service=True, ), response=StructureType( @@ -1237,7 +1289,7 @@ def validate_iterator( attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("ns", "S_1_0.dsdl"), has_parent_service=True, ), fixed_port_id=None, @@ -1251,7 +1303,7 @@ def validate_iterator( attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("ns", "XX_1_0.dsdl"), has_parent_service=True, ), response=StructureType( @@ -1260,8 +1312,31 @@ def validate_iterator( attributes=[], deprecated=True, fixed_port_id=None, - source_file_path=Path(), - has_parent_service=False, + source_file_path=Path("ns", "XX_1_0.dsdl"), + has_parent_service=True, + ), + fixed_port_id=None, + ) + + with raises(ValueError): # Request/response consistency error (internal failure) + ServiceType( + request=StructureType( + name="ns.XX.Request", + version=Version(1, 0), + attributes=[], + deprecated=False, + fixed_port_id=None, + source_file_path=Path("ns", "XX_1_0.dsdl"), + has_parent_service=True, + ), + response=StructureType( + name="ns.XX.Response", + version=Version(1, 0), + attributes=[], + deprecated=False, + fixed_port_id=None, + source_file_path=Path("ns", "YY_1_0.dsdl"), + has_parent_service=True, ), fixed_port_id=None, ) @@ -1273,7 +1348,7 @@ def validate_iterator( attributes=[], deprecated=False, fixed_port_id=None, - source_file_path=Path(), + source_file_path=Path("e", "E_0_1.dsdl"), has_parent_service=False, ) validate_iterator(e, []) diff --git a/pydsdl/_test.py b/pydsdl/_test.py index 3e4048f..ca9c373 100644 --- a/pydsdl/_test.py +++ b/pydsdl/_test.py @@ -2,6 +2,7 @@ # This software is distributed under the terms of the MIT License. # Author: Pavel Kirienko +# cSpell: words iceb # pylint: disable=global-statement,protected-access,too-many-statements,consider-using-with,redefined-outer-name import tempfile @@ -62,6 +63,7 @@ def parse_definition( ) -> _serializable.CompositeType: return definition.read( lookup_definitions, + [], print_output_handler=lambda line, text: print("Output from line %d:" % line, text), allow_unregulated_fixed_port_id=False, ) @@ -314,7 +316,7 @@ def _unittest_comments(wrkspc: Workspace) -> None: uint8 CHARACTER = '#' # comment on constant int8 a # comment on field - int8 aprime + int8 a_prime @assert 1 == 1 # toss one in for confusion void2 # comment on padding field saturated int64[<33] b @@ -422,7 +424,7 @@ def _unittest_error(wrkspc: Workspace) -> None: def standalone(rel_path: str, definition: str, allow_unregulated: bool = False) -> _serializable.CompositeType: return wrkspc.parse_new(rel_path, definition + "\n").read( - [], lambda *_: None, allow_unregulated + [], [], lambda *_: None, allow_unregulated ) # pragma: no branch with raises(_error.InvalidDefinitionError, match="(?i).*port ID.*"): @@ -754,20 +756,20 @@ def print_handler(line_number: int, text: str) -> None: wrkspc.parse_new( "ns/A.1.0.dsdl", "# line number 1\n" "# line number 2\n" "@print 2 + 2 == 4 # line number 3\n" "# line number 4\n" "@sealed\n", - ).read([], print_handler, False) + ).read([], [], print_handler, False) assert printed_items assert printed_items[0] == 3 assert printed_items[1] == "true" - wrkspc.parse_new("ns/B.1.0.dsdl", "@print false\n@sealed").read([], print_handler, False) + wrkspc.parse_new("ns/B.1.0.dsdl", "@print false\n@sealed").read([], [], print_handler, False) assert printed_items assert printed_items[0] == 1 assert printed_items[1] == "false" wrkspc.parse_new( "ns/Offset.1.0.dsdl", "@print _offset_ # Not recorded\n" "uint8 a\n" "@print _offset_\n" "@extent 800\n" - ).read([], print_handler, False) + ).read([], [], print_handler, False) assert printed_items assert printed_items[0] == 3 assert printed_items[1] == "{8}" diff --git a/setup.cfg b/setup.cfg index 8d934ba..c58e783 100644 --- a/setup.cfg +++ b/setup.cfg @@ -35,7 +35,7 @@ include = pydsdl* # -------------------------------------------------- PYTEST -------------------------------------------------- [tool:pytest] -testpaths = pydsdl +testpaths = pydsdl test norecursedirs = third_party python_files = *.py python_classes = _UnitTest diff --git a/test/test_public_types.py b/test/test_public_types.py new file mode 100644 index 0000000..8463f97 --- /dev/null +++ b/test/test_public_types.py @@ -0,0 +1,47 @@ +# Copyright (C) OpenCyphal Development Team +# Copyright Amazon.com Inc. or its affiliates. +# SPDX-License-Identifier: MIT + +# pylint: disable=redefined-outer-name +# pylint: disable=logging-fstring-interpolation +import cProfile +import io +import pstats +from pathlib import Path +from pstats import SortKey + +import pydsdl + + +def _unittest_public_types_namespaces(public_types: Path) -> None: + """ + Sanity check to ensure that the public types can be read. This also allows us to debug + against a real dataset. + """ + pr = cProfile.Profile() + pr.enable() + _ = pydsdl.read_namespace(public_types) + pr.disable() + s = io.StringIO() + sortby = SortKey.TIME + ps = pstats.Stats(pr, stream=s).sort_stats(sortby) + ps.print_stats() + print(s.getvalue()) + + +def _unittest_public_types_files(public_types: Path) -> None: + """ + Sanity check to ensure that the public types can be read. This also allows us to debug + against a real dataset. + """ + pr = cProfile.Profile() + pr.enable() + node_types = list(public_types.glob("node/**/*.dsdl")) + assert len(node_types) > 0 + _ = pydsdl.read_files(node_types, {public_types}) + pr.disable() + s = io.StringIO() + sortby = SortKey.TIME + ps = pstats.Stats(pr, stream=s).sort_stats(sortby) + ps.print_stats() + print(s.getvalue()) From 9eb4725d3651c31463597242351df72b08406d94 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Wed, 19 Jun 2024 10:34:12 -0700 Subject: [PATCH 02/11] Update docs/requirements.txt Co-authored-by: Pavel Kirienko --- docs/requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 3532611..6ac611d 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,3 +1,3 @@ -sphinx >= 5.0 -sphinx_rtd_theme >= 1.0.0 +sphinx == 7.3.7 +sphinx_rtd_theme == 2.0.0 sphinx-computron >= 0.2, < 2.0 From a44b5ea8a1622febb66d057515d3a942a7523dd1 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Wed, 19 Jun 2024 10:34:21 -0700 Subject: [PATCH 03/11] Update pydsdl/_data_type_builder.py Co-authored-by: Pavel Kirienko --- pydsdl/_data_type_builder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pydsdl/_data_type_builder.py b/pydsdl/_data_type_builder.py index 63b3ba4..2769311 100644 --- a/pydsdl/_data_type_builder.py +++ b/pydsdl/_data_type_builder.py @@ -198,7 +198,6 @@ 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) From 6fc0ad8c9a8cb1c8390a54452ce369ba018d2155 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 20 Jun 2024 11:16:43 -0700 Subject: [PATCH 04/11] Update pydsdl/_dsdl.py Co-authored-by: Pavel Kirienko --- pydsdl/_dsdl.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pydsdl/_dsdl.py b/pydsdl/_dsdl.py index 4e2f83b..0c8654c 100644 --- a/pydsdl/_dsdl.py +++ b/pydsdl/_dsdl.py @@ -140,11 +140,8 @@ def read( 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 get_definition_ordering_rank(d: DSDLFile | CompositeType) -> tuple[str, int, int]: + return d.full_name, -d.version.major, -d.version.minor def file_sort(file_list: Iterable[SortedFileT]) -> SortedFileList[SortedFileT]: From 8f97fb4af09512f90a504152da77e113c24308a0 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 20 Jun 2024 14:49:42 -0700 Subject: [PATCH 05/11] All comments except mypy futures. --- conftest.py | 8 -- pydsdl/_data_type_builder.py | 2 +- pydsdl/_dsdl.py | 84 +++++-------- pydsdl/_dsdl_definition.py | 226 +++++++++++++++++++++++++++++++++-- pydsdl/_namespace.py | 145 ++-------------------- pydsdl/_namespace_reader.py | 11 +- setup.cfg | 2 +- test/test_public_types.py | 47 -------- 8 files changed, 264 insertions(+), 261 deletions(-) delete mode 100644 test/test_public_types.py diff --git a/conftest.py b/conftest.py index d73f2d7..fbc7eb1 100644 --- a/conftest.py +++ b/conftest.py @@ -61,11 +61,3 @@ def temp_dsdl_factory(request: pytest.FixtureRequest) -> Any: # pylint: disable request.addfinalizer(f._test_path_finalizer) # pylint: disable=protected-access return f - - -@pytest.fixture -def public_types() -> Path: - """ - Path to the public regulated data types directory used for tests. - """ - return Path(".dsdl-test") / "uavcan" diff --git a/pydsdl/_data_type_builder.py b/pydsdl/_data_type_builder.py index 2769311..8a2d630 100644 --- a/pydsdl/_data_type_builder.py +++ b/pydsdl/_data_type_builder.py @@ -222,7 +222,7 @@ def resolve_versioned_data_type(self, name: str, version: _serializable.Version) target_definition = found[0] for visitor in self._definition_visitors: - visitor(self._definition, target_definition) + visitor.on_definition(self._definition, target_definition) assert isinstance(target_definition, DsdlFileBuildable) assert target_definition.full_name == full_name diff --git a/pydsdl/_dsdl.py b/pydsdl/_dsdl.py index 0c8654c..ed6e958 100644 --- a/pydsdl/_dsdl.py +++ b/pydsdl/_dsdl.py @@ -4,7 +4,7 @@ from abc import ABC, abstractmethod from pathlib import Path -from typing import Any, Callable, Iterable, List, Optional, Set, Tuple, TypeVar, Union +from typing import Any, Callable, Iterable, List, Optional, Tuple, TypeVar, Union from ._serializable import CompositeType, Version @@ -129,18 +129,27 @@ def read( raise NotImplementedError() -DefinitionVisitor = Callable[[DsdlFile, DsdlFileBuildable], None] -""" -Called by the parser after if finds a dependent type but before it parses a file in a lookup namespace. -:param DsdlFile argument 0: The target DSDL file that has dependencies the parser is searching for. -:param DsdlFile argument 1: The dependency of target_dsdl_file file the parser is about to parse. -""" +class DefinitionVisitor(ABC): + """ + A visitor that is notified about discovered dependencies. + """ + + @abstractmethod + def on_definition(self, target_dsdl_file: DsdlFile, dependency_dsdl_file: DsdlFileBuildable) -> None: + """ + Called by the parser after if finds a dependent type but before it parses a file in a lookup namespace. + :param target_dsdl_file: The target DSDL file that has dependencies the parser is searching for. + :param dependency_dsdl_file: The dependency of target_dsdl_file file the parser is about to parse. + """ + raise NotImplementedError() + SortedFileT = TypeVar("SortedFileT", DsdlFile, DsdlFileBuildable, CompositeType) SortedFileList = List[SortedFileT] """A list of DSDL files sorted by name, newest version first.""" -def get_definition_ordering_rank(d: DSDLFile | CompositeType) -> tuple[str, int, int]: + +def get_definition_ordering_rank(d: Union[DsdlFile, CompositeType]) -> Tuple[str, int, int]: return d.full_name, -d.version.major, -d.version.minor @@ -148,7 +157,7 @@ def file_sort(file_list: Iterable[SortedFileT]) -> SortedFileList[SortedFileT]: """ Sorts a list of DSDL files lexicographically by name, newest version first. """ - return list(sorted(file_list, key=FileSortKey)) + return list(sorted(file_list, key=get_definition_ordering_rank)) def normalize_paths_argument_to_list( @@ -167,16 +176,16 @@ def _convert(arg: Any) -> Path: raise TypeError(f"Invalid type: {type(arg)}") return Path(arg) if isinstance(arg, str) else arg - return [_convert(arg) for arg in namespaces_or_namespace] + value_set = set() + def _filter_duplicate_paths(arg: Any) -> bool: + if arg in value_set: + return False + value_set.add(arg) + return True -def normalize_paths_argument_to_set( - namespaces_or_namespace: Union[None, Path, str, Iterable[Union[Path, str]]], -) -> Set[Path]: - """ - Normalizes the input argument to a set of paths. - """ - return set(normalize_paths_argument_to_list(namespaces_or_namespace)) + converted = [_convert(arg) for arg in namespaces_or_namespace] + return list(filter(_filter_duplicate_paths, converted)) # +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ @@ -210,6 +219,10 @@ def _unittest_dsdl_normalize_paths_argument_to_list() -> None: result = normalize_paths_argument_to_list(["path/to/namespace1", Path("path/to/namespace2")]) assert result == [Path("path/to/namespace1"), Path("path/to/namespace2")] + # Test de-duplication + result = normalize_paths_argument_to_list(["path/to/namespace1", "path/to/namespace1"]) + assert result == [Path("path/to/namespace1")] + # Test with invalid argument type with assert_raises(TypeError): normalize_paths_argument_to_list(42) # type: ignore @@ -217,40 +230,3 @@ def _unittest_dsdl_normalize_paths_argument_to_list() -> None: # Test with invalid argument type with assert_raises(TypeError): normalize_paths_argument_to_list([42]) # type: ignore - - -def _unittest_dsdl_normalize_paths_argument_to_set() -> None: - - from pytest import raises as assert_raises - - # Test with None argument - result = normalize_paths_argument_to_set(None) - assert result == set() - - # Test with single string argument - result = normalize_paths_argument_to_set("path/to/namespace") - assert result == {Path("path/to/namespace")} - - # Test with single Path argument - result = normalize_paths_argument_to_set(Path("path/to/namespace")) - assert result == {Path("path/to/namespace")} - - # Test with list of strings argument - result = normalize_paths_argument_to_set(["path/to/namespace1", "path/to/namespace2"]) - assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} - - # Test with list of Path arguments - result = normalize_paths_argument_to_set([Path("path/to/namespace1"), Path("path/to/namespace2")]) - assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} - - # Test with mixed list of strings and Path arguments - result = normalize_paths_argument_to_set(["path/to/namespace1", Path("path/to/namespace2")]) - assert result == {Path("path/to/namespace1"), Path("path/to/namespace2")} - - # Test with invalid argument type - with assert_raises(TypeError): - normalize_paths_argument_to_set(42) # type: ignore - - # Test with invalid argument type - with assert_raises(TypeError): - normalize_paths_argument_to_set([42]) # type: ignore diff --git a/pydsdl/_dsdl_definition.py b/pydsdl/_dsdl_definition.py index f8f7dad..e82a9c8 100644 --- a/pydsdl/_dsdl_definition.py +++ b/pydsdl/_dsdl_definition.py @@ -5,7 +5,7 @@ import logging import time from pathlib import Path -from typing import Callable, Iterable, List, Optional +from typing import Callable, Iterable, List, Optional, Type from . import _parser from ._data_type_builder import DataTypeBuilder @@ -25,14 +25,97 @@ def __init__(self, text: str, path: Path): super().__init__(text=text, path=Path(path)) +class DsdlPathInferenceError(InvalidDefinitionError): + """ + Raised when the namespace, type, fixed port ID, or version cannot be inferred from a file path. + """ + + def __init__(self, text: str, dsdl_path: Path, valid_dsdl_roots: List[Path]): + super().__init__(text=text, path=Path(dsdl_path)) + self.valid_dsdl_roots = valid_dsdl_roots[:] if valid_dsdl_roots is not None else None + + class DSDLDefinition(DsdlFileBuildable): """ A DSDL type definition source abstracts the filesystem level details away, presenting a higher-level interface that operates solely on the level of type names, namespaces, fixed identifiers, and so on. Upper layers that operate on top of this abstraction do not concern themselves with the file system at all. + + :param file_path: The path to the DSDL file. + :param root_namespace_path: The path to the root namespace directory. `file_path` must be a descendant of this path. + See `from_first_in` for a more flexible way to create a DSDLDefinition object. """ + @classmethod + def _infer_path_to_root_from_first_found(cls, dsdl_path: Path, valid_dsdl_roots: List[Path]) -> Path: + """ + See `from_first_in` for documentation on this logic. + :return The path to the root namespace directory. + """ + if valid_dsdl_roots is None: + raise ValueError("valid_dsdl_roots was None") + + if dsdl_path.is_absolute() and len(valid_dsdl_roots) == 0: + raise DsdlPathInferenceError( + f"dsdl_path ({dsdl_path}) is absolute and the provided valid root names are empty. The DSDL root of " + "an absolute path cannot be inferred without this information.", + dsdl_path, + valid_dsdl_roots, + ) + + if len(valid_dsdl_roots) == 0: + # if we have no valid roots we can only infer the root of the path. We require the path to be relative + # to avoid accidental inferences given that dsdl file trees starting from a filesystem root are rare. + return Path(dsdl_path.parts[0]) + + # INFERENCE 1: The strongest inference is when the path is relative to a known root. + resolved_dsdl_path = dsdl_path.resolve(strict=False) if dsdl_path.is_absolute() else None + for path_to_root in valid_dsdl_roots: + # First we try the paths as-is... + try: + _ = dsdl_path.relative_to(path_to_root) + except ValueError: + pass + else: + return path_to_root + # then we try resolving the root path if it is absolute + if path_to_root.is_absolute() and resolved_dsdl_path is not None: + path_to_root_resolved = path_to_root.resolve(strict=False) + try: + _ = resolved_dsdl_path.relative_to(path_to_root_resolved).parent + except ValueError: + pass + else: + return path_to_root_resolved + + # INFERENCE 2: A weaker, but valid inference is when the path is a child of a known root folder name. + root_parts = [x.parts[-1] for x in valid_dsdl_roots if len(x.parts) == 1] + parts = list(dsdl_path.parent.parts) + for i, part in list(enumerate(parts)): + if part in root_parts: + return Path().joinpath(*parts[: i + 1]) + # +1 to include the root folder + raise DsdlPathInferenceError(f"No valid root found in path {str(dsdl_path)}", dsdl_path, valid_dsdl_roots) + + @classmethod + def from_first_in(cls: Type["DSDLDefinition"], dsdl_path: Path, valid_dsdl_roots: List[Path]) -> "DSDLDefinition": + """ + Creates a DSDLDefinition object by inferring the path to the namespace root of a DSDL file given a set + of valid roots. The logic used prefers an instance of dsdl_path found to exist under a valid root but + will degrade to pure-path string matching if no file is found. Because this logic uses the first root path + that passes one of these two inferences the order of the valid_dsdl_roots list matters. + + :param dsdl_path: The path to the alleged DSDL file. + :param valid_dsdl_roots: The ordered set of valid root names or paths under which the type must reside. + This argument is accepted as a list for ordering but no de-duplication is performed + as the caller is expected to provide a correct set of paths. + :return A new DSDLDefinition object + :raises DsdlPathInferenceError: If the namespace root cannot be inferred from the provided information. + """ + return cls(dsdl_path, cls._infer_path_to_root_from_first_found(dsdl_path, valid_dsdl_roots)) + def __init__(self, file_path: Path, root_namespace_path: Path): + """ """ # Normalizing the path and reading the definition text self._file_path = Path(file_path) del file_path @@ -44,12 +127,7 @@ def __init__(self, file_path: Path, root_namespace_path: Path): if CompositeType.NAME_COMPONENT_SEPARATOR in self._root_namespace_path.name: raise FileNameFormatError("Invalid namespace name", path=self._root_namespace_path) - # Determining the relative path within the root namespace directory - try: - relative_path = self._root_namespace_path.name / self._file_path.relative_to(self._root_namespace_path) - except ValueError: - # the file is not under the same root path so we'll have to make an assumption that the - relative_path = Path(self._root_namespace_path.name) / self._file_path.name + relative_path = self._root_namespace_path.name / self._file_path.relative_to(self._root_namespace_path) # Parsing the basename, e.g., 434.GetTransportStatistics.0.1.dsdl basename_components = relative_path.name.split(".")[:-1] @@ -232,7 +310,7 @@ def __str__(self) -> str: # +-[UNIT TESTS]------------------------------------------------------------------------------------------------------+ -def _unittest_dsdl_definition_read_non_existant() -> None: +def _unittest_dsdl_definition_read_non_existent() -> None: from pytest import raises as expect_raises target = Path("root", "ns", "Target.1.1.dsdl") @@ -246,8 +324,138 @@ def print_output(line_number: int, text: str) -> None: # pragma: no cover def _unittest_dsdl_definition_read_text(temp_dsdl_factory) -> None: # type: ignore + from pytest import raises as expect_raises + target_root = Path("root", "ns") target_file_path = Path(target_root / "Target.1.1.dsdl") dsdl_file = temp_dsdl_factory.new_file(target_root / target_file_path, "@sealed") - target_definition = DSDLDefinition(dsdl_file, target_root) + with expect_raises(ValueError): + target_definition = DSDLDefinition(dsdl_file, target_root) + # we test first that we can't create the object until we have a target_root that contains the dsdl_file + + target_definition = DSDLDefinition(dsdl_file, dsdl_file.parent.parent) assert "@sealed" == target_definition.text + + +def _unittest_type_from_path_inference() -> None: + from pytest import raises as expect_raises + + # pylint: disable=protected-access + + dsdl_file = Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl") + path_to_root = DSDLDefinition._infer_path_to_root_from_first_found(dsdl_file, [Path("/repo/uavcan")]) + namespace_parts = dsdl_file.parent.relative_to(path_to_root.parent).parts + + assert path_to_root == Path("/repo/uavcan") + assert namespace_parts == ("uavcan", "foo", "bar") + + # The simplest inference made is when relative dsdl paths are provided with no additional information. In this + # case the method assumes that the relative path is the correct and complete namespace of the type: + + # relative path + root = DSDLDefinition._infer_path_to_root_from_first_found(Path("uavcan/foo/bar/435.baz.1.0.dsdl"), []) + assert root == Path("uavcan") + + # The root namespace is not inferred in an absolute path without additional data: + + with expect_raises(DsdlPathInferenceError): + _ = DSDLDefinition._infer_path_to_root_from_first_found(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), []) + + with expect_raises(ValueError): + _ = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), None # type: ignore + ) + + # If an absolute path is provided along with a path-to-root "hint" then the former must be relative to the + # latter: + + # dsdl file path is not contained within the root path + with expect_raises(DsdlPathInferenceError): + _ = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/not-a-repo")] + ) + + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/repo/uavcan")] + ) + assert root == Path("/repo/uavcan") + + # The priority is given to paths that are relative to the root when both simple root names and paths are provided: + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("foo"), Path("/repo/uavcan")] + ) + assert root == Path("/repo/uavcan") + + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("foo"), Path("repo/uavcan")] + ) + assert root == Path("repo/uavcan") + + # Finally, the method will infer the root namespace from simple folder names if no additional information is + # provided: + + valid_roots = [Path("uavcan"), Path("cyphal")] + + # absolute dsdl path using valid roots + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots + ) + assert root == Path("/repo/uavcan") + + # relative dsdl path using valid roots + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots + ) + assert root == Path("repo/uavcan") + + # absolute dsdl path using valid roots but an invalid file path + with expect_raises(DsdlPathInferenceError): + _ = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots + ) + + # relative dsdl path using valid roots but an invalid file path + with expect_raises(DsdlPathInferenceError): + _ = DSDLDefinition._infer_path_to_root_from_first_found(Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots) + + # relative dsdl path with invalid root fragments + invalid_root_fragments = [Path("cyphal", "acme")] + with expect_raises(DsdlPathInferenceError): + _ = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), invalid_root_fragments + ) + + # In this example, foo/bar might look like a valid root path but it is not relative to repo/uavcan/foo/bar and is + # not considered after relative path inference has failed because it is not a simple root name. + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("foo/bar"), Path("foo")] + ) + assert root == Path("repo/uavcan/foo") + + # when foo/bar is placed within the proper, relative path it is considered as a valid root and is preferred over + # the simple root name "foo": + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("repo/uavcan/foo/bar"), Path("foo")] + ) + assert root == Path("repo/uavcan/foo/bar") + + # Sometimes the root paths have crap in them and need to be resolved: + + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/path/to/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/path/to/repo/uavcan/../uavcan")] + ) + assert root == Path("/path/to/repo/uavcan") + + # Let's ensure ordering here + + root = DSDLDefinition._infer_path_to_root_from_first_found( + Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("repo/uavcan"), Path("repo/uavcan/foo")] + ) + assert root == Path("repo/uavcan") + + +def _unittest_from_first_in() -> None: + dsdl_def = DSDLDefinition.from_first_in( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/repo/uavcan/foo/..")] + ) + assert dsdl_def.full_name == "uavcan.foo.bar.baz" diff --git a/pydsdl/_namespace.py b/pydsdl/_namespace.py index dd77c43..1e19f9d 100644 --- a/pydsdl/_namespace.py +++ b/pydsdl/_namespace.py @@ -13,7 +13,7 @@ from . import _dsdl_definition, _error, _serializable from ._dsdl import DsdlFileBuildable, PrintOutputHandler, SortedFileList from ._dsdl import file_sort as dsdl_file_sort -from ._dsdl import normalize_paths_argument_to_list, normalize_paths_argument_to_set +from ._dsdl import normalize_paths_argument_to_list from ._namespace_reader import DsdlDefinitions, read_definitions _logger = logging.getLogger(__name__) @@ -75,12 +75,6 @@ class SealingConsistencyError(_error.InvalidDefinitionError): """ -class DsdlPathInferenceError(_error.InvalidDefinitionError): - """ - Raised when the namespace, type, fixed port ID, or version cannot be inferred from a file path. - """ - - # +--[PUBLIC API]-----------------------------------------------------------------------------------------------------+ @@ -220,7 +214,7 @@ def read_files( # Normalize paths and remove duplicates. Resolve symlinks to avoid ambiguities. target_dsdl_definitions = _construct_dsdl_definitions_from_files( normalize_paths_argument_to_list(dsdl_files), - normalize_paths_argument_to_set(root_namespace_directories_or_names), + normalize_paths_argument_to_list(root_namespace_directories_or_names), ) if len(target_dsdl_definitions) == 0: _logger.info("No DSDL files found in the specified directories") @@ -307,10 +301,10 @@ def _construct_lookup_directories_path_list( """ Intermediate transformation and validation of inputs into a list of lookup directories as paths. - :param root_namespace_directory: The path of the root namespace directory that will be read. + :param root_namespace_directories: The path of the root namespace directory that will be read. For example, ``dsdl/uavcan`` to read the ``uavcan`` namespace. - :param lookup_directories: List of other namespace directories containing data type definitions that are + :param lookup_directories_path_list: List of other namespace directories containing data type definitions that are referred to from the target root namespace. For example, if you are reading a vendor-specific namespace, the list of lookup directories should always include a path to the standard root namespace ``uavcan``, otherwise the types defined in the vendor-specific namespace won't be able to use data types from the @@ -349,20 +343,20 @@ def _construct_lookup_directories_path_list( def _construct_dsdl_definitions_from_files( dsdl_files: List[Path], - valid_roots: Set[Path], + valid_roots: List[Path], ) -> SortedFileList[DsdlFileBuildable]: """ """ output = set() # type: Set[DsdlFileBuildable] for fp in dsdl_files: - root_namespace_path = _infer_path_to_root(fp, valid_roots) - if fp.suffix == DSDL_FILE_SUFFIX_LEGACY: + resolved_fp = fp.resolve(strict=False) + if resolved_fp.suffix == DSDL_FILE_SUFFIX_LEGACY: _logger.warning( "File uses deprecated extension %r, please rename to use %r: %s", DSDL_FILE_SUFFIX_LEGACY, DSDL_FILE_SUFFIX, - fp, + resolved_fp, ) - output.add(_dsdl_definition.DSDLDefinition(fp, root_namespace_path)) + output.add(_dsdl_definition.DSDLDefinition.from_first_in(resolved_fp, list(valid_roots))) return dsdl_file_sort(output) @@ -597,46 +591,6 @@ def check_each(path_tuple_with_result: Tuple[Tuple[Path, Path], List[int]]) -> b ) -def _infer_path_to_root(dsdl_path: Path, valid_dsdl_roots_or_path_to_root: Set[Path]) -> Path: - """ - Infer the path to the namespace root of a DSDL file path. - :param dsdl_path: The path to the alleged DSDL file. - :param valid_dsdl_roots_or_path_to_root: The set of valid root names or paths under which the type must reside. - :return The path to the root namespace directory. - :raises DsdlPathInferenceError: If the namespace root cannot be inferred from the provided information. - """ - if valid_dsdl_roots_or_path_to_root is None: - raise _error.InternalError("valid_dsdl_roots_or_path_to_root was None") - - if dsdl_path.is_absolute() and len(valid_dsdl_roots_or_path_to_root) == 0: - raise DsdlPathInferenceError( - f"dsdl_path ({dsdl_path}) is absolute and the provided valid root names are empty. The DSDL root of " - "an absolute path cannot be inferred without this information.", - ) - - if len(valid_dsdl_roots_or_path_to_root) == 0: - # if we have no valid roots we can only infer the root of the path. We require the path to be relative - # to avoid accidental inferences given that dsdl file trees starting from a filesystem root are rare. - return Path(dsdl_path.parts[0]) - - # The strongest inference is when the path is relative to a known root. - for path_to_root in valid_dsdl_roots_or_path_to_root: - try: - _ = dsdl_path.relative_to(path_to_root) - except ValueError: - continue - return path_to_root - - # A weaker, but valid inference is when the path is a child of a known root folder name. - root_parts = {x.parts[-1] for x in valid_dsdl_roots_or_path_to_root if len(x.parts) == 1} - parts = list(dsdl_path.parent.parts) - for i, part in list(enumerate(parts)): - if part in root_parts: - return Path().joinpath(*parts[: i + 1]) - # +1 to include the root folder - raise DsdlPathInferenceError(f"No valid root found in path {str(dsdl_path)}") - - # +--[ UNIT TESTS ]---------------------------------------------------------------------------------------------------+ @@ -848,87 +802,6 @@ def _unittest_issue_71() -> None: # https://github.com/OpenCyphal/pydsdl/issues assert len(read_namespace(link, [real, link])) == 1 -def _unittest_type_from_path_inference() -> None: - from pytest import raises as expect_raises - - # To determine the namespace do - - dsdl_file = Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl") - path_to_root = _infer_path_to_root(dsdl_file, {Path("/repo/uavcan")}) - namespace_parts = dsdl_file.parent.relative_to(path_to_root.parent).parts - - assert path_to_root == Path("/repo/uavcan") - assert namespace_parts == ("uavcan", "foo", "bar") - - # The simplest inference made is when relative dsdl paths are provided with no additional information. In this - # case the method assumes that the relative path is the correct and complete namespace of the type: - - # relative path - root = _infer_path_to_root(Path("uavcan/foo/bar/435.baz.1.0.dsdl"), set()) - assert root == Path("uavcan") - - # The root namespace is not inferred in an absolute path without additional data: - - with expect_raises(DsdlPathInferenceError): - _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), set()) - - with expect_raises(_error.InternalError): - _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), None) # type: ignore - - # If an absolute path is provided along with a path-to-root "hint" then the former must be relative to the - # latter: - - # dsdl file path is not contained within the root path - with expect_raises(DsdlPathInferenceError): - _ = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("/not-a-repo")}) - - root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("/repo/uavcan")}) - assert root == Path("/repo/uavcan") - - # The priority is given to paths that are relative to the root when both simple root names and paths are provided: - root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo"), Path("/repo/uavcan")}) - assert root == Path("/repo/uavcan") - - root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo"), Path("repo/uavcan")}) - assert root == Path("repo/uavcan") - - # Finally, the method will infer the root namespace from simple folder names if no additional information is - # provided: - - valid_roots = {Path("uavcan"), Path("cyphal")} - - # absolute dsdl path using valid roots - root = _infer_path_to_root(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots) - assert root == Path("/repo/uavcan") - - # relative dsdl path using valid roots - root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots) - assert root == Path("repo/uavcan") - - # absolute dsdl path using valid roots but an invalid file path - with expect_raises(DsdlPathInferenceError): - _ = _infer_path_to_root(Path("/repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots) - - # relative dsdl path using valid roots but an invalid file path - with expect_raises(DsdlPathInferenceError): - _ = _infer_path_to_root(Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots) - - # relative dsdl path with invalid root fragments - invalid_root_fragments = {Path("cyphal", "acme")} - with expect_raises(DsdlPathInferenceError): - _ = _infer_path_to_root(Path("repo/crap/foo/bar/435.baz.1.0.dsdl"), invalid_root_fragments) - - # In this example, foo/bar might look like a valid root path but it is not relative to repo/uavcan/foo/bar and is - # not considered after relative path inference has failed because it is not a simple root name. - root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("foo/bar"), Path("foo")}) - assert root == Path("repo/uavcan/foo") - - # when foo/bar is placed within the proper, relative path it is considered as a valid root and is preferred over - # the simple root name "foo": - root = _infer_path_to_root(Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), {Path("repo/uavcan/foo/bar"), Path("foo")}) - assert root == Path("repo/uavcan/foo/bar") - - def _unittest_type_read_files_example(temp_dsdl_factory) -> None: # type: ignore # let's test the comments for the read function diff --git a/pydsdl/_namespace_reader.py b/pydsdl/_namespace_reader.py index 63180de..7295e0f 100644 --- a/pydsdl/_namespace_reader.py +++ b/pydsdl/_namespace_reader.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import Dict, List, NamedTuple, Optional, Set, cast -from ._dsdl import DsdlFile, DsdlFileBuildable, PrintOutputHandler, SortedFileList +from ._dsdl import DefinitionVisitor, DsdlFile, DsdlFileBuildable, PrintOutputHandler, SortedFileList from ._dsdl import file_sort as dsdl_file_sort from ._error import FrontendError, InternalError from ._serializable._composite import CompositeType @@ -32,9 +32,10 @@ def _read_definitions( _pending_definitions: Set[DsdlFileBuildable] = set() - def callback(_: DsdlFile, dependent_type: DsdlFileBuildable) -> None: - if dependent_type.file_path not in file_pool: - _pending_definitions.add(dependent_type) + class _Callback(DefinitionVisitor): + def on_definition(self, _: DsdlFile, dependency_dsdl_file: DsdlFileBuildable) -> None: + if dependency_dsdl_file.file_path not in file_pool: + _pending_definitions.add(dependency_dsdl_file) def print_handler(file: Path, line: int, message: str) -> None: if print_output_handler is not None: @@ -61,7 +62,7 @@ def print_handler(file: Path, line: int, message: str) -> None: try: new_composite_type = target_definition.read( lookup_definitions, - [callback], + [_Callback()], functools.partial(print_handler, target_definition.file_path), allow_unregulated_fixed_port_id, ) diff --git a/setup.cfg b/setup.cfg index c58e783..8d934ba 100644 --- a/setup.cfg +++ b/setup.cfg @@ -35,7 +35,7 @@ include = pydsdl* # -------------------------------------------------- PYTEST -------------------------------------------------- [tool:pytest] -testpaths = pydsdl test +testpaths = pydsdl norecursedirs = third_party python_files = *.py python_classes = _UnitTest diff --git a/test/test_public_types.py b/test/test_public_types.py deleted file mode 100644 index 8463f97..0000000 --- a/test/test_public_types.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright (C) OpenCyphal Development Team -# Copyright Amazon.com Inc. or its affiliates. -# SPDX-License-Identifier: MIT - -# pylint: disable=redefined-outer-name -# pylint: disable=logging-fstring-interpolation -import cProfile -import io -import pstats -from pathlib import Path -from pstats import SortKey - -import pydsdl - - -def _unittest_public_types_namespaces(public_types: Path) -> None: - """ - Sanity check to ensure that the public types can be read. This also allows us to debug - against a real dataset. - """ - pr = cProfile.Profile() - pr.enable() - _ = pydsdl.read_namespace(public_types) - pr.disable() - s = io.StringIO() - sortby = SortKey.TIME - ps = pstats.Stats(pr, stream=s).sort_stats(sortby) - ps.print_stats() - print(s.getvalue()) - - -def _unittest_public_types_files(public_types: Path) -> None: - """ - Sanity check to ensure that the public types can be read. This also allows us to debug - against a real dataset. - """ - pr = cProfile.Profile() - pr.enable() - node_types = list(public_types.glob("node/**/*.dsdl")) - assert len(node_types) > 0 - _ = pydsdl.read_files(node_types, {public_types}) - pr.disable() - s = io.StringIO() - sortby = SortKey.TIME - ps = pstats.Stats(pr, stream=s).sort_stats(sortby) - ps.print_stats() - print(s.getvalue()) From 9998ec6b73602ac9aa5d067701efb05acf917e3e Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 20 Jun 2024 15:20:03 -0700 Subject: [PATCH 06/11] Mypy seems okay with old syntax. --- noxfile.py | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/noxfile.py b/noxfile.py index b74b4c1..f9e73d2 100644 --- a/noxfile.py +++ b/noxfile.py @@ -82,29 +82,28 @@ def pristine(session): @nox.session(python=PYTHONS, reuse_venv=True) def lint(session): - if is_oldest_python(session): - # we run mypy and pylint only on the oldest Python version to ensure maximum compatibility - session.install( - "mypy ~= 1.10", - "types-parsimonious", - "pylint ~= 3.2", - ) - session.run( - "mypy", - "--strict", - f"--config-file={ROOT_DIR / 'setup.cfg'}", - "pydsdl", - env={ - "MYPYPATH": str(THIRD_PARTY_DIR), - }, - ) - session.run( - "pylint", - str(ROOT_DIR / "pydsdl"), - env={ - "PYTHONPATH": str(THIRD_PARTY_DIR), - }, - ) + # we run mypy and pylint only on the oldest Python version to ensure maximum compatibility + session.install( + "mypy ~= 1.10", + "types-parsimonious", + "pylint ~= 3.2", + ) + session.run( + "mypy", + "--strict", + f"--config-file={ROOT_DIR / 'setup.cfg'}", + "pydsdl", + env={ + "MYPYPATH": str(THIRD_PARTY_DIR), + }, + ) + session.run( + "pylint", + str(ROOT_DIR / "pydsdl"), + env={ + "PYTHONPATH": str(THIRD_PARTY_DIR), + }, + ) if is_latest_python(session): # we run black only on the newest Python version to ensure that the code is formatted with the latest version session.install("black ~= 24.4") From 22fa2376cf6eb6dbd24f524e383852ebda213b23 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Tue, 25 Jun 2024 10:30:22 -0700 Subject: [PATCH 07/11] Making DsdlPathInferenceError extend DsdlPathInferenceError --- pydsdl/_dsdl_definition.py | 4 ++-- pydsdl/_namespace.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pydsdl/_dsdl_definition.py b/pydsdl/_dsdl_definition.py index e82a9c8..4a5ceeb 100644 --- a/pydsdl/_dsdl_definition.py +++ b/pydsdl/_dsdl_definition.py @@ -10,7 +10,7 @@ from . import _parser from ._data_type_builder import DataTypeBuilder from ._dsdl import DefinitionVisitor, DsdlFileBuildable -from ._error import FrontendError, InternalError, InvalidDefinitionError +from ._error import FrontendError, InternalError, InvalidDefinitionError, UndefinedDataTypeError from ._serializable import CompositeType, Version _logger = logging.getLogger(__name__) @@ -25,7 +25,7 @@ def __init__(self, text: str, path: Path): super().__init__(text=text, path=Path(path)) -class DsdlPathInferenceError(InvalidDefinitionError): +class DsdlPathInferenceError(UndefinedDataTypeError): """ Raised when the namespace, type, fixed port ID, or version cannot be inferred from a file path. """ diff --git a/pydsdl/_namespace.py b/pydsdl/_namespace.py index 1e19f9d..f142539 100644 --- a/pydsdl/_namespace.py +++ b/pydsdl/_namespace.py @@ -275,7 +275,7 @@ def _complete_read_function( ", ".join(set(sorted(map(lambda t: t.root_namespace, lookup_dsdl_definitions)))), ) - # This is the biggie. All the rest of the wranging is just to get to this point. This will take the + # This is the biggie. All the rest of the wrangling is just to get to this point. This will take the # most time and memory. definitions = read_definitions( target_dsdl_definitions, lookup_dsdl_definitions, print_output_handler, allow_unregulated_fixed_port_id From ba289345e1a6663d803308542a80409ceb007384 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Tue, 25 Jun 2024 10:45:17 -0700 Subject: [PATCH 08/11] Whoops! --- pydsdl/_dsdl_definition.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pydsdl/_dsdl_definition.py b/pydsdl/_dsdl_definition.py index 4a5ceeb..a8af059 100644 --- a/pydsdl/_dsdl_definition.py +++ b/pydsdl/_dsdl_definition.py @@ -8,9 +8,9 @@ from typing import Callable, Iterable, List, Optional, Type from . import _parser -from ._data_type_builder import DataTypeBuilder +from ._data_type_builder import DataTypeBuilder, UndefinedDataTypeError from ._dsdl import DefinitionVisitor, DsdlFileBuildable -from ._error import FrontendError, InternalError, InvalidDefinitionError, UndefinedDataTypeError +from ._error import FrontendError, InternalError, InvalidDefinitionError from ._serializable import CompositeType, Version _logger = logging.getLogger(__name__) From 4ee7a13cebe43989b588c194e377a49dadacc2b6 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Tue, 25 Jun 2024 11:28:39 -0700 Subject: [PATCH 09/11] Fixing docs on 3.8 --- docs/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 6ac611d..81ca5c8 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,3 +1,3 @@ -sphinx == 7.3.7 +sphinx == 7.1.2 # this is the last version that supports Python 3.8 sphinx_rtd_theme == 2.0.0 sphinx-computron >= 0.2, < 2.0 From 2c983b897ef741b78d1fe9c9e1756a5afe390427 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Tue, 25 Jun 2024 13:30:37 -0700 Subject: [PATCH 10/11] Fixing paths for Windows tests --- pydsdl/_dsdl_definition.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/pydsdl/_dsdl_definition.py b/pydsdl/_dsdl_definition.py index a8af059..bbdf075 100644 --- a/pydsdl/_dsdl_definition.py +++ b/pydsdl/_dsdl_definition.py @@ -342,11 +342,11 @@ def _unittest_type_from_path_inference() -> None: # pylint: disable=protected-access - dsdl_file = Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl") - path_to_root = DSDLDefinition._infer_path_to_root_from_first_found(dsdl_file, [Path("/repo/uavcan")]) + dsdl_file = Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve() + path_to_root = DSDLDefinition._infer_path_to_root_from_first_found(dsdl_file, [Path("/repo/uavcan").resolve()]) namespace_parts = dsdl_file.parent.relative_to(path_to_root.parent).parts - assert path_to_root == Path("/repo/uavcan") + assert path_to_root == Path("/repo/uavcan").resolve() assert namespace_parts == ("uavcan", "foo", "bar") # The simplest inference made is when relative dsdl paths are provided with no additional information. In this @@ -359,11 +359,13 @@ def _unittest_type_from_path_inference() -> None: # The root namespace is not inferred in an absolute path without additional data: with expect_raises(DsdlPathInferenceError): - _ = DSDLDefinition._infer_path_to_root_from_first_found(Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), []) + _ = DSDLDefinition._infer_path_to_root_from_first_found( + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), [] + ) with expect_raises(ValueError): _ = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), None # type: ignore + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), None # type: ignore ) # If an absolute path is provided along with a path-to-root "hint" then the former must be relative to the @@ -372,19 +374,19 @@ def _unittest_type_from_path_inference() -> None: # dsdl file path is not contained within the root path with expect_raises(DsdlPathInferenceError): _ = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/not-a-repo")] + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), [Path("/not-a-repo").resolve()] ) root = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/repo/uavcan")] + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), [Path("/repo/uavcan").resolve()] ) - assert root == Path("/repo/uavcan") + assert root == Path("/repo/uavcan").resolve() # The priority is given to paths that are relative to the root when both simple root names and paths are provided: root = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("foo"), Path("/repo/uavcan")] + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), [Path("foo"), Path("/repo/uavcan").resolve()] ) - assert root == Path("/repo/uavcan") + assert root == Path("/repo/uavcan").resolve() root = DSDLDefinition._infer_path_to_root_from_first_found( Path("repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("foo"), Path("repo/uavcan")] @@ -398,9 +400,9 @@ def _unittest_type_from_path_inference() -> None: # absolute dsdl path using valid roots root = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), valid_roots + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), valid_roots ) - assert root == Path("/repo/uavcan") + assert root == Path("/repo/uavcan").resolve() # relative dsdl path using valid roots root = DSDLDefinition._infer_path_to_root_from_first_found( @@ -411,7 +413,7 @@ def _unittest_type_from_path_inference() -> None: # absolute dsdl path using valid roots but an invalid file path with expect_raises(DsdlPathInferenceError): _ = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/repo/crap/foo/bar/435.baz.1.0.dsdl"), valid_roots + Path("/repo/crap/foo/bar/435.baz.1.0.dsdl").resolve(), valid_roots ) # relative dsdl path using valid roots but an invalid file path @@ -442,9 +444,10 @@ def _unittest_type_from_path_inference() -> None: # Sometimes the root paths have crap in them and need to be resolved: root = DSDLDefinition._infer_path_to_root_from_first_found( - Path("/path/to/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/path/to/repo/uavcan/../uavcan")] + Path("/path/to/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), + [Path("/path/to/repo/uavcan/../uavcan").resolve()], ) - assert root == Path("/path/to/repo/uavcan") + assert root == Path("/path/to/repo/uavcan").resolve() # Let's ensure ordering here @@ -456,6 +459,6 @@ def _unittest_type_from_path_inference() -> None: def _unittest_from_first_in() -> None: dsdl_def = DSDLDefinition.from_first_in( - Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl"), [Path("/repo/uavcan/foo/..")] + Path("/repo/uavcan/foo/bar/435.baz.1.0.dsdl").resolve(), [Path("/repo/uavcan/foo/..").resolve()] ) assert dsdl_def.full_name == "uavcan.foo.bar.baz" From e4f8a6c001aa91fa7297042c6d6534ca9af408c3 Mon Sep 17 00:00:00 2001 From: Scott Dixon Date: Thu, 27 Jun 2024 12:23:12 -0700 Subject: [PATCH 11/11] Fix for Issue #104 This changes our logic for detecting duplicate definitions on case-sensitive file-systems to only operate on files of interest and to fix this detection logic per issue #104. If globular searches find duplicates we do not waste time detecting this until/unless these duplicates are actually considered when parsing target definitions. --- pydsdl/_data_type_builder.py | 31 ++++++++++-- pydsdl/_namespace.py | 83 ++++++------------------------ pydsdl/_test.py | 97 +++++++++++++++++++++++++++--------- 3 files changed, 117 insertions(+), 94 deletions(-) diff --git a/pydsdl/_data_type_builder.py b/pydsdl/_data_type_builder.py index 8a2d630..ffcd66c 100644 --- a/pydsdl/_data_type_builder.py +++ b/pydsdl/_data_type_builder.py @@ -34,6 +34,18 @@ class MissingSerializationModeError(_error.InvalidDefinitionError): pass +class DataTypeCollisionError(_error.InvalidDefinitionError): + """ + Raised when there are conflicting data type definitions. + """ + + +class DataTypeNameCollisionError(DataTypeCollisionError): + """ + Raised when type collisions are caused by naming conflicts. + """ + + _logger = logging.getLogger(__name__) @@ -196,7 +208,11 @@ def resolve_versioned_data_type(self, name: str, version: _serializable.Version) _logger.debug("The full name of a relatively referred type %r reconstructed as %r", name, full_name) del name - found = list(filter(lambda d: d.full_name == full_name and d.version == version, self._lookup_definitions)) + found = list( + filter( + lambda d: d.full_name.lower() == full_name.lower() 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] @@ -217,8 +233,17 @@ def resolve_versioned_data_type(self, name: str, version: _serializable.Version) error_description += " Please make sure that you specified the directories correctly." raise UndefinedDataTypeError(error_description) - if len(found) > 1: # pragma: no cover - raise _error.InternalError("Conflicting definitions: %r" % found) + if len(found) > 1: + if ( + found[0].full_name != found[1].full_name and found[0].full_name.lower() == found[1].full_name.lower() + ): # pragma: no cover + # This only happens if the file system is case-sensitive. + raise DataTypeNameCollisionError( + "Full name of this definition differs from %s only by letter case, " + "which is not permitted" % found[0].file_path, + path=found[1].file_path, + ) + raise DataTypeCollisionError("Conflicting definitions: %r" % found) target_definition = found[0] for visitor in self._definition_visitors: diff --git a/pydsdl/_namespace.py b/pydsdl/_namespace.py index f142539..4608e59 100644 --- a/pydsdl/_namespace.py +++ b/pydsdl/_namespace.py @@ -26,18 +26,6 @@ class RootNamespaceNameCollisionError(_error.InvalidDefinitionError): """ -class DataTypeCollisionError(_error.InvalidDefinitionError): - """ - Raised when there are conflicting data type definitions. - """ - - -class DataTypeNameCollisionError(DataTypeCollisionError): - """ - Raised when there are conflicting data type names. - """ - - class NestedRootNamespaceError(_error.InvalidDefinitionError): """ Nested root namespaces are not allowed. This exception is thrown when this rule is violated. @@ -259,9 +247,6 @@ def _complete_read_function( lookup_dsdl_definitions = _construct_dsdl_definitions_from_namespaces(lookup_directories_path_list) - # Check for collisions against the lookup definitions also. - _ensure_no_collisions(target_dsdl_definitions, lookup_dsdl_definitions) - _logger.debug("Lookup DSDL definitions are listed below:") for x in lookup_dsdl_definitions: _logger.debug(_LOG_LIST_ITEM_PREFIX + str(x)) @@ -385,44 +370,6 @@ def _construct_dsdl_definitions_from_namespaces( return dsdl_file_sort([_dsdl_definition.DSDLDefinition(*p) for p in source_file_paths]) -def _ensure_no_collisions( - target_definitions: List[DsdlFileBuildable], - lookup_definitions: List[DsdlFileBuildable], -) -> None: - for tg in target_definitions: - tg_full_namespace_period = tg.full_namespace.lower() + "." - tg_full_name_period = tg.full_name.lower() + "." - for lu in lookup_definitions: - lu_full_namespace_period = lu.full_namespace.lower() + "." - lu_full_name_period = lu.full_name.lower() + "." - # This is to allow the following messages to coexist happily: - # zubax/non_colliding/iceberg/Ice.0.1.dsdl - # zubax/non_colliding/IceB.0.1.dsdl - # The following is still not allowed: - # zubax/colliding/iceberg/Ice.0.1.dsdl - # zubax/colliding/Iceberg.0.1.dsdl - if tg.full_name != lu.full_name and tg.full_name.lower() == lu.full_name.lower(): - raise DataTypeNameCollisionError( - "Full name of this definition differs from %s only by letter case, " - "which is not permitted" % lu.file_path, - path=tg.file_path, - ) - if (tg_full_namespace_period).startswith(lu_full_name_period): - raise DataTypeNameCollisionError( - "The namespace of this type conflicts with %s" % lu.file_path, path=tg.file_path - ) - if (lu_full_namespace_period).startswith(tg_full_name_period): - raise DataTypeNameCollisionError( - "This type conflicts with the namespace of %s" % lu.file_path, path=tg.file_path - ) - if ( - tg_full_name_period == lu_full_name_period - and tg.version == lu.version - and not tg.file_path.samefile(lu.file_path) - ): # https://github.com/OpenCyphal/pydsdl/issues/94 - raise DataTypeCollisionError("This type is redefined in %s" % lu.file_path, path=tg.file_path) - - def _ensure_no_fixed_port_id_collisions(types: List[_serializable.CompositeType]) -> None: for a in types: for b in types: @@ -864,20 +811,22 @@ def _unittest_read_files_empty_args() -> None: assert len(transitive) == 0 -def _unittest_ensure_no_collisions(temp_dsdl_factory) -> None: # type: ignore - from pytest import raises as expect_raises - - # gratuitous coverage of the collision check where other tests don't cover some edge cases +def _unittest_ensure_no_namespace_name_collisions_or_nested_root_namespaces() -> None: + """gratuitous coverage of the collision check where other tests don't cover some edge cases.""" _ensure_no_namespace_name_collisions_or_nested_root_namespaces([], False) - with expect_raises(DataTypeNameCollisionError): - _ensure_no_collisions( - [_dsdl_definition.DSDLDefinition(Path("a/b.1.0.dsdl"), Path("a"))], - [_dsdl_definition.DSDLDefinition(Path("a/B.1.0.dsdl"), Path("a"))], - ) - with expect_raises(DataTypeNameCollisionError): - _ensure_no_collisions( - [_dsdl_definition.DSDLDefinition(Path("a/b/c.1.0.dsdl"), Path("a"))], - [_dsdl_definition.DSDLDefinition(Path("a/b.1.0.dsdl"), Path("a"))], - ) +def _unittest_issue_104(temp_dsdl_factory) -> None: # type: ignore + """demonstrate that removing _ensure_no_collisions is okay""" + + thing_1_0 = Path("a/b/thing.1.0.dsdl") + thing_type_1_0 = Path("a/b/thing/thingtype.1.0.dsdl") + + file_at_root = temp_dsdl_factory.new_file(Path("a/Nothing.1.0.dsdl"), "@sealed\n") + thing_file = temp_dsdl_factory.new_file(thing_1_0, "@sealed\na.b.thing.thingtype.1.0 thing\n") + _ = temp_dsdl_factory.new_file(thing_type_1_0, "@sealed\n") + + direct, transitive = read_files(thing_file, file_at_root.parent, file_at_root.parent) + + assert len(direct) == 1 + assert len(transitive) == 1 diff --git a/pydsdl/_test.py b/pydsdl/_test.py index ca9c373..b7e4c4c 100644 --- a/pydsdl/_test.py +++ b/pydsdl/_test.py @@ -10,6 +10,7 @@ from pathlib import Path from textwrap import dedent import pytest # This is only safe to import in test files! +from . import _data_type_builder from . import _expression from . import _error from . import _parser @@ -24,6 +25,8 @@ class Workspace: def __init__(self) -> None: self._tmp_dir = tempfile.TemporaryDirectory(prefix="pydsdl-test-") + name = self._tmp_dir.name.upper() + self._is_case_sensitive = not Path(name).exists() @property def directory(self) -> Path: @@ -57,6 +60,10 @@ def drop(self, rel_path_glob: str) -> None: for g in self.directory.glob(rel_path_glob): g.unlink() + @property + def is_fs_case_sensitive(self) -> bool: + return self._is_case_sensitive + def parse_definition( definition: _dsdl_definition.DSDLDefinition, lookup_definitions: Sequence[_dsdl_definition.DSDLDefinition] @@ -1095,7 +1102,7 @@ def print_handler(d: Path, line: int, text: str) -> None: """ ), ) - with raises(_namespace.DataTypeNameCollisionError): + with raises(_namespace.FixedPortIDCollisionError): _namespace.read_namespace( wrkspc.directory / "zubax", [ @@ -1104,30 +1111,27 @@ def print_handler(d: Path, line: int, text: str) -> None: ) # Do again to test single lookup-directory override - with raises(_namespace.DataTypeNameCollisionError): + with raises(_namespace.FixedPortIDCollisionError): _namespace.read_namespace(wrkspc.directory / "zubax", wrkspc.directory / "zubax") - try: - (wrkspc.directory / "zubax/colliding/iceberg/300.Ice.30.0.dsdl").unlink() - wrkspc.new( - "zubax/COLLIDING/300.Iceberg.30.0.dsdl", - dedent( - """ - @extent 1024 - --- - @extent 1024 + (wrkspc.directory / "zubax/colliding/iceberg/300.Ice.30.0.dsdl").unlink() + wrkspc.new( + "zubax/COLLIDING/300.Iceberg.30.0.dsdl", + dedent( """ - ), - ) - with raises(_namespace.DataTypeNameCollisionError, match=".*letter case.*"): - _namespace.read_namespace( + @extent 1024 + --- + @extent 1024 + """ + ), + ) + with raises(_namespace.FixedPortIDCollisionError): + _namespace.read_namespace( + wrkspc.directory / "zubax", + [ wrkspc.directory / "zubax", - [ - wrkspc.directory / "zubax", - ], - ) - except _namespace.FixedPortIDCollisionError: # pragma: no cover - pass # We're running on a platform where paths are not case-sensitive. + ], + ) # Test namespace can intersect with type name (wrkspc.directory / "zubax/COLLIDING/300.Iceberg.30.0.dsdl").unlink() @@ -1160,6 +1164,52 @@ def print_handler(d: Path, line: int, text: str) -> None: assert "zubax.noncolliding.Iceb" in [x.full_name for x in parsed] +def _unittest_collision_on_case_sensitive_filesystem(wrkspc: Workspace) -> None: + from pytest import raises + + if not wrkspc.is_fs_case_sensitive: # pragma: no cover + pytest.skip("This test is only relevant on case-sensitive filesystems.") + + # Empty namespace. + assert [] == _namespace.read_namespace(wrkspc.directory) + + wrkspc.new( + "atlantic/ships/Titanic.1.0.dsdl", + dedent( + """ + greenland.colliding.IceBerg.1.0[<=2] bergs + @sealed + """ + ), + ) + + wrkspc.new( + "greenland/colliding/IceBerg.1.0.dsdl", + dedent( + """ + @sealed + """ + ), + ) + + wrkspc.new( + "greenland/COLLIDING/IceBerg.1.0.dsdl", + dedent( + """ + @sealed + """ + ), + ) + + with raises(_data_type_builder.DataTypeNameCollisionError, match=".*letter case.*"): + _namespace.read_namespace( + wrkspc.directory / "atlantic", + [ + wrkspc.directory / "greenland", + ], + ) + + def _unittest_parse_namespace_versioning(wrkspc: Workspace) -> None: from pytest import raises @@ -1264,8 +1314,7 @@ def _unittest_parse_namespace_versioning(wrkspc: Workspace) -> None: ), ) - with raises(_namespace.DataTypeCollisionError): - _namespace.read_namespace((wrkspc.directory / "ns"), []) + _namespace.read_namespace((wrkspc.directory / "ns"), []) wrkspc.drop("ns/Spartans.30.2.dsdl") @@ -1613,7 +1662,7 @@ def _unittest_issue94(wrkspc: Workspace) -> None: wrkspc.new("outer_b/ns/Foo.1.0.dsdl", "@sealed") # Conflict! wrkspc.new("outer_a/ns/Bar.1.0.dsdl", "Foo.1.0 fo\n@sealed") # Which Foo.1.0? - with raises(_namespace.DataTypeCollisionError): + with raises(_data_type_builder.DataTypeCollisionError): _namespace.read_namespace( wrkspc.directory / "outer_a" / "ns", [wrkspc.directory / "outer_b" / "ns"],