Add Error Prone static analysis - #2634

Here's CI log in my fork: https://github.com/vlsi/assertj-core/runs/6645822760?check_suite_focus=true#step:5:46

Interesting @vlsi! the team needs to have a look at the rules but looks promising

Of course, I don't suggest enabling and fixing all the bug patterns, however, I believe the errors produced by the default configuration should be fixed.

Even some of the "warnings by default" are worth fixing as well.

For instance:

1. https://github.com/assertj/assertj-core/blob/b44460623b9c0e83c2b311c1fc6b7bffa1a077b9/src/main/java/org/assertj/core/description/TextDescription.java#L54-L56

Error:  /../assertj-core/assertj-core/src/main/java/org/assertj/core/description/TextDescription.java:[56,24] [ArrayHashCode] hashcode method on array does not hash array contents
    (see https://errorprone.info/bugpattern/ArrayHashCode)
  Did you mean 'return Objects.hash(value, Arrays.hashCode(args));'?

2. https://github.com/assertj/assertj-core/blob/8a70b403490ba5799fc06b90dc8096c729b4da0e/src/main/java/org/assertj/core/internal/Iterables.java#L1315-L1321

Error:  /../assertj-core/assertj-core/src/main/java/org/assertj/core/internal/Iterables.java:[1321,31] [ReturnValueIgnored] Return value of 'orElseThrow' must be used
    (see https://errorprone.info/bugpattern/ReturnValueIgnored)

it might be questionable, however, I believe Stream.anyMatch would indeed be better:

    if (stream(actual).anyMatch(predicate)) {
      throw failures.failure(info, anyElementShouldMatch(actual, predicateDescription));
    }

I remembered errorprone because it can detect MethodCanBeStatic (it verifies the method is private, so it does not break backward compatibility).

Then, there's https://github.com/palantir/assertj-automation which you might want to apply and promote :)

@joel-costigliola , one more question: for now, I include errorprone Maven plugin right into assertj-core, however, you might want to add it to the base configuration (https://github.com/assertj/assertj-parent-pom ?)

In my experience, the mere presence of assertj/assertj-parent-pom creates too much friction since Maven does not support cross-repository changes. I can't really have a pull request that would change assertj-parent-pom and apply the changed assertj-parent-pom into assertj-core.

We had the exact same issue with https://github.com/pgjdbc/pgjdbc-parent-poms when the PostgreSQL JDBC driver was Maven-based. Moving to Gradle was a huge relief so we could keep all the things in a single repository.

Even if you keep multiple repositories (e.g. keep assertj-core, assertj-parent-whatever, etc), Gradle makes it possible to use tools like https://melix.github.io/includegit-gradle-plugin/ when you can have local testing and even pull requests that cross repository bounds: https://twitter.com/CedricChampeau/status/1499019789033000962

Of course, I would yield to the team's decision, however, I strongly believe tools like ErrorProne, Cherckerframework, AutoStyle would improve contributor and developer experience.
At the same time, Maven makes many of those improvements impossible or extremely hard.

I can't really have a pull request that would change assertj-parent-pom and apply the changed assertj-parent-pom into assertj-core.

We had the exact same issue with https://github.com/pgjdbc/pgjdbc-parent-poms when the PostgreSQL JDBC driver was Maven-based. Moving to Gradle was a huge relief so we could keep all the things in a single repository.

Personally, I don't see how this is related to Maven. What you're mentioning is a multi-module setup, something we already plan to tackle in #2424. Even if we would switch to Gradle, most likely we would target the same setup.

I agree Gradle would allow multi-repository development but IMHO the fact that it's possible doesn't necessarily mean it's the direction we should take.