Skip to content

Update Error Prone to v2.50.0 - #4847

Open
solrbot wants to merge 6 commits into
apache:mainfrom
solrbot:renovate/error-prone
Open

Update Error Prone to v2.50.0#4847
solrbot wants to merge 6 commits into
apache:mainfrom
solrbot:renovate/error-prone

Conversation

@solrbot

@solrbot solrbot commented Sep 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This PR contains the following updates:

Package Type Update Change
com.google.errorprone:error_prone_core (source) dependencies minor 2.41.0 → 2.50.0
com.google.errorprone:error_prone_annotations (source) dependencies minor 2.41.0 → 2.50.0

Release Notes

google/error-prone (com.google.errorprone:error_prone_core)

v2.50.0: Error Prone 2.50.0

New checks:

Closed issues: #​5553, #​5649, #​5778

Full changelog: google/error-prone@v2.49.0...v2.50.0

v2.49.0: Error Prone 2.49.0

This release includes several changes to Matcher APIs, and removed some deprecated or problematic APIs:

  • Remove deprecated MethodMatchers.withSignature API, which relies on fragile toString behaviour. Alternatives for matching on method signatures with varargs and type parameters were added in a98a1c5.
  • Removed variableType(Matcher) API. Matchers.variableType(Matcher) uses VariableTree#getType to match variable types, which own't work for lambda parameters with inferred types after JDK-8268850. The recommended replacement is variableType(TypePredicate).
  • Make enclosingPackage return an optional. Module elements are not enclosed by a package, checks using enclosingPackage shouldn't assume an enclosing package exists when processing arbitrary elements.
  • New FieldMatchers API, similar to MethodMatchers (1dd9c3a).

New checks:

Closed issues: #​2283, #​3503, #​5210, #​5289, #​5548, #​5548, #​5554, #​5609, #​5614, #​5656

Full changelog: google/error-prone@v2.48.0...v2.49.0

v2.48.0: Error Prone 2.48.0

Changes:

  • Added support for passing flags with command-line argument files (@-files) (8e84edf)

New checks:

Closed issues: #​5529, #​5537, #​5522, #​5521

Full changelog: google/error-prone@v2.47.0...v2.48.0

v2.47.0: Error Prone 2.47.0

New checks:

Closed issues: #​1811, #​4168, #​5459, #​5460

Full changelog: google/error-prone@v2.46.0...v2.47.0

v2.46.0: Error Prone 2.46.0

Changes:

New checks:

Full changelog: google/error-prone@v2.45.0...v2.46.0

v2.45.0: Error Prone 2.45.0

Changes:

  • Improved compatibility with latest JDK 26 EA builds.

New checks:

Closed issues: #​5335

Full changelog: google/error-prone@v2.44.0...v2.45.0

v2.44.0: Error Prone 2.44.0

Changes

Closed issues: #​5218, #​5278

Full changelog: google/error-prone@v2.43.0...v2.44.0

v2.43.0: Error Prone 2.43.0

The minimum supported JDK version to run Error Prone is now JDK 21 (#​4867).

Changes:

  • -XepPatchChecks now skips disabled checks (#​4943)
  • AndroidJdkLibsChecker has been removed, the recommended replacement for Android code is Android Lint's NewApi check

New checks:

Closed issues: #​4943, #​5102, #​5107, #​5121, #​5158, #​5217, #​5239

Full changelog: google/error-prone@v2.42.0...v2.43.0

v2.42.0: Error Prone 2.42.0

New checks:

Changes:

  • The return type of ASTHelpers.asFlagSet has changed. The previous type was EnumSet<Flags.Flag>, where Flags.Flag is an enum in the javac class Flags. A recent JDK change has replaced that enum with a new top-level enum called FlagsEnum. It is not possible to change ASTHelpers.asFlagSet in a way that would be type-safe and compatible with the enums from JDKs both before and after the change. Instead, the method now returns ImmutableSet<String>, where the strings come from the toString() of the enum constants. That means they are "native", "abstract", etc.
  • Flag IO.print[ln]() in SystemOut.

Full changelog: google/error-prone@v2.41.0...v2.42.0

Configuration

📅 Schedule: (UTC)

  • Branch creation
    • "before 9am on the first day of the month"
  • Automerge
    • At any time (no schedule defined)

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about these updates again.

  • If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot

@solrbot solrbot added the exempt-stale Prevent a PR from going stale label Sep 1, 2026
@solrbot solrbot added this to the 10.x milestone Sep 1, 2026
@solrbot
solrbot force-pushed the renovate/error-prone branch from b4e2a97 to ce979af Compare September 1, 2026 14:08
@solrbot
solrbot force-pushed the renovate/error-prone branch from 9244f7b to a342c87 Compare September 2, 2026 23:15
solrbot and others added 5 commits September 2, 2026 23:15
@solrbot
Add changelog entry for PR#4847
930634c
@epugh
Merge remote-tracking branch 'upstream/main' into pr/4847
473ebca
@epugh
Skip our noggit code, it's not "really" ours...
ecec2bd
@epugh @claude
Fix Error Prone 2.50.0 findings: NotJavadoc, TypeParameterQualifier, …
0743dd4 
…dead code

Error Prone 2.50.0 (PR apache#4847) flags several categories of real issues
across the codebase, fixed here (ReferenceEquality suppressions are
handled separately in the next commit):

- NotJavadoc: /** */ comments attached to methods inside local/anonymous
  classes, or stacked duplicate /** */ blocks, converted to regular
  comments or merged as appropriate.
- TypeParameterQualifier: T::toString replaced with Object::toString in
  RequestReplicaListTransformerGenerator, since a generic type parameter
  used as an unbound method reference qualifier is erased to Object
  anyway.
- A latent bug in CommandOperation.getStr(String, String) where the
  ROOT_OBJ branch returned null instead of the default value when the
  underlying object was null, inconsistent with the sibling branch and
  the single-arg overload.
- Excluded vendored Tika sources and noggit from Error Prone checks,
  matching existing precedent for other vendored/generated code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@epugh @claude
Suppress Error Prone ReferenceEquality findings across the codebase
3c713f1 
Error Prone 2.50.0's ReferenceEquality check is a purely syntactic
heuristic: it flags any == / != between two objects of a type that
overrides equals(), regardless of whether the comparison is actually
a deliberate identity check. It cannot distinguish that from a genuine
missed-.equals() bug.

Every occurrence suppressed here was individually reviewed and is a
legitimate identity comparison: sentinel objects, "same instance means
no-op" idioms, singleton comparisons (e.g. Calcite Convention, an
IdentityNormalizer instance), heavyweight resource identity (SolrCore,
Thread, JettySolrRunner), or identity semantics already documented via
System.identityHashCode() in the corresponding hashCode(). Each
suppression is scoped to the smallest enclosing method or constructor
and, where the reasoning wasn't already evident from context, annotated
with a short comment explaining why identity (not value) equality is
correct there.

This is broken out as its own commit since the sheer number of
suppressions may warrant a different overall approach (e.g. a targeted
excludedPaths/allowlist) rather than suppressing case-by-case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@epugh

epugh commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The new ErrorProne brought a set of new rules, some that are great, and one that maybe needs discussion. This commit is all reasonable seeming fixes/cleanups: 0743dd4, however the suppression of ReferenceEquality was a LOT! See 3c713f1 and I don't know what we think about this...

We could also disable it via

// '-Xep:ReferenceEquality:WARN', // too many false positives on intentional identity checks
//     (sentinels, singletons, resource/thread identity); see PR #4847

I think I am leaning towards this, but would love other opinoins!

@epugh
epugh requested review from dsmiley, epugh and janhoy and removed request for epugh September 3, 2026 17:35
@solrbot

solrbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@epugh

epugh commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

FYI https://errorprone.info/bugpattern/ReferenceEquality

dsmiley
dsmiley reviewed Sep 4, 2026
View reviewed changes

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The reference equality thing is a shame. I wish there was an annotation that could either be used on a type (say Aliases) or on field with a constant initializer, that means -- this thing is used with reference equality. Basically -- opt-in. Instead, with error-prone, we can only flag a method that uses equality somewhere inside, without clarity on what line had the issue; and it only applies to that method.

Any way, I actually think it's a fairly positive change because it should catch bugs. On the other hand, we may want to reconsider hard-line static analysis of error-prone as we consider adopting SonarQube soon, which raises matters but doesn't block us on anything. Error-prone blocks us and/or compells an annotation, so I suppose it's fair to say error-prone doesn't block either. It does force an annotation for a suppression... which maybe has some enduring quality value of communicating to a future reader that this reference equality is intentional. That brings me back to basically agreeing with what's here.

Comment thread solr/core/src/java/org/apache/solr/handler/component/StatsField.java
countDistinct(false, distinctValues),
percentiles(true) {
/** special for percentiles * */
/* special for percentiles */

@dsmiley dsmiley Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I disagree with NotJavadoc rule. The creator of the rule apparently thinks the only point of Javadoc is to publish it. I rarely look at published javadoc nowadays. I view javadoc as the normal way that a class/field/member is documented. And it even looks fairly nice nowadays in IntelliJ with the rendered view.

@dsmiley dsmiley Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Further argument: Imagine I have some class with javadoc... and then I do a refactoring that turns it into an anonymous inner class. Why should I then be forced to turn javadocs to non-javadoc? Seems arbitrary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsmiley dsmiley dsmiley left review comments
@janhoy janhoy Awaiting requested review from janhoy
@epugh epugh Awaiting requested review from epugh

Assignees

No one assigned

Labels

admin-ui cat:api cat:cli cat:cloud cat:index cat:metrics cat:packagemanager cat:schema cat:search cat:security client:solrj dependencies Dependency upgrades documentation Improvements or additions to documentation exempt-stale Prevent a PR from going stale jetty-server module:clustering module:cross-dc module:cuvs module:extraction module:gcs-repository module:jwt-auth module:langid module:language-models module:ltr module:opentelemetry module:s3-repository module:scripting module:sql test-framework tests tool:build

Projects

None yet

Milestone

10.x

Development

Successfully merging this pull request may close these issues.

3 participants

@solrbot @epugh @dsmiley