Skip to content

Commit

Permalink
Fix ApplyTypeAnnotationsVisitor behavior on attribute assignments. (#903
Browse files Browse the repository at this point in the history
)

* Fixes an issue where ApplyTypeAnnotationsVisitor would crash on code
  like `SomeClass.some_attribute = 42` with a "Name is not a valid
  identifier" error message.
* Changes the above-mentioned error message to include the bad name in
  the message, for easier debugging.
* Adds tests for all valid assignment targets, as described here:
  https://libcst.readthedocs.io/en/latest/nodes.html#libcst.BaseAssignTargetExpression.
  • Loading branch information
rchen152 committed Apr 5, 2023
1 parent 4f810db commit f936db2
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 2 deletions.
2 changes: 1 addition & 1 deletion libcst/_nodes/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def _validate(self) -> None:
if len(self.value) == 0:
raise CSTValidationError("Cannot have empty name identifier.")
if not self.value.isidentifier():
raise CSTValidationError("Name is not a valid identifier.")
raise CSTValidationError(f"Name {self.value!r} is not a valid identifier.")

def _codegen_impl(self, state: CodegenState) -> None:
with self._parenthesize(state):
Expand Down
2 changes: 1 addition & 1 deletion libcst/codemod/visitors/_apply_type_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ def _annotate_single_target(
self.qualifier.append(name)
if (
self._qualifier_name() in self.annotations.attributes
and not isinstance(only_target, cst.Subscript)
and not isinstance(only_target, (cst.Attribute, cst.Subscript))
):
annotation = self.annotations.attributes[self._qualifier_name()]
self.qualifier.pop()
Expand Down
88 changes: 88 additions & 0 deletions libcst/codemod/visitors/tests/test_apply_type_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1745,3 +1745,91 @@ def test_signature_matching_with_always_qualify(
self.run_test_case_with_flags(
stub=stub, before=before, after=after, always_qualify_annotations=True
)

@data_provider(
{
"attribute": (
"""
class C:
x: int
""",
"""
class C:
x = 0
C.x = 1
""",
"""
class C:
x: int = 0
C.x = 1
""",
),
"subscript": (
"""
d: dict[str, int]
""",
"""
d = {}
d["k"] = 0
""",
"""
d: dict[str, int] = {}
d["k"] = 0
""",
),
"starred": (
"""
a: int
b: list[int]
""",
"""
a, *b = [1, 2, 3]
""",
"""
a: int
b: list[int]
a, *b = [1, 2, 3]
""",
),
"name": (
"""
a: int
""",
"""
a = 0
""",
"""
a: int = 0
""",
),
"list": (
"""
a: int
""",
"""
[a] = [0]
""",
"""
a: int
[a] = [0]
""",
),
"tuple": (
"""
a: int
""",
"""
(a,) = [0]
""",
"""
a: int
(a,) = [0]
""",
),
}
)
def test_valid_assign_expressions(self, stub: str, before: str, after: str) -> None:
self.run_simple_test_case(stub=stub, before=before, after=after)

0 comments on commit f936db2

Please sign in to comment.