Fix enclosing attribute for attributes in call arguments

[Kronuz]

Summary

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.

Test Plan

Run tests

[jimmylai]

@zsol for the failed test case, looks like we intended to remove the x.y and keep x.y.z.
If that's the case, then this is not a bug. Can you confirm?

LibCST/libcst/codemod/commands/tests/test_remove_unused_imports.py

Lines 51 to 63 in 6a5e739

	def test_dotted_imports(self) -> None:
	before = """
	import a.b, a.b.c
	import e.f
	import g.h
	import x.y, x.y.z
	def foo() -> None:
	a.b
	e.g
	g.h.i
	x.y.z
	"""

@Kronuz what's your use case that current code doesn't work?

In general, shadow a name in a loop is confusing and may cause bug. If it's possible, we should develop a lint rule to detect it.

[Kronuz]

I added a test which fails without this:

LibCST/libcst/codemod/commands/tests/test_remove_unused_imports.py

Lines 80 to 88 in 1c75064

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

I'm thinking the real reason could also be the incorrect detection of __top_level_attribute for c, detecting d.x as "enclosing_attribute", and ultimately leading to lose c's access when name gets reassigned from c to d. I'll dig into this hypothesis.

[zsol]

Yes that name shadowing was intentional. The first name assignment is the default, and then the loop is there to find a better, more specific name.

[jimmylai]

y is an undefined name. Shall we just update it to make it valid use case?

[jimmylai]

@Kronuz, when @zsol added the variable name shadow, he tries to remove the x.y and keep x.y.z, that means for Attribute access x.y.z we want to associate it with x.y.z rather than x.y or both. Can you keep this test work while making it also work for your new test case?

LibCST/libcst/codemod/commands/tests/test_remove_unused_imports.py

Lines 51 to 63 in 6a5e739

	def test_dotted_imports(self) -> None:
	before = """
	import a.b, a.b.c
	import e.f
	import g.h
	import x.y, x.y.z
	def foo() -> None:
	a.b
	e.g
	g.h.i
	x.y.z
	"""

[Kronuz]

@jimmylai, I think I know what the real problem is. I’ll try to fix it!

[Kronuz]

@jimmylai, fixed!

[jimmylai]

@Kronuz the fix broke a test for rename codemod.
https://app.circleci.com/pipelines/github/Instagram/LibCST/1309/workflows/5b05474b-e290-4de4-9de2-886cd5d911c9/jobs/8019

LibCST/libcst/codemod/commands/tests/test_rename.py

Lines 52 to 67 in 030df06

	def test_rename_repeated_name_with_asname(self) -> None:
	before = """
	from foo import foo as bla
	def test() -> None:
	bla.bla(5)
	"""
	after = """
	from baz import qux
	def test() -> None:
	qux.bla(5)
	"""
	self.assertCodemod(
	before, after, old_name="foo.foo", new_name="baz.qux",
	)

You can run tox -e py37 to run all tests locally before submit changes to save you some time.
There is also a formatting error and you can run tox -e autofix to fix it.

[Kronuz]

You can run tox -e py37 to run all tests locally before submit changes to save you some time.
There is also a formatting error and you can run tox -e autofix to fix it.

Done! sorry about that :P

These fixes will be needed for the linter to work.

[codecov-commenter]

Codecov Report

Merging #362 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #362   +/-   ##
=======================================
  Coverage   94.05%   94.05%           
=======================================
  Files         231      231           
  Lines       22093    22101    +8     
=======================================
+ Hits        20779    20787    +8     
  Misses       1314     1314           

Impacted Files	Coverage Δ
...demod/commands/tests/test_remove_unused_imports.py	100.00% <100.00%> (ø)
libcst/metadata/scope_provider.py	96.78% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e788a2...3e66bdd. Read the comment docs.

[zsol]

LGTM!