diff --git a/pydsdl/_data_type_builder.py b/pydsdl/_data_type_builder.py index 96128d0..d7812fb 100644 --- a/pydsdl/_data_type_builder.py +++ b/pydsdl/_data_type_builder.py @@ -35,6 +35,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__) @@ -197,7 +209,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] @@ -218,8 +234,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 065c739..ea6bc9e 100644 --- a/pydsdl/_namespace.py +++ b/pydsdl/_namespace.py @@ -27,18 +27,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. @@ -260,9 +248,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)) @@ -386,45 +371,7 @@ 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[ReadableDSDLFile], - lookup_definitions: list[ReadableDSDLFile], -) -> 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: +def _ensure_no_fixed_port_id_collisions(types: List[_serializable.CompositeType]) -> None: for a in types: for b in types: different_names = a.full_name != b.full_name @@ -864,22 +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 +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) - _ = temp_dsdl_factory - # 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) +def _unittest_issue_104(temp_dsdl_factory) -> None: # type: ignore + """demonstrate that removing _ensure_no_collisions is okay""" - 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"))], - ) + thing_1_0 = Path("a/b/thing.1.0.dsdl") + thing_type_1_0 = Path("a/b/thing/thingtype.1.0.dsdl") - 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"))], - ) + 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 d3dbecb..83eb14d 100644 --- a/pydsdl/_test.py +++ b/pydsdl/_test.py @@ -11,6 +11,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 @@ -25,6 +26,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: @@ -58,6 +61,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] @@ -1096,7 +1103,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", [ @@ -1105,30 +1112,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() @@ -1161,6 +1165,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 @@ -1265,8 +1315,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") @@ -1614,7 +1663,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"],