Add `rewrite` support for `errorprone.refasterrules`

[Pankraz76]

1. What are you trying to do?

Add rewrite support for errorprone.refasterrules

I’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 rewrite rules. This would complement Checkstyle’s static analysis capabilities by addressing semantic bugs rather than stylistic issues.

Motivation

Error Prone’s refaster/rewrite rules can automatically:

• Fix common bug patterns (e.g., String.equals() misuse)
• Modernize code (e.g., JDK migration helpers)
• Enforce best practices (e.g., null-check improvements)

Real-world adoptions show tangible benefits:

• Issue #17487: Introduce Rewrite covering S1144: Unused "private" methods should be removed checkstyle/checkstyle#17545
• Fix #5953: [java] PoC: Add rewrite support for errorprone.refasterrules pmd/pmd#5956
• Introduce Rewrite & PMD covering S1144: Unused "private" methods should be removed opensearch-project/OpenSearch#18791
• PoC: Add rewrite support for errorprone.refasterrules diffplug/spotless#2576
• PoC: Add rewrite support for errorprone.refasterrules apache/kafka#20219

Proposal

1. Add support for errorprone.refasterrules-based rewrites
2. Implement with opt-in adoption (no breaking changes)
3. Include suppression mechanisms for API constraints

Discussion Points

• Need consensus on:
• Scope of auto-fixes
• Preferred suppression strategy
• Integration approach

Next Steps

I’m happy to:

• Prepare a PoC demonstrating the value
• Collaborate on implementation strategy
• Address any concerns about compatibility

relates to:

• implement IDE agnostic configuration with editorconfig.org checkstyle/checkstyle#16543

2. What's the best code you can write to accomplish that without the new feature?

• Add staticanalysis.UnnecessaryParentheses #7916

3. What would that same code look like if we added your feature?

current state diff:

• Add staticanalysis.UnnecessaryParentheses #7916

(Optional) What would the method signatures for your feature look like?

Concrete Use Cases

checkstyle/checkstyle#17487

Packages

No response

Checklist

• I agree to follow the code of conduct.

• I have read and understood the contribution guidelines.

• I have read and understood Guava's philosophy, and I strongly believe that this proposal aligns with it.

• I have visited the idea graveyard, and did not see anything similar to this idea.

[cpovirk]

Thanks.

• Adding missing @Override annotations is pretty much always great.
• Ditto removing unused imports. Something seems broken with the package-info changes in Add staticanalysis.UnnecessaryParentheses #7916, but I can ignore those.
• Our CI is already slower than we'd like as it is, so I probably wouldn't want to enable more checking there.
• Adding an optional way to perform the rewrite in pom.xml is probably fine, though I acknowledge the danger of bit rot if we're not enabling it during CI.

I will pull in the Java changes from the PR, and you can let me know what (if anything) would need to change in the pom.xml changes in order to make them opt-in only.

[Pankraz76]

Thanks for the kind feedback.

• danger of bit rot if we're not enabling it during CI

Yes, the goal is convention (over config princip), imposing a gate to close the broken window effect.

So long story short. Having different scale and CI, the actual computing time highly depends. As the code base seems to be not that huge, it might be a simple task running just a few minutes, justifying the cost facing the constant benefit.

Assuming here still something to migrate, like everywhere, with Java 17 and the upcoming 25 release.

https://docs.openrewrite.org/recipes/java/migrate/upgradetojava21

Even with more recipes, the time will not build up that much, which is the goal—aiming for error-prone solutions that go beyond trivial, but address real issues.

The real effort for the plugin is to build the kind of DOM tree, gaining all the info needed to make changes.

---

Another approach could be to run it on schedule, like Dependa.
This way, the code would be checked periodically—not instantly fixed, but once in a while, as the result is kind of the same, lacking an important factor.

This introduce a costly review burden, as discussions about bugs, flaws, and fixes would still arise, or worse overseen.

Alternatively, delegating this to static code analysis could let the system "heal itself," reducing friction and allowing more focus on actual code rather than recurring complaints about repetitive patterns.

[Pankraz76]

nice tooling, thanks for merge.

[cpovirk]

I see that the pom.xml changes don't appear to affect the behavior of mvn clean install by default, so we could merge those changes to make it possible to run the rewrites without affecting CI or typical builds.

When I do run rewrite:dryRun, I see an increase in build time of 15-30 seconds for each of our four modules. (I'm not sure if the exact current CI config works, since it appears to try to run on guava-bom, too, though I may be misreading it.) That's enough that I wouldn't want it on for CI or normal builds.

Much of the reason that I'm comfortable with our current setup is that we have static analysis enabled internally (mostly https://errorprone.info/). Thus, I'm not too worried about lacking it here. (In fact, I've been considering turning Error Prone off in our external CI, again motivated by improving build speed.) As you've noticed, that doesn't mean that we've gone through to clean up everything that it identifies (though we've made progress).

One other note: We're also iffy on removing the Callable import from AbstractLoadingCache: The Javadoc contains a reference {@link #get(Object, Callable)}, and even though javadoc seems to be able to generate the link without the import, it seems as if it ought to be necessary.

I'll probably end up merging the pom.xml changes so that we have the ability to run rewrites easily in the future (again, barring bit rot).

[Pankraz76]

run on guava-bom

considering scope of maven bestpractices it might be considered a win. Of course we could exclude all other recipes in this module, by using profiles applying different configs following the needs.

https://docs.openrewrite.org/recipes/maven/bestpractices

One other note: We're also iffy on removing the Callable import from AbstractLoadingCache: The Javadoc contains a reference {@link #get(Object, Callable)}, and even though javadoc seems to be able to generate the link without the import, it seems as if it ought to be necessary.

thanks for the hint, its an known bug. @timtebeek

• Remove fully qualified JavaDoc references from TypesInUse for RemoveUnusedImports openrewrite/rewrite#5738

[cpovirk]

My overall views here are converging onto what I wrote just now in #7916 (comment). I will see what changes from #7930 jump out to me as likely to improve our development speed in the future. But there are a large number of changes in that PR that we can't take (mostly for Android reasons), and I need to focus my Guava time elsewhere.