[substitutions] Improve error messages with include stack trace (#15874)

Co-authored-by: J. Nick Koston <nick@home-assistant.io>

tests/component_tests/packages/test_packages.py

@@ -46,7 +46,7 @@ from esphome.const import (
 )
 from esphome.core import CORE
 from esphome.util import OrderedDict
-from esphome.yaml_util import IncludeFile, add_context, load_yaml
+from esphome.yaml_util import DocumentPath, IncludeFile, add_context, load_yaml
 
 # Test strings
 TEST_DEVICE_NAME = "test_device_name"
@@ -1113,7 +1113,7 @@ def test_packages_include_file_resolves_to_list(mock_resolve_include) -> None:
     """When packages: is an IncludeFile that resolves to a list, it is processed correctly."""
     include_file = MagicMock(spec=IncludeFile)
     package_content = {CONF_WIFI: {CONF_SSID: TEST_PACKAGE_WIFI_SSID}}
-    mock_resolve_include.return_value = ([package_content], None)
+    mock_resolve_include.return_value = [package_content]
 
     config = {CONF_PACKAGES: include_file}
     result = do_packages_pass(config)
@@ -1127,7 +1127,7 @@ def test_packages_include_file_resolves_to_dict(mock_resolve_include) -> None:
     """When packages: is an IncludeFile that resolves to a dict, it is processed correctly."""
     include_file = MagicMock(spec=IncludeFile)
     package_content = {CONF_WIFI: {CONF_SSID: TEST_PACKAGE_WIFI_SSID}}
-    mock_resolve_include.return_value = ({"network": package_content}, None)
+    mock_resolve_include.return_value = {"network": package_content}
 
     config = {CONF_PACKAGES: include_file}
     result = do_packages_pass(config)
@@ -1142,7 +1142,7 @@ def test_packages_include_file_resolves_to_invalid_type_raises(
 ) -> None:
     """When packages: is an IncludeFile that resolves to an invalid type, cv.Invalid is raised."""
     include_file = MagicMock(spec=IncludeFile)
-    mock_resolve_include.return_value = ("not_a_dict_or_list", None)
+    mock_resolve_include.return_value = "not_a_dict_or_list"
 
     config = {CONF_PACKAGES: include_file}
     with pytest.raises(
@@ -1215,7 +1215,9 @@ def test_named_dict_with_include_files_no_false_deprecation_warning(
 
     call_count = 0
 
-    def failing_callback(package_config: dict, context: object) -> dict:
+    def failing_callback(
+        package_config: dict, context: object, path: DocumentPath | None = None
+    ) -> dict:
         nonlocal call_count
         call_count += 1
         if call_count == 1:
@@ -1251,7 +1253,9 @@ def test_validate_deprecated_false_raises_directly(
 
     call_count = 0
 
-    def failing_callback(package_config: dict, context: object) -> dict:
+    def failing_callback(
+        package_config: dict, context: object, path: DocumentPath | None = None
+    ) -> dict:
         nonlocal call_count
         call_count += 1
         if call_count == 1:
@@ -1283,7 +1287,9 @@ def test_error_on_first_declared_package_still_detected() -> None:
 
     call_count = 0
 
-    def fail_on_last(package_config: dict, context: object) -> dict:
+    def fail_on_last(
+        package_config: dict, context: object, path: DocumentPath | None = None
+    ) -> dict:
         nonlocal call_count
         call_count += 1
         # Reverse iteration: third_pkg (1), second_pkg (2), first_pkg (3)
@@ -1312,7 +1318,9 @@ def test_deprecated_single_package_fallback_still_works(
 
     attempt = 0
 
-    def fail_then_succeed(package_config: dict, context: object) -> dict:
+    def fail_then_succeed(
+        package_config: dict, context: object, path: DocumentPath | None = None
+    ) -> dict:
         nonlocal attempt
         attempt += 1
         if attempt == 1:

tests/unit_tests/test_substitutions.py

@@ -659,7 +659,7 @@ def test_resolve_package_max_depth_exceeded(tmp_path: Path) -> None:
         cv.Invalid,
         match=f"Maximum include nesting depth \\({MAX_INCLUDE_DEPTH}\\) exceeded",
     ):
-        processor.resolve_package(package_config, substitutions.ContextVars())
+        processor.resolve_package(package_config, substitutions.ContextVars(), [])
 
 
 def test_include_filename_substitution_undefined_var(tmp_path: Path) -> None:
@@ -690,7 +690,7 @@ def test_raise_first_undefined_logs_extras_at_debug(
         caplog.at_level(logging.DEBUG, logger="esphome.components.substitutions"),
         pytest.raises(cv.Invalid) as exc_info,
     ):
-        substitutions.raise_first_undefined(errors, None, "package definition")
+        substitutions.raise_first_undefined(errors, "package definition")
 
     # First error is surfaced as the cv.Invalid message.
     raised = str(exc_info.value)
@@ -706,7 +706,7 @@ def test_raise_first_undefined_logs_extras_at_debug(
 
 def test_raise_first_undefined_noop_on_empty() -> None:
     """An empty errors list is a no-op — no exception, no log."""
-    substitutions.raise_first_undefined([], None, "package definition")
+    substitutions.raise_first_undefined([], "package definition")
 
 
 def test_do_substitution_pass_included_substitutions_must_be_mapping(
@@ -778,4 +778,43 @@ def test_resolve_package_undefined_var_in_include_filename(tmp_path: Path) -> No
     )
     processor = _PackageProcessor({}, None, False)
     with pytest.raises(cv.Invalid, match="unresolved substitutions"):
-        processor.resolve_package(package_config, substitutions.ContextVars())
+        processor.resolve_package(package_config, substitutions.ContextVars(), [])
+
+
+def test_resolve_include_error_shows_expanded_from_when_substituted(
+    tmp_path: Path,
+) -> None:
+    """When a substituted filename fails to load, the error includes '(expanded from ...)'."""
+    parent = tmp_path / "main.yaml"
+    parent.write_text("")
+
+    def failing_loader(_path: Path) -> None:
+        raise EsphomeError("File not found")
+
+    include = yaml_util.IncludeFile(parent, "${device}.yaml", None, failing_loader)
+    context = substitutions.ContextVars({"device": "my_device"})
+
+    with pytest.raises(cv.Invalid) as exc_info:
+        substitutions.resolve_include(include, [], context)
+
+    msg = str(exc_info.value)
+    assert "my_device.yaml" in msg
+    assert "expanded from '${device}.yaml'" in msg
+
+
+def test_resolve_include_error_no_expanded_from_for_literal_filename(
+    tmp_path: Path,
+) -> None:
+    """When a literal filename fails to load, the error has no 'expanded from' clause."""
+    parent = tmp_path / "main.yaml"
+    parent.write_text("")
+
+    def failing_loader(_path: Path) -> None:
+        raise EsphomeError("File not found")
+
+    include = yaml_util.IncludeFile(parent, "literal.yaml", None, failing_loader)
+
+    with pytest.raises(cv.Invalid) as exc_info:
+        substitutions.resolve_include(include, [], substitutions.ContextVars())
+
+    assert "expanded from" not in str(exc_info.value)

tests/unit_tests/test_yaml_util.py

@@ -9,8 +9,9 @@ from esphome import core, yaml_util
 from esphome.components import substitutions
 from esphome.config_helpers import Extend, Remove
 import esphome.config_validation as cv
-from esphome.core import EsphomeError
+from esphome.core import DocumentLocation, DocumentRange, EsphomeError
 from esphome.util import OrderedDict
+from esphome.yaml_util import ESPHomeDataBase, format_path, make_data_base
 
 
 @pytest.fixture(autouse=True)
@@ -712,3 +713,181 @@ def test_yaml_merge_chain_include_depth_exceeded() -> None:
     yaml_text = "base:\n  <<: !include loop.yaml\n"
     with pytest.raises(EsphomeError, match="Maximum include chain depth"):
         yaml_util.parse_yaml(parent, io.StringIO(yaml_text), self_referencing_loader)
+
+
+def _located(value, doc: str, line: int, col: int):
+    """Return *value* wrapped with a fake ESPHomeDataBase source location."""
+    loc = DocumentLocation(doc, line, col)
+    obj = make_data_base(value)
+    if isinstance(obj, ESPHomeDataBase):
+        obj._esp_range = DocumentRange(loc, loc)
+    return obj
+
+
+def test_format_path_no_location_info_returns_flat_path():
+    """Plain path items with no esp_range produce a simple flat 'In:' line."""
+    result = format_path(["wifi", "ssid"], None)
+    assert result == "In: wifi->ssid"
+
+
+def test_format_path_no_location_info_current_obj_adds_file():
+    """When path has no location but current_obj does, its location is shown."""
+    obj = _located("${var}", "main.yaml", 5, 10)
+    result = format_path(["wifi", "ssid"], obj)
+    assert result == "In: wifi->ssid in main.yaml 6:11"
+
+
+def test_format_path_single_frame_no_include_boundary():
+    """All located keys from the same document → single 'In:' line, no 'Included from'."""
+    path = ["packages", _located("pkg1", "root.yaml", 5, 2)]
+    result = format_path(path, None)
+    assert result.startswith("In: packages->pkg1 in root.yaml 6:3")
+    assert "Included from" not in result
+
+
+def test_format_path_two_frames_shows_included_from():
+    """Keys from two different documents produce 'In:' + one 'Included from' line."""
+    path = [
+        "packages",
+        _located("device", "root.yaml", 10, 2),
+        "packages",
+        _located("inner", "hardware.yaml", 3, 2),
+    ]
+    result = format_path(path, None)
+    assert "In: packages->inner in hardware.yaml 4:3" in result
+    assert "Included from packages->device in root.yaml 11:3" in result
+
+
+def test_format_path_three_frames_full_include_stack():
+    """Three document levels produce two 'Included from' lines in correct order."""
+    path = [
+        "packages",
+        _located("device", "root.yaml", 10, 2),
+        "packages",
+        _located("_wifi_", "hardware.yaml", 43, 2),
+        "packages",
+        _located("_roam_", "wifi.yaml", 25, 2),
+    ]
+    result = format_path(path, None)
+    lines = result.splitlines()
+    assert lines[0].startswith("In: packages->_roam_ in wifi.yaml")
+    assert lines[1].startswith("  Included from packages->_wifi_ in hardware.yaml")
+    assert lines[2].startswith("  Included from packages->device in root.yaml")
+
+
+def test_format_path_current_obj_overrides_innermost_location():
+    """current_obj's esp_range replaces the key's column for the 'In:' line."""
+    path = ["packages", _located("pkg1", "root.yaml", 5, 2)]
+    # Value (the expression) sits at column 10, not column 2 like the key
+    value = _located("${undefined}", "root.yaml", 5, 10)
+    result = format_path(path, value)
+    assert "6:11" in result
+    assert "6:3" not in result
+
+
+def test_format_path_empty_path_with_no_location():
+    """Empty path with no location info returns 'In: '."""
+    result = format_path([], None)
+    assert result == "In: "
+
+
+def test_format_path_integer_path_items_formatted_as_subscript():
+    """Integer indices are rendered as [n] subscripts in the flat fallback."""
+    result = format_path(["packages", 0], None)
+    assert result == "In: packages[0]"
+
+
+def test_format_path_integer_list_index_attached_to_previous_frame():
+    """A list index between two include boundaries attaches to the outer frame."""
+    path = [
+        "packages",
+        _located("packages", "main.yaml", 5, 0),
+        0,
+        _located("packages", "level1.yaml", 2, 0),
+        0,
+        _located("esphome", "level2.yaml", 0, 0),
+        _located("name", "level2.yaml", 1, 8),
+    ]
+    result = format_path(path, None)
+    lines = result.splitlines()
+    assert lines[0].startswith("In: esphome->name in level2.yaml")
+    assert "packages[0]" in lines[1] and "level1.yaml" in lines[1]
+    assert "packages[0]" in lines[2] and "main.yaml" in lines[2]
+
+
+def test_format_path_trailing_unlocated_string_after_located_key():
+    """Plain string keys after the last located key must still appear in output."""
+    path = [_located("packages", "main.yaml", 5, 0), "sub", "key"]
+    result = format_path(path, None)
+    assert result == "In: packages->sub->key in main.yaml 6:1"
+
+
+def test_format_path_trailing_unlocated_int_attaches_to_current_frame():
+    """Trailing ints attach to the open frame's last key (subscript), strings
+    buffer until end-of-path and then flush behind."""
+    path = [_located("packages", "main.yaml", 5, 0), 0, "sub"]
+    result = format_path(path, None)
+    # Int attaches to 'packages' as [0] subscript; trailing 'sub' is flushed
+    # at end and appears after.
+    assert result == "In: packages[0]->sub in main.yaml 6:1"
+
+
+def test_format_path_only_trailing_unlocated_strings_are_preserved():
+    """Trailing pending items must not be silently dropped after the last frame."""
+    path = [
+        _located("packages", "main.yaml", 5, 0),
+        _located("inner", "hardware.yaml", 3, 0),
+        "tail1",
+        "tail2",
+    ]
+    result = format_path(path, None)
+    lines = result.splitlines()
+    assert lines[0] == "In: inner->tail1->tail2 in hardware.yaml 4:1"
+    assert lines[1] == "  Included from packages in main.yaml 6:1"
+
+
+def test_format_path_leading_int_with_no_current_doc_goes_to_pending():
+    """An int before any located key is buffered and shown in the first frame."""
+    path = [0, _located("name", "main.yaml", 1, 0)]
+    result = format_path(path, None)
+    # Leading ints have no preceding name to subscript onto, so they render
+    # as bare [n] in the formatted segment.
+    assert result == "In: [0]->name in main.yaml 2:1"
+
+
+def test_format_path_only_unlocated_int_returns_flat_fallback():
+    """Path with only an int and no location info renders via the flat fallback."""
+    result = format_path([0], None)
+    assert result == "In: [0]"
+
+
+def test_format_path_current_obj_in_different_doc_than_innermost_frame():
+    """current_obj's location is preferred even when its document differs from the frame's."""
+    path = [_located("packages", "root.yaml", 1, 0)]
+    value = _located("${var}", "other.yaml", 9, 4)
+    result = format_path(path, value)
+    # Innermost line uses current_obj's mark (other.yaml 10:5), not the key's.
+    assert result == "In: packages in other.yaml 10:5"
+
+
+def test_format_path_current_obj_without_location_falls_back_to_key():
+    """An ESPHomeDataBase current_obj with no esp_range falls back to the key's location."""
+
+    class _NoRange(ESPHomeDataBase, str):
+        pass
+
+    obj = _NoRange.__new__(_NoRange, "value")
+    str.__init__(obj)
+    # No _esp_range set on this instance.
+    assert obj.esp_range is None
+
+    path = [_located("packages", "main.yaml", 5, 2)]
+    result = format_path(path, obj)
+    assert result == "In: packages in main.yaml 6:3"
+
+
+def test_format_path_empty_path_with_located_current_obj():
+    """An empty path with a located current_obj still surfaces the location."""
+    obj = _located("${var}", "main.yaml", 0, 0)
+    result = format_path([], obj)
+    assert result == "In:  in main.yaml 1:1"