fix: issue with list-errors and mismatched config types (#2151)

src/ape/api/config.py

@@ -3,7 +3,7 @@
 from enum import Enum
 from functools import cached_property
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Optional, TypeVar, cast
+from typing import Any, Optional, TypeVar, cast
 
 import yaml
 from ethpm_types import PackageManifest, PackageMeta, Source
@@ -24,9 +24,6 @@
 )
 from ape.utils.misc import load_config
 
-if TYPE_CHECKING:
-    from ape.managers.config import ConfigManager
-
 ConfigItemType = TypeVar("ConfigItemType")
 
 
@@ -56,9 +53,6 @@ class PluginConfig(BaseSettings):
     a config API must register a subclass of this class.
     """
 
-    # NOTE: This is probably partially initialized at the time of assignment
-    _config_manager: Optional["ConfigManager"]
-
     model_config = SettingsConfigDict(extra="allow")
 
     @classmethod
@@ -74,7 +68,11 @@ def update(root: dict, value_map: dict):
 
             return root
 
-        return cls(**update(default_values, overrides))
+        data = update(default_values, overrides)
+        try:
+            return cls.model_validate(data)
+        except ValidationError as err:
+            raise ConfigError(str(err)) from err
 
     @only_raise_attribute_error
     def __getattr__(self, attr_name: str) -> Any:
@@ -139,6 +137,85 @@ class DeploymentConfig(PluginConfig):
     """
 
 
+def _get_problem_with_config(errors: list, path: Path) -> Optional[str]:
+    # Attempt to find line numbers in the config matching.
+    cfg_content = Source(content=path.read_text(encoding="utf8")).content
+    if not cfg_content:
+        # Likely won't happen?
+        return None
+
+    err_map: dict = {}
+    for error in errors:
+        if not (location := error.get("loc")):
+            continue
+
+        lineno = None
+        loc_idx = 0
+        depth = len(location)
+        list_counter = 0
+        for no, line in cfg_content.items():
+            loc = location[loc_idx]
+            clean_line = line.lstrip()
+
+            if isinstance(loc, int):
+                if not clean_line.startswith("- "):
+                    # Not a list item.
+                    continue
+
+                elif list_counter != loc:
+                    # Still search for index.
+                    list_counter += 1
+                    continue
+
+                # If we get here, we have found the index.
+                list_counter = 0
+
+            elif not clean_line.startswith(f"{loc}:"):
+                continue
+
+            # Look for the next location up the tree.
+            loc_idx += 1
+
+            # Even if we don't find the full path for some reason
+            # (perhaps it's missing), we still have the last lineno noticed.
+            lineno = no
+
+            if loc_idx < depth:
+                continue
+
+            # Found.
+            break
+
+        if lineno is not None:
+            err_map[lineno] = error
+        # else: we looped through the whole file and didn't find anything.
+
+    if not err_map:
+        # raise ValidationError as-is (no line numbers found for some reason).
+        return None
+
+    error_strs: list[str] = []
+    for line_no, cfg_err in err_map.items():
+        sub_message = cfg_err.get("msg", cfg_err)
+        line_before = line_no - 1 if line_no > 1 else None
+        line_after = line_no + 1 if line_no < len(cfg_content) else None
+        lines = []
+        if line_before is not None:
+            lines.append(f"   {line_before}: {cfg_content[line_before]}")
+        lines.append(f"-->{line_no}: {cfg_content[line_no]}")
+        if line_after is not None:
+            lines.append(f"   {line_after}: {cfg_content[line_after]}")
+
+        file_preview = "\n".join(lines)
+        sub_message = f"{sub_message}\n{file_preview}\n"
+        error_strs.append(sub_message)
+
+    # NOTE: Using reversed because later pydantic errors
+    #   appear earlier in the list.
+    final_msg = "\n".join(reversed(error_strs)).strip()
+    return f"'{clean_path(path)}' is invalid!\n{final_msg}"
+
+
 class ApeConfig(ExtraAttributesMixin, BaseSettings, ManagerAccessMixin):
     """
     The top-level config.
@@ -282,65 +359,10 @@ def validate_file(cls, path: Path, **overrides) -> "ApeConfig":
                 # TODO: Support JSON configs here.
                 raise  # The validation error as-is
 
-            # Several errors may have been collected from the config.
-            all_errors = err.errors()
-            # Attempt to find line numbers in the config matching.
-            cfg_content = Source(content=path.read_text(encoding="utf8")).content
-            if not cfg_content:
-                # Likely won't happen?
-                raise  # The validation error as-is
-
-            err_map: dict = {}
-            for error in all_errors:
-                if not (location := error.get("loc")):
-                    continue
-
-                lineno = None
-                loc_idx = 0
-                depth = len(location)
-                for no, line in cfg_content.items():
-                    loc = location[loc_idx]
-                    if not line.lstrip().startswith(f"{loc}:"):
-                        continue
-
-                    # Look for the next location up the tree.
-                    loc_idx += 1
-                    if loc_idx < depth:
-                        continue
-
-                    # Found.
-                    lineno = no
-                    break
-
-                if lineno is not None and loc_idx >= depth:
-                    err_map[lineno] = error
-                # else: we looped through the whole file and didn't find anything.
-
-            if not err_map:
-                # raise ValidationError as-is (no line numbers found for some reason).
-                raise
-
-            error_strs: list[str] = []
-            for line_no, cfg_err in err_map.items():
-                sub_message = cfg_err.get("msg", cfg_err)
-                line_before = line_no - 1 if line_no > 1 else None
-                line_after = line_no + 1 if line_no < len(cfg_content) else None
-                lines = []
-                if line_before is not None:
-                    lines.append(f"   {line_before}: {cfg_content[line_before]}")
-                lines.append(f"-->{line_no}: {cfg_content[line_no]}")
-                if line_after is not None:
-                    lines.append(f"   {line_after}: {cfg_content[line_after]}")
-
-                file_preview = "\n".join(lines)
-                sub_message = f"{sub_message}\n{file_preview}\n"
-                error_strs.append(sub_message)
-
-            # NOTE: Using reversed because later pydantic errors
-            #   appear earlier in the list.
-            final_msg = "\n".join(reversed(error_strs)).strip()
-            final_msg = f"'{clean_path(path)}' is invalid!\n{final_msg}"
-            raise ConfigError(final_msg)
+            if final_msg := _get_problem_with_config(err.errors(), path):
+                raise ConfigError(final_msg)
+            else:
+                raise ConfigError(str(err)) from err
 
     @classmethod
     def from_manifest(cls, manifest: PackageManifest, **overrides) -> "ApeConfig":

src/ape/managers/config.py

@@ -151,14 +151,14 @@ def _merge_configs(base: dict, secondary: dict) -> dict:
         if key not in secondary:
             result[key] = value
 
-        elif not isinstance(value, dict):
+        elif not isinstance(value, dict) or not isinstance(secondary[key], dict):
             # Is a primitive value found in both configs.
             # Must use the second one.
             result[key] = secondary[key]
 
         else:
             # Merge the dictionaries.
-            sub = _merge_configs(base[key], secondary[key])
+            sub = _merge_configs(value, secondary[key])
             result[key] = sub
 
     # Add missed keys from secondary.

tests/functional/test_config.py

@@ -3,7 +3,6 @@
 from typing import Optional, Union
 
 import pytest
-from pydantic import ValidationError
 from pydantic_settings import SettingsConfigDict
 
 from ape.api.config import ApeConfig, ConfigEnum, PluginConfig
@@ -68,6 +67,33 @@ def test_validate_file_expands_env_vars():
             del os.environ[env_var_name]
 
 
+def test_validate_file_shows_linenos():
+    with create_tempdir() as temp_dir:
+        file = temp_dir / "ape-config.yaml"
+        file.write_text("name: {'test': 123}")
+
+        expected = (
+            f"'{temp_dir / 'ape-config.yaml'}' is invalid!"
+            "\nInput should be a valid string\n-->1: name: {'test': 123}"
+        )
+        with pytest.raises(ConfigError) as err:
+            _ = ApeConfig.validate_file(file)
+
+        assert expected in str(err.value)
+
+
+def test_validate_file_shows_linenos_handles_lists():
+    with create_tempdir() as temp_dir:
+        file = temp_dir / "ape-config.yaml"
+        file.write_text("deployments:\n  ethereum:\n   sepolia:\n      - foo: bar")
+        with pytest.raises(ConfigError) as err:
+            _ = ApeConfig.validate_file(file)
+
+        assert str(file) in str(err.value)
+        assert "sepolia:" in str(err.value)
+        assert "-->4" in str(err.value)
+
+
 def test_deployments(networks_connected_to_tester, owner, vyper_contract_container, project):
     _ = networks_connected_to_tester  # Connection needs to lookup config.
 
@@ -300,6 +326,15 @@ def test_merge_configs_short_circuits():
     assert merge_configs(ex, {}) == merge_configs({}, ex) == ex
 
 
+def test_merge_configs_wrong_type():
+    cfg_0 = {"foo": 123}
+    cfg_1 = {"foo": {"bar": 123}}
+    actual = merge_configs(cfg_0, cfg_1)
+    assert actual["foo"] == {"bar": 123}
+    actual = merge_configs(cfg_1, cfg_0)
+    assert actual["foo"] == 123
+
+
 def test_plugin_config_getattr_and_getitem(config):
     config = config.get_config("ethereum")
     assert config.mainnet is not None
@@ -389,14 +424,14 @@ def test_get_config_unknown_plugin(config):
 def test_get_config_invalid_plugin_config(project):
     with project.temp_config(node={"ethereum": [1, 2]}):
         # Show project's ApeConfig model works.
-        with pytest.raises(ValidationError):
+        with pytest.raises(ConfigError):
             project.config.get_config("node")
 
         # Show the manager-wrapper also works
         # (simple wrapper for local project's config,
         # but at one time pointlessly overrode the `get_config()`
         # which caused issues).
-        with pytest.raises(ValidationError):
+        with pytest.raises(ConfigError):
             project.config_manager.get_config("node")