Add rewrite support for RemoveUnusedImports
#131611
Conversation
elasticsearchmachine
added
v9.2.0
external-contributor
|
@elastic/es-delivery Can you help here? |
tmp ignored with |
| id 'elasticsearch.eclipse' | ||
| id 'elasticsearch.versions' | ||
| id 'elasticsearch.formatting' | ||
| //id 'elasticsearch.rewrite' |
| It is usually more useful, and just as fast, to just reformat the project. You | ||
| can do this with: | ||
|
|
||
| ./gradlew spotlessApply |
| # log some dependency verification info to console | ||
| org.gradle.dependency.verification.console=verbose | ||
| # fixme undo lenient | ||
| org.gradle.dependency.verification=lenient |
There was a problem hiding this comment.
why does
gradle --write-verification-metadata sha256
not include dependencies ?
|
Pinging @elastic/es-delivery (Team:Delivery) |
|
Hey @Pankraz76, thank you this Pull Request. We currently dont use rewrite or openrewrite to manage our code quality and code quality checks. instead we rely on other tools as stated in the CONTRIBUTING.md. That said we won't merge the pull request as it breaks many of our workflows and how our process of writing and maintaining high quality code in this project. As you seem passionate about code quality feel free to raise a ticket to discuss open issues you might see in our setup. |
|
I have seen your stack its lacking automation and real capable tools like PMD or spotbugs. Rewrite goes beyond could even migrate the whole project to new java version. Automation is the only way on such scale like elastic is. If you want to fix potential security issues by using errorprone, let me know. |
|
Just see the 91 unused imports spotless missed. This tool is very limited as not type aware. It can not find methods fields or anything that rewrite can. So you stack tells that you going in a good direction where rewrite is going further, so its the natural evolvement no only to fix whitespaces but real bugs with automation. |
|
I know we have a gap to close in our build-tool and build-tools-internal projects. likely those are the projects where you find those imports. |
|
We are constantly revisiting our tooling but its always a trade of overhead and value we get out of it. We have Rewrite on our radar as I contributed to that project before and been close with the authors. It is great and we have used its great support for automatic code migration but at this point we have other priorities iin spending our time. |
its for free, as almost the same effort, as you know rewrite already. If we point out unused methods with PMD or rewrite is not big difference, assuming currently its not done. Assuming your code migration is not totally depending on deprecations, its likely incomplete/broken again, if not rewritten and checked constantly. No one prevents the old code pattern if the migration is done only once. |
|
As its not only about wildcards assuming security fixes done by file recipe are something important for elastic as well: |
With raising pull request like this against projects you haven't been involved with in the past, you need to understand that this stuff is definitely not free. It requires rework and adoption to our developer workflows, brings in significant maintenance costs and additional costs to our ci pipelines that are quite significant on the first glance. Anyhow. I would recommend before imvesting such effort in the future you raise those discussions either in a github issue or reach out on other channels. Trying to force a third party tool on a 120+ Developer team of this seniority level without willing to have a conversation first is usually not successful. Not at elasticsearch and likely not on other open source projects. As someone who seems to be passionate about openrewrite and eager about helping open source teams, I would focus first on revisiting projects that already use it and help them being more successful with this. |
Of course not—but costs aren’t decreasing either. If you want to avoid scrapping the software entirely, a rewrite is the most cost-effective solution to these issues. As for the unused imports, the system isn’t introducing new functionality; it’s just trying to complete unfinished work (which, realistically, is a perpetual task). This remains an ongoing effort. You emphasize high quality, yet multiple unresolved issues persist. For instance, Spotless fails because the convention-over-configuration principle was overlooked. And thats just the beginning. But its all good just wanted you inform you guys. Im not considering this project. |
4 participants
PoC for:
rewritesupport forerrorprone.refasterrules#132969