558 usethis tool coverage breaks existing config in some cases (#560)

* Add failing test to reproduce bug in `usethis tool coverage` breaking TOML config

* Switch to using tmp_path in test_after_codespell test

* Add lower-level failing tests

* Add lower level tests

* Finish merge

* Use better strategy for handling high levels of nesting in tomlkit workaround

* Tweak tests to accomodate slightly different config structure

* Simplify logic and handle edge case of root level config

* Empty commit to trigger CI

* Add additional check to failing test

* Fix indexing

* Handle merge case

* Use "seeding" approach to fine-tune workaround

* Massaging logic to pass tests

* Restrict workarond

* Modify test to reflect updated behaviour

* Bump tomlkit

* Pass tests

* Pass tests

* Tweak logic to pass tests

* Revert "Bump tomlkit"

This reverts commit f17264c.

* Re-enable PLR rule

Author: nathanjmcdougall

src/usethis/_integrations/file/toml/io_.py

@@ -11,7 +11,7 @@
 from pydantic import TypeAdapter
 from tomlkit import TOMLDocument
 from tomlkit.exceptions import TOMLKitError
-from typing_extensions import assert_never
+from typing_extensions import Never, assert_never
 
 from usethis._integrations.file.toml.errors import (
     TOMLDecodeError,
@@ -34,6 +34,8 @@
     from pathlib import Path
     from typing import ClassVar
 
+    from tomlkit.container import Container
+    from tomlkit.items import Item
     from typing_extensions import Self
 
     from usethis._io import Key
@@ -145,7 +147,7 @@ def set_value(
                 return
 
         d, parent = toml_document, {}
-        shared_keys = []
+        shared_keys: list[str] = []
         try:
             # Index our way into each ID key.
             # Eventually, we should land at a final dict, which is the one we are setting.
@@ -155,42 +157,24 @@ def set_value(
                 d, parent = d[key], d
                 shared_keys.append(key)
         except KeyError:
-            # The old configuration should be kept for all ID keys except the
-            # final/deepest one which shouldn't exist anyway since we checked as much,
-            # above. For example, if there is [tool.ruff] then we shouldn't overwrite it
-            # with [tool.deptry]; they should coexist. So under the "tool" key, we need
-            # to "merge" the two dicts.
-
-            if len(keys) <= 3:
-                contents = value
-                for key in reversed(keys):
-                    contents = {key: contents}
-                toml_document = mergedeep.merge(toml_document, contents)  # type: ignore[reportAssignmentType]
-                assert isinstance(toml_document, TOMLDocument)
-            else:
-                # Note that this alternative logic is just to avoid a bug:
-                # https://github.com/nathanjmcdougall/usethis-python/issues/507
-                TypeAdapter(dict).validate_python(d)
-                assert isinstance(d, dict)
-
-                unshared_keys = keys[len(shared_keys) :]
-
-                d[_get_unified_key(unshared_keys)] = value
+            _set_value_in_existing(
+                toml_document=toml_document,
+                current_container=d,
+                keys=keys,
+                current_keys=shared_keys,
+                value=value,
+            )
         else:
             if not exists_ok:
                 # The configuration is already present, which is not allowed.
-                if keys:
-                    msg = f"Configuration value '{print_keys(keys)}' is already set."
-                else:
-                    msg = "Configuration value at root level is already set."
-                raise TOMLValueAlreadySetError(msg)
+                _raise_already_set(keys)
             else:
                 # The configuration is already present, but we're allowed to overwrite it.
                 TypeAdapter(dict).validate_python(parent)
                 assert isinstance(parent, dict)
                 parent[keys[-1]] = value
 
-        self.commit(toml_document)
+        self.commit(toml_document)  # type: ignore[reportAssignmentType]
 
     def __delitem__(self, keys: Sequence[Key]) -> None:
         """Delete a value in the TOML file.
@@ -322,6 +306,50 @@ def remove_from_list(self, *, keys: Sequence[Key], values: list[Any]) -> None:
         self.commit(toml_document)
 
 
+def _set_value_in_existing(
+    *,
+    toml_document: TOMLDocument,
+    current_container: TOMLDocument | Item | Container,
+    keys: Sequence[Key],
+    current_keys: Sequence[Key],
+    value: Any,
+) -> None:
+    # The old configuration should be kept for all ID keys except the
+    # final/deepest one which shouldn't exist anyway since we checked as much,
+    # above. For example, if there is [tool.ruff] then we shouldn't overwrite it
+    # with [tool.deptry]; they should coexist. So under the "tool" key, we need
+    # to "merge" the two dicts.
+
+    if len(keys) <= 3:
+        contents = value
+        for key in reversed(keys):
+            contents = {key: contents}
+        toml_document = mergedeep.merge(toml_document, contents)  # type: ignore[reportAssignmentType]
+        assert isinstance(toml_document, TOMLDocument)
+    else:
+        # Note that this alternative logic is just to avoid a bug:
+        # https://github.com/nathanjmcdougall/usethis-python/issues/507
+        TypeAdapter(dict).validate_python(current_container)
+        assert isinstance(current_container, dict)
+
+        unshared_keys = keys[len(current_keys) :]
+
+        if len(current_keys) == 1:
+            # In this case, we need to "seed" the section to avoid another bug:
+            # https://github.com/nathanjmcdougall/usethis-python/issues/558
+
+            placeholder = {keys[0]: {keys[1]: {}}}
+            toml_document = mergedeep.merge(toml_document, placeholder)  # type: ignore[reportArgumentType]
+
+            contents = value
+            for key in reversed(unshared_keys[1:]):
+                contents = {key: contents}
+
+            current_container[keys[1]] = contents  # type: ignore[reportAssignmentType]
+        else:
+            current_container[_get_unified_key(unshared_keys)] = value
+
+
 def _validate_keys(keys: Sequence[Key]) -> list[str]:
     """Validate the keys.
 
@@ -348,6 +376,15 @@ def _validate_keys(keys: Sequence[Key]) -> list[str]:
     return so_far_keys
 
 
+def _raise_already_set(keys: Sequence[Key]) -> Never:
+    """Raise an error if the configuration is already set."""
+    if keys:
+        msg = f"Configuration value '{print_keys(keys)}' is already set."
+    else:
+        msg = "Configuration value at root level is already set."
+    raise TOMLValueAlreadySetError(msg)
+
+
 def _get_unified_key(keys: Sequence[Key]) -> tomlkit.items.Key:
     keys = _validate_keys(keys)
 

tests/usethis/_core/test_core_tool.py

@@ -272,14 +272,17 @@ def test_from_nothing(
                 assert Dependency(
                     name="coverage", extras=frozenset({"toml"})
                 ) in get_deps_from_group("test")
-                out, err = capfd.readouterr()
-                assert not err
-                assert out == (
-                    "✔ Adding dependency 'coverage' to the 'test' group in 'pyproject.toml'.\n"
-                    "☐ Install the dependency 'coverage'.\n"
-                    "✔ Adding coverage config to 'pyproject.toml'.\n"
-                    "☐ Run 'uv run coverage help' to see available coverage commands.\n"
-                )
+
+                assert ["tool", "uv", "default-groups"] in PyprojectTOMLManager()
+
+            out, err = capfd.readouterr()
+            assert not err
+            assert out == (
+                "✔ Adding dependency 'coverage' to the 'test' group in 'pyproject.toml'.\n"
+                "☐ Install the dependency 'coverage'.\n"
+                "✔ Adding coverage config to 'pyproject.toml'.\n"
+                "☐ Run 'uv run coverage help' to see available coverage commands.\n"
+            )
 
         @pytest.mark.usefixtures("_vary_network_conn")
         def test_no_pyproject_toml(
@@ -387,6 +390,36 @@ class .*\\bProtocol\\):
 """
             )
 
+    @pytest.mark.usefixtures("_vary_network_conn")
+    def test_after_codespell(self, tmp_path: Path):
+        # To check the config is valid
+        # https://github.com/nathanjmcdougall/usethis-python/issues/558
+
+        # Arrange
+        (tmp_path / "pyproject.toml").write_text("""\
+[project]
+name = "example"
+version = "0.1.0"
+description = "Add your description here"
+
+[dependency-groups]
+dev = [
+    "codespell>=2.4.1",
+]
+                                                    
+[tool.codespell]
+ignore-regex = ["[A-Za-z0-9+/]{100,}"]
+""")
+
+        # Act
+        with change_cwd(tmp_path), files_manager():
+            use_coverage()
+
+            # Assert
+            assert ["tool", "coverage"] in PyprojectTOMLManager()
+        content = (tmp_path / "pyproject.toml").read_text()
+        assert "[tool.coverage]" in content
+
     class TestRemove:
         def test_unused(self, uv_init_dir: Path, capfd: pytest.CaptureFixture[str]):
             with change_cwd(uv_init_dir), PyprojectTOMLManager():

tests/usethis/_integrations/file/pyproject_toml/test_pyproject_toml_io_.py

@@ -269,6 +269,27 @@ def test_update_not_exists_ok(self, tmp_path: Path):
                     keys=["tool", "usethis", "key"], value="value2", exists_ok=False
                 )
 
+        def test_coverage_to_codespell(self, tmp_path: Path):
+            # https://github.com/nathanjmcdougall/usethis-python/issues/558
+
+            # Arrange
+            (tmp_path / "pyproject.toml").write_text("""\
+[tool.codespell]
+ignore-regex = ["[A-Za-z0-9+/]{100,}"]
+""")
+
+            # Act
+            with change_cwd(tmp_path), PyprojectTOMLManager() as file_manager:
+                file_manager.set_value(
+                    keys=["tool", "coverage", "run", "source"], value=["."]
+                )
+
+                # Assert
+                assert ["tool", "coverage"] in PyprojectTOMLManager()
+
+            with change_cwd(tmp_path), PyprojectTOMLManager() as file_manager:
+                assert ["tool", "coverage"] in PyprojectTOMLManager()
+
     class TestDel:
         def test_already_missing(self, tmp_path: Path):
             # Arrange

tests/usethis/_integrations/sonarqube/test_sonarqube_config.py

@@ -38,17 +38,20 @@ def test_file_not_exists(self, uv_init_dir: Path):
         # If the file does not exist, we should construct based on information in
         # the repo.
 
+        # Arrange
         with change_cwd(uv_init_dir), PyprojectTOMLManager():
-            # Arrange
             PyprojectTOMLManager().set_value(
                 keys=["tool", "usethis", "sonarqube", "project-key"], value="foobar"
             )
             PyprojectTOMLManager().set_value(
                 keys=["tool", "coverage", "xml", "output"], value="coverage.xml"
             )
             python_pin("3.12")
+        content = (uv_init_dir / "pyproject.toml").read_text()
+        assert "xml" in content
 
-            # Act
+        # Act
+        with change_cwd(uv_init_dir), PyprojectTOMLManager():
             result = get_sonar_project_properties()
 
         # Assert
@@ -79,7 +82,10 @@ def test_different_python_version(self, tmp_path: Path):
             PyprojectTOMLManager().set_value(
                 keys=["tool", "coverage", "xml", "output"], value="coverage.xml"
             )
+        content = (tmp_path / "pyproject.toml").read_text()
+        assert "xml" in content
 
+        with change_cwd(tmp_path), PyprojectTOMLManager():
             # Act
             result = get_sonar_project_properties()
 

tests/usethis/_interface/test_tool.py

@@ -4,6 +4,8 @@
 from typer.testing import CliRunner
 
 from usethis._config import usethis_config
+from usethis._config_file import files_manager
+from usethis._integrations.file.pyproject_toml.io_ import PyprojectTOMLManager
 from usethis._integrations.uv.call import call_uv_subprocess
 from usethis._interface.tool import ALL_TOOL_COMMANDS, app
 from usethis._subprocess import SubprocessFailedError, call_subprocess
@@ -55,6 +57,42 @@ def test_runs(self, tmp_path: Path):
             assert result.exit_code == 0, result.output
             call_subprocess(["coverage", "run", "."])
 
+    @pytest.mark.usefixtures("_vary_network_conn")
+    def test_after_codespell(self, tmp_path: Path):
+        # To check the config is valid
+        # https://github.com/nathanjmcdougall/usethis-python/issues/558
+
+        # Arrange
+        (tmp_path / "pyproject.toml").write_text("""\
+[project]
+name = "example"
+version = "0.1.0"
+description = "Add your description here"
+
+[dependency-groups]
+dev = [
+    "codespell>=2.4.1",
+]
+                                                    
+[tool.codespell]
+ignore-regex = ["[A-Za-z0-9+/]{100,}"]
+""")
+
+        runner = CliRunner()
+        with change_cwd(tmp_path):
+            # Act
+            if not usethis_config.offline:
+                result = runner.invoke(app, ["coverage"])
+            else:
+                result = runner.invoke(app, ["coverage", "--offline"])
+
+            # Assert
+            assert result.exit_code == 0, result.output
+            with files_manager():
+                assert ["tool", "coverage"] in PyprojectTOMLManager()
+
+        assert "[tool.coverage]" in (tmp_path / "pyproject.toml").read_text()
+
 
 class TestDeptry:
     @pytest.mark.usefixtures("_vary_network_conn")

tests/usethis/_tool/impl/test_coverage.py

@@ -0,0 +1,38 @@
+from pathlib import Path
+
+from usethis._config_file import files_manager
+from usethis._integrations.file.pyproject_toml.io_ import PyprojectTOMLManager
+from usethis._test import change_cwd
+from usethis._tool.impl.coverage import CoverageTool
+
+
+class TestCoverageTool:
+    class TestAddConfigs:
+        def test_after_codespell(self, tmp_path: Path):
+            # To check the config is valid
+            # https://github.com/nathanjmcdougall/usethis-python/issues/558
+
+            # Arrange
+            (tmp_path / "pyproject.toml").write_text("""\
+[project]
+name = "example"
+version = "0.1.0"
+description = "Add your description here"
+
+[dependency-groups]
+dev = [
+    "codespell>=2.4.1",
+]
+                                                    
+[tool.codespell]
+ignore-regex = ["[A-Za-z0-9+/]{100,}"]
+""")
+
+            # Act
+            with change_cwd(tmp_path), files_manager():
+                CoverageTool().add_configs()
+
+            # Assert
+            with change_cwd(tmp_path), files_manager():
+                assert ["tool", "coverage"] in PyprojectTOMLManager()
+            assert "[tool.coverage]" in (tmp_path / "pyproject.toml").read_text()