From bb1f11550377fdbd2e2b848f540302a3b7ec468a Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Thu, 18 Nov 2021 18:03:54 +0000 Subject: [PATCH 1/6] Add ImportAssignment class and record it from Scope --- libcst/metadata/scope_provider.py | 34 +++++++++++++++- libcst/metadata/tests/test_scope_provider.py | 43 ++++++++++++++++---- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/libcst/metadata/scope_provider.py b/libcst/metadata/scope_provider.py index f69974a7e..87c7ce406 100644 --- a/libcst/metadata/scope_provider.py +++ b/libcst/metadata/scope_provider.py @@ -208,6 +208,23 @@ class BuiltinAssignment(BaseAssignment): pass +class ImportAssignment(Assignment): + """An assignment records the import node and it's alias""" + + as_name: cst.CSTNode + + def __init__( + self, + name: str, + scope: "Scope", + node: cst.CSTNode, + index: int, + as_name: cst.CSTNode, + ): + super().__init__(name, scope, node, index) + self.as_name = as_name + + class Assignments: """A container to provide all assignments in a scope.""" @@ -405,6 +422,19 @@ def record_assignment(self, name: str, node: cst.CSTNode) -> None: Assignment(name=name, scope=self, node=node, index=self._assignment_count) ) + def record_import_assignment( + self, name: str, node: cst.CSTNode, as_name: cst.CSTNode + ) -> None: + self._assignments[name].add( + ImportAssignment( + name=name, + scope=self, + node=node, + as_name=as_name, + index=self._assignment_count, + ) + ) + def record_access(self, name: str, access: Access) -> None: self._accesses[name].add(access) @@ -826,11 +856,13 @@ def _visit_import_alike(self, node: Union[cst.Import, cst.ImportFrom]) -> bool: asname = name.asname if asname is not None: name_values = _gen_dotted_names(cst.ensure_type(asname.name, cst.Name)) + import_node_asname = asname.name else: name_values = _gen_dotted_names(name.name) + import_node_asname = name.name for name_value, _ in name_values: - self.scope.record_assignment(name_value, node) + self.scope.record_import_assignment(name_value, node, import_node_asname) return False def visit_Import(self, node: cst.Import) -> Optional[bool]: diff --git a/libcst/metadata/tests/test_scope_provider.py b/libcst/metadata/tests/test_scope_provider.py index 56dc0853d..11b4ade58 100644 --- a/libcst/metadata/tests/test_scope_provider.py +++ b/libcst/metadata/tests/test_scope_provider.py @@ -14,6 +14,7 @@ from libcst.metadata import MetadataWrapper from libcst.metadata.scope_provider import ( Assignment, + ImportAssignment, BuiltinAssignment, BuiltinScope, ClassScope, @@ -192,17 +193,24 @@ def test_import(self) -> None: len(scope_of_module[in_scope]), 1, f"{in_scope} should be in scope." ) - assignment = cast(Assignment, list(scope_of_module[in_scope])[0]) + assignment = cast(ImportAssignment, list(scope_of_module[in_scope])[0]) self.assertEqual( assignment.name, in_scope, - f"Assignment name {assignment.name} should equal to {in_scope}.", + f"ImportAssignment name {assignment.name} should equal to {in_scope}.", ) import_node = ensure_type(m.body[idx], cst.SimpleStatementLine).body[0] self.assertEqual( assignment.node, import_node, - f"The node of Assignment {assignment.node} should equal to {import_node}", + f"The node of ImportAssignment {assignment.node} should equal to {import_node}", + ) + alias = import_node.names[0] + as_name = alias.asname.name if alias.asname else alias.name + self.assertEqual( + assignment.as_name, + as_name, + f"The alias name of ImportAssignment {assignment.as_name} should equal to {as_name}", ) def test_dotted_import_access(self) -> None: @@ -221,7 +229,7 @@ def test_dotted_import_access(self) -> None: self.assertTrue("a" in scope_of_module) self.assertEqual(scope_of_module.accesses["a"], set()) - a_b_c_assignment = cast(Assignment, list(scope_of_module["a.b.c"])[0]) + a_b_c_assignment = cast(ImportAssignment, list(scope_of_module["a.b.c"])[0]) a_b_c_access = list(a_b_c_assignment.references)[0] self.assertEqual(scope_of_module.accesses["a.b.c"], {a_b_c_access}) self.assertEqual(a_b_c_access.node, call.func) @@ -261,7 +269,9 @@ def test_dotted_import_with_call_access(self) -> None: self.assertTrue("os.path" in scope_of_module) self.assertTrue("os" in scope_of_module) - os_path_join_assignment = cast(Assignment, list(scope_of_module["os.path"])[0]) + os_path_join_assignment = cast( + ImportAssignment, list(scope_of_module["os.path"])[0] + ) os_path_join_assignment_references = list(os_path_join_assignment.references) self.assertNotEqual(len(os_path_join_assignment_references), 0) os_path_join_access = os_path_join_assignment_references[0] @@ -289,21 +299,36 @@ def test_import_from(self) -> None: for alias in import_aliases: self.assertEqual(scopes[alias], scope_of_module) - for idx, in_scope in [(0, "a"), (0, "b_renamed"), (1, "c"), (2, "d")]: + for idx, in_scope, imported_object_idx in [ + (0, "a", 0), + (0, "b_renamed", 1), + (1, "c", 0), + (2, "d", 0), + ]: self.assertEqual( len(scope_of_module[in_scope]), 1, f"{in_scope} should be in scope." ) - import_assignment = cast(Assignment, list(scope_of_module[in_scope])[0]) + import_assignment = cast( + ImportAssignment, list(scope_of_module[in_scope])[0] + ) self.assertEqual( import_assignment.name, in_scope, - f"The name of Assignment {import_assignment.name} should equal to {in_scope}.", + f"The name of ImportAssignment {import_assignment.name} should equal to {in_scope}.", ) import_node = ensure_type(m.body[idx], cst.SimpleStatementLine).body[0] self.assertEqual( import_assignment.node, import_node, - f"The node of Assignment {import_assignment.node} should equal to {import_node}", + f"The node of ImportAssignment {import_assignment.node} should equal to {import_node}", + ) + + alias = import_node.names[imported_object_idx] + as_name = alias.asname.name if alias.asname else alias.name + self.assertEqual( + import_assignment.as_name, + as_name, + f"The alias name of ImportAssignment {import_assignment.as_name} should equal to {as_name}", ) for not_in_scope in ["foo", "bar", "foo.bar", "b"]: From 5a08e55b0fe8542013902f545dbd28471b34c65e Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Thu, 18 Nov 2021 19:16:38 +0000 Subject: [PATCH 2/6] Add overrides for LocalScope and ClassScope --- libcst/metadata/scope_provider.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/libcst/metadata/scope_provider.py b/libcst/metadata/scope_provider.py index 87c7ce406..ee6106609 100644 --- a/libcst/metadata/scope_provider.py +++ b/libcst/metadata/scope_provider.py @@ -450,6 +450,12 @@ def _record_assignment_as_parent(self, name: str, node: cst.CSTNode) -> None: """Overridden by ClassScope to forward 'nonlocal' assignments from child scopes.""" self.record_assignment(name, node) + def _record_import_assignment_as_parent( + self, name: str, node: cst.CSTNode, as_name: cst.CSTNode + ) -> None: + """Overridden by ClassScope to forward 'nonlocal' assignments from child scopes.""" + self.record_import_assignment(name, node, as_name) + @abc.abstractmethod def __contains__(self, name: str) -> bool: """Check if the name str exist in current scope by ``name in scope``.""" @@ -670,6 +676,16 @@ def record_assignment(self, name: str, node: cst.CSTNode) -> None: else: super().record_assignment(name, node) + def record_import_assignment( + self, name: str, node: cst.CSTNode, as_name: cst.CSTNode + ) -> None: + if name in self._scope_overwrites: + self._scope_overwrites[name]._record_import_assignment_as_parent( + name, node, as_name + ) + else: + super().record_import_assignment(name, node, as_name) + def __contains__(self, name: str) -> bool: if name in self._scope_overwrites: return name in self._scope_overwrites[name] @@ -718,6 +734,11 @@ def inner_fn(): """ self.parent._record_assignment_as_parent(name, node) + def _record_import_assignment_as_parent( + self, name: str, node: cst.CSTNode, as_name: cst.CSTNode + ) -> None: + self.parent._record_import_assignment_as_parent(name, node, as_name) + def _getitem_from_self_or_parent(self, name: str) -> Set[BaseAssignment]: """ Class variables are only accessible using ClassName.attribute, cls.attribute, or @@ -862,7 +883,9 @@ def _visit_import_alike(self, node: Union[cst.Import, cst.ImportFrom]) -> bool: import_node_asname = name.name for name_value, _ in name_values: - self.scope.record_import_assignment(name_value, node, import_node_asname) + self.scope.record_import_assignment( + name_value, node, import_node_asname + ) return False def visit_Import(self, node: cst.Import) -> Optional[bool]: From 51a33c67f439cd6e32dfe081a84765fc9bcf1de1 Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Thu, 18 Nov 2021 19:46:21 +0000 Subject: [PATCH 3/6] Clean scope_provider code and use ImportAssignment class in unusued_imports codemod --- .../visitors/_gather_unused_imports.py | 3 +- libcst/metadata/__init__.py | 3 +- libcst/metadata/scope_provider.py | 48 +++++++------------ 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/libcst/codemod/visitors/_gather_unused_imports.py b/libcst/codemod/visitors/_gather_unused_imports.py index 89f378447..2dc439ac3 100644 --- a/libcst/codemod/visitors/_gather_unused_imports.py +++ b/libcst/codemod/visitors/_gather_unused_imports.py @@ -134,8 +134,7 @@ def is_in_use(self, scope: cst.metadata.Scope, alias: cst.ImportAlias) -> bool: for assignment in scope[name_or_alias]: if ( - isinstance(assignment, cst.metadata.Assignment) - and isinstance(assignment.node, (cst.ImportFrom, cst.Import)) + isinstance(assignment, cst.metadata.ImportAssignment) and len(assignment.references) > 0 ): return True diff --git a/libcst/metadata/__init__.py b/libcst/metadata/__init__.py index 55a48cb5d..c94dd8f68 100644 --- a/libcst/metadata/__init__.py +++ b/libcst/metadata/__init__.py @@ -41,6 +41,7 @@ ComprehensionScope, FunctionScope, GlobalScope, + ImportAssignment, QualifiedName, QualifiedNameSource, Scope, @@ -63,7 +64,7 @@ "BaseAssignment", "Assignment", "BuiltinAssignment", - "BuiltinScope", + "ImportAssignment" "BuiltinScope", "Access", "Scope", "GlobalScope", diff --git a/libcst/metadata/scope_provider.py b/libcst/metadata/scope_provider.py index ee6106609..63fdf7361 100644 --- a/libcst/metadata/scope_provider.py +++ b/libcst/metadata/scope_provider.py @@ -418,14 +418,14 @@ def __init__(self, parent: "Scope") -> None: self._assignment_count = 0 def record_assignment(self, name: str, node: cst.CSTNode) -> None: - self._assignments[name].add( + self._add_assignment( Assignment(name=name, scope=self, node=node, index=self._assignment_count) ) def record_import_assignment( self, name: str, node: cst.CSTNode, as_name: cst.CSTNode ) -> None: - self._assignments[name].add( + self._add_assignment( ImportAssignment( name=name, scope=self, @@ -446,15 +446,12 @@ def _contains_in_self_or_parent(self, name: str) -> bool: """Overridden by ClassScope to hide it's assignments from child scopes.""" return name in self - def _record_assignment_as_parent(self, name: str, node: cst.CSTNode) -> None: - """Overridden by ClassScope to forward 'nonlocal' assignments from child scopes.""" - self.record_assignment(name, node) + def _add_assignment(self, assignment: "BaseAssignment") -> None: + self._assignments[assignment.name].add(assignment) - def _record_import_assignment_as_parent( - self, name: str, node: cst.CSTNode, as_name: cst.CSTNode - ) -> None: + def _add_assignment_as_parent(self, assignment): """Overridden by ClassScope to forward 'nonlocal' assignments from child scopes.""" - self.record_import_assignment(name, node, as_name) + self._add_assignment(assignment) @abc.abstractmethod def __contains__(self, name: str) -> bool: @@ -611,15 +608,15 @@ def __getitem__(self, name: str) -> Set[BaseAssignment]: return self._assignments[name] return set() - def record_assignment(self, name: str, node: cst.CSTNode) -> None: - raise NotImplementedError("assignments in builtin scope are not allowed") - def record_global_overwrite(self, name: str) -> None: raise NotImplementedError("global overwrite in builtin scope are not allowed") def record_nonlocal_overwrite(self, name: str) -> None: raise NotImplementedError("declarations in builtin scope are not allowed") + def _add_assignment(self, assignment: "BaseAssignment") -> None: + raise NotImplementedError("assignments in builtin scope are not allowed") + class GlobalScope(Scope): """ @@ -670,21 +667,13 @@ def record_global_overwrite(self, name: str) -> None: def record_nonlocal_overwrite(self, name: str) -> None: self._scope_overwrites[name] = self.parent - def record_assignment(self, name: str, node: cst.CSTNode) -> None: - if name in self._scope_overwrites: - self._scope_overwrites[name]._record_assignment_as_parent(name, node) - else: - super().record_assignment(name, node) - - def record_import_assignment( - self, name: str, node: cst.CSTNode, as_name: cst.CSTNode - ) -> None: - if name in self._scope_overwrites: - self._scope_overwrites[name]._record_import_assignment_as_parent( - name, node, as_name + def _add_assignment(self, assignment: "BaseAssignment") -> None: + if assignment.name in self._scope_overwrites: + self._scope_overwrites[assignment.name]._add_assignment_as_parent( + assignment ) else: - super().record_import_assignment(name, node, as_name) + super()._add_assignment(assignment) def __contains__(self, name: str) -> bool: if name in self._scope_overwrites: @@ -717,7 +706,7 @@ class ClassScope(LocalScope): When a class is defined, it creates a ClassScope. """ - def _record_assignment_as_parent(self, name: str, node: cst.CSTNode) -> None: + def _add_assignment_as_parent(self, assignment: "BaseAssignment") -> None: """ Forward the assignment to parent. @@ -732,12 +721,7 @@ def inner_fn(): # hidden from its children. """ - self.parent._record_assignment_as_parent(name, node) - - def _record_import_assignment_as_parent( - self, name: str, node: cst.CSTNode, as_name: cst.CSTNode - ) -> None: - self.parent._record_import_assignment_as_parent(name, node, as_name) + self.parent._add_assignment_as_parent(assignment) def _getitem_from_self_or_parent(self, name: str) -> Set[BaseAssignment]: """ From 21d14714b34f654d0a1d3873ac48e9a5eb17ec2e Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Thu, 18 Nov 2021 19:50:03 +0000 Subject: [PATCH 4/6] fix typeo --- libcst/metadata/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libcst/metadata/__init__.py b/libcst/metadata/__init__.py index c94dd8f68..0603f0988 100644 --- a/libcst/metadata/__init__.py +++ b/libcst/metadata/__init__.py @@ -64,7 +64,8 @@ "BaseAssignment", "Assignment", "BuiltinAssignment", - "ImportAssignment" "BuiltinScope", + "ImportAssignment", + "BuiltinScope", "Access", "Scope", "GlobalScope", From aa71b08004601413893996e37259e9cd990c7359 Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Thu, 18 Nov 2021 21:03:20 +0000 Subject: [PATCH 5/6] Add missing types --- libcst/metadata/scope_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcst/metadata/scope_provider.py b/libcst/metadata/scope_provider.py index 63fdf7361..e996b18ea 100644 --- a/libcst/metadata/scope_provider.py +++ b/libcst/metadata/scope_provider.py @@ -449,7 +449,7 @@ def _contains_in_self_or_parent(self, name: str) -> bool: def _add_assignment(self, assignment: "BaseAssignment") -> None: self._assignments[assignment.name].add(assignment) - def _add_assignment_as_parent(self, assignment): + def _add_assignment_as_parent(self, assignment: "BaseAssignment") -> None: """Overridden by ClassScope to forward 'nonlocal' assignments from child scopes.""" self._add_assignment(assignment) From d29ab7ef11ba5875ffc90bb09b2c7472be2992f4 Mon Sep 17 00:00:00 2001 From: Giorgi Megreli Date: Fri, 19 Nov 2021 11:06:46 +0000 Subject: [PATCH 6/6] Fix fixit errors --- libcst/metadata/scope_provider.py | 1 + 1 file changed, 1 insertion(+) diff --git a/libcst/metadata/scope_provider.py b/libcst/metadata/scope_provider.py index e996b18ea..e3590f0ca 100644 --- a/libcst/metadata/scope_provider.py +++ b/libcst/metadata/scope_provider.py @@ -447,6 +447,7 @@ def _contains_in_self_or_parent(self, name: str) -> bool: return name in self def _add_assignment(self, assignment: "BaseAssignment") -> None: + assignment.scope = self self._assignments[assignment.name].add(assignment) def _add_assignment_as_parent(self, assignment: "BaseAssignment") -> None: