treat collections in packages as immutable (#663)

justifying the use of a shallow copy in Package.clone()

Co-authored-by: Randy Döring <[email protected]>

src/poetry/core/factory.py

@@ -19,6 +19,8 @@
 if TYPE_CHECKING:
     from collections.abc import Mapping
 
+    from packaging.utils import NormalizedName
+
     from poetry.core.packages.dependency import Dependency
     from poetry.core.packages.dependency_group import DependencyGroup
     from poetry.core.packages.project_package import ProjectPackage
@@ -131,11 +133,13 @@ def configure_package(
 
         package.root_dir = root
 
-        for author in config.get("authors", []):
-            package.authors.append(combine_unicode(author))
+        package.authors = [
+            combine_unicode(author) for author in config.get("authors", [])
+        ]
 
-        for maintainer in config.get("maintainers", []):
-            package.maintainers.append(combine_unicode(maintainer))
+        package.maintainers = [
+            combine_unicode(maintainer) for maintainer in config.get("maintainers", [])
+        ]
 
         package.description = config.get("description", "")
         package.homepage = config.get("homepage")
@@ -177,19 +181,22 @@ def configure_package(
                 package=package, group="dev", dependencies=config["dev-dependencies"]
             )
 
+        package_extras: dict[NormalizedName, list[Dependency]] = {}
         extras = config.get("extras", {})
         for extra_name, requirements in extras.items():
             extra_name = canonicalize_name(extra_name)
-            package.extras[extra_name] = []
+            package_extras[extra_name] = []
 
             # Checking for dependency
             for req in requirements:
                 req = Dependency(req, "*")
 
                 for dep in package.requires:
                     if dep.name == req.name:
-                        dep.in_extras.append(extra_name)
-                        package.extras[extra_name].append(dep)
+                        dep._in_extras = [*dep._in_extras, extra_name]
+                        package_extras[extra_name].append(dep)
+
+        package.extras = package_extras
 
         if "build" in config:
             build = config["build"]

src/poetry/core/masonry/utils/module.py

@@ -3,6 +3,8 @@
 from pathlib import Path
 from typing import TYPE_CHECKING
 from typing import Any
+from typing import Mapping
+from typing import Sequence
 
 
 if TYPE_CHECKING:
@@ -18,8 +20,8 @@ def __init__(
         self,
         name: str,
         directory: str = ".",
-        packages: list[dict[str, Any]] | None = None,
-        includes: list[dict[str, Any]] | None = None,
+        packages: Sequence[Mapping[str, Any]] = (),
+        includes: Sequence[Mapping[str, Any]] = (),
     ) -> None:
         from poetry.core.masonry.utils.include import Include
         from poetry.core.masonry.utils.package_include import PackageInclude
@@ -30,8 +32,6 @@ def __init__(
         self._is_package = False
         self._path = Path(directory)
         self._includes: list[Include] = []
-        packages = packages or []
-        includes = includes or []
 
         if not packages:
             # It must exist either as a .py file or a directory, but not both

src/poetry/core/packages/dependency.py

@@ -7,6 +7,7 @@
 from contextlib import suppress
 from pathlib import Path
 from typing import TYPE_CHECKING
+from typing import Sequence
 from typing import TypeVar
 
 from packaging.utils import canonicalize_name
@@ -61,6 +62,9 @@ def __init__(
             features=extras,
         )
 
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
+
         self._constraint: VersionConstraint
         self._pretty_constraint: str
         self.constraint = constraint  # type: ignore[assignment]
@@ -79,7 +83,7 @@ def __init__(
         self._transitive_python_constraint: VersionConstraint | None = None
         self._transitive_marker: BaseMarker | None = None
 
-        self._in_extras: list[NormalizedName] = []
+        self._in_extras: Sequence[NormalizedName] = []
 
         self._activated = not self._optional
 
@@ -184,13 +188,15 @@ def marker(self, marker: str | BaseMarker) -> None:
             # If we have extras, the dependency is optional
             self.deactivate()
 
+            new_in_extras = []
             for or_ in markers["extra"]:
                 for op, extra in or_:
                     if op == "==":
-                        self.in_extras.append(canonicalize_name(extra))
+                        new_in_extras.append(canonicalize_name(extra))
                     elif op == "" and "||" in extra:
                         for _extra in extra.split(" || "):
-                            self.in_extras.append(canonicalize_name(_extra))
+                            new_in_extras.append(canonicalize_name(_extra))
+            self._in_extras = [*self._in_extras, *new_in_extras]
 
         # Recalculate python versions.
         self._python_versions = "*"
@@ -235,7 +241,7 @@ def extras(self) -> frozenset[NormalizedName]:
         return self._features
 
     @property
-    def in_extras(self) -> list[NormalizedName]:
+    def in_extras(self) -> Sequence[NormalizedName]:
         return self._in_extras
 
     @property

src/poetry/core/packages/directory_dependency.py

@@ -34,6 +34,8 @@ def __init__(
             base=base,
             extras=extras,
         )
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
         self._develop = develop
 
         # cache this function to avoid multiple IO reads and parsing

src/poetry/core/packages/file_dependency.py

@@ -36,6 +36,8 @@ def __init__(
             subdirectory=directory,
             extras=extras,
         )
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
 
     @property
     def directory(self) -> str | None:

src/poetry/core/packages/package.py

@@ -1,12 +1,13 @@
 from __future__ import annotations
 
-import copy
 import re
 import warnings
 
 from contextlib import contextmanager
 from typing import TYPE_CHECKING
 from typing import ClassVar
+from typing import Mapping
+from typing import Sequence
 from typing import TypeVar
 
 from poetry.core.constraints.version import parse_constraint
@@ -93,30 +94,33 @@ def __init__(
             features=features,
         )
 
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
+
         self._set_version(version)
 
         self.description = ""
 
-        self._authors: list[str] = []
-        self._maintainers: list[str] = []
+        self.authors: Sequence[str] = []
+        self.maintainers: Sequence[str] = []
 
         self.homepage: str | None = None
         self.repository_url: str | None = None
         self.documentation_url: str | None = None
-        self.keywords: list[str] = []
+        self.keywords: Sequence[str] = []
         self._license: License | None = None
         self.readmes: tuple[Path, ...] = ()
 
-        self.extras: dict[NormalizedName, list[Dependency]] = {}
+        self.extras: Mapping[NormalizedName, Sequence[Dependency]] = {}
 
-        self._dependency_groups: dict[str, DependencyGroup] = {}
+        self._dependency_groups: Mapping[str, DependencyGroup] = {}
 
         # Category is heading towards deprecation.
         self._category = "main"
-        self.files: list[dict[str, str]] = []
+        self.files: Sequence[Mapping[str, str]] = []
         self.optional = False
 
-        self.classifiers: list[str] = []
+        self.classifiers: Sequence[str] = []
 
         self._python_versions = "*"
         self._python_constraint = parse_constraint("*")
@@ -177,10 +181,6 @@ def full_pretty_version(self) -> str:
         ref = self._source_resolved_reference or self._source_reference
         return f"{self.pretty_version} {ref}"
 
-    @property
-    def authors(self) -> list[str]:
-        return self._authors
-
     @property
     def author_name(self) -> str | None:
         return self._get_author()["name"]
@@ -189,10 +189,6 @@ def author_name(self) -> str | None:
     def author_email(self) -> str | None:
         return self._get_author()["email"]
 
-    @property
-    def maintainers(self) -> list[str]:
-        return self._maintainers
-
     @property
     def maintainer_name(self) -> str | None:
         return self._get_maintainer()["name"]
@@ -238,10 +234,10 @@ def _set_version(self, version: str | Version) -> None:
         self._version = version
 
     def _get_author(self) -> dict[str, str | None]:
-        if not self._authors:
+        if not self.authors:
             return {"name": None, "email": None}
 
-        m = AUTHOR_REGEX.match(self._authors[0])
+        m = AUTHOR_REGEX.match(self.authors[0])
 
         if m is None:
             raise ValueError(
@@ -255,10 +251,10 @@ def _get_author(self) -> dict[str, str | None]:
         return {"name": name, "email": email}
 
     def _get_maintainer(self) -> dict[str, str | None]:
-        if not self._maintainers:
+        if not self.maintainers:
             return {"name": None, "email": None}
 
-        m = AUTHOR_REGEX.match(self._maintainers[0])
+        m = AUTHOR_REGEX.match(self.maintainers[0])
 
         if m is None:
             raise ValueError(
@@ -314,7 +310,7 @@ def license(self, value: str | License | None) -> None:
     def all_classifiers(self) -> list[str]:
         from poetry.core.constraints.version import Version
 
-        classifiers = copy.copy(self.classifiers)
+        classifiers = list(self.classifiers)
 
         # Automatically set python classifiers
         if self.python_versions == "*":
@@ -439,7 +435,9 @@ def dependency_group_names(self, include_optional: bool = False) -> set[str]:
         }
 
     def add_dependency_group(self, group: DependencyGroup) -> None:
-        self._dependency_groups[group.name] = group
+        groups = dict(self._dependency_groups)
+        groups[group.name] = group
+        self._dependency_groups = groups
 
     def has_dependency_group(self, name: str) -> bool:
         return name in self._dependency_groups
@@ -469,23 +467,28 @@ def without_dependency_groups(self: T, groups: Collection[str]) -> T:
         """
         Returns a clone of the package with the given dependency groups excluded.
         """
-        package = self.clone()
+        updated_groups = {
+            group_name: group
+            for group_name, group in self._dependency_groups.items()
+            if group_name not in groups
+        }
 
-        for group_name in groups:
-            if group_name in package._dependency_groups:
-                del package._dependency_groups[group_name]
+        package = self.clone()
+        package._dependency_groups = updated_groups
 
         return package
 
     def without_optional_dependency_groups(self: T) -> T:
         """
         Returns a clone of the package without optional dependency groups.
         """
+        updated_groups = {
+            group_name: group
+            for group_name, group in self._dependency_groups.items()
+            if not group.is_optional()
+        }
         package = self.clone()
-
-        for group_name, group in self._dependency_groups.items():
-            if group.is_optional():
-                del package._dependency_groups[group_name]
+        package._dependency_groups = updated_groups
 
         return package
 
@@ -500,11 +503,13 @@ def with_dependency_groups(
 
         If `only` is set to True, then only the given groups will be selected.
         """
+        updated_groups = {
+            group_name: group
+            for group_name, group in self._dependency_groups.items()
+            if group_name in groups or not only and not group.is_optional()
+        }
         package = self.clone()
-
-        for group_name, group in self._dependency_groups.items():
-            if (only or group.is_optional()) and group_name not in groups:
-                del package._dependency_groups[group_name]
+        package._dependency_groups = updated_groups
 
         return package
 

src/poetry/core/packages/path_dependency.py

@@ -32,6 +32,8 @@ def __init__(
         subdirectory: str | None = None,
         extras: Iterable[str] | None = None,
     ) -> None:
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
         assert source_type in ("file", "directory")
         self._path = path
         self._base = base or Path.cwd()

src/poetry/core/packages/project_package.py

@@ -4,6 +4,8 @@
 
 from typing import TYPE_CHECKING
 from typing import Any
+from typing import Mapping
+from typing import Sequence
 
 from poetry.core.constraints.version import parse_constraint
 from poetry.core.version.markers import parse_marker
@@ -34,11 +36,14 @@ def __init__(
 
         super().__init__(name, version)
 
-        self.build_config: dict[str, Any] = {}
-        self.packages: list[dict[str, Any]] = []
-        self.include: list[dict[str, Any]] = []
-        self.exclude: list[dict[str, Any]] = []
-        self.custom_urls: dict[str, str] = {}
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
+
+        self.build_config: Mapping[str, Any] = {}
+        self.packages: Sequence[Mapping[str, Any]] = []
+        self.include: Sequence[Mapping[str, Any]] = []
+        self.exclude: Sequence[Mapping[str, Any]] = []
+        self.custom_urls: Mapping[str, str] = {}
 
         if self._python_versions == "*":
             self._python_constraint = parse_constraint("~2.7 || >=3.4")

src/poetry/core/packages/specification.py

@@ -29,6 +29,9 @@ def __init__(
     ) -> None:
         from packaging.utils import canonicalize_name
 
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
+
         self._pretty_name = name
         self._name = canonicalize_name(name)
         self._source_type = source_type
@@ -175,7 +178,7 @@ def is_same_package_as(self, other: PackageSpecification) -> bool:
         return self.is_same_source_as(other)
 
     def clone(self: T) -> T:
-        return copy.deepcopy(self)
+        return copy.copy(self)
 
     def with_features(self: T, features: Iterable[str]) -> T:
         package = self.clone()

src/poetry/core/packages/url_dependency.py

@@ -21,6 +21,8 @@ def __init__(
         optional: bool = False,
         extras: Iterable[str] | None = None,
     ) -> None:
+        # Attributes must be immutable for clone() to be safe!
+        # (For performance reasons, clone only creates a copy instead of a deep copy).
         self._url = url
         self._directory = directory