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

Add recepie RemoveRedundantImports #5052

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

punkratz312
Copy link

Add new recepie RemoveRedundantImports to fix: https://checkstyle.sourceforge.io/checks/imports/redundantimport.html

@punkratz312 punkratz312 marked this pull request as ready for review February 18, 2025 09:06
@punkratz312
Copy link
Author

@timtebeek is this any good?

@timtebeek
Copy link
Contributor

Thanks for kicking this off @punkratz312 ! Couple quick immediate thoughts:

  • we might need to add a precondition to skip languages where one might add an alias for an import, or restrict to known languages. We have such preconditions in rewrite-static-analysis; to be determined if we move those here, or move this recipe there
  • it'd be interesting to see if our OrderImport recipe already supports this, or could be extended to support this. I think it might make sense to have this functionality there as opposed to a recipe that folks have to deliberately run separately. What are your thoughts on that?
  • the recipe file header and class javadoc appear outdated copies. Perhaps good to clear that up if we do continue with this PR.
  • In terms of implementation it might make sense to use ListUtils.map or ListUtils.filter instead of the usage of the Streams API

Let's discuss before making further changes just so we're on the same page :)

@punkratz312
Copy link
Author

punkratz312 commented Feb 18, 2025

Yes, indeed, it's actually working. I've double-checked it with both positive and negative tests.

Of course, it's very much related, so it's justifiable to have this together.

This dedicated approach would only make sense if there is a need or desire to address only the DRY violations and not to change the entire import tree, potentially due to ordering the imports as well.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants