Issue #17487: Add rewrite support for errorprone.refasterrules
#17490
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Pankraz76
force-pushed
the
openrewrite
branch
9 times, most recently
from
2aca6b4 to
e6e7b4a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Pankraz76
force-pushed
the
openrewrite
branch
3 times, most recently
from
af8fb66 to
78b6650
Compare
Pankraz76
changed the title
rewrite support for errorprone.refasterrulesrewrite support for errorprone.refasterrules
Pankraz76
force-pushed
the
openrewrite
branch
4 times, most recently
from
6f91533 to
bf4d80f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Pankraz76
force-pushed
the
openrewrite
branch
4 times, most recently
from
163e83b to
24d6679
Compare
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
It's been awhile since I looked at checkstyle, so my recollections might be out of date but I think of it as very focused tool for finding code style issues exclusively. Two things I don't think it does and would be surprised to find out it does:
- Change the code, at all.
- Find non-style issues.
Checkstyle's purpose is to generate warnings about style issues, nothing more.
That is, I don't want it to be spotbugs or pmd or sptoless. These are separate tools and they work better independently. I prefer the unix-like approach of combining lots of small utilities rather than packaging everything into a single all-purpose-tool.
|
Thank you for reaching out. This focuses on the internal code for the check itself. The changes won’t alter the check’s external behavior—only improve development performance and quality by helping identify and fix bugs/flaws. I’ve already had to resolved 70 check-related issues manually, when contributing, which is the nature of check, we all know about. To clarify, this change won’t be exposed in the delivered Let me know if you have questions or need further details. Best regards |
|
Is this only affecting the source code in this repo? Or is it changing the code of projects that use checkstyle? I'd rather not have this tool changing any code. I want it to report problems. I specifically want it to not attempt to fix them. |
we also not changing PMD or kafka, by using auto correction. Its just another static code analysis tool, finally going beyond pointing out, but really fixing the issues. Increasing dev time, reducing styling efforts. |
this would be for some ppl. quite nice, idk if its even possible, unless customers use checkstyle pom as parent. You asking for this: But no, don´t worry. Its, likely, not going to happen at all, but (assuming) definitely not through check and/or with this PR. |
…RulesRecipes`
…RulesRecipes`
Pankraz76
changed the title
rewrite support for errorprone.refasterrulesrewrite support for errorprone.refasterrules
|
Kindly consider dedicated recipe changes. Some even dont find anything, which is the overall goal of currently approached analysis. |
|
Not sure how we end up in this conversation
Checkstyle is far above just style. But by name we have bunch for style. Overlap with PMD is significant. Other tools les in overlap. We keep modules on whatever users like to validate.
We develop https://github.com/checkstyle/checkstyle-openrewrite-recipes it will help users who want all to be automated. It will be just another tool that user should manually plug in build to do a work. |
this is already "done" by the cleanup recipe. It can parse the check config and apply accordingly. But its not ready yet. Have not seen it being complain, when applied, once. Maybe due to my mistake or its some kind of bug.
might be an easy approach for PMD as well. |
|
from @Anmol202005: please abandon dryrun, it does not work for us. and use approach I suggested above to just check diff after CI command execution , most CI already have it. |
considering dedicated test run on CI comparing both, its seems to be quite a similar result as expected, due to the same workload. This makes the extra configuration questionable. Its "just" cosmetics, how you want to handle it is up to you.
Might want to go (c)lean here consider to convent over config principle. |
|
closing to stop starting start finishing. |
The reality is that Spotless automates code formatting checks, making tools like Checkstyle painless to use. Since it automatically fixes violations, it becomes a seamless part of the development workflow (e.g., in Maven you committed to). The hesitation is confusing when you compare it to SpotBugs or PMD. The critical difference is that those tools only identify issues—they don't automatically fix them. Spotless removes the manual effort entirely, which is generally a good thing considering maven parent. |
5 participants
#17487
Add
rewritesupport forerrorprone.refasterrulesI’d like to propose integrating Google’s Error Prone and its Picnic extension (demonstrated in Automating Away Bugs with Error Prone | PlatformCon 2023) to enable automated bug fixes via
rewriterules. This would complement Checkstyle’s static analysis capabilities by addressing semantic bugs rather than stylistic issues.Motivation
Error Prone’s
refaster/rewriterules can automatically:String.equals()misuse)Real-world adoptions show tangible benefits:
Proposal
errorprone.refasterrules-based rewritesDiscussion Points
Next Steps
I’m happy to: