add support for PEP 621: poetry remove (#9135)

Author: radoering

src/poetry/console/commands/remove.py

@@ -7,6 +7,7 @@
 from cleo.helpers import argument
 from cleo.helpers import option
 from packaging.utils import canonicalize_name
+from poetry.core.packages.dependency import Dependency
 from poetry.core.packages.dependency_group import MAIN_GROUP
 from tomlkit.toml_document import TOMLDocument
 
@@ -66,39 +67,45 @@ def handle(self) -> int:
             group = self.option("group", self.default_group)
 
         content: dict[str, Any] = self.poetry.file.read()
-        poetry_content = content["tool"]["poetry"]
+        project_content = content.get("project", {})
+        poetry_content = content.get("tool", {}).get("poetry", {})
 
         if group is None:
-            removed = []
+            # remove from all groups
+            removed = set()
             group_sections = [
-                (group_name, group_section.get("dependencies", {}))
-                for group_name, group_section in poetry_content.get("group", {}).items()
+                (
+                    MAIN_GROUP,
+                    project_content.get("dependencies", []),
+                    poetry_content.get("dependencies", {}),
+                )
             ]
+            group_sections.extend(
+                (group_name, [], group_section.get("dependencies", {}))
+                for group_name, group_section in poetry_content.get("group", {}).items()
+            )
 
-            for group_name, section in [
-                (MAIN_GROUP, poetry_content["dependencies"]),
-                *group_sections,
-            ]:
-                removed += self._remove_packages(packages, section, group_name)
-                if group_name != MAIN_GROUP:
-                    if not section:
-                        del poetry_content["group"][group_name]
-                    else:
-                        poetry_content["group"][group_name]["dependencies"] = section
+            for group_name, project_section, poetry_section in group_sections:
+                removed |= self._remove_packages(
+                    packages, project_section, poetry_section, group_name
+                )
+                if group_name != MAIN_GROUP and not poetry_section:
+                    del poetry_content["group"][group_name]
         elif group == "dev" and "dev-dependencies" in poetry_content:
             # We need to account for the old `dev-dependencies` section
             removed = self._remove_packages(
-                packages, poetry_content["dev-dependencies"], "dev"
+                packages, [], poetry_content["dev-dependencies"], "dev"
             )
 
             if not poetry_content["dev-dependencies"]:
                 del poetry_content["dev-dependencies"]
         else:
-            removed = []
+            removed = set()
             if "group" in poetry_content:
                 if group in poetry_content["group"]:
                     removed = self._remove_packages(
                         packages,
+                        [],
                         poetry_content["group"][group].get("dependencies", {}),
                         group,
                     )
@@ -109,23 +116,21 @@ def handle(self) -> int:
         if "group" in poetry_content and not poetry_content["group"]:
             del poetry_content["group"]
 
-        removed_set = set(removed)
-        not_found = set(packages).difference(removed_set)
+        not_found = set(packages).difference(removed)
         if not_found:
             raise ValueError(
                 "The following packages were not found: " + ", ".join(sorted(not_found))
             )
 
         # Refresh the locker
-        content["tool"]["poetry"] = poetry_content
         self.poetry.locker.set_pyproject_data(content)
         self.installer.set_locker(self.poetry.locker)
         self.installer.set_package(self.poetry.package)
         self.installer.dry_run(self.option("dry-run", False))
         self.installer.verbose(self.io.is_verbose())
         self.installer.update(True)
         self.installer.execute_operations(not self.option("lock"))
-        self.installer.whitelist(removed_set)
+        self.installer.whitelist(removed)
 
         status = self.installer.run()
 
@@ -136,17 +141,27 @@ def handle(self) -> int:
         return status
 
     def _remove_packages(
-        self, packages: list[str], section: dict[str, Any], group_name: str
-    ) -> list[str]:
-        removed = []
+        self,
+        packages: list[str],
+        project_section: list[str],
+        poetry_section: dict[str, Any],
+        group_name: str,
+    ) -> set[str]:
+        removed = set()
         group = self.poetry.package.dependency_group(group_name)
-        section_keys = list(section.keys())
 
         for package in packages:
-            for existing_package in section_keys:
-                if canonicalize_name(existing_package) == canonicalize_name(package):
-                    del section[existing_package]
-                    removed.append(package)
-                    group.remove_dependency(package)
+            normalized_name = canonicalize_name(package)
+            for requirement in project_section.copy():
+                if Dependency.create_from_pep_508(requirement).name == normalized_name:
+                    project_section.remove(requirement)
+                    removed.add(package)
+            for existing_package in list(poetry_section):
+                if canonicalize_name(existing_package) == normalized_name:
+                    del poetry_section[existing_package]
+                    removed.add(package)
+
+        for package in removed:
+            group.remove_dependency(package)
 
         return removed

tests/console/commands/test_remove.py

@@ -2,6 +2,7 @@
 
 from typing import TYPE_CHECKING
 from typing import Any
+from typing import Callable
 from typing import cast
 
 import pytest
@@ -31,25 +32,103 @@
 @pytest.fixture
 def poetry_with_up_to_date_lockfile(
     project_factory: ProjectFactory, fixture_dir: FixtureDirGetter
-) -> Poetry:
-    source = fixture_dir("up_to_date_lock")
+) -> Callable[[str], Poetry]:
+    def get_poetry(fixture_name: str) -> Poetry:
+        source = fixture_dir(fixture_name)
 
-    poetry = project_factory(
-        name="foobar",
-        pyproject_content=(source / "pyproject.toml").read_text(encoding="utf-8"),
-        poetry_lock_content=(source / "poetry.lock").read_text(encoding="utf-8"),
-    )
+        poetry = project_factory(
+            name="foobar",
+            pyproject_content=(source / "pyproject.toml").read_text(encoding="utf-8"),
+            poetry_lock_content=(source / "poetry.lock").read_text(encoding="utf-8"),
+        )
+
+        assert isinstance(poetry.locker, TestLocker)
+        poetry.locker.locked(True)
+        return poetry
 
-    assert isinstance(poetry.locker, TestLocker)
-    poetry.locker.locked(True)
-    return poetry
+    return get_poetry
 
 
 @pytest.fixture()
 def tester(command_tester_factory: CommandTesterFactory) -> CommandTester:
     return command_tester_factory("remove")
 
 
+def test_remove_from_project_and_poetry(
+    tester: CommandTester,
+    app: PoetryTestApplication,
+    repo: TestRepository,
+    installed: Repository,
+) -> None:
+    repo.add_package(Package("foo", "2.0.0"))
+    repo.add_package(Package("bar", "1.0.0"))
+
+    pyproject: dict[str, Any] = app.poetry.file.read()
+
+    project_dependencies: dict[str, Any] = tomlkit.parse(
+        """\
+[project]
+dependencies = [
+    "foo>=2.0",
+    "bar>=1.0",
+]
+"""
+    )
+
+    poetry_dependencies: dict[str, Any] = tomlkit.parse(
+        """\
+[tool.poetry.dependencies]
+foo = "^2.0.0"
+bar = "^1.0.0"
+
+"""
+    )
+
+    pyproject["project"]["dependencies"] = project_dependencies["project"][
+        "dependencies"
+    ]
+    pyproject["tool"]["poetry"]["dependencies"] = poetry_dependencies["tool"]["poetry"][
+        "dependencies"
+    ]
+    pyproject = cast("TOMLDocument", pyproject)
+    app.poetry.file.write(pyproject)
+
+    app.poetry.package.add_dependency(Factory.create_dependency("foo", "^2.0.0"))
+    app.poetry.package.add_dependency(Factory.create_dependency("bar", "^1.0.0"))
+
+    tester.execute("foo")
+
+    pyproject = app.poetry.file.read()
+    pyproject = cast("dict[str, Any]", pyproject)
+    project_dependencies = pyproject["project"]["dependencies"]
+    assert "foo>=2.0" not in project_dependencies
+    assert "bar>=1.0" in project_dependencies
+    poetry_dependencies = pyproject["tool"]["poetry"]["dependencies"]
+    assert "foo" not in poetry_dependencies
+    assert "bar" in poetry_dependencies
+
+    expected_project_string = """\
+dependencies = [
+    "bar>=1.0",
+]
+"""
+    expected_poetry_string = """\
+
+[tool.poetry.dependencies]
+bar = "^1.0.0"
+
+"""
+    pyproject = cast("TOMLDocument", pyproject)
+    string_content = pyproject.as_string()
+    if "\r\n" in string_content:
+        # consistent line endings
+        expected_project_string = expected_project_string.replace("\n", "\r\n")
+        expected_poetry_string = expected_poetry_string.replace("\n", "\r\n")
+
+    assert expected_project_string in string_content
+    assert expected_poetry_string in string_content
+
+
 def test_remove_without_specific_group_removes_from_all_groups(
     tester: CommandTester,
     app: PoetryTestApplication,
@@ -110,7 +189,7 @@ def test_remove_without_specific_group_removes_from_all_groups(
     assert expected in string_content
 
 
-def test_remove_without_specific_group_removes_from_specific_groups(
+def test_remove_with_specific_group_removes_from_specific_groups(
     tester: CommandTester,
     app: PoetryTestApplication,
     repo: TestRepository,
@@ -169,7 +248,7 @@ def test_remove_without_specific_group_removes_from_specific_groups(
     assert expected in string_content
 
 
-def test_remove_does_not_live_empty_groups(
+def test_remove_does_not_keep_empty_groups(
     tester: CommandTester,
     app: PoetryTestApplication,
     repo: TestRepository,
@@ -299,33 +378,41 @@ def test_remove_command_should_not_write_changes_upon_installer_errors(
     assert app.poetry.file.read().as_string() == original_content
 
 
+@pytest.mark.parametrize(
+    "fixture_name", ["up_to_date_lock", "up_to_date_lock_non_package"]
+)
 def test_remove_with_dry_run_keep_files_intact(
-    poetry_with_up_to_date_lockfile: Poetry,
+    fixture_name: str,
+    poetry_with_up_to_date_lockfile: Callable[[str], Poetry],
     repo: TestRepository,
     command_tester_factory: CommandTesterFactory,
 ) -> None:
-    tester = command_tester_factory("remove", poetry=poetry_with_up_to_date_lockfile)
+    poetry = poetry_with_up_to_date_lockfile(fixture_name)
+    tester = command_tester_factory("remove", poetry=poetry)
 
-    original_pyproject_content = poetry_with_up_to_date_lockfile.file.read()
-    original_lockfile_content = poetry_with_up_to_date_lockfile._locker.lock_data
+    original_pyproject_content = poetry.file.read()
+    original_lockfile_content = poetry._locker.lock_data
 
     repo.add_package(get_package("docker", "4.3.1"))
 
     tester.execute("docker --dry-run")
 
-    assert poetry_with_up_to_date_lockfile.file.read() == original_pyproject_content
-    assert (
-        poetry_with_up_to_date_lockfile._locker.lock_data == original_lockfile_content
-    )
+    assert poetry.file.read() == original_pyproject_content
+    assert poetry._locker.lock_data == original_lockfile_content
 
 
+@pytest.mark.parametrize(
+    "fixture_name", ["up_to_date_lock", "up_to_date_lock_non_package"]
+)
 def test_remove_performs_uninstall_op(
-    poetry_with_up_to_date_lockfile: Poetry,
+    fixture_name: str,
+    poetry_with_up_to_date_lockfile: Callable[[str], Poetry],
     command_tester_factory: CommandTesterFactory,
     installed: Repository,
 ) -> None:
     installed.add_package(get_package("docker", "4.3.1"))
-    tester = command_tester_factory("remove", poetry=poetry_with_up_to_date_lockfile)
+    poetry = poetry_with_up_to_date_lockfile(fixture_name)
+    tester = command_tester_factory("remove", poetry=poetry)
 
     tester.execute("docker")
 
@@ -343,13 +430,18 @@ def test_remove_performs_uninstall_op(
     assert tester.io.fetch_output() == expected
 
 
+@pytest.mark.parametrize(
+    "fixture_name", ["up_to_date_lock", "up_to_date_lock_non_package"]
+)
 def test_remove_with_lock_does_not_perform_uninstall_op(
-    poetry_with_up_to_date_lockfile: Poetry,
+    fixture_name: str,
+    poetry_with_up_to_date_lockfile: Callable[[str], Poetry],
     command_tester_factory: CommandTesterFactory,
     installed: Repository,
 ) -> None:
     installed.add_package(get_package("docker", "4.3.1"))
-    tester = command_tester_factory("remove", poetry=poetry_with_up_to_date_lockfile)
+    poetry = poetry_with_up_to_date_lockfile(fixture_name)
+    tester = command_tester_factory("remove", poetry=poetry)
 
     tester.execute("docker --lock")