Improved assignment handling with sorting

Author: amyreese

dowsing/setuptools/setup_py_parsing.py

@@ -8,7 +8,12 @@
 from typing import Any, Dict, Optional
 
 import libcst as cst
-from libcst.metadata import ParentNodeProvider, QualifiedNameProvider, ScopeProvider
+from libcst.metadata import (
+    ParentNodeProvider,
+    PositionProvider,
+    QualifiedNameProvider,
+    ScopeProvider,
+)
 
 from ..types import Distribution
 from .setup_and_metadata import SETUP_ARGS
@@ -124,7 +129,12 @@ def leave_Call(
 
 
 class SetupCallAnalyzer(cst.CSTVisitor):
-    METADATA_DEPENDENCIES = (ScopeProvider, ParentNodeProvider, QualifiedNameProvider)
+    METADATA_DEPENDENCIES = (
+        ScopeProvider,
+        ParentNodeProvider,
+        QualifiedNameProvider,
+        PositionProvider,
+    )
 
     # TODO names resulting from other than 'from setuptools import setup'
     # TODO wrapper funcs that modify args
@@ -179,7 +189,7 @@ def visit_Call(self, node: cst.Call) -> Optional[bool]:
     PRETEND_ARGV = ["setup.py", "bdist_wheel"]
 
     def evaluate_in_scope(
-        self, item: cst.CSTNode, scope: Any, target_name: str = ""
+        self, item: cst.CSTNode, scope: Any, target_name: str = "", target_line: int = 0
     ) -> Any:
         qnames = self.get_metadata(QualifiedNameProvider, item)
 
@@ -192,19 +202,30 @@ def evaluate_in_scope(
         elif isinstance(item, cst.Name):
             name = item.value
             assignments = scope[name]
-            for a in assignments:
-                # TODO: Only assignments "before" this node matter if in the
-                # same scope; really if we had a call graph and walked the other
-                # way, we could have a better idea of what has already happened.
-
+            assignment_nodes = sorted(
+                (
+                    (self.get_metadata(PositionProvider, a.node).start.line, a.node)
+                    for a in assignments
+                    if a.node
+                ),
+                reverse=True,
+            )
+            # Walk assignments from bottom to top, evaluating them recursively.
+            # When recursing, only look at assignments above the "target line".
+            for lineno, node in assignment_nodes:
                 # Assign(
                 #   targets=[AssignTarget(target=Name(value="v"))],
                 #   value=SimpleString(value="'x'"),
                 # )
                 # TODO or an import...
                 # TODO builtins have BuiltinAssignment
+
+                # we have recursed, likey due to `x = x + y` assignment or similar
+                # self-referential evaluation, and can't
+                if target_name and target_name == name and lineno >= target_line:
+                    continue
+
                 try:
-                    node = a.node
                     if node:
                         parent = self.get_metadata(ParentNodeProvider, node)
                         if parent:
@@ -214,35 +235,38 @@ def evaluate_in_scope(
                     else:
                         raise KeyError
                 except (KeyError, AttributeError):
-                    return "??"
-
-                # This presumes a single assignment
-                if not isinstance(gp, cst.Assign) or len(gp.targets) != 1:
-                    return "??"  # TooComplicated(repr(gp))
+                    continue
 
                 try:
                     scope = self.get_metadata(ScopeProvider, gp)
                 except KeyError:
                     # module scope isn't in the dict
-                    return "??"
+                    continue
 
-                # we have recursed, likey due to `x = x + y` assignment or similar
-                # self-referential evaluation
-                if target_name and target_name == name:
-                    return "??"
+                # This presumes a single assignment
+                if isinstance(gp, cst.Assign) and len(gp.targets) == 1:
+                    result = self.evaluate_in_scope(gp.value, scope, name, lineno)
+                elif isinstance(parent, cst.AugAssign):
+                    result = self.evaluate_in_scope(parent, scope, name, lineno)
+                else:
+                    # too complicated?
+                    continue
 
                 # keep trying assignments until we get something other than ??
-                result = self.evaluate_in_scope(gp.value, scope, name)
-                if result and result != "??":
+                if result != "??":
                     return result
+
             # give up
             return "??"
         elif isinstance(item, (cst.Tuple, cst.List)):
             lst = []
             for el in item.elements:
                 lst.append(
                     self.evaluate_in_scope(
-                        el.value, self.get_metadata(ScopeProvider, el), target_name
+                        el.value,
+                        self.get_metadata(ScopeProvider, el),
+                        target_name,
+                        target_line,
                     )
                 )
             if isinstance(item, cst.Tuple):
@@ -260,10 +284,12 @@ def evaluate_in_scope(
             for arg in item.args:
                 if isinstance(arg.keyword, cst.Name):
                     args[names.index(arg.keyword.value)] = self.evaluate_in_scope(
-                        arg.value, scope, target_name
+                        arg.value, scope, target_name, target_line
                     )
                 else:
-                    args[i] = self.evaluate_in_scope(arg.value, scope, target_name)
+                    args[i] = self.evaluate_in_scope(
+                        arg.value, scope, target_name, target_line
+                    )
                     i += 1
 
             # TODO clear ones that are still default
@@ -277,7 +303,7 @@ def evaluate_in_scope(
             for arg in item.args:
                 if isinstance(arg.keyword, cst.Name):
                     d[arg.keyword.value] = self.evaluate_in_scope(
-                        arg.value, scope, target_name
+                        arg.value, scope, target_name, target_line
                     )
                 # TODO something with **kwargs
             return d
@@ -286,19 +312,19 @@ def evaluate_in_scope(
             for el2 in item.elements:
                 if isinstance(el2, cst.DictElement):
                     d[self.evaluate_in_scope(el2.key, scope)] = self.evaluate_in_scope(
-                        el2.value, scope, target_name
+                        el2.value, scope, target_name, target_line
                     )
             return d
         elif isinstance(item, cst.Subscript):
-            lhs = self.evaluate_in_scope(item.value, scope, target_name)
+            lhs = self.evaluate_in_scope(item.value, scope, target_name, target_line)
             if isinstance(lhs, str):
                 # A "??" entry, propagate
                 return "??"
 
             # TODO: Figure out why this is Sequence
             if isinstance(item.slice[0].slice, cst.Index):
                 rhs = self.evaluate_in_scope(
-                    item.slice[0].slice.value, scope, target_name
+                    item.slice[0].slice.value, scope, target_name, target_line
                 )
                 try:
                     if isinstance(lhs, dict):
@@ -312,8 +338,8 @@ def evaluate_in_scope(
                 # LOG.warning(f"Omit2 {type(item.slice[0].slice)!r}")
                 return "??"
         elif isinstance(item, cst.BinaryOperation):
-            lhs = self.evaluate_in_scope(item.left, scope, target_name)
-            rhs = self.evaluate_in_scope(item.right, scope, target_name)
+            lhs = self.evaluate_in_scope(item.left, scope, target_name, target_line)
+            rhs = self.evaluate_in_scope(item.right, scope, target_name, target_line)
             if lhs == "??" or rhs == "??":
                 return "??"
             if isinstance(item.operator, cst.Add):
@@ -323,6 +349,18 @@ def evaluate_in_scope(
                     return "??"
             else:
                 return "??"
+        elif isinstance(item, cst.AugAssign):
+            lhs = self.evaluate_in_scope(item.target, scope, target_name, target_line)
+            rhs = self.evaluate_in_scope(item.value, scope, target_name, target_line)
+            if lhs == "??" or rhs == "??":
+                return "??"
+            if isinstance(item.operator, cst.AddAssign):
+                try:
+                    return lhs + rhs
+                except Exception:
+                    return "??"
+            else:
+                return "??"
         else:
             # LOG.warning(f"Omit1 {type(item)!r}")
             return "??"

dowsing/tests/setuptools.py

@@ -344,3 +344,32 @@ def test_add_items(self) -> None:
         self.assertEqual(d.name, "aaaa1111")
         self.assertEqual(d.packages, ["a", "b", "c"])
         self.assertEqual(d.classifiers, "??")
+
+    def test_self_reference_assignments(self) -> None:
+        d = self._read(
+            """\
+from setuptools import setup
+
+version = "base"
+name = "foo"
+name += "bar"
+version = version + ".suffix"
+
+classifiers = [
+    "123",
+    "abc",
+]
+
+if True:
+    classifiers = classifiers + ["xyz"]
+
+setup(
+    name=name,
+    version=version,
+    classifiers=classifiers,
+)
+            """
+        )
+        self.assertEqual(d.name, "foobar")
+        self.assertEqual(d.version, "base.suffix")
+        self.assertListEqual(d.classifiers, ["123", "abc", "xyz"])