Names in subscript expressions have the wrong expression context

[zsol]

I stumbled on this by running RemoveImportsVisitor on a large codebase and saw that
RemoveImportsVisitor incorrectly removes imports corresponding to some symbols used in assignments.

Example repro:

from a import b
b[0] = False

[zsol]

Looks like this is because in the above example, b[0] is used in ExpressionContext.STORE, so it's recorded as an assignment in ScopeProvider. Technically, b is not being assigned to, so that's incorrect. Consider also

from a import identity
x = [1]
identity(x)[0] = 2

Currently ExpressionContextProvider returns LOAD for the name identity. That's the source of this bug.

I think ExpressionContextProvider should only return STORE for Name nodes inside a Subscript expression. @jimmylai any thoughts on this?

[zsol]

To clarify, this line tests a buggy behavior, it should verify that a has a LOAD context:
https://github.com/Instagram/LibCST/blob/master/libcst/metadata/tests/test_expression_context_provider.py#L151

[zsol]

Looks like the ast module already does this:

>>> import ast
>>> m = ast.parse("from a import b; b[0] = False")
>>> m.body[1].targets[0].value.ctx
<_ast.Load object at 0x7f25ce22a3d0>

[jimmylai]

Great finding! I didn't carefully make it right for subscript name when building ExpressionContext . Thanks for reporting this and fix it.