From 6477669de676c2baa1b5c0a2566cfd00ad42fc72 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 28 May 2020 20:59:20 -0400 Subject: [PATCH] Fix AST parsing when repeated assignments occur - Additionally fix handling of `os.environ` based version numbers and other assignments - Fixes #246 Signed-off-by: Dan Ryan --- news/246.bugfix.rst | 1 + src/requirementslib/models/setup_info.py | 20 +++++++------- .../setup.py | 26 +++++++++++++++++++ .../__init__.py | 0 tests/unit/test_setup_info.py | 18 +++++++++++++ 5 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 news/246.bugfix.rst create mode 100644 tests/fixtures/setup_py/package_with_repeated_assignments/setup.py create mode 100644 tests/fixtures/setup_py/package_with_repeated_assignments/src/test_package_with_repeated_assignments/__init__.py diff --git a/news/246.bugfix.rst b/news/246.bugfix.rst new file mode 100644 index 00000000..7ffcdd1c --- /dev/null +++ b/news/246.bugfix.rst @@ -0,0 +1 @@ +Fixed an issue in the AST parser which caused failures when parsing ``setup.py`` files with assignments (e.g. ``variable = some_value``) to the same name more than once, followed by operations on those variables (e.g. ``new_value = variable + other_variable``). diff --git a/src/requirementslib/models/setup_info.py b/src/requirementslib/models/setup_info.py index f0d40f29..5979dbdd 100644 --- a/src/requirementslib/models/setup_info.py +++ b/src/requirementslib/models/setup_info.py @@ -729,14 +729,16 @@ def unmap_binops(self): self.binOps_map[binop] = ast_unparse(binop, analyzer=self) def match_assignment_str(self, match): - return next( - iter(k for k in self.assignments if getattr(k, "id", "") == match), None - ) + matches = [k for k in self.assignments if getattr(k, "id", "") == match] + if matches: + return matches[-1] + return None def match_assignment_name(self, match): - return next( - iter(k for k in self.assignments if getattr(k, "id", "") == match.id), None - ) + matches = [k for k in self.assignments if getattr(k, "id", "") == match.id] + if matches: + return matches[-1] + return None def generic_unparse(self, item): if any(isinstance(item, k) for k in AST_BINOP_MAP.keys()): @@ -771,7 +773,7 @@ def unparse_Subscript(self, item): if isinstance(item.slice, ast.Index): try: unparsed = unparsed[self.unparse(item.slice.value)] - except KeyError: + except (KeyError, TypeError): # not everything can be looked up before runtime unparsed = item return unparsed @@ -838,7 +840,7 @@ def unparse_Compare(self, item): if isinstance(item.left, ast.Attribute) or isinstance(item.left, ast.Str): import importlib - left = unparse(item.left) + left = self.unparse(item.left) if "." in left: name, _, val = left.rpartition(".") left = getattr(importlib.import_module(name), val, left) @@ -1002,7 +1004,7 @@ def ast_unparse(item, initial_mapping=False, analyzer=None, recurse=True): # no if isinstance(item.slice, ast.Index): try: unparsed = unparsed[unparse(item.slice.value)] - except KeyError: + except (KeyError, TypeError): # not everything can be looked up before runtime unparsed = item elif any(isinstance(item, k) for k in AST_BINOP_MAP.keys()): diff --git a/tests/fixtures/setup_py/package_with_repeated_assignments/setup.py b/tests/fixtures/setup_py/package_with_repeated_assignments/setup.py new file mode 100644 index 00000000..d979fdd1 --- /dev/null +++ b/tests/fixtures/setup_py/package_with_repeated_assignments/setup.py @@ -0,0 +1,26 @@ +import os + +from setuptools import find_packages, setup + +thisdir = os.path.abspath(os.path.dirname(__file__)) +version = os.environ["PACKAGE_VERSION"] + + +def my_function(other_list): + entry = {"key": [{"matches": ["string 1", "string 2", "some_string"]}]} + matches = entry["key"][0]["matches"] + matches = [x for x in matches if "some_string" not in x] + entry["key"][0]["matches"] = matches + other_list + + +setup( + name="test_package_with_repeated_assignments", + version=version, + description="Test package with repeated assignments and version from environment", + long_description="This is a package", + install_requires=["six",], + package_dir={"": "src"}, + packages=find_packages("src"), + include_package_data=True, + zip_safe=False, +) diff --git a/tests/fixtures/setup_py/package_with_repeated_assignments/src/test_package_with_repeated_assignments/__init__.py b/tests/fixtures/setup_py/package_with_repeated_assignments/src/test_package_with_repeated_assignments/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/test_setup_info.py b/tests/unit/test_setup_info.py index 1f2a24f3..5e8511af 100644 --- a/tests/unit/test_setup_info.py +++ b/tests/unit/test_setup_info.py @@ -252,6 +252,24 @@ def test_ast_parser_handles_binops(setup_py_dir): assert analyzer.parse_setup_function() == parsed +def test_ast_parser_handles_repeated_assignments(setup_py_dir): + target = setup_py_dir.joinpath( + "package_with_repeated_assignments/setup.py" + ).as_posix() + parsed = ast_parse_setup_py(target) + analyzer = ast_parse_file(target) + assert parsed["name"] == "test_package_with_repeated_assignments" + assert isinstance(parsed["version"], str) is False + assert parsed["install_requires"] == ["six"] + analyzer_parsed = analyzer.parse_setup_function() + # the versions in this instance are AST objects as they come from + # os.environ and will need to be parsed downstream from here, so + # equality comparisons will fail + analyzer_parsed.pop("version") + parsed.pop("version") + assert analyzer_parsed == parsed + + def test_setup_cfg_parser(setup_cfg_dir): setup_path = setup_cfg_dir / "package_with_multiple_extras/setup.cfg" if six.PY2: