Add `rewrite` support for `errorprone.refasterrules`

[Pankraz76]

Add rewrite support for errorprone.refasterrules

I propose integrating Google Error Prone (https://github.com/google/error-prone) with its Picnic extension (demo: https://www.youtube.com/watch?v=6llnrUtVlrE) to enable automated fixes via rewrite rules.
This would complement Checkstyle by addressing semantic bugs rather than just stylistic issues.

Motivation

Error Prone’s refaster/rewrite rules can:

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

Adoption examples:

• https://github.com/diffplug/spotless/pull/2641
• https://github.com/apache/maven/pull/11159
• https://github.com/apache/maven-parent/pull/496
• https://github.com/checkstyle/checkstyle/issues/17487
• https://github.com/apache/kafka/pull/20219
• https://github.com/diffplug/spotless/pull/2576

Proposal

1. Support errorprone.refasterrules rewrites
2. Keep adoption opt-in (no breaking changes)
3. Provide suppression mechanisms for API constraints

Open Questions

• Scope of auto-fixes
• Suppression strategy
• Integration approach

Next Steps

I can:

• Prepare a PoC
• Help with implementation design
• Address compatibility concerns

[Pankraz76]

Earlier objections are resolved. The tool is already in productive use across major projects we depend on, so we’re already benefiting from it indirectly.

Why not adopt it directly and reduce the manual burden?

[jbduncan]

My biggest objection right now is how slow the OpenRewrite Gradle plugin is - developer experience is important to me - so if it were me, I would just run it once and accept any fixes that it suggests.

However, I'm not a JUnit maintainer, so I'll leave it to the team to decide next steps. :)

[Pankraz76]

Thanks for the nice argumentation and layout of the issue.

Initially, it’s even faster (2 sec.) than Spot, which is quite a surprise. In the end, though, it’s quite an investment. PMD is also not much faster or slower at this scale.

• https://github.com/diffplug/spotless/pull/2636

Spot, on the other hand, contains almost every recipe we can imagine, and it doesn’t slow down development.

• https://github.com/diffplug/spotless/pull/2651

[jbduncan]

Initially, it’s even faster (2 sec.) than Spot, which is quite a surprise.

Oh, really?

In my personal project, Spotless runs orders of magnitude faster than OpenRewrite, especially on subsequent re-runs.

But if there are CI stats to prove it and the team has anecdotal evidence that OpenRewrite is fast enough for local development, then I'm more open to it being used in JUnit. :)

[marcphilipp]

As a first step, I'd suggest adding tech.picnic.error-prone-support:error-prone-contrib to see if that reports any findings.

[Pankraz76]

You’re asking for https://error-prone.picnic.tech/#gradle, right?
Unfortunately, using Google Error Prone also Picnic is quite a hassle and not very user-friendly compared to the Rewrite integration.

In the end, it’s the same — but it’s much more suitable to use Rewrite directly as the vehicle to deliver the Refaster rules.

When adding some Error Prone recipes, we can already see certain changes.
Fortunately, some of the recipes you added — like the security ones — remain clean,
so the changes are still intact.

• https://github.com/junit-team/junit-framework/pull/5003

[marcphilipp]

You’re asking for error-prone.picnic.tech#gradle, right? Unfortunately, using Google Error Prone also Picnic is quite a hassle and not very user-friendly compared to the Rewrite integration.

Yes, we have already set up and configured Error Prone so adding another dependency should be simple.

[Pankraz76]

I tried following the tutorial, but unfortunately it didn’t lead to any results:

• https://github.com/junit-team/junit-framework/pull/5006

Yes it seems reasonable to extend, since Error Prone is already included.

The previous arguments against using Rewrite no longer apply, as it’s already successfully adopted in other projects. In fact, JUnit is indirectly benefiting from Rewrite already through its use in Spotless and Checkstyle.

How to configure this properly?

[jbduncan]

The previous arguments against using Rewrite no longer apply, as it’s already successfully adopted in other projects.

Apart from the (to be tested) significant slowdown, of course. 😉

[Pankraz76]

Working now! With Gradle, I had the chance to learn how to apply it.

Somehow, only the picnic rules are being applied—and not even all of them—so I’m not sure why.
@rickie do you have any suggestions on how to proceed?

This is now the rewrite equivalent without relying on rewrite. The goal was automation and workflow improvements, so I’d still consider this a win.

It’s opt-in: by default nothing happens. If a developer wants to apply the opinionated fixes from Error Prone, they can choose to activate them.

Thanks for the learning and the journey so far!

• https://github.com/junit-team/junit-framework/pull/5006

[rickie]

Hey, sorry for my late response, I see you already put some effort into introducing some of the Error Prone Support checks, very cool!

Somehow, only the picnic rules are being applied—and not even all of them—so I’m not sure why. @rickie do you have any suggestions on how to proceed?

Yes I can help review and support where needed.

We also introduced Error Prone Support to Checkstyle. The approach we took there was introduce one check per PR, which can result in many small and easily reviewable PRs. I'd recommend to do the same here. We can simply take it step by step.

Unfortunately, using Google Error Prone also Picnic is quite a hassle and not very user-friendly compared to the Rewrite integration. In the end, it’s the same — but it’s much more suitable to use Rewrite directly as the vehicle to deliver the Refaster rules.

Given there is already a set up of Error Prone it's not so hard to add more of our rules to the build.

As Error Prone has minimal build time impact (as it runs compile-time as part of the build) it is more integrated into the developer workflow. OpenRewrite is good for the one-off migrations / applications of many rules. After that we prefer running the Refaster rules and Error Prone checks with Error Prone.

---

I saw a comment about patching - which is one of the key reasons to use Error Prone IMO. Let's see what the comment there is about and if we can configure that to make it work!

[Pankraz76]

key reasons to use Error Prone IMO

yes, agree upon.

In my observation its not working as expected yet, as patching blocks out all other checks, making a configuration mandatory, effectively preventing the convention approach:

• https://github.com/diffplug/spotless/pull/2664
• https://github.com/junit-team/junit-framework/pull/5006#issuecomment-3384697369
• https://github.com/junit-team/junit-framework/pull/5043

Will work this out together, thanks.

[Pankraz76]

Is this approach something to consider?

• https://github.com/Pankraz76/junit5/pull/13

It seems much more user-friendly to integrate it with rewrite, rather than fiddling around with raw args and params.

Also, this could be a good starting point to integrate more improvements, such as the Gradle wrapper bump.

I’m also quite surprised that the Java 17 migration only revealed one trivial issue.

I think the reasoning made against the plugin might no longer be relevant, as the newer version is stable and much faster — making it a good candidate for a tryout.
It’s already working well in Spotless and Checkstyle.

This would hopefully also be in favor of @vlsi.

[marcphilipp]

I think the reasoning made against the plugin might no longer be relevant, as the newer version is stable and much faster — making it a good candidate for a tryout.

We have nothing against the plugin per-se but want to avoid added runtime and complexity (as I pointed out in https://github.com/junit-team/junit-framework/pull/5193#issuecomment-3649208617).

@Pankraz76 Are there additional ErrorProne/Refaster rules you would like us to consider? If so, please list them here so we can decide which we want to try before you put in the work and submit PRs.

[Pankraz76]

We have nothing against the plugin per-se but want to avoid added runtime and complexity (as I pointed out in #5193 (comment)).

its 5 mins and the total build time is not longer at all, so id see the argument any good sorry.

• https://docs.openrewrite.org/recipes/java/testing/junit5/cleanupassertions
• https://github.com/gradle/gradle/pull/35878
• https://error-prone.picnic.tech/bugpatterns/StaticImport
• https://docs.openrewrite.org/recipes/java/testing/junit5/junit5bestpractices

all of them are complain and lacking within junit at the same time.

Considering cleanupassertions have done the assertion stuff always actual and assert stuff wrong the last 15 years and also junit is not complain considering this tiny flaw.

Yes A = B is the same like B = A but still is makes some kind of difference.

Considering the convention principle there is something to gain for everyone.

[Pankraz76]

Kindly request some feedback on this as an example. Of course there is some picnic stuff for junit as well. But its quite big as its composite for good reason this needs extra effort. This kind of change needs to go small as its about convention principle. (Whatever these convention might be)

Is this something to consider?

Thanks.

• https://github.com/Pankraz76/junit5/pull/15