Skip to content

Commit

Permalink
Fix enclosing attribute for attributes in call arguments
Browse files Browse the repository at this point in the history
Fixes enclosed arguments like `c.d` in `x.y(c.d()).z()` were badly being
resolved as `x.y` instead.

This also clarifies the intent in `infer_accesses()` so it no longer shadows
variable `name` and also fixes the case where no node is actually found
in the scope.
  • Loading branch information
Kronuz committed Aug 7, 2020
1 parent 2e788a2 commit 3e66bdd
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
10 changes: 10 additions & 0 deletions libcst/codemod/commands/tests/test_remove_unused_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ def foo() -> None:

self.assertCodemod(before, after)

def test_enclosed_attributes(self) -> None:
before = """
from a.b import c
import x
def foo() -> None:
x.y(c.d()).z()
"""
self.assertCodemod(before, before)

def test_access_in_assignment(self) -> None:
before = """
from a import b
Expand Down
25 changes: 17 additions & 8 deletions libcst/metadata/scope_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ def __init__(self, provider: "ScopeProvider") -> None:
self.provider: ScopeProvider = provider
self.scope: Scope = GlobalScope()
self.__deferred_accesses: List[Tuple[Access, Optional[cst.Attribute]]] = []
self.__top_level_attribute: Optional[cst.Attribute] = None
self.__top_level_attribute_stack: List[Optional[cst.Attribute]] = [None]

@contextmanager
def _new_scope(
Expand Down Expand Up @@ -686,21 +686,29 @@ def visit_ImportFrom(self, node: cst.ImportFrom) -> Optional[bool]:
return self._visit_import_alike(node)

def visit_Attribute(self, node: cst.Attribute) -> Optional[bool]:
if self.__top_level_attribute is None:
self.__top_level_attribute = node
if self.__top_level_attribute_stack[-1] is None:
self.__top_level_attribute_stack[-1] = node
node.value.visit(self) # explicitly not visiting attr
if self.__top_level_attribute is node:
self.__top_level_attribute = None
if self.__top_level_attribute_stack[-1] is node:
self.__top_level_attribute_stack[-1] = None
return False

def visit_Call(self, node: cst.Call) -> Optional[bool]:
self.__top_level_attribute_stack.append(None)

def leave_Call(self, original_node: cst.Call) -> None:
self.__top_level_attribute_stack.pop()

def visit_Name(self, node: cst.Name) -> Optional[bool]:
# not all Name have ExpressionContext
context = self.provider.get_metadata(ExpressionContextProvider, node, None)
if context == ExpressionContext.STORE:
self.scope.record_assignment(node.value, node)
elif context in (ExpressionContext.LOAD, ExpressionContext.DEL):
access = Access(node, self.scope)
self.__deferred_accesses.append((access, self.__top_level_attribute))
self.__deferred_accesses.append(
(access, self.__top_level_attribute_stack[-1])
)

def visit_FunctionDef(self, node: cst.FunctionDef) -> Optional[bool]:
self.scope.record_assignment(node.name.value, node)
Expand Down Expand Up @@ -842,9 +850,10 @@ def infer_accesses(self) -> None:
if enclosing_attribute is not None:
# if _gen_dotted_names doesn't generate any values, fall back to
# the original name node above
for name, node in _gen_dotted_names(enclosing_attribute):
if name in access.scope:
for attr_name, node in _gen_dotted_names(enclosing_attribute):
if attr_name in access.scope:
access.node = node
name = attr_name
break

scope_name_accesses[(access.scope, name)].add(access)
Expand Down

0 comments on commit 3e66bdd

Please sign in to comment.