Skip to content

Commit

Permalink
Cache the scope name prefix to prevent scope traversal in a tight loop (
Browse files Browse the repository at this point in the history
#708)

* Cache the scope name prefix to prevent scope traversal in a tight loop

* Adding pyre-fixme. this attribute iclearly has a type in the base class.

* Clarify why we do join(filter(None,...
  • Loading branch information
lpetre committed Jun 21, 2022
1 parent 306a5f8 commit 42164f8
Showing 1 changed file with 24 additions and 19 deletions.
43 changes: 24 additions & 19 deletions libcst/metadata/scope_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,24 +217,12 @@ def _index(self) -> int:
return self.__index

def get_qualified_names_for(self, full_name: str) -> Set[QualifiedName]:
scope = self.scope
name_prefixes = []
while scope:
if isinstance(scope, ClassScope):
name_prefixes.append(scope.name)
elif isinstance(scope, FunctionScope):
name_prefixes.append(f"{scope.name}.<locals>")
elif isinstance(scope, ComprehensionScope):
name_prefixes.append("<comprehension>")
elif not isinstance(scope, (GlobalScope, BuiltinScope)):
raise Exception(f"Unexpected Scope: {scope}")

scope = scope.parent if scope.parent != scope else None

parts = [*reversed(name_prefixes)]
if full_name:
parts.append(full_name)
return {QualifiedName(".".join(parts), QualifiedNameSource.LOCAL)}
return {
QualifiedName(
self.scope._maybe_dotted_name(full_name),
QualifiedNameSource.LOCAL,
)
}


# even though we don't override the constructor.
Expand Down Expand Up @@ -409,6 +397,7 @@ class Scope(abc.ABC):
_assignment_count: int
_accesses_by_name: MutableMapping[str, Set[Access]]
_accesses_by_node: MutableMapping[cst.CSTNode, Set[Access]]
_name_prefix: str

def __init__(self, parent: "Scope") -> None:
super().__init__()
Expand All @@ -418,6 +407,7 @@ def __init__(self, parent: "Scope") -> None:
self._assignment_count = 0
self._accesses_by_name = defaultdict(set)
self._accesses_by_node = defaultdict(set)
self._name_prefix = ""

def record_assignment(self, name: str, node: cst.CSTNode) -> None:
target = self._find_assignment_target(name)
Expand Down Expand Up @@ -591,6 +581,11 @@ def accesses(self) -> Accesses:
"""Return an :class:`~libcst.metadata.Accesses` contains all accesses in current scope."""
return Accesses(self._accesses_by_name)

# makes a dot separated name but filters out empty strings
def _maybe_dotted_name(self, *args: Optional[str]) -> str:
# filter(None, ...) removes all falsey values (ie empty string)
return ".".join(filter(None, [self._name_prefix, *args]))


class BuiltinScope(Scope):
"""
Expand Down Expand Up @@ -667,6 +662,8 @@ def __init__(
self.name = name
self.node = node
self._scope_overwrites = {}
# pyre-fixme[4]: Attribute `_name_prefix` of class `LocalScope` has type `str` but no type is specified.
self._name_prefix = self._make_name_prefix()

def record_global_overwrite(self, name: str) -> None:
self._scope_overwrites[name] = self.globals
Expand Down Expand Up @@ -695,6 +692,9 @@ def __getitem__(self, name: str) -> Set[BaseAssignment]:
else:
return self.parent._getitem_from_self_or_parent(name)

def _make_name_prefix(self) -> str:
return self.parent._maybe_dotted_name(self.name, "<locals>")


# even though we don't override the constructor.
class FunctionScope(LocalScope):
Expand Down Expand Up @@ -741,6 +741,9 @@ def _contains_in_self_or_parent(self, name: str) -> bool:
"""
return self.parent._contains_in_self_or_parent(name)

def _make_name_prefix(self) -> str:
return self.parent._maybe_dotted_name(self.name)


# even though we don't override the constructor.
class ComprehensionScope(LocalScope):
Expand All @@ -755,7 +758,9 @@ class ComprehensionScope(LocalScope):
# TODO: Assignment expressions (Python 3.8) will complicate ComprehensionScopes,
# and will require us to handle such assignments as non-local.
# https://www.python.org/dev/peps/pep-0572/#scope-of-the-target
pass

def _make_name_prefix(self) -> str:
return self.parent._maybe_dotted_name("<comprehension>")


# Generates dotted names from an Attribute or Name node:
Expand Down

0 comments on commit 42164f8

Please sign in to comment.