From cb1198af61d321f5fe0b88ca544171c4c146b630 Mon Sep 17 00:00:00 2001
From: Johan Herland <johan.herland@tweag.io>
Date: Mon, 17 Jun 2024 12:48:23 +0200
Subject: [PATCH] parse_setup_py: Handle string dependency when we expect list
 of strings

As described in issue #429, the s3transfer package comes with setup.py
where the `extras_require` dictionary does not map to a list of strings
(extra requirements), but instead map to a string directly.

When s3transfer is installed with this extra (e.g. `pip install
s3transfer[crt]`), the setuptools machinery handles this variation
gracefully, i.e. it interprets the string as if it was a single-element
list containing the same string.

An experiment reveals that the same interpretation occurs if a string is
passed directly in the `install_requires` argument.

This commit teaches FawltyDeps to do the same.

Also, if parse_one_req() fails to parse a requirement string, catch that
exception (likely an `InvalidRequirement` or some other subclass of
ValueError), and convert it into a DependencyParsingError, which is
handled more gracefully (i.e. print a warning message and continue).
---
 fawltydeps/extract_declared_dependencies.py   | 24 ++++++++++++-------
 ...t_extract_declared_dependencies_success.py | 15 ++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/fawltydeps/extract_declared_dependencies.py b/fawltydeps/extract_declared_dependencies.py
index c596323f..763b0b85 100644
--- a/fawltydeps/extract_declared_dependencies.py
+++ b/fawltydeps/extract_declared_dependencies.py
@@ -9,7 +9,7 @@
 from dataclasses import replace
 from pathlib import Path
 from tempfile import NamedTemporaryFile
-from typing import Callable, Iterable, Iterator, NamedTuple, Optional, Tuple
+from typing import Callable, Iterable, Iterator, NamedTuple, Optional, Tuple, Union
 
 from pip_requirements_parser import RequirementsFile  # type: ignore[import]
 from pkg_resources import Requirement
@@ -79,24 +79,32 @@ def parse_setup_py(path: Path) -> Iterator[DeclaredDependency]:  # noqa: C901
     # resolve any variable references in the arguments to the setup() call.
     tracked_vars = VariableTracker(source)
 
-    def _extract_deps_from_setup_call(  # noqa: C901
+    def _extract_deps_from_value(
+        value: Union[str, Iterable[str]],
+        node: ast.AST,
+    ) -> Iterator[DeclaredDependency]:
+        if isinstance(value, str):  # expected list, but got string
+            value = [value]  # parse as if a single-element list is given
+        try:
+            for item in value:
+                yield parse_one_req(item, source)
+        except ValueError as e:  # parse_one_req() failed
+            raise DependencyParsingError(node) from e
+
+    def _extract_deps_from_setup_call(
         node: ast.Call,
     ) -> Iterator[DeclaredDependency]:
         for keyword in node.keywords:
             try:
                 if keyword.arg == "install_requires":
                     value = tracked_vars.resolve(keyword.value)
-                    if not isinstance(value, list):
-                        raise DependencyParsingError(keyword.value)
-                    for item in value:
-                        yield parse_one_req(item, source)
+                    yield from _extract_deps_from_value(value, keyword.value)
                 elif keyword.arg == "extras_require":
                     value = tracked_vars.resolve(keyword.value)
                     if not isinstance(value, dict):
                         raise DependencyParsingError(keyword.value)
                     for items in value.values():
-                        for item in items:
-                            yield parse_one_req(item, source)
+                        yield from _extract_deps_from_value(items, keyword.value)
             except (DependencyParsingError, CannotResolve) as exc:
                 if sys.version_info >= (3, 9):
                     unparsed_content = ast.unparse(exc.node)
diff --git a/tests/test_extract_declared_dependencies_success.py b/tests/test_extract_declared_dependencies_success.py
index 4f7928cc..14dec4a1 100644
--- a/tests/test_extract_declared_dependencies_success.py
+++ b/tests/test_extract_declared_dependencies_success.py
@@ -302,6 +302,21 @@ def setup(**kwargs):
             ["pandas", "click"],
             id="legacy_encoding__succeeds",
         ),
+        pytest.param(
+            """\
+            from setuptools import setup
+
+            setup(
+                extras_require={
+                    'crt1': 'botocore1>=1.33.2,<2.0a.0',
+                    'crt2': 'botocore2[crt]',
+                    'crt3': ['botocore3'],
+                },
+            )
+            """,
+            ["botocore1", "botocore2", "botocore3"],
+            id="extras_with_varying_types",
+        ),
     ],
 )
 def test_parse_setup_py(write_tmp_files, file_content, expect_deps):