diff --git a/src/pf1e_simulator/map.py b/src/pf1e_simulator/map.py index eff99da..df26dd5 100644 --- a/src/pf1e_simulator/map.py +++ b/src/pf1e_simulator/map.py @@ -12,10 +12,14 @@ from typing import TYPE_CHECKING import yaml from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator +from yaml.nodes import ScalarNode if TYPE_CHECKING: + from collections.abc import Hashable from pathlib import Path + from yaml.nodes import MappingNode + Pos = tuple[int, int] # (row, col), 0-indexed _ELEVATION_CHARS = frozenset("0123456789.") # digit = height, '.' = ground level @@ -38,6 +42,29 @@ class MapValidationError(Exception): self.reason = reason +class _NoDuplicateLoader(yaml.SafeLoader): + """SafeLoader that rejects duplicate mapping keys (PyYAML is last-wins).""" + + +def _construct_mapping( + loader: _NoDuplicateLoader, node: MappingNode, *, deep: bool = False +) -> dict[Hashable, object]: + seen: set[str] = set() + for key_node, _ in node.value: + if isinstance(key_node, ScalarNode): + key = loader.construct_scalar(key_node) + if key in seen: + msg = f"duplicate mapping key: {key!r}" + raise yaml.constructor.ConstructorError(None, None, msg, key_node.start_mark) + seen.add(key) + return yaml.constructor.SafeConstructor.construct_mapping(loader, node, deep) + + +_NoDuplicateLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _construct_mapping +) + + class TerrainType(BaseModel): """What one square means for movement and sight lines.""" @@ -177,7 +204,13 @@ def load_map(path: Path) -> MapSpec: msg = f"cannot read file: {exc}" raise MapValidationError(path, msg) from exc try: - data: object = yaml.safe_load(text) + # _NoDuplicateLoader subclasses SafeLoader: no arbitrary object + # instantiation, it only adds duplicate-key rejection (S506 is a + # false positive for this loader). + data: object = yaml.load( + text, + Loader=_NoDuplicateLoader, # noqa: S506 + ) except yaml.YAMLError as exc: msg = f"invalid YAML: {exc}" raise MapValidationError(path, msg) from exc diff --git a/tests/test_map.py b/tests/test_map.py index 5745b46..68f2ec0 100644 --- a/tests/test_map.py +++ b/tests/test_map.py @@ -67,6 +67,27 @@ def test_minimal_map_without_optional_layers(tmp_path: Path) -> None: assert zone_cells(spec, "A") == [] +def test_rejects_duplicate_yaml_keys(tmp_path: Path) -> None: + """PyYAML is silently last-wins on duplicate mapping keys; a map that + declares `name` (or a legend entry) twice must fail, not keep one.""" + body = _minimal_map() + 'name: "Test bis"\n' + with pytest.raises(MapValidationError, match="duplicate mapping key"): + load_map(_write(tmp_path, body)) + + +def test_rejects_duplicate_legend_keys(tmp_path: Path) -> None: + body = _minimal_map().replace( + ' ".": { type: floor, move_cost: 1 }\n', + ' ".": { type: floor, move_cost: 1 }\n ".": { type: wall, move_cost: null }\n', + ) + with pytest.raises(MapValidationError, match="duplicate mapping key"): + load_map(_write(tmp_path, body)) + + +def test_accepts_map_without_duplicates(tmp_path: Path) -> None: + assert load_map(_write(tmp_path, _minimal_map())).name == "Test" + + def test_rejects_ragged_rows(tmp_path: Path) -> None: body = _minimal_map().replace(" #.#\n", " #.##\n") with pytest.raises(MapValidationError):