Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - Remove comment and white space lines #100

Open
Tracked by #272
hakancelikdev opened this issue Sep 12, 2020 · 5 comments · May be fixed by #257
Open
Tracked by #272

[BUG] - Remove comment and white space lines #100

hakancelikdev opened this issue Sep 12, 2020 · 5 comments · May be fixed by #257
Assignees
Labels
changelog Changes should be written to the changelog file. enhancement New feature or request test
Milestone

Comments

@hakancelikdev
Copy link
Owner

Code

# Python imports
import os

from typing import Union

# Local imports
from .pgen2 import token
from .pgen2 import driver

from .pgen2.grammar import Grammar
os, Union, driver, Grammar

Expected situation

 import os

 from typing import Union

# Local imports
-from .pgen2 import token
 from .pgen2 import driver

 from .pgen2.grammar import Grammar

os, Union, driver, Grammar

Actual situation

 import os

 from typing import Union
-
-# Local imports
-from .pgen2 import token
 from .pgen2 import driver

 from .pgen2.grammar import Grammar
os, Union, driver, Grammar
@hakancelikdev hakancelikdev added changelog Changes should be written to the changelog file. needs test labels Sep 12, 2020
@hadialqattan
Copy link
Contributor

hadialqattan commented Sep 14, 2020

I did some code debugging, I ended with a bug on LibCST.RemoveFromParent() function!

from typing import Union

import libcst as cst


class RemoveUnusedImportTransformer(cst.CSTTransformer):
    def leave_ImportFrom(
        self, original_node: cst.ImportFrom, updated_node: cst.ImportFrom
    ) -> Union[cst.RemovalSentinel, cst.ImportFrom]:
        if updated_node.names[0].name.value == "token":  # Hard coded name.
            return cst.RemoveFromParent()  # Here we have a bug!
        return updated_node


def refactor_string(source: str) -> str:
    transformer = RemoveUnusedImportTransformer()
    cst_tree = cst.parse_module(source)
    return cst_tree.visit(transformer).code


if __name__ == "__main__":
    refactored_code = refactor_string(
        """# Python imports
import os

from typing import Union

# Local imports
from .pgen2 import token
from .pgen2 import driver

from .pgen2.grammar import Grammar

os, Union, driver, Grammar"""
    )
    print(refactored_code)

Output:

# Python imports
import os

from typing import Union
from .pgen2 import driver

from .pgen2.grammar import Grammar

os, Union, driver, Grammar

@hadialqattan
Copy link
Contributor

If that's the case, I'm gonna open an issue on the LibCST repo.

@hakancelikdev
Copy link
Owner Author

If that's the case, I'm gonna open an issue on the LibCST repo.

Yes I think the issue need to open.

@hadialqattan
Copy link
Contributor

Please take a look at Instagram/LibCST#390

@hadialqattan
Copy link
Contributor

hadialqattan commented Sep 14, 2020

I think we should implement a method that preserves node comment/empty-line from removal!

This was referenced Dec 5, 2022
@hakancelikdev hakancelikdev unpinned this issue Dec 5, 2022
@hakancelikdev hakancelikdev moved this from 🏗 In progress to 🔖 Ready in unimport's backlog Dec 5, 2022
@hakancelikdev hakancelikdev moved this from 🔖 Ready to 🆕 New in unimport's backlog Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog Changes should be written to the changelog file. enhancement New feature or request test
Projects
Status: 🆕 New
Development

Successfully merging a pull request may close this issue.

2 participants