From e7a7aa150dab1f645520bf7807f477a9e8400384 Mon Sep 17 00:00:00 2001 From: Gunni Date: Tue, 13 Oct 2026 01:36:57 +0000 Subject: [PATCH] [yaml_util] Raise a clear error for a non-mapping `defaults:` in a package (#19425) Co-authored-by: Claude Opus 5 Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/yaml_util.py | 15 ++++++- tests/unit_tests/test_substitutions.py | 58 ++++++++++++++++++++++++++ tests/unit_tests/test_yaml_util.py | 23 ++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index f02819a3ed..b5e2ec17e8 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -191,10 +191,23 @@ def add_context(value: Any, context_vars: dict[str, Any] | None) -> Any: If the value is already tagged, the new context vars are merged with existing ones, with new vars taking precedence. Returns the value tagged with ConfigContext. Returns the original value if value is not a list/string/dict. + + Raises EsphomeError if a `defaults:` key is present but is not a mapping. """ if isinstance(value, dict) and CONF_DEFAULTS in value: + defaults = value.pop(CONF_DEFAULTS) + if defaults is None: + raise EsphomeError( + f"'{CONF_DEFAULTS}' is empty; " + f"remove the key or give it at least one value" + ) + if not isinstance(defaults, dict): + raise EsphomeError( + f"'{CONF_DEFAULTS}' must be a key to value mapping, " + f"got {type(defaults)}" + ) context_vars = { - **value.pop(CONF_DEFAULTS), + **defaults, **(context_vars or {}), } diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index 198b002d43..b0d3656101 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -988,6 +988,64 @@ def test_resolve_include_error_shows_expanded_from_when_substituted( assert "expanded from '${device}.yaml'" in msg +def test_resolve_include_empty_defaults_names_file(tmp_path: Path) -> None: + """An empty `defaults:` in an included file surfaces as cv.Invalid naming that file. + + add_context() raises EsphomeError, so resolve_include's existing handler can + wrap it with the filename and document path. A bare TypeError would escape + that handler and reach the user as an unhandled traceback. + """ + parent = tmp_path / "main.yaml" + parent.write_text("") + + def loader(_path: Path) -> dict[str, Any]: + return {"defaults": None, "ota": [{"platform": "esphome"}]} + + include = yaml_util.IncludeFile(parent, "_network/ota.yaml", loader) + + with pytest.raises(cv.Invalid) as exc_info: + substitutions.resolve_include(include, [], substitutions.ContextVars()) + + msg = str(exc_info.value) + assert "_network/ota.yaml" in msg + assert "'defaults' is empty" in msg + + +def test_do_packages_pass_nested_empty_defaults_names_file(tmp_path: Path) -> None: + """An empty `defaults:` two package levels deep still names the offending file. + + Mirrors a real layout: the top-level config includes a network package whose + own `packages:` block includes the file with the empty `defaults:`. + """ + parent = tmp_path / "main.yaml" + parent.write_text("") + + def inner_loader(_path: Path) -> dict[str, Any]: + return {"defaults": None, "ota": [{"platform": "esphome"}]} + + def outer_loader(_path: Path) -> dict[str, Any]: + return { + "packages": { + "otas": yaml_util.IncludeFile(parent, "_network/ota.yaml", inner_loader) + } + } + + config = OrderedDict( + { + "packages": { + "wifis": yaml_util.IncludeFile(parent, "network.yaml", outer_loader) + } + } + ) + + with pytest.raises(cv.Invalid) as exc_info: + do_packages_pass(config) + + msg = str(exc_info.value) + assert "_network/ota.yaml" in msg + assert "'defaults' is empty" in msg + + def test_resolve_include_error_no_expanded_from_for_literal_filename( tmp_path: Path, ) -> None: diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index c30b03467f..c3276823ec 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -569,6 +569,29 @@ def test_config_context_defaults_explicit_vars_override() -> None: assert "defaults" not in tagged +def test_config_context_defaults_empty_raises() -> None: + """Test that an empty defaults: key raises EsphomeError, not a bare TypeError. + + A ``defaults:`` block with an empty (or comments-only) body parses as None. + Unpacking it used to fail with ``TypeError: 'NoneType' object is not a + mapping``, which escapes the EsphomeError handler in resolve_include and + reaches the user as an unhandled traceback naming no file. + """ + data = {"defaults": None, "key": "value"} + + with pytest.raises(EsphomeError, match="'defaults' is empty"): + yaml_util.add_context(data, None) + + +@pytest.mark.parametrize("defaults", ["oops", ["a", "b"], 42]) +def test_config_context_defaults_non_mapping_raises(defaults) -> None: + """Test that a non-mapping defaults: value raises EsphomeError naming the type.""" + data = {"defaults": defaults, "key": "value"} + + with pytest.raises(EsphomeError, match="'defaults' must be a key to value mapping"): + yaml_util.add_context(data, None) + + def test_represent_extend() -> None: """Test that Extend objects are dumped as plain !extend scalars.""" assert yaml_util.dump({"key": Extend("my_id")}) == "key: !extend 'my_id'\n"