Repository: sarugaku/requirementslib

Issue:
AST parsing fails when setup.py is called with fully qualified functions
`setuptools.setup()` for example is not captured by the AST parser, but it should be

Discussion:

Patch:
diff --git a/news/163.bugfix.rst b/news/163.bugfix.rst
new file mode 100644
index 0000000..5ad4e4f
--- /dev/null
+++ b/news/163.bugfix.rst
@@ -0,0 +1,1 @@
+Added support to the AST parser for discovering non-standard invocations of ``setup`` in ``setup.py``, e.g. using the fully qualified function name.
diff --git a/src/requirementslib/models/setup_info.py b/src/requirementslib/models/setup_info.py
index b3251f7..4888169 100644
--- a/src/requirementslib/models/setup_info.py
+++ b/src/requirementslib/models/setup_info.py
@@ -665,13 +665,31 @@ def ast_unparse(item, initial_mapping=False, analyzer=None, recurse=True):  # no
             unparsed = item
     elif six.PY3 and isinstance(item, ast.NameConstant):
         unparsed = item.value
+    elif isinstance(item, ast.Attribute):
+        attr_name = getattr(item, "value", None)
+        attr_attr = getattr(item, "attr", None)
+        name = unparse(attr_name) if attr_name is not None else attr_attr
+        if initial_mapping:
+            unparsed = item
+        elif name and attr_attr:
+            if not initial_mapping and isinstance(name, six.string_types):
+                unparsed = ".".join([item for item in (name, attr_attr) if item])
+            else:
+                unparsed = item
+        elif attr_attr and not name:
+            unparsed = attr_attr
+        else:
+            unparsed = name
     elif isinstance(item, ast.Call):
         unparsed = {}
         if isinstance(item.func, ast.Name):
-            name = unparse(item.func)
-            unparsed[name] = {}
+            func_name = unparse(item.func)
+        elif isinstance(item.func, ast.Attribute):
+            func_name = unparse(item.func)
+        if func_name:
+            unparsed[func_name] = {}
             for keyword in item.keywords:
-                unparsed[name].update(unparse(keyword))
+                unparsed[func_name].update(unparse(keyword))
     elif isinstance(item, ast.keyword):
         unparsed = {unparse(item.arg): unparse(item.value)}
     elif isinstance(item, ast.Assign):
@@ -712,7 +730,14 @@ def ast_parse_setup_py(path):
     ast_analyzer.visit(tree)
     setup = {}  # type: Dict[Any, Any]
     for k, v in ast_analyzer.function_map.items():
-        if isinstance(k, ast.Name) and k.id == "setup":
+        fn_name = ""
+        if isinstance(k, ast.Name):
+            fn_name = k.id
+        elif isinstance(k, ast.Attribute):
+            fn = ast_unparse(k)
+            if isinstance(fn, six.string_types):
+                _, _, fn_name = fn.rpartition(".")
+        if fn_name == "setup":
             setup = v
     cleaned_setup = ast_unparse(setup, analyzer=ast_analyzer)
     return cleaned_setup


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index a9f87b8..209831b 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -12,6 +12,8 @@ import vistir
 import requirementslib.utils
 from requirementslib.models.setup_info import SetupInfo
 
+CURRENT_FILE = vistir.compat.Path(__file__).absolute()
+
 
 def check_for_mercurial():
     c = vistir.misc.run(
@@ -120,6 +122,21 @@ def artifact_dir():
     )
 
 
+@pytest.fixture(scope="session")
+def fixture_dir():
+    return CURRENT_FILE.parent.joinpath("fixtures")
+
+
+@pytest.fixture(scope="session")
+def setup_py_dir(fixture_dir):
+    return fixture_dir / "setup_py"
+
+
+@pytest.fixture(scope="session")
+def setup_cfg_dir(fixture_dir):
+    return fixture_dir / "setup_cfg"
+
+
 @pytest.fixture
 def test_artifact(artifact_dir, pathlib_tmpdir, request):
     name = request.param["name"]
diff --git a/tests/fixtures/setup_py/package_using_fully_qualified_setuptools/setup.py b/tests/fixtures/setup_py/package_using_fully_qualified_setuptools/setup.py
new file mode 100644
index 0000000..e388b00
--- /dev/null
+++ b/tests/fixtures/setup_py/package_using_fully_qualified_setuptools/setup.py
@@ -0,0 +1,22 @@
+import os
+
+import setuptools
+
+thisdir = os.path.abspath(os.path.dirname(__file__))
+version = "1.0.0"
+
+testing_extras = ["coverage", "flaky"]
+
+setuptools.setup(
+    name="test_package",
+    version=version,
+    description="The Backend HTTP Server",
+    long_description="This is a package",
+    install_requires=["six"],
+    tests_require=testing_extras,
+    extras_require={"testing": testing_extras},
+    package_dir={"": "src"},
+    packages=["test_package"],
+    include_package_data=True,
+    zip_safe=False,
+)
diff --git a/tests/fixtures/setup_py/package_using_fully_qualified_setuptools/src/test_package/__init__.py b/tests/fixtures/setup_py/package_using_fully_qualified_setuptools/src/test_package/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fixtures/setup_py/package_with_extras_as_variable/setup.py b/tests/fixtures/setup_py/package_with_extras_as_variable/setup.py
new file mode 100644
index 0000000..21d5301
--- /dev/null
+++ b/tests/fixtures/setup_py/package_with_extras_as_variable/setup.py
@@ -0,0 +1,22 @@
+import os
+
+from setuptools import find_packages, setup
+
+thisdir = os.path.abspath(os.path.dirname(__file__))
+version = "1.0.0"
+
+testing_extras = ["coverage", "flaky"]
+
+setup(
+    name="test_package",
+    version=version,
+    description="The Backend HTTP Server",
+    long_description="This is a package",
+    install_requires=["six"],
+    tests_require=testing_extras,
+    extras_require={"testing": testing_extras},
+    package_dir={"": "src"},
+    packages=["test_package"],
+    include_package_data=True,
+    zip_safe=False,
+)
diff --git a/tests/fixtures/setup_py/package_with_extras_as_variable/src/test_package/__init__.py b/tests/fixtures/setup_py/package_with_extras_as_variable/src/test_package/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fixtures/setup_py/package_with_multiple_extras/setup.py b/tests/fixtures/setup_py/package_with_multiple_extras/setup.py
new file mode 100644
index 0000000..bdae619
--- /dev/null
+++ b/tests/fixtures/setup_py/package_with_multiple_extras/setup.py
@@ -0,0 +1,22 @@
+import os
+
+from setuptools import find_packages, setup
+
+thisdir = os.path.abspath(os.path.dirname(__file__))
+version = "1.0.0"
+
+testing_extras = ["coverage", "flaky"]
+
+setup(
+    name="test_package",
+    version=version,
+    description="The Backend HTTP Server",
+    long_description="This is a package",
+    install_requires=["six"],
+    tests_require=testing_extras,
+    extras_require={"testing": testing_extras, "dev": ["parver", "invoke", "wheel"]},
+    package_dir={"": "src"},
+    packages=["test_package"],
+    include_package_data=True,
+    zip_safe=False,
+)
diff --git a/tests/fixtures/setup_py/package_with_multiple_extras/src/test_package/__init__.py b/tests/fixtures/setup_py/package_with_multiple_extras/src/test_package/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fixtures/setup_py/package_with_one_extra/setup.py b/tests/fixtures/setup_py/package_with_one_extra/setup.py
new file mode 100644
index 0000000..21d5301
--- /dev/null
+++ b/tests/fixtures/setup_py/package_with_one_extra/setup.py
@@ -0,0 +1,22 @@
+import os
+
+from setuptools import find_packages, setup
+
+thisdir = os.path.abspath(os.path.dirname(__file__))
+version = "1.0.0"
+
+testing_extras = ["coverage", "flaky"]
+
+setup(
+    name="test_package",
+    version=version,
+    description="The Backend HTTP Server",
+    long_description="This is a package",
+    install_requires=["six"],
+    tests_require=testing_extras,
+    extras_require={"testing": testing_extras},
+    package_dir={"": "src"},
+    packages=["test_package"],
+    include_package_data=True,
+    zip_safe=False,
+)
diff --git a/tests/fixtures/setup_py/package_with_one_extra/src/test_package/__init__.py b/tests/fixtures/setup_py/package_with_one_extra/src/test_package/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/unit/test_setup_info.py b/tests/unit/test_setup_info.py
index f4dde47..2fb825b 100644
--- a/tests/unit/test_setup_info.py
+++ b/tests/unit/test_setup_info.py
@@ -1,6 +1,7 @@
 # -*- coding=utf-8 -*-
 
 import os
+import shutil
 import sys
 
 import pip_shims.shims
@@ -8,6 +9,7 @@ import pytest
 import vistir
 
 from requirementslib.models.requirements import Requirement
+from requirementslib.models.setup_info import ast_parse_setup_py
 
 
 @pytest.mark.parametrize(
@@ -124,52 +126,25 @@ setup(
         assert sorted(list(setup_dict.get("requires").keys())) == ["raven"]
 
 
-def test_extras(pathlib_tmpdir):
+@pytest.mark.parametrize(
+    "setup_py_name, extras, dependencies",
+    [
+        (
+            "package_with_multiple_extras",
+            ["testing", "dev"],
+            ["coverage", "flaky", "invoke", "parver", "six", "wheel"],
+        ),
+        ("package_with_one_extra", ["testing"], ["coverage", "flaky", "six"]),
+    ],
+)
+def test_extras(pathlib_tmpdir, setup_py_dir, setup_py_name, extras, dependencies):
     """Test named extras as a dependency"""
     setup_dir = pathlib_tmpdir.joinpath("test_package")
-    setup_dir.mkdir()
+    shutil.copytree(setup_py_dir.joinpath(setup_py_name).as_posix(), setup_dir.as_posix())
     assert setup_dir.is_dir()
-    setup_py = setup_dir.joinpath("setup.py")
-    setup_py.write_text(
-        u"""
-import os
-from setuptools import setup, find_packages
-
-thisdir = os.path.abspath(os.path.dirname(__file__))
-version = "1.0.0"
-
-testing_extras = [
-    'coverage',
-    'flaky',
-]
-
-setup(
-    name='test_package',
-    version=version,
-    description="The Backend HTTP Server",
-    long_description="This is a package",
-    install_requires=[
-        'six',
-    ],
-    tests_require=testing_extras,
-    extras_require={
-        'testing': testing_extras,
-    },
-    package_dir={"": "src"},
-    packages=['test_package'],
-    include_package_data=True,
-    zip_safe=False,
-)
-    """.strip()
-    )
-    src_dir = setup_dir.joinpath("src")
-    src_dir.mkdir()
-    pkg_dir = src_dir.joinpath("test_package")
-    pkg_dir.mkdir()
-    pkg_dir.joinpath("__init__.py").write_text(u"")
     pipfile_entry = {
         "path": "./{0}".format(setup_dir.name),
-        "extras": ["testing"],
+        "extras": extras,
         "editable": True,
     }
     setup_dict = None
@@ -178,8 +153,56 @@ setup(
         assert r.name == "test-package"
         r.req.setup_info.get_info()
         setup_dict = r.req.setup_info.as_dict()
-        assert sorted(list(setup_dict.get("requires").keys())) == [
-            "coverage",
-            "flaky",
-            "six",
-        ], setup_dict
+        assert sorted(list(setup_dict.get("requires").keys())) == dependencies
+
+
+def test_ast_parser_finds_variables(setup_py_dir):
+    parsed = ast_parse_setup_py(
+        setup_py_dir.joinpath("package_with_extras_as_variable/setup.py").as_posix()
+    )
+    expected = {
+        "name": "test_package",
+        "version": "1.0.0",
+        "description": "The Backend HTTP Server",
+        "long_description": "This is a package",
+        "install_requires": ["six"],
+        "tests_require": ["coverage", "flaky"],
+        "extras_require": {"testing": ["coverage", "flaky"]},
+        "package_dir": {"": "src"},
+        "packages": ["test_package"],
+        "include_package_data": True,
+        "zip_safe": False,
+    }
+    for k, v in expected.items():
+        assert k in parsed
+        if isinstance(v, bool):
+            assert str(parsed[k]) == str(v), parsed[k]
+        else:
+            assert parsed[k] == v, parsed[k]
+
+
+def test_ast_parser_finds_fully_qualified_setup(setup_py_dir):
+    parsed = ast_parse_setup_py(
+        setup_py_dir.joinpath(
+            "package_using_fully_qualified_setuptools/setup.py"
+        ).as_posix()
+    )
+    expected = {
+        "name": "test_package",
+        "version": "1.0.0",
+        "description": "The Backend HTTP Server",
+        "long_description": "This is a package",
+        "install_requires": ["six"],
+        "tests_require": ["coverage", "flaky"],
+        "extras_require": {"testing": ["coverage", "flaky"]},
+        "package_dir": {"": "src"},
+        "packages": ["test_package"],
+        "include_package_data": True,
+        "zip_safe": False,
+    }
+    for k, v in expected.items():
+        assert k in parsed
+        if isinstance(v, bool):
+            assert str(parsed[k]) == str(v), parsed[k]
+        else:
+            assert parsed[k] == v, parsed[k]