Implement ImportManager for managing imports #5458
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
MBoegers
left a comment
MBoegers
left a comment
There was a problem hiding this comment.
Sorry for the long run.. Overall looks good, just some ideas around the namings and annotations
| import static org.openrewrite.Tree.randomId; | ||
| import static org.openrewrite.java.ImportManager.ImportStatus.*; | ||
|
|
||
| public class ImportManager { |
There was a problem hiding this comment.
Can we find a better name than Manager? I think of a manager like something that not only knows about something but also can alter them.
We ususally use the suffix Util for helper, but I get that it is not a classic Util as it maintains state.
Maybe ImportReport as it can answer questions?
There was a problem hiding this comment.
I agree with you. What about ImportAnalyzer?
| import static org.openrewrite.Tree.randomId; | ||
| import static org.openrewrite.java.ImportManager.ImportStatus.*; | ||
|
|
||
| public class ImportManager { |
There was a problem hiding this comment.
Would be final reasonable?
There was a problem hiding this comment.
IMO final could be optional considering it can be used for others JVM based languages with few modifications.
| IMPORT_AMBIGUITY, | ||
| } | ||
|
|
||
| @Getter |
| @Override | ||
| public J visitFieldAccess(J.FieldAccess fieldAccess, Set<JavaType.FullyQualified> types) { | ||
| if (fieldAccess.getType() instanceof JavaType.FullyQualified) { | ||
| if (fieldAccess.isFullyQualifiedClassReference(((JavaType.FullyQualified) fieldAccess.getType()).getFullyQualifiedName())) { |
There was a problem hiding this comment.
We haveTypeUtils.asFullyQualified defined which does the same.
| return identifier; | ||
| } | ||
| JavaType.Variable variable = identifier.getFieldType(); | ||
| if (variable != null && variable.getOwner() instanceof JavaType.Class && variable.hasFlags(Flag.Static)) { |
There was a problem hiding this comment.
You have ImportManager.isStaticClassVariable() which does the same.
github-project-automation
bot
moved this from Ready to Review
to In Progress
in OpenRewrite
|
@MBoegers incorporated feedback. PTAL |
|
This PR is stale because it has been open for 90 days with no activity. Remove |
|
@timtebeek @MBoegers Got a stale notification for this pr. I believe I have already incorporated all feedbacks. PTAL |
3 participants
What's changed?
Introduced the ImportManager to handle import management. It provides APIs to:
This utility can be leveraged by existing visitors AddImport and RemoveImport, and we can also create a new visitor (e.g., FixImport, see #5435) to automatically add or remove imports based on usage. We can also build recipes like RemoveUnusedImport on top of this.
Additionally, ImportManager can support APIs to determine whether a list of imports can be safely folded or to unfold wildcard imports. Currently not implemented but could be easily built on top of the existing structure.
What's your motivation?
While working on #5439, I noticed that AddImport misses some edge cases. I attempted to address import ambiguity, but the solution still doesn't cover all scenarios. This utility class provides a centralized and robust solution to handle all such cases.
Additional context
Checklist