Update com.google.errorprone to 2.50.0

[janhoy]

Upgrades Error Prone 2.41.0 → 2.50.0.

Most of this work done with Claude Fable 5, with an explaining comment for each new suppression.

The new/stricter bug patterns flagged ~196 findings, all resolved so the build passes with -Werror:

• ReferenceEquality (~160): each site reviewed individually. Intentional identity comparisons (unique sentinels like END_OBJ/NO_OP/DELETED, cached singletons, same-instance fast paths, Calcite copy() conventions) are suppressed with a justifying comment; genuine value comparisons rewritten to equals()/Objects.equals().
• JdkObsolete (~20): findings suppressed with TODO comments; the actual modernization to java.nio.charset.Charset APIs is in Modernize String-based charset APIs to java.nio.charset.Charset #4606. The Tika-copied ToTextContentHandler keeps its checked-exception API (suppressed permanently).
• NotJavadoc (~15): stray/duplicate javadoc on anonymous/local-class methods and commented-out code converted to regular comments.
• TypeParameterQualifier (1 error): T::toString → Object::toString in RequestReplicaListTransformerGenerator.

Per review feedback, this PR is kept mechanical (suppressions and behavior-neutral rewrites only). The real, behavior-changing fixes that the upgrade surfaced are split into separate PRs, and the corresponding sites here are suppressed with TODO comments pointing at them (to be dropped on rebase once they merge):

• Fix reference-equality comparison bugs found by Error Prone 2.50.0 #4605 — three latent reference-equality bugs (ZkMaintenanceUtils single-file upload comparing Path with ==, HttpShardHandler comparing a request-context Boolean with ==, ZkShardTermsTest.waitFor comparing boxed values by reference)
• Modernize String-based charset APIs to java.nio.charset.Charset #4606 — charset API modernization (String charset names → java.nio.charset.Charset), including handling of invalid client-supplied charset names

[janhoy]

Copilot found a real behavior regression from the Charset.fromName() fixes, and it affected more places than the ones flagged. The Error Prone fixes replaced String-based charset APIs (which throw the checked UnsupportedEncodingException) with Charset.forName(), which throws unchecked IllegalArgumentException. Since the charset name usually comes from a client-supplied Content-Type header or user config, an invalid name would now escape as a runtime exception instead of being handled as an I/O error.

Fixed in 282d84b:

• Added a small helper ContentStreamBase.charsetForName(String) that converts the unchecked exceptions back to UnsupportedEncodingException (with cause), matching the legacy JDK API semantics.
• Switched all 9 affected call sites introduced on this branch to use it (ContentStreamBase, JsonMapResponseParser, JacksonDataBindResponseParser, DefaultSampleDocumentsLoader, XMLLoader, TextQueryResponseWriter, FileBasedSpellChecker). Every site already declared throws IOException or sat in a catch (IOException), so this restores the exact pre-upgrade behavior.
• HttpSolrClient is left as-is — it already catches IOException | IllegalArgumentException and wraps both in RemoteSolrException.

[Copilot]

Pull request overview

Copilot reviewed 164 out of 164 changed files in this pull request and generated no new comments.

[epugh]

I stuggle to really figure out how to evaluate this (other than check the tests pass!). I noticed just a bit of code change, and I expected more.. I guess maybe I expected more "this pattern you have been using is a bad pattern and you should refactor/change it", did any of that come out? versus just adding @SuppressWarnings... Are any of the suppressed warnings actually bad patterns that we have been using that should be maybe fixed in follow up tickets????/

[dsmiley]

As this PR has real fixes amongst many more stylistic / best-practices... I feel the real fixes should be another PR. A large PR should hopefully be filled with boring easy-to-review stuff.

[janhoy]

There were hundreds of warnings caused by the new / stronger errorprone rules. Claude found most of the invocations to be legit, such as == for comparing objects instead of .equals() since we look for same-instance proof. For other things like Charset.forName() there are real code changes.

It could be argued that we handle both "warn" and "error" in the same PR. But also "warn" recommendations are worth fixing. But what benefit does splitting into two PRs give us further down the road?

@epugh Any particular reason you expected more real code change as a result of the new rules? I's not that we haven't been fixing errorprone feeback earlier and have a backlog. This is only reacting to the new/tighter rules in this partifular version bump.

[epugh]

@epugh Any particular reason you expected more real code change as a result of the new rules? I's not that we haven't been fixing errorprone feeback earlier and have a backlog. This is only reacting to the new/tighter rules in this partifular version bump.

I guess I was thinking that there would be more "modern java coding practices should be used" that this would flag by doing a suppression of a warning. I think all of this looks good.

[janhoy]

@dsmiley I first thought you by "real fixes" meant mandatory / ERROR type fixes. But all errorprone rules cause build errors.

So you just want this PR to "flag" the places where we need "real" fixes, and then do those as follow-ups. I can prepare such a split.

[janhoy]

Split done. I propose we merge the two "fixes" PRs first, then rebase this and remove TODO comments, check errorprone again and then merge this.

[dsmiley]

IMO "dependency_update" usage should be restricted to dependencies shipped with a binary Solr distribution. If it's broader than that; it's uninteresting / less useful and takes more human judgement to gauge... "do I care about this" (as a user).

I appreciate you are making these static analysis improvements. Weather they are found by Sonar, IntelliJ, Errorprone, or you woke up one day and had to scratch that itch without any tool saying so -- who cares. The changes themselves is the substance that may or may not be changelog worthy. That a dependency we do not include with Solr got bumped and found them isn't relevant.

[janhoy]

Let's discuss on dev@ how to improve how we handle changelogs in general. It's three aspects to this:

• Changelog entries included with normal PRs filed by humans - training contributors into writing them correctly and for the right audience
• Changelog entries auto written for solrbot PRs (all of them dependency_update)
• What does the RM do with all the files in changelog/unreleased/* during a release? Does it include weeding, filtering, rewriting, deduplicating, consolidating changelogs (like a RM typically did with CHANGES.txt)? Could be a nice LLM skill...