Detect new usage of imported names in functions

pyff/pyff.py

@@ -1,14 +1,40 @@
 """Functions for comparison of various Python entities"""
 
 from ast import FunctionDef, parse, NodeVisitor, Module, dump
-from typing import cast, Set, Dict
+from typing import cast, Set, Dict, Iterable
 from collections import defaultdict
 from itertools import zip_longest
 
 import pyff.pyfference as pf
 from pyff.summary import ClassSummary
 
-def _pyff_function_ast(first: FunctionDef, second: FunctionDef) -> pf.FunctionPyfference:
+class ExternalNamesExtractor(NodeVisitor):
+    """Collects information about imported name usage in function"""
+    def __init__(self, imported_names: Iterable[str]) -> None:
+        self.imported_names: Iterable[str] = imported_names
+        self.names: Set[str] = set()
+
+    def visit_Name(self, node): # pylint: disable=invalid-name
+        """Compare all names against a list of imported names"""
+        if node.id in self.imported_names:
+            self.names.add(node.id)
+
+def _compare_import_usage(old: FunctionDef, new: FunctionDef,
+                          old_imports: Iterable[str] = None,
+                          new_imports: Iterable[str] = None) -> Iterable[str]:
+    first_walker = ExternalNamesExtractor(old_imports)
+    second_walker = ExternalNamesExtractor(new_imports)
+
+    first_walker.visit(old)
+    second_walker.visit(new)
+
+    appeared = second_walker.names - first_walker.names
+
+    return appeared
+
+def _pyff_function_ast(first: FunctionDef, second: FunctionDef,
+                       old_imports: Iterable[str] = None,
+                       new_imports: Iterable[str] = None) -> pf.FunctionPyfference:
     """Return differences between two Python function ASTs, or None if they are identical"""
     names = None
     if first.name != second.name:
@@ -20,8 +46,14 @@ def _pyff_function_ast(first: FunctionDef, second: FunctionDef) -> pf.FunctionPy
             implementation = True
             break
 
+    if old_imports or new_imports:
+        appeared_import_usage = _compare_import_usage(first, second, old_imports, new_imports)
+    else:
+        appeared_import_usage = None
+
     if names or implementation:
-        return pf.FunctionPyfference(name=first.name, names=names, implementation=implementation)
+        return pf.FunctionPyfference(name=first.name, names=names, implementation=implementation,
+                                     appeared_import_usage=appeared_import_usage)
 
     return None
 
@@ -77,6 +109,12 @@ class MethodsExtractor(NodeVisitor):
     def __init__(self) -> None:
         self.names: Set[str] = set()
         self.functions: Dict[str, FunctionDef] = {}
+        self.imported_names: Set[str] = set()
+
+    def visit_ImportFrom(self, node): # pylint: disable=invalid-name
+        """Save imported names"""
+        for name in node.names:
+            self.imported_names.add(name.name)
 
     def visit_ClassDef(self, node): # pylint: disable=invalid-name
         """Prevent this visitor from inspecting classes"""
@@ -99,7 +137,9 @@ def _pyff_functions(first_ast: Module, second_ast: Module) -> pf.FunctionsPyffer
     differences = {}
     for function in both:
         difference = _pyff_function_ast(first_walker.functions[function],
-                                        second_walker.functions[function])
+                                        second_walker.functions[function],
+                                        first_walker.imported_names,
+                                        second_walker.imported_names)
         if difference:
             differences[function] = difference
 
@@ -126,7 +166,9 @@ def visit_ImportFrom(self, node): # pylint: disable=invalid-name
         """Save information about `from x import y` statements"""
         self.modules[node.module].update([node.name for node in node.names])
 
-def pyff_function(first: str, second: str) -> pf.FunctionPyfference:
+def pyff_function(first: str, second: str,
+                  old_imports: Iterable[str] = None,
+                  new_imports: Iterable[str] = None) -> pf.FunctionPyfference:
     """Return differences between two Python functions, or None if they are identical"""
     first_ast = parse(first).body
     second_ast = parse(second).body
@@ -136,7 +178,8 @@ def pyff_function(first: str, second: str) -> pf.FunctionPyfference:
     if len(second_ast) != 1 or not isinstance(second_ast[0], FunctionDef):
         raise ValueError(f"Second argument does not seem to be a single Python function: {second}")
 
-    return _pyff_function_ast(cast(FunctionDef, first_ast[0]), cast(FunctionDef, second_ast[0]))
+    return _pyff_function_ast(cast(FunctionDef, first_ast[0]), cast(FunctionDef, second_ast[0]),
+                              old_imports, new_imports)
 
 def pyff_module(first: str, second: str) -> pf.ModulePyfference:
     """Return difference between two Python modules, or None if they are identical"""

pyff/pyfference.py

@@ -8,11 +8,12 @@
 class FunctionPyfference:  # pylint: disable=too-few-public-methods
     """Holds differences between Python function definitions"""
     def __init__(self, name: str, names: Tuple[str, str] = None,
-                 implementation: bool = None) -> None:
+                 implementation: bool = None, appeared_import_usage: Iterable[str] = None) -> None:
         self.name = name
         self.names: Change = None
         self.changes: List = []
         self.implementation: bool = implementation
+        self.appeared_import_usage: Iterable[str] = appeared_import_usage
 
         if names:
             self.names = Change(names[0], names[1])
@@ -25,14 +26,19 @@ def __len__(self):
         return len(self.changes)
 
     def __str__(self):
+        suffix = ""
+        if self.appeared_import_usage:
+            names = [f"'{name}'" for name in self.appeared_import_usage]
+            suffix = ", newly uses external names " + ", ".join(names)
+
         if self.names and self.implementation:
             old = self.names.old
             new = self.names.new
-            return f"Function '{old}' was renamed to '{new}' and its implementation changed"
+            return f"Function '{old}' renamed to '{new}' and its implementation changed" + suffix
         elif self.names:
-            return f"Function '{self.names.old}' was renamed to '{self.names.new}'"
+            return f"Function '{self.names.old}' renamed to '{self.names.new}'"
         elif self.implementation:
-            return f"Function '{self.name}' changed implementation"
+            return f"Function '{self.name}' changed implementation" + suffix
 
         return ""
 

tests/unit/test_functions.py

@@ -3,10 +3,11 @@
 from pytest import raises
 from pyff.pyff import pyff_function
 
-TRIVIAL_FUNCTION = """def function(): pass"""
-TRIVIAL_FUNCTION_2 = """def function2(): pass"""
+TRIVIAL_FUNCTION = """def func(): pass"""
+TRIVIAL_FUNCTION_2 = """def func2(): pass"""
 
-IMPLEMENTED_FUNCTION = """def function(): return None"""
+IMPLEMENTED_FUNCTION = """def func(): return None"""
+FUNCTION_W_EXTERNAL_NAME = """def func(): parser = ArgumentParser()"""
 
 def test_trivial_function():
     difference = pyff_function(TRIVIAL_FUNCTION, TRIVIAL_FUNCTION)
@@ -15,10 +16,10 @@ def test_trivial_function():
 def test_name_change():
     difference = pyff_function(TRIVIAL_FUNCTION, TRIVIAL_FUNCTION_2)
     assert len(difference) == 1
-    assert difference.name == "function"
+    assert difference.name == "func"
     assert difference.names is not None
-    assert difference.names.old == "function"
-    assert difference.names.new == "function2"
+    assert difference.names.old == "func"
+    assert difference.names.new == "func2"
 
 def test_not_functions():
     no_func = "a = 1"
@@ -36,4 +37,13 @@ def test_changed_implementation():
     assert len(difference) == 1
     assert difference.names is None
     assert difference.implementation is not None
-    assert str(difference) == "Function 'function' changed implementation"
+    assert str(difference) == "Function 'func' changed implementation"
+
+def test_changed_implementation_external_name(): # pylint: disable=invalid-name
+    difference = pyff_function(TRIVIAL_FUNCTION, FUNCTION_W_EXTERNAL_NAME, old_imports=[],
+                               new_imports=["ArgumentParser"])
+    assert len(difference) == 1
+    assert difference.names is None
+    assert difference.implementation is not None
+    assert (str(difference) ==
+            "Function 'func' changed implementation, newly uses external names 'ArgumentParser'")

tests/unit/test_modules.py

@@ -24,6 +24,10 @@ def func():
 def func():
     return None"""
 
+IMPORT_USAGE_MODULE = """import sys
+from os import path
+def func():
+    return path()"""
 
 def test_trivial_module():
     difference = pyff_module(TRIVIAL_MODULE, TRIVIAL_MODULE)
@@ -46,3 +50,10 @@ def test_module_with_changed_function(): # pylint: disable=invalid-name
     assert difference is not None
     assert len(difference) == 1
     assert str(difference) == "Function 'func' changed implementation"
+
+def test_module_with_new_external_names_usage(): # pylint: disable=invalid-name
+    difference = pyff_module(IMPORT_MODULE, IMPORT_USAGE_MODULE)
+    assert difference is not None
+    assert len(difference) == 1
+    assert (str(difference) ==
+            "Function 'func' changed implementation, newly uses external names 'path'")

tests/unit/test_pyfference.py

@@ -9,7 +9,7 @@ def test_function_name_changed():
     assert fpyff.names.new == "second"
     assert fpyff.implementation is None
     assert len(fpyff) == 1
-    assert str(fpyff) == "Function 'first' was renamed to 'second'"
+    assert str(fpyff) == "Function 'first' renamed to 'second'"
 
 def test_function_same():
     fpyff = pf.FunctionPyfference(name="func")
@@ -30,7 +30,7 @@ def test_function_implementation_changed(): # pylint: disable=invalid-name
 def test_function_everything_changed(): # pylint: disable=invalid-name
     fpyff = pf.FunctionPyfference(name="first", names=("first", "second"), implementation=True)
     assert len(fpyff) == 2
-    assert str(fpyff) == "Function 'first' was renamed to 'second' and its implementation changed"
+    assert str(fpyff) == "Function 'first' renamed to 'second' and its implementation changed"
 
 def test_new_from_import():
     mpyff = pf.FromImportPyfference(new={'os': ['path', 'getenv']})