This seems like a good addition to Rewrite, as in some aspects it is superior and has its unique benefits—like every tool has. Of course, they cover the same topic, likeMissingOverrideAnnotation, so I'm wondering why we see this changed again. Maybe it's some bug or config issue; it seems to have a great potential when the right tools make the equation round up. It's faster than Rewrite, even though the configuration is not straightforward opt-in like Rewrite. We will work this out and learn on the job while extending the tool.

It's behaving just like Rewrite—failing the build and suggesting changes. The opt-in is quite unique and will not be communicated like in Rewrite, but the changes should be small, like the changesets.

Do you like to see this going on?@nedtwigg Thanks.

This was referencedOct 8, 2025

resolve temporary constraint exclude oferrorprone("com.google.guava:guava")junit-team/junit-framework#5039

Closed

Addrewrite support forerrorprone.refasterrulesjunit-team/junit-framework#5002

Open

rickie reviewed

Oct 9, 2025

View reviewed changes

Copy link

rickie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I could be missing some context or another issue / PR in this repository, so please let me know if I miss something.

Not sure about the preference of the maintainers, but we might benefit from a slightly different approach. Now we are enabling a lot of checks in one PR. That makes the PR slightly harder to review. We could benefit from an approach outlined in this comment in JUnit repository.

If we use that approach, reviewing a PR is a lot easier, as all changes relate toone check. In this case there are quite a few that make changes, which makes it harder.

WDYT about that approach?

Pankraz76 force-pushed thepr-error-prone-SanityCheck branch froma7f79d2 to0e2141aCompare

October 9, 2025 13:51

Pankraz76 changed the title~~Adderror-prone.picnic.tech featuringStaticImport~~Adderror-prone.picnic.tech featuringConstantNaming

Oct 9, 2025

Pankraz76 commented

Oct 9, 2025

View reviewed changes

gradle/error-prone.gradle

		// 'StaticImport',
		)
		errorproneArgs.add('-XepOpt:Refaster:NamePattern=^(?!.Rules\\$).')
		if (!getenv().containsKey('CI')&& getenv('IN_PLACE')?.toBoolean()) {

Copy link

ContributorAuthor

Pankraz76Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

activatingIN_PLACE allowing removal ofallErrorsAsWarnings resulting in local passing, on CI, without patching of course, positive failing build.

How to automate prone without having this glitch?@rickie Patching only cares for the XepPatchChecks like the documentation tells. XepPatchLocation Only works in paid, despite having it seen in gradle not being the case its a very random plugin behaviour sometimes.

Apply errorprone suggestions usingPatchLocation:IN_PLACE forDefaultCharset gradle/gradle#35107

Copy link

rickieOct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Only works in paid, despite having it seen in gradle not being the case its a very random plugin behaviour sometimes.

I don't fully understand this comment, can you elaborate?

I agree the usage of theXepPatchChecks and location is a bit finicky and requires some effort to get right. You work on a lot of PRs which is cool to see. But right now I don't have time to dive into all of them and find the exact cause. Will try to also look tomorrow, but can't make promises right now.

I once created a demo project (it's in Maven though) but perhaps you can look into that, and based on that try to configure it for one of the projects you're working on:https://github.com/rickie/error-prone-demo.

Pankraz76 force-pushed thepr-error-prone-SanityCheck branch from0e2141a to5d58113Compare

October 9, 2025 14:23

Pankraz76 commented

Oct 9, 2025

View reviewed changes

gradle/error-prone.gradle Outdated


		tasks.withType(JavaCompile).configureEach {
		options.errorprone {
		allErrorsAsWarnings=true// ModuleHelper.java:37: warning: Unsafe is internal proprietary API and may be removed in a future release

Copy link

ContributorAuthor

Pankraz76Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

somehow this does not suppress/avoid:

FAILURE: Build failed with an exception.* What went wrong:Execution failed for task ':lib:compileJava'.> Compilation failed; see the compiler output below.  /Users/vincent.potucek/IdeaProjects/spotless/lib/src/main/java/com/diffplug/spotless/java/ModuleHelper.java:37: warning: Unsafe is internal proprietary API and may be removed in a future release  import sun.misc.Unsafe;                 ^  4 errors  100 warningsBUILD FAILED in 10s

Copy link

rickieOct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

allErrorsAsWarnings this flag is anError Prone flag (docs). It doesn't mean that all the errors from the build are being lowered in severity, it's about the checks from Error Prone that are configured with severityERROR are lowered toWARNING.

In this case there is something else - unrelated to this flag - that is causing the error about thesun.misc.Unsafe. I'm not sure from the top of my head what this error is or why we get it here, but make sure to not misinterpret this flag.