Removing error-prone from our build

[nedtwigg]

We have recently merged a ton of automated refactors, I believe some by OpenRewrite and some by error-prone. Right now we have:

• spotless
• spotbugs
• errorprone
• openrewrite

We just received an excellent PR tackling a very hard and long-requested feature:

• new step to expand java wildcard imports #2744

It failed CI. I took a look and

• https://github.com/diffplug/spotless/actions/runs/19546227953/job/55965599249
• literally 3000 lines of warnings in the build (!!!)
• I clone it locally
• Looks like this is the failure

* What went wrong:
Execution failed for task ':lib:compileJavaParserJava'.
> Compilation failed; see the compiler output below.
  /Users/ntwigg/Documents/dev/spotless/lib/src/javaParser/java/com/diffplug/spotless/glue/javaParser/ExpandWildcardsFormatterFunc.java:95: error: [CollectorMutability] Avoid `Collectors.to{List,Map,Set}` in favor of collectors that emphasize (im)mutability
                                .collect(toMap(Function.identity(),
                                              ^
      (see https://error-prone.picnic.tech/bugpatterns/CollectorMutability)
    Did you mean '.collect(toImmutableMap(Function.identity(),' or 't -> new TreeSet<>(Comparator.comparing(ImportDeclaration::getNameAsString)), (a, b) -> { throw new IllegalStateException(); }, HashMap::new));'?
  1 error

I like immutability, but the warning above is not easy to fix. We don't have guava, toMap is a good choice here.

@Pankraz76 The feature in the PR is more important than the rule-check. When rule-checking is consuming more reviewer time than actual PRs, something is wrong. For this reason, I removed error-prone fom the build in the above-mentioned PR.

• 06b33ff

[Pankraz76]

I agree — it isn’t very pleasant or kind to use, or integrate, and sometimes it feels overwhelming and spam-like.

[Pankraz76]

• literally 3000 lines of warnings in the build (!!!)

-XepDisableAllWarnings might was the missing key and should be the cure of this madness.

Idk whats the point, beside the spamming yes, as warnings are just optional?

[jbduncan]

I haven't been paying attention to our ErrorProne setup, so I don't know which rules we had previously enabled.

Usually, the rules that come out-of-the-box and are enabled by default are a good baseline, because they're based on Google's internal lessons on which rules produce many false positives and which don't. Are we seeing problems from these rules, or any other rules that we've enabled, or both?

The way I see it, it would be worth reintroducing ErrorProne with just the enabled-by-default rules and slowly reintroduce others like the Picnic ones over time.

What do you think, both?

[Pankraz76]

thanks for your diplomatic support in guiding/guarding the overall quality goals.

-XepDisableAllWarnings might was the missing key and should be the cure of this madness.

This should fix the massiv output, also referring in:

• [prone] apply -XepDisableAllWarnings #2754
  • [prone] fix warning StaticImport, NonStaticImport #2743