Skip to content

Commit

Permalink
fix RemoveImportsVisitor crash when ImportAlias is inserted without c…
Browse files Browse the repository at this point in the history
…omma (#397)

The comment preserving logic introduced in #392 assumed that in an ImportFrom node, ImportAliases have a comma property (except for the last one). That's only true if the ImportFrom node is parsed from actual source, but isn't necessarily true if it's constructed manually.
  • Loading branch information
zsol authored Oct 1, 2020
1 parent 34c1826 commit 1d22a29
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 6 deletions.
18 changes: 13 additions & 5 deletions libcst/codemod/visitors/_remove_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,12 +374,20 @@ def _process_importfrom_aliases(
if len(names_to_keep) != 0:
# there is a previous import alias
prev = names_to_keep[-1]
names_to_keep[-1] = prev.with_deep_changes(
whitespace_after=_merge_whitespace_after(
prev.comma.whitespace_after,
comma.whitespace_after,
if isinstance(prev.comma, cst.Comma):
prev = prev.with_deep_changes(
prev.comma,
whitespace_after=_merge_whitespace_after(
prev.comma.whitespace_after,
comma.whitespace_after,
),
)
)
else:
# The previous alias didn't have a trailing comma. This can
# occur if the alias was generated, instead of being parsed
# from source.
prev = prev.with_changes(comma=comma)
names_to_keep[-1] = prev
else:
# No previous import alias, need to attach comment to `ImportFrom`.
# We can only do this if there was a leftparen on the import
Expand Down
28 changes: 27 additions & 1 deletion libcst/codemod/visitors/tests/test_remove_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import libcst as cst
import libcst.matchers as m
from libcst.codemod import CodemodContext, CodemodTest, VisitorBasedCodemodCommand
from libcst.codemod.visitors import RemoveImportsVisitor
from libcst.codemod.visitors import AddImportsVisitor, RemoveImportsVisitor
from libcst.metadata import (
QualifiedName,
QualifiedNameProvider,
Expand Down Expand Up @@ -80,6 +80,12 @@ def test_remove_fromimport_keeping_standalone_comment(self) -> None:
short,
this_stays_too
)
from fourth import (
a,
# comment
b,
c
)
"""
after = """
from foo import (
Expand All @@ -92,6 +98,10 @@ def test_remove_fromimport_keeping_standalone_comment(self) -> None:
from third import (
this_stays_too
)
from fourth import (
a,
c
)
"""
self.assertCodemod(
before,
Expand All @@ -101,6 +111,7 @@ def test_remove_fromimport_keeping_standalone_comment(self) -> None:
("loooong", "short", None),
("loooong", "bar", None),
("third", "short", None),
("fourth", "b", None),
],
)

Expand Down Expand Up @@ -887,6 +898,21 @@ def visit_ImportFrom(self, node: cst.ImportFrom) -> None:
RemoveImportTransformer(CodemodContext()).transform_module(module).code,
)

def test_remove_import_alias_after_inserting(self) -> None:
before = "from foo import bar, baz"
after = "from foo import quux, baz"

class AddRemoveTransformer(VisitorBasedCodemodCommand):
def visit_Module(self, node: cst.Module) -> None:
AddImportsVisitor.add_needed_import(self.context, "foo", "quux")
RemoveImportsVisitor.remove_unused_import(self.context, "foo", "bar")

module = cst.parse_module(self.make_fixture_data(before))
self.assertCodeEqual(
AddRemoveTransformer(CodemodContext()).transform_module(module).code,
after,
)

def test_remove_comma(self) -> None:
"""
Trailing commas should be removed if and only if the last alias is removed.
Expand Down

0 comments on commit 1d22a29

Please sign in to comment.