locker: handle nested extras requirement

Previously, when using locked repository, incorrect dependency instance
was created when a dependency's extra requirement activated a
nested extra. This change ensures that these are correctly
loaded.

As part of this change new lock files write PEP 508 serialised form of
extra dependencies in order to reuse core logic to parse specification
of extra requirement.

Resolves: #3224

poetry/packages/locker.py

@@ -24,12 +24,14 @@ from tomlkit.exceptions import TOMLKitError
 
 import poetry.repositories
 
+from poetry.core.packages import dependency_from_pep_508
 from poetry.core.packages.package import Dependency
 from poetry.core.packages.package import Package
 from poetry.core.semver import parse_constraint
 from poetry.core.semver.version import Version
 from poetry.core.toml.file import TOMLFile
 from poetry.core.version.markers import parse_marker
+from poetry.core.version.requirements import InvalidRequirement
 from poetry.packages import DependencyPackage
 from poetry.utils._compat import OrderedDict
 from poetry.utils._compat import Path
@@ -142,11 +144,18 @@ class Locker(object):
                     package.extras[name] = []
 
                     for dep in deps:
-                        m = re.match(r"^(.+?)(?:\s+\((.+)\))?$", dep)
-                        dep_name = m.group(1)
-                        constraint = m.group(2) or "*"
-
-                        package.extras[name].append(Dependency(dep_name, constraint))
+                        try:
+                            dependency = dependency_from_pep_508(dep)
+                        except InvalidRequirement:
+                            # handle lock files with invalid PEP 508
+                            m = re.match(r"^(.+?)(?:\[(.+?)])?(?:\s+\((.+)\))?$", dep)
+                            dep_name = m.group(1)
+                            extras = m.group(2) or ""
+                            constraint = m.group(3) or "*"
+                            dependency = Dependency(
+                                dep_name, constraint, extras=extras.split(",")
+                            )
+                        package.extras[name].append(dependency)
 
             if "marker" in info:
                 package.marker = parse_marker(info["marker"])
@@ -543,8 +552,10 @@ class Locker(object):
         if package.extras:
             extras = {}
             for name, deps in package.extras.items():
+                # TODO: This should use dep.to_pep_508() once this is fixed
+                # https://github.com/python-poetry/poetry-core/pull/102
                 extras[name] = [
-                    str(dep) if not dep.constraint.is_any() else dep.name
+                    dep.base_pep_508_name if not dep.constraint.is_any() else dep.name
                     for dep in deps
                 ]
 

tests/installation/fixtures/with-dependencies-extras.test

@@ -18,7 +18,7 @@ python-versions = "*"
 C = {version = "^1.0", optional = true}
 
 [package.extras]
-foo = ["C (^1.0)"]
+foo = ["C (>=1.0,<2.0)"]
 
 [[package]]
 name = "C"

tests/installation/fixtures/with-dependencies-nested-extras.test

+[[package]]
+name = "A"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+B = {version = "^1.0", optional = true, extras = ["C"]}
+
+[package.extras]
+B = ["B[C] (>=1.0,<2.0)"]
+
+[[package]]
+name = "B"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+C = {version = "^1.0", optional = true}
+
+[package.extras]
+C = ["C (>=1.0,<2.0)"]
+
+[[package]]
+name = "C"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[metadata]
+python-versions = "*"
+lock-version = "1.1"
+content-hash = "123456789"
+
+[metadata.files]
+"A" = []
+"B" = []
+"C" = []

tests/installation/test_installer.py

@@ -639,6 +639,35 @@ def test_run_with_dependencies_extras(installer, locker, repo, package):
     assert locker.written_data == expected
 
 
+def test_run_with_dependencies_nested_extras(installer, locker, repo, package):
+    package_a = get_package("A", "1.0")
+    package_b = get_package("B", "1.0")
+    package_c = get_package("C", "1.0")
+
+    dependency_c = Factory.create_dependency("C", {"version": "^1.0", "optional": True})
+    dependency_b = Factory.create_dependency(
+        "B", {"version": "^1.0", "optional": True, "extras": ["C"]}
+    )
+    dependency_a = Factory.create_dependency("A", {"version": "^1.0", "extras": ["B"]})
+
+    package_b.extras = {"C": [dependency_c]}
+    package_b.add_dependency(dependency_c)
+
+    package_a.add_dependency(dependency_b)
+    package_a.extras = {"B": [dependency_b]}
+
+    repo.add_package(package_a)
+    repo.add_package(package_b)
+    repo.add_package(package_c)
+
+    package.add_dependency(dependency_a)
+
+    installer.run()
+    expected = fixture("with-dependencies-nested-extras")
+
+    assert locker.written_data == expected
+
+
 def test_run_does_not_install_extras_if_not_requested(installer, locker, repo, package):
     package.extras["foo"] = [get_dependency("D")]
     package_a = get_package("A", "1.0")

tests/packages/test_locker.py

@@ -142,6 +142,136 @@ cachecontrol = []
     assert lockfile_dep.name == "lockfile"
 
 
+def test_locker_properly_loads_nested_extras(locker):
+    content = """\
+[[package]]
+name = "a"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+b = {version = "^1.0", optional = true, extras = "c"}
+
+[package.extras]
+b = ["b[c] (>=1.0,<2.0)"]
+
+[[package]]
+name = "b"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+c = {version = "^1.0", optional = true}
+
+[package.extras]
+c = ["c (>=1.0,<2.0)"]
+
+[[package]]
+name = "c"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[metadata]
+python-versions = "*"
+lock-version = "1.1"
+content-hash = "123456789"
+
+[metadata.files]
+"a" = []
+"b" = []
+"c" = []
+"""
+
+    locker.lock.write(tomlkit.parse(content))
+
+    repository = locker.locked_repository()
+    assert 3 == len(repository.packages)
+
+    packages = repository.find_packages(get_dependency("a", "1.0"))
+    assert len(packages) == 1
+
+    package = packages[0]
+    assert len(package.requires) == 1
+    assert len(package.extras) == 1
+
+    dependency_b = package.extras["b"][0]
+    assert dependency_b.name == "b"
+    assert dependency_b.extras == frozenset({"c"})
+
+    packages = repository.find_packages(dependency_b)
+    assert len(packages) == 1
+
+    package = packages[0]
+    assert len(package.requires) == 1
+    assert len(package.extras) == 1
+
+    dependency_c = package.extras["c"][0]
+    assert dependency_c.name == "c"
+    assert dependency_c.extras == frozenset()
+
+    packages = repository.find_packages(dependency_c)
+    assert len(packages) == 1
+
+
+def test_locker_properly_loads_extras_legacy(locker):
+    content = """\
+[[package]]
+name = "a"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[package.dependencies]
+b = {version = "^1.0", optional = true}
+
+[package.extras]
+b = ["b (^1.0)"]
+
+[[package]]
+name = "b"
+version = "1.0"
+description = ""
+category = "main"
+optional = false
+python-versions = "*"
+
+[metadata]
+python-versions = "*"
+lock-version = "1.1"
+content-hash = "123456789"
+
+[metadata.files]
+"a" = []
+"b" = []
+"""
+
+    locker.lock.write(tomlkit.parse(content))
+
+    repository = locker.locked_repository()
+    assert 2 == len(repository.packages)
+
+    packages = repository.find_packages(get_dependency("a", "1.0"))
+    assert len(packages) == 1
+
+    package = packages[0]
+    assert len(package.requires) == 1
+    assert len(package.extras) == 1
+
+    dependency_b = package.extras["b"][0]
+    assert dependency_b.name == "b"
+
+
 def test_lock_packages_with_null_description(locker, root):
     package_a = get_package("A", "1.0.0")
     package_a.description = None