Repository: symbol-management/symbol-exporter

Issue:
Constants should have empty volume
```python
import numpy as np

a = np.ones(5)
```

 `numpy.ones` is not hidden by the module - symbol is used at module import - so we should consider that part of the surface area and not the volume.

Above code should not have a volume in `z.symbols[a]`

Currently it is:
```python
z.symbols == {
        "a": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "constant"}
    }
```

The proposal is:
```python
z.symbols == {
        "MODULE_NAME.a": {"lineno": 4, "symbols_in_volume": set(), "type": "constant"},
        "MODULE_NAME": {"lineno": None, "symbols_in_volume": {"numpy.ones"}, "type": "module"}
    }
```

Discussion:

Patch:
diff --git a/symbol_exporter/ast_symbol_extractor.py b/symbol_exporter/ast_symbol_extractor.py
index cc60d83..0f138cd 100644
--- a/symbol_exporter/ast_symbol_extractor.py
+++ b/symbol_exporter/ast_symbol_extractor.py
@@ -8,19 +8,16 @@ builtin_symbols = set(dir(builtins))
 
 
 class SymbolFinder(ast.NodeVisitor):
-    def __init__(self, module_name=None):
-        if module_name:
-            self.current_symbol_stack = [module_name]
-            self.symbols = {
-                module_name: {
-                    "type": "module",
-                    "lineno": None,
-                    "symbols_in_volume": set(),
-                }
+    def __init__(self, module_name):
+        self._module_name = module_name
+        self.current_symbol_stack = [module_name]
+        self.symbols = {
+            module_name: {
+                "type": "module",
+                "lineno": None,
+                "symbols_in_volume": set(),
             }
-        else:
-            self.current_symbol_stack = []
-            self.symbols = {}
+        }
         self.imported_symbols = []
         self.attr_stack = []
         self.used_symbols = set()
@@ -60,11 +57,11 @@ class SymbolFinder(ast.NodeVisitor):
         # TODO: handle multiple assignments
         # TODO: handle inside class
         # TODO: handle self?
-        if len(node.targets) == 1 and len(self.current_symbol_stack) == 0:
+        if len(node.targets) == 1 and len(self.current_symbol_stack) == 1:
             for target in node.targets:
                 if hasattr(target, "id"):
                     self.current_symbol_stack.append(target.id)
-                    self.symbols[target.id] = {
+                    self.symbols[self._symbol_stack_to_symbol_name()] = {
                         "type": "constant",
                         "lineno": node.lineno,
                         "symbols_in_volume": set(),
@@ -81,7 +78,7 @@ class SymbolFinder(ast.NodeVisitor):
         if (hasattr(node.func, "id")
                 and node.func.id not in self.aliases
                 and node.func.id not in builtin_symbols
-                and node.func.id not in self.symbols):
+                and not self._symbol_in_surface_area(node.func.id)):
             self.undeclared_symbols.add(node.func.id)
         tmp_stack = self.attr_stack.copy()
         self.attr_stack.clear()
@@ -111,26 +108,41 @@ class SymbolFinder(ast.NodeVisitor):
         # self.aliases.pop("self")
 
     def visit_Name(self, node: ast.Name) -> Any:
+        def get_symbol_name(name):
+            return self._symbol_in_surface_area(name) or ".".join([name] + list(reversed(self.attr_stack)))
+
         name = self.aliases.get(node.id, node.id)
         if name in builtin_symbols:
             self.used_builtins.add(name)
         if self._symbol_previously_seen(name):
-            symbol_name = ".".join([name] + list(reversed(self.attr_stack)))
-            # Hack for now until we remove constants from the symbols dict.
-            # Can remove if statement once https://github.com/symbol-management/symbol-exporter/issues/26 is resolved
-            if not self.symbols.get(symbol_name, {}).get("type") == "constant":
+            symbol_name = get_symbol_name(name)
+            if not self._is_constant(symbol_name):
                 self.used_symbols.add(symbol_name)
             # Do not add myself to my own volume.
             # A previously declared symbol in the module is being referenced.
-            if symbol_name != self._symbol_stack_to_symbol_name():
-                self.symbols[self._symbol_stack_to_symbol_name()]["symbols_in_volume"].add(
-                    symbol_name
-                )
+            surface_symbol = self._symbol_stack_to_symbol_name()
+            if symbol_name != surface_symbol:
+                if self._is_constant(surface_symbol):
+                    self.symbols[self._module_name]["symbols_in_volume"].add(symbol_name)
+                else:
+                    self.symbols[surface_symbol]["symbols_in_volume"].add(
+                        symbol_name
+                    )
         self.generic_visit(node)
 
+    def _is_constant(self, symbol_name):
+        return self.symbols.get(symbol_name, {}).get("type") == "constant"
+
     def _symbol_previously_seen(self, symbol):
         return (symbol in self.imported_symbols or symbol in self.undeclared_symbols
-                or symbol in builtin_symbols or symbol in self.symbols)
+                or symbol in builtin_symbols or self._symbol_in_surface_area(symbol))
+
+    def _symbol_in_surface_area(self, symbol):
+        fully_qualified_symbol_name = f"{self._module_name}.{symbol}"
+        if fully_qualified_symbol_name in self.symbols:
+            return fully_qualified_symbol_name
+        else:
+            return None
 
 # 1. get all the imports and their aliases (which includes imported things)
 # 2. walk the ast find all usages of those aliases and log all the names and attributes used


Test patch:
diff --git a/tests/test_ast_symbol_extractor.py b/tests/test_ast_symbol_extractor.py
index be90d41..9d7af0b 100644
--- a/tests/test_ast_symbol_extractor.py
+++ b/tests/test_ast_symbol_extractor.py
@@ -14,13 +14,14 @@ def f():
     return xyz.i
 """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"xyz": "abc.xyz"}
     assert z.imported_symbols == ["abc.xyz"]
     assert z.used_symbols == {"abc.xyz.i"}
     assert z.symbols == {
-        "f": {"lineno": 4, "symbols_in_volume": {"abc.xyz.i"}, "type": "function"}
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.f": {"lineno": 4, "symbols_in_volume": {"abc.xyz.i"}, "type": "function"}
     }
 
 
@@ -32,13 +33,14 @@ def f():
     return l.i
 """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"xyz": "abc.xyz", "l": "abc.xyz"}
     assert z.imported_symbols == ["abc.xyz"]
     assert z.used_symbols == {"abc.xyz.i"}
     assert z.symbols == {
-        "f": {"lineno": 4, "symbols_in_volume": {"abc.xyz.i"}, "type": "function"}
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.f": {"lineno": 4, "symbols_in_volume": {"abc.xyz.i"}, "type": "function"}
     }
 
 
@@ -50,13 +52,14 @@ def f():
     return np.ones(np.twos().three)
 """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
     assert z.used_symbols == {"numpy.ones", "numpy.twos"}
     assert z.symbols == {
-        "f": {
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.f": {
             "lineno": 4,
             "symbols_in_volume": {"numpy.ones", "numpy.twos"},
             "type": "function",
@@ -71,13 +74,14 @@ import numpy as np
 z = np.ones(5)
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
     assert z.used_symbols == {"numpy.ones"}
     assert z.symbols == {
-        "z": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "constant"}
+        'mm': {'lineno': None, 'symbols_in_volume': {"numpy.ones"}, 'type': 'module'},
+        "mm.z": {"lineno": 4, "symbols_in_volume": set(), "type": "constant"}
     }
 
 
@@ -89,13 +93,14 @@ class ABC():
     a = np.ones(5)
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
     assert z.used_symbols == {"numpy.ones"}
     assert z.symbols == {
-        "ABC": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "class"}
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.ABC": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "class"}
     }
 
 
@@ -110,14 +115,15 @@ class ABC():
         return np.twos(10)
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
     assert z.used_symbols == {"numpy.ones", "numpy.twos"}
     assert z.symbols == {
-        "ABC": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "class"},
-        "ABC.xyz": {
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.ABC": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "class"},
+        "mm.ABC.xyz": {
             "lineno": 7,
             "symbols_in_volume": {"numpy.twos"},
             "type": "function",
@@ -137,15 +143,16 @@ def test_import_adds_symbols():
     z = np.ones(5)
     """
     tree = ast.parse(dedent(code))
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     # TODO: should we add a key in the metadata to say that a symbol is
     #  a reference to another symbol?
     assert z.symbols == {
-        "np": {},
-        "xyz": {},
-        "l": {},
-        "z": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "constant"},
+        "mm.np": {},
+        "mm.xyz": {},
+        "mm.l": {},
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'},
+        "mm.z": {"lineno": 4, "symbols_in_volume": {"numpy.ones"}, "type": "constant"},
     }
 
 
@@ -155,13 +162,15 @@ import numpy as np
 from abc import *
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
     assert not z.used_symbols
     assert z.star_imports == {"abc"}
-    assert not z.symbols
+    assert z.symbols == {
+        'mm': {'lineno': None, 'symbols_in_volume': set(), 'type': 'module'}
+    }
 
 
 def test_undeclared_symbols():
@@ -176,7 +185,7 @@ a = np.ones(5)
 b = twos(10)
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"np": "numpy"}
     assert z.imported_symbols == ["numpy"]
@@ -184,13 +193,17 @@ b = twos(10)
     assert z.undeclared_symbols == {"twos"}
     assert z.star_imports == {"abc", "xyz"}
     assert z.symbols == {
-        "a": {
+        'mm': {
+            'lineno': None,
+            'symbols_in_volume': {"numpy.ones", "twos"},
+            'type': 'module'},
+        "mm.a": {
             "lineno": 8,
-            "symbols_in_volume": {"numpy.ones"},
+            "symbols_in_volume": set(),
             "type": "constant"},
-        "b": {
+        "mm.b": {
             "lineno": 9,
-            "symbols_in_volume": {"twos"},
+            "symbols_in_volume": set(),
             "type": "constant"},
     }
 
@@ -202,13 +215,14 @@ from abc import twos
 b = twos(10)
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"twos": "abc.twos"}
     assert z.imported_symbols == ["abc.twos"]
     assert z.used_symbols == {"abc.twos"}
     assert z.symbols == {
-        "b": {"lineno": 4, "symbols_in_volume": {"abc.twos"}, "type": "constant"}
+        'mm': {'lineno': None, 'symbols_in_volume': {"abc.twos"}, 'type': 'module'},
+        "mm.b": {"lineno": 4, "symbols_in_volume": set(), "type": "constant"}
     }
     assert not z.undeclared_symbols
 
@@ -220,14 +234,15 @@ from abc import twos
 b = len([])
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"twos": "abc.twos"}
     assert z.imported_symbols == ["abc.twos"]
     assert z.used_symbols == {"len"}
     assert z.used_builtins == {"len"}
     assert z.symbols == {
-        "b": {"lineno": 4, "symbols_in_volume": {"len"}, "type": "constant"}
+        'mm': {'lineno': None, 'symbols_in_volume': {"len"}, 'type': 'module'},
+        "mm.b": {"lineno": 4, "symbols_in_volume": set(), "type": "constant"}
     }
     assert not z.undeclared_symbols
 
@@ -242,14 +257,15 @@ def f():
 g = f()
     """
     tree = ast.parse(code)
-    z = SymbolFinder()
+    z = SymbolFinder(module_name="mm")
     z.visit(tree)
     assert z.aliases == {"twos": "abc.twos"}
     assert z.imported_symbols == ["abc.twos"]
-    assert z.used_symbols == {"f"}
+    assert z.used_symbols == {"mm.f"}
     assert not z.used_builtins
     assert z.symbols == {
-        "f": {"lineno": 4, "symbols_in_volume": set(), "type": "function"},
-        'g': {'lineno': 7, 'symbols_in_volume': {'f'}, 'type': 'constant'}
+        'mm': {'lineno': None, 'symbols_in_volume': {'mm.f'}, 'type': 'module'},
+        "mm.f": {"lineno": 4, "symbols_in_volume": set(), "type": "function"},
+        'mm.g': {'lineno': 7, 'symbols_in_volume': set(), 'type': 'constant'}
     }
     assert not z.undeclared_symbols