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.

Sign up for GitHub

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

Jump to bottom

support relative imports in AddImportsVisitor #578

Closed
martindemello opened this issue Jan 4, 2022 · 3 comments · Fixed by #585
Closed

support relative imports in AddImportsVisitor #578

martindemello opened this issue Jan 4, 2022 · 3 comments · Fixed by #585
Labels
enhancement New feature or request

Comments

@martindemello
Copy link
Contributor

The definition of an import is currently (module, obj, asname), leaving no way to handle relative imports in codemods that use the visitor.
@martindemello
Copy link
Contributor Author

also while looking at this, i feel like the code would be a lot easier to work with if the list of imports were converted from a tuple to a dataclass within the public methods, and then the dataclass was passed around internally. i've noticed that the libcst codebase makes heavy use of tuples as a general style - would a refactoring of this one visitor to use a dataclass be rejected on consistency grounds? if not i'll submit a PR for that first, and then add relative import support on top of it.

@martindemello
Copy link
Contributor Author

forgot to mention that i did find a workaround - manually adding dots to the module name before calling the visitor works, but it would be nicer to have it handle the relative level as part of the method call.

@zsol
Copy link
Member

zsol commented Jan 5, 2022

I'm in favor of refactoring that particular tuple to dataclasses. Happy to accept a PR for all the issues mentioned here.

@zsol zsol added the enhancement New feature or request label Jan 5, 2022
@martindemello martindemello mentioned this issue Jan 6, 2022
Merged
@zsol zsol closed this as completed in #585 Jan 8, 2022
zsol pushed a commit that referenced this issue Jan 8, 2022
@martindemello
Support relative imports in AddImportsVisitor. (#585)
8652974 
* Support relative imports in AddImportsVisitor.

* Adds an Import dataclass to represent a single imported object
* Refactors AddImportsVisitor to pass around Import objects
* Separates out the main logic in get_absolute_module_for_import so that
  it can be used to resolve relative module names outside of a cst.Import
  node
* Resolves relative module names in AddImportsVisitor if we have a
  current module name set.

Fixes #578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support relative imports in AddImportsVisitor.
2 participants
@martindemello @zsol