From 02019dd16c60fd0cbb0ec3eb80b85bbda42fc4e9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 22 Jun 2025 21:04:42 +0200 Subject: [PATCH] validate sooner --- esphome/core/config.py | 55 +++++++++++++++++----------- tests/unit_tests/core/test_config.py | 19 +++++++--- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/esphome/core/config.py b/esphome/core/config.py index 4d28a81229..7358276754 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -109,13 +109,10 @@ def validate_hostname(config): def validate_id_hash_collisions(config: dict) -> dict: - """Validate that there are no hash collisions between IDs of the same type.""" - from esphome.helpers import fnv1a_32bit_hash - - # Check area hash collisions + """Validate that there are no hash collisions between IDs.""" area_hashes: dict[int, str] = {} - # Check main area if present + # Check main area if CONF_AREA in config: area_id: core.ID = config[CONF_AREA][CONF_ID] if area_id.id: @@ -125,29 +122,45 @@ def validate_id_hash_collisions(config: dict) -> dict: # Check areas list for area in config.get(CONF_AREAS, []): area_id: core.ID = area[CONF_ID] - if area_id.id: - area_hash = fnv1a_32bit_hash(area_id.id) - if area_hash in area_hashes: - raise cv.Invalid( - f"Area ID '{area_id.id}' with hash {area_hash} collides with " - f"existing area ID '{area_hashes[area_hash]}'", - path=[CONF_AREAS, area_id.id], - ) + if not area_id.id: + continue + + area_hash = fnv1a_32bit_hash(area_id.id) + if area_hash not in area_hashes: area_hashes[area_hash] = area_id.id + continue + + # Skip exact duplicates (handled by IDPassValidationStep) + if area_id.id == area_hashes[area_hash]: + continue + + raise cv.Invalid( + f"Area ID '{area_id.id}' with hash {area_hash} collides with " + f"existing area ID '{area_hashes[area_hash]}'", + path=[CONF_AREAS, area_id.id], + ) # Check device hash collisions device_hashes: dict[int, str] = {} for device in config.get(CONF_DEVICES, []): device_id: core.ID = device[CONF_ID] - if device_id.id: - device_hash = fnv1a_32bit_hash(device_id.id) - if device_hash in device_hashes: - raise cv.Invalid( - f"Device ID '{device_id.id}' with hash {device_hash} collides with " - f"existing device ID '{device_hashes[device_hash]}'", - path=[CONF_DEVICES, device_id.id], - ) + if not device_id.id: + continue + + device_hash = fnv1a_32bit_hash(device_id.id) + if device_hash not in device_hashes: device_hashes[device_hash] = device_id.id + continue + + # Skip exact duplicates (handled by IDPassValidationStep) + if device_id.id == device_hashes[device_hash]: + continue + + raise cv.Invalid( + f"Device ID '{device_id.id}' with hash {device_hash} collides " + f"with existing device ID '{device_hashes[device_hash]}'", + path=[CONF_DEVICES, device_id.id], + ) return config diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 11a80e4cc5..ed442b93fa 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -172,11 +172,8 @@ def test_area_id_collision( # Check for the specific error message in stdout captured = capsys.readouterr() - # Since duplicate IDs have the same hash, our hash collision detection catches this - assert ( - "Area ID 'duplicate_id' with hash 1805131238 collides with existing area ID 'duplicate_id'" - in captured.out - ) + # Exact duplicates are now caught by IDPassValidationStep + assert "ID duplicate_id redefined! Check esphome->area->id." in captured.out def test_device_without_area(yaml_file: Callable[[str], str]) -> None: @@ -241,3 +238,15 @@ def test_area_id_hash_collision( "Area ID 'd6ka' with hash 3082558663 collides with existing area ID 'test_2258'" in captured.out ) + + +def test_device_duplicate_id( + yaml_file: Callable[[str], str], capsys: pytest.CaptureFixture[str] +) -> None: + """Test that duplicate device IDs are detected by IDPassValidationStep.""" + result = load_config_from_fixture(yaml_file, "device_duplicate_id.yaml") + assert result is None + + # Check for the specific error message from IDPassValidationStep + captured = capsys.readouterr() + assert "ID duplicate_device redefined!" in captured.out