Use `safe_extra` to standardize extra dependencies (#4122)

Co-authored-by: Bjorn Neergaard <bjorn@neersighted.com>

src/poetry/puzzle/provider.py

@@ -36,6 +36,7 @@ from poetry.packages.package_collection import PackageCollection
 from poetry.puzzle.exceptions import OverrideNeeded
 from poetry.repositories.exceptions import PackageNotFound
 from poetry.utils.helpers import download_file
+from poetry.utils.helpers import safe_extra
 from poetry.vcs.git import Git
 
 
@@ -563,6 +564,7 @@ class Provider:
         # to the current package
         if package.dependency.extras:
             for extra in package.dependency.extras:
+                extra = safe_extra(extra)
                 if extra not in package.extras:
                     continue
 
@@ -585,7 +587,9 @@ class Provider:
                 (dep.is_optional() and dep.name not in optional_dependencies)
                 or (
                     dep.in_extras
-                    and not set(dep.in_extras).intersection(package.dependency.extras)
+                    and not set(dep.in_extras).intersection(
+                        {safe_extra(extra) for extra in package.dependency.extras}
+                    )
                 )
             ):
                 continue

src/poetry/utils/helpers.py

@@ -157,3 +157,15 @@ def pluralize(count: int, word: str = "") -> str:
     if count == 1:
         return word
     return word + "s"
+
+
+def safe_extra(extra: str) -> str:
+    """Convert an arbitrary string to a standard 'extra' name.
+
+    Any runs of non-alphanumeric characters are replaced with a single '_',
+    and the result is always lowercased.
+
+    See
+    https://github.com/pypa/setuptools/blob/452e13c/pkg_resources/__init__.py#L1423-L1431.
+    """
+    return re.sub("[^A-Za-z0-9.-]+", "_", extra).lower()

tests/console/commands/test_add.py

@@ -200,8 +200,12 @@ Package operations: 1 install, 0 updates, 0 removals
     assert tester.command.installer.executor.installations_count == 1
 
 
+@pytest.mark.parametrize("extra_name", ["msgpack", "MsgPack"])
 def test_add_constraint_with_extras(
-    app: PoetryTestApplication, repo: TestRepository, tester: CommandTester
+    app: PoetryTestApplication,
+    repo: TestRepository,
+    tester: CommandTester,
+    extra_name: str,
 ):
     cachy1 = get_package("cachy", "0.1.0")
     cachy1.extras = {"msgpack": [get_dependency("msgpack-python")]}
@@ -212,7 +216,7 @@ def test_add_constraint_with_extras(
     repo.add_package(cachy1)
     repo.add_package(get_package("msgpack-python", "0.5.3"))
 
-    tester.execute("cachy[msgpack]>=0.1.0,<0.2.0")
+    tester.execute(f"cachy[{extra_name}]>=0.1.0,<0.2.0")
 
     expected = """\
 
@@ -327,11 +331,13 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert tester.command.installer.executor.installations_count == 2
 
 
+@pytest.mark.parametrize("extra_name", ["foo", "FOO"])
 def test_add_git_constraint_with_extras(
     app: PoetryTestApplication,
     repo: TestRepository,
     tester: CommandTester,
     tmp_venv: VirtualEnv,
+    extra_name: str,
 ):
     tester.command.set_env(tmp_venv)
 
@@ -339,7 +345,7 @@ def test_add_git_constraint_with_extras(
     repo.add_package(get_package("cleo", "0.6.5"))
     repo.add_package(get_package("tomlkit", "0.5.5"))
 
-    tester.execute("git+https://github.com/demo/demo.git[foo,bar]")
+    tester.execute(f"git+https://github.com/demo/demo.git[{extra_name},bar]")
 
     expected = """\
 
@@ -364,7 +370,7 @@ Package operations: 4 installs, 0 updates, 0 removals
     assert "demo" in content["dependencies"]
     assert content["dependencies"]["demo"] == {
         "git": "https://github.com/demo/demo.git",
-        "extras": ["foo", "bar"],
+        "extras": [extra_name, "bar"],
     }
 
 
@@ -562,8 +568,12 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert content["dependencies"]["demo"] == {"path": path}
 
 
+@pytest.mark.parametrize("extra_name", ["msgpack", "MsgPack"])
 def test_add_constraint_with_extras_option(
-    app: PoetryTestApplication, repo: TestRepository, tester: CommandTester
+    app: PoetryTestApplication,
+    repo: TestRepository,
+    tester: CommandTester,
+    extra_name: str,
 ):
     cachy2 = get_package("cachy", "0.2.0")
     cachy2.extras = {"msgpack": [get_dependency("msgpack-python")]}
@@ -574,7 +584,7 @@ def test_add_constraint_with_extras_option(
     repo.add_package(cachy2)
     repo.add_package(get_package("msgpack-python", "0.5.3"))
 
-    tester.execute("cachy=0.2.0 --extras msgpack")
+    tester.execute(f"cachy=0.2.0 --extras {extra_name}")
 
     expected = """\
 
@@ -597,7 +607,7 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert "cachy" in content["dependencies"]
     assert content["dependencies"]["cachy"] == {
         "version": "0.2.0",
-        "extras": ["msgpack"],
+        "extras": [extra_name],
     }
 
 
@@ -641,10 +651,12 @@ Package operations: 2 installs, 0 updates, 0 removals
     }
 
 
+@pytest.mark.parametrize("extra_name", ["foo", "FOO"])
 def test_add_url_constraint_wheel_with_extras(
     app: PoetryTestApplication,
     repo: TestRepository,
     tester: CommandTester,
+    extra_name: str,
     mocker: MockerFixture,
 ):
     repo.add_package(get_package("pendulum", "1.4.4"))
@@ -653,7 +665,7 @@ def test_add_url_constraint_wheel_with_extras(
 
     tester.execute(
         "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
-        "[foo,bar]"
+        f"[{extra_name},bar]"
     )
 
     expected = """\
@@ -684,7 +696,7 @@ Package operations: 4 installs, 0 updates, 0 removals
         "url": (
             "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
         ),
-        "extras": ["foo", "bar"],
+        "extras": [extra_name, "bar"],
     }
 
 
@@ -1165,11 +1177,13 @@ Package operations: 1 install, 0 updates, 0 removals
     assert len(installer.installs) == 1
 
 
+@pytest.mark.parametrize("extra_name", ["msgpack", "MsgPack"])
 def test_add_constraint_with_extras_old_installer(
     app: PoetryTestApplication,
     repo: TestRepository,
     installer: NoopInstaller,
     old_tester: CommandTester,
+    extra_name: str,
 ):
     cachy1 = get_package("cachy", "0.1.0")
     cachy1.extras = {"msgpack": [get_dependency("msgpack-python")]}
@@ -1180,7 +1194,7 @@ def test_add_constraint_with_extras_old_installer(
     repo.add_package(cachy1)
     repo.add_package(get_package("msgpack-python", "0.5.3"))
 
-    old_tester.execute("cachy[msgpack]>=0.1.0,<0.2.0")
+    old_tester.execute(f"cachy[{extra_name}]>=0.1.0,<0.2.0")
 
     expected = """\
 
@@ -1298,17 +1312,19 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert len(installer.installs) == 2
 
 
+@pytest.mark.parametrize("extra_name", ["foo", "FOO"])
 def test_add_git_constraint_with_extras_old_installer(
     app: PoetryTestApplication,
     repo: TestRepository,
     installer: NoopInstaller,
     old_tester: CommandTester,
+    extra_name: str,
 ):
     repo.add_package(get_package("pendulum", "1.4.4"))
     repo.add_package(get_package("cleo", "0.6.5"))
     repo.add_package(get_package("tomlkit", "0.5.5"))
 
-    old_tester.execute("git+https://github.com/demo/demo.git[foo,bar]")
+    old_tester.execute(f"git+https://github.com/demo/demo.git[{extra_name},bar]")
 
     expected = """\
 
@@ -1334,7 +1350,7 @@ Package operations: 4 installs, 0 updates, 0 removals
     assert "demo" in content["dependencies"]
     assert content["dependencies"]["demo"] == {
         "git": "https://github.com/demo/demo.git",
-        "extras": ["foo", "bar"],
+        "extras": [extra_name, "bar"],
     }
 
 
@@ -1523,11 +1539,13 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert content["dependencies"]["demo"] == {"path": path}
 
 
+@pytest.mark.parametrize("extra_name", ["msgpack", "MsgPack"])
 def test_add_constraint_with_extras_option_old_installer(
     app: PoetryTestApplication,
     repo: TestRepository,
     installer: NoopInstaller,
     old_tester: CommandTester,
+    extra_name: str,
 ):
     cachy2 = get_package("cachy", "0.2.0")
     cachy2.extras = {"msgpack": [get_dependency("msgpack-python")]}
@@ -1538,7 +1556,7 @@ def test_add_constraint_with_extras_option_old_installer(
     repo.add_package(cachy2)
     repo.add_package(get_package("msgpack-python", "0.5.3"))
 
-    old_tester.execute("cachy=0.2.0 --extras msgpack")
+    old_tester.execute(f"cachy=0.2.0 --extras {extra_name}")
 
     expected = """\
 
@@ -1562,7 +1580,7 @@ Package operations: 2 installs, 0 updates, 0 removals
     assert "cachy" in content["dependencies"]
     assert content["dependencies"]["cachy"] == {
         "version": "0.2.0",
-        "extras": ["msgpack"],
+        "extras": [extra_name],
     }
 
 
@@ -1608,11 +1626,13 @@ Package operations: 2 installs, 0 updates, 0 removals
     }
 
 
+@pytest.mark.parametrize("extra_name", ["foo", "FOO"])
 def test_add_url_constraint_wheel_with_extras_old_installer(
     app: PoetryTestApplication,
     repo: TestRepository,
     installer: NoopInstaller,
     old_tester: CommandTester,
+    extra_name: str,
 ):
     repo.add_package(get_package("pendulum", "1.4.4"))
     repo.add_package(get_package("cleo", "0.6.5"))
@@ -1620,7 +1640,7 @@ def test_add_url_constraint_wheel_with_extras_old_installer(
 
     old_tester.execute(
         "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
-        "[foo,bar]"
+        f"[{extra_name},bar]"
     )
 
     expected = """\
@@ -1650,7 +1670,7 @@ Package operations: 4 installs, 0 updates, 0 removals
         "url": (
             "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
         ),
-        "extras": ["foo", "bar"],
+        "extras": [extra_name, "bar"],
     }
 
 

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

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

tests/installation/test_installer.py

@@ -1001,11 +1001,11 @@ def test_run_with_dependencies_nested_extras(
     )
     dependency_a = Factory.create_dependency("A", {"version": "^1.0", "extras": ["B"]})
 
-    package_b.extras = {"C": [dependency_c]}
+    package_b.extras = {"c": [dependency_c]}
     package_b.add_dependency(dependency_c)
 
     package_a.add_dependency(dependency_b)
-    package_a.extras = {"B": [dependency_b]}
+    package_a.extras = {"b": [dependency_b]}
 
     repo.add_package(package_a)
     repo.add_package(package_b)

tests/utils/test_helpers.py

@@ -5,6 +5,7 @@ import pytest
 from poetry.core.utils.helpers import parse_requires
 
 from poetry.utils.helpers import canonicalize_name
+from poetry.utils.helpers import safe_extra
 
 
 def test_parse_requires():
@@ -77,3 +78,10 @@ test_canonicalize_name_cases = [
 def test_canonicalize_name(test: str, expected: str):
     canonicalized_name = canonicalize_name(test)
     assert canonicalized_name == expected
+
+
+def test_safe_extra():
+    extra = "pandas.CSVDataSet"
+    result = safe_extra(extra)
+    expected = "pandas.csvdataset"
+    assert result == expected