Update com.google.errorprone:* to 2.31.0 (#2862)

* Update com.google.errorprone:* to 2.31.0 * Remove impossible null comparison * Suppress wanted method references * Fix ClassCanBeStatic warnings * Remove null checks for primitive data types (cherry picked from commit f6c5af9)
apache · Nov 13, 2024 · 64625f6 · 64625f6
1 parent 968aac2
commit 64625f6
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 16 deletions.
diff --git a/gradle/validation/error-prone.gradle b/gradle/validation/error-prone.gradle
@@ -179,6 +179,7 @@ allprojects { prj ->
             '-Xep:MathRoundIntLong:ERROR',
             // '-Xep:MislabeledAndroidString:OFF', // we don't use android
             '-Xep:MisplacedScopeAnnotations:ERROR',
+            // '-Xep:MissingRuntimeRetention:ERROR', // todo check if useful or comment why not
             // '-Xep:MissingSuperCall:OFF', // we don't use this annotation
             // '-Xep:MissingTestCall:OFF', // we don't use this annotation
             '-Xep:MisusedDayOfYear:ERROR',
@@ -218,12 +219,15 @@ allprojects { prj ->
             '-Xep:RandomCast:ERROR',
             '-Xep:RandomModInteger:ERROR',
             // '-Xep:RectIntersectReturnValueIgnored:OFF', // we don't use android
+            // '-Xep:RedundantSetterCall:ERROR', // todo check if useful or comment why not
             // '-Xep:RequiredModifiers:OFF', // we don't use this annotation
             // '-Xep:RestrictedApiChecker:OFF', // we don't use this annotation
             // '-Xep:ReturnValueIgnored:OFF', // todo there are problems that should be fixed
+            // '-Xep:SelfAssertion:ERROR', // todo check if useful or comment why not
             '-Xep:SelfAssignment:ERROR',
             '-Xep:SelfComparison:ERROR',
             '-Xep:SelfEquals:ERROR',
+            // '-Xep:SetUnrecognized:ERROR', // todo check if useful or comment why not
             // '-Xep:ShouldHaveEvenArgs:OFF', // we don't use truth
             '-Xep:SizeGreaterThanOrEqualsZero:ERROR',
             '-Xep:StreamToString:ERROR',
@@ -236,7 +240,6 @@ allprojects { prj ->
             // '-Xep:ThrowIfUncheckedKnownChecked:OFF', // we don't use this annotation
             '-Xep:ThrowNull:ERROR',
             '-Xep:TreeToString:ERROR',
-            // '-Xep:TruthSelfEquals:OFF', // we don't use truth
             '-Xep:TryFailThrowable:ERROR',
             '-Xep:TypeParameterQualifier:ERROR',
             '-Xep:UnicodeDirectionalityCharacters:ERROR',
@@ -265,6 +268,7 @@ allprojects { prj ->
             '-Xep:AssertionFailureIgnored:WARN',
             '-Xep:AssistedInjectAndInjectOnSameConstructor:WARN',
             '-Xep:AttemptedNegativeZero:WARN',
+            // '-Xep:AutoValueBoxedValues:WARN', // todo check if useful or comment why not
             // '-Xep:AutoValueFinalMethods:OFF', // we don't use autovalue
             // '-Xep:AutoValueImmutableFields:OFF', // we don't use autovalue
             // '-Xep:AutoValueSubclassLeaked:OFF', // we don't use autovalue
@@ -285,6 +289,7 @@ allprojects { prj ->
             '-Xep:ChainedAssertionLosesContext:WARN',
             '-Xep:CharacterGetNumericValue:WARN',
             '-Xep:ClassCanBeStatic:WARN',
+            // '-Xep:ClassInitializationDeadlock:WARN', // todo check if useful or comment why not
             '-Xep:ClassNewInstance:WARN',
             // '-Xep:CloseableProvides:OFF', // we don't use this annotation
             '-Xep:ClosingStandardOutputStreams:WARN',
@@ -296,6 +301,8 @@ allprojects { prj ->
             '-Xep:DateChecker:WARN',
             '-Xep:DateFormatConstant:WARN',
             // '-Xep:DefaultCharset:OFF', // we have forbiddenapis for that
+            //'-Xep:DeeplyNested:WARN', // todo check if useful or comment why not
+            //'-Xep:DefaultLocale:WARN', // todo check if useful or comment why not
             '-Xep:DefaultPackage:WARN',
             '-Xep:DeprecatedVariable:WARN',
             '-Xep:DirectInvocationOnMock:WARN',
@@ -309,6 +316,7 @@ allprojects { prj ->
             '-Xep:EmptyBlockTag:WARN',
             // '-Xep:EmptyCatch:OFF', // todo check if useful or comment why not - might be handled by ECJ?
             // '-Xep:EmptySetMultibindingContributions:OFF', // we don't use this annotation
+            // '-Xep:EnumOrdinal:WARN', // todo check if useful or comment why not
             '-Xep:EqualsGetClass:WARN',
             '-Xep:EqualsIncompatibleType:WARN',
             '-Xep:EqualsUnsafeCast:WARN',
@@ -330,6 +338,7 @@ allprojects { prj ->
             // '-Xep:FragmentNotInstantiable:OFF', // we don't use android
             // '-Xep:FutureReturnValueIgnored:OFF', // todo there are problems that should be fixed
             '-Xep:GetClassOnEnum:WARN',
+            // '-Xep:GuiceNestedCombine:WARN', // todo check if useful or comment why not
             '-Xep:HidingField:WARN',
             '-Xep:ICCProfileGetInstance:WARN',
             '-Xep:IdentityHashMapUsage:WARN',
@@ -383,6 +392,7 @@ allprojects { prj ->
             '-Xep:JodaPlusMinusLong:WARN',
             '-Xep:JodaTimeConverterManager:WARN',
             '-Xep:JodaWithDurationAddedLong:WARN',
+            // '-Xep:JUnitIncompatibleType:WARN', // todo check if useful or comment why not
             // '-Xep:LabelledBreakTarget:OFF', // stylistic
             '-Xep:LiteEnumValueOf:WARN',
             '-Xep:LiteProtoToString:WARN',
@@ -403,10 +413,12 @@ allprojects { prj ->
             // '-Xep:MissingSummary:OFF', // style preference that we don't want to enforce
             // '-Xep:MixedMutabilityReturnType:OFF', // todo check if useful or comment why not
             '-Xep:MockNotUsedInProduction:WARN',
+            // '-Xep:MockitoDoSetup:WARN', // todo check if useful or comment why not
             '-Xep:ModifiedButNotUsed:WARN',
             '-Xep:ModifyCollectionInEnhancedForLoop:WARN',
             '-Xep:ModifySourceCollectionInStream:WARN',
             '-Xep:MultimapKeys:WARN',
+            // '-Xep:MultipleNullnessAnnotations:WARN', // todo check if useful or comment why not
             '-Xep:MultipleParallelOrSequentialCalls:WARN',
             '-Xep:MultipleUnaryOperatorsInMethodCall:WARN',
             // '-Xep:MutableGuiceModule:OFF', // we don't use guice
@@ -428,7 +440,9 @@ allprojects { prj ->
             '-Xep:NullableOptional:WARN',
             // '-Xep:NullablePrimitive:OFF', // we don't use this annotation
             // '-Xep:NullablePrimitiveArray:OFF', // we don't use this annotation
+            // '-Xep:NullableTypeParameter:WARN', // todo check if useful or comment why not
             // '-Xep:NullableVoid:OFF', // we don't use this annotation
+            // '-Xep:NullableWildcard:WARN', // todo check if useful or comment why not
             '-Xep:ObjectEqualsForPrimitives:WARN',
             // '-Xep:ObjectToString:OFF', // todo check if useful or comment why not
             '-Xep:ObjectsHashCodePrimitive:WARN',
@@ -442,6 +456,7 @@ allprojects { prj ->
             '-Xep:Overrides:WARN',
             // '-Xep:OverridesGuiceInjectableMethod:OFF', // we don't use guice
             '-Xep:ParameterName:WARN',
+            // '-Xep:PatternMatchingInstanceof:WARN', // todo check if useful or comment why not
             '-Xep:PreconditionsCheckNotNullRepeated:WARN',
             '-Xep:PrimitiveAtomicReference:WARN',
             '-Xep:ProtectedMembersInFinalClass:WARN',
@@ -459,6 +474,7 @@ allprojects { prj ->
             // '-Xep:SameNameButDifferent:OFF', // todo check if useful or comment why not
             '-Xep:SelfAlwaysReturnsThis:WARN',
             // '-Xep:ShortCircuitBoolean:OFF', // todo check if useful or comment why not
+            // '-Xep:StatementSwitchToExpressionSwitch:WARN', // todo check if useful or comment why not
             // '-Xep:StaticAssignmentInConstructor:OFF', // we assign SolrTestCaseJ4.configString in many tests, difficult to untangle
             '-Xep:StaticAssignmentOfThrowable:WARN',
             // '-Xep:StaticGuardedByInstance:OFF', // todo check if useful or comment why not
@@ -469,9 +485,12 @@ allprojects { prj ->
             '-Xep:StringCharset:WARN',
             '-Xep:StringFormatWithLiteral:WARN',
             // '-Xep:StringSplitter:OFF', // todo check if useful or comment why not - might be able to use forbidden-apis for this?
+            // '-Xep:SunApi:WARN', // todo check if useful or comment why not
+            // '-Xep:SuperCallToObjectMethod:WARN', // todo check if useful or comment why not
             '-Xep:SuperEqualsIsObjectEquals:WARN',
             // '-Xep:SwigMemoryLeak:OFF', // we don't use swig
             // '-Xep:SynchronizeOnNonFinalField:OFF', // todo check if useful or comment why not
+            // '-Xep:SystemConsoleNull:WARN', // todo check if useful or comment why not
             // '-Xep:ThreadJoinLoop:OFF', // todo check if useful or comment why not
             // '-Xep:ThreadLocalUsage:OFF', // todo check if useful or comment why not
             // '-Xep:ThreadPriorityCheck:OFF', // todo check if useful or comment why not
@@ -493,6 +512,7 @@ allprojects { prj ->
             // '-Xep:UnicodeEscape:OFF', // can't enable since Lucene/Solr tests use unicode a bunch
             // '-Xep:UnnecessaryAssignment:OFF', // we don't use these annotations
             '-Xep:UnnecessaryAsync:WARN',
+            // '-Xep:UnnecessaryBreakInSwitch:WARN', // todo check if useful or comment why not
             '-Xep:UnnecessaryLambda:WARN',
             '-Xep:UnnecessaryLongToIntConversion:WARN',
             '-Xep:UnnecessaryMethodInvocationMatcher:WARN',
@@ -513,6 +533,7 @@ allprojects { prj ->
             // '-Xep:UseBinds:OFF', // we don't use this annotation
             // '-Xep:UseCorrectAssertInTests:OFF', // we inherit from LuceneTestCase which extends Assert
             '-Xep:VariableNameSameAsType:WARN',
+            // '-Xep:VoidUsed:WARN', // todo check if useful or comment why not
             // '-Xep:WaitNotInLoop:OFF', // todo check if useful or comment why not
             // '-Xep:WakelockReleasedDangerously:OFF', // we don't use android
             // '-Xep:WithSignatureDiscouraged:OFF', // we aren't using this error-prone internal api

diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageListeningClassLoader.java b/solr/core/src/java/org/apache/solr/pkg/PackageListeningClassLoader.java
@@ -105,6 +105,9 @@ public SolrPackageLoader.SolrPackage.Version findPackageVersion(
     return theVersion;
   }
 
+  // Allow method reference to return a reference to a functional interface (Mapwriter),
+  // rather than a reference to a PgkVersion object
+  @SuppressWarnings("UnnecessaryMethodReference")
   @Override
   public MapWriter getPackageVersion(PluginInfo.ClassName cName) {
     if (cName.pkg == null) return null;

diff --git a/.../core/src/java/org/apache/solr/update/processor/ClassificationUpdateProcessorFactory.java b/.../core/src/java/org/apache/solr/update/processor/ClassificationUpdateProcessorFactory.java
@@ -92,8 +92,7 @@ public void init(final NamedList<?> args) {
       String algorithmString = params.get(ALGORITHM_PARAM);
       Algorithm classificationAlgorithm;
       try {
-        if (algorithmString == null
-            || Algorithm.valueOf(algorithmString.toUpperCase(Locale.ROOT)) == null) {
+        if (algorithmString == null) {
           classificationAlgorithm = DEFAULT_ALGORITHM;
         } else {
           classificationAlgorithm = Algorithm.valueOf(algorithmString.toUpperCase(Locale.ROOT));

diff --git a/solr/core/src/test/org/apache/solr/handler/admin/IndexSizeEstimatorTest.java b/solr/core/src/test/org/apache/solr/handler/admin/IndexSizeEstimatorTest.java
@@ -227,8 +227,6 @@ public void testIntegration() throws Exception {
           (k, v) -> {
             double size = fromHumanReadableUnits((String) v);
             double sampledSize = fromHumanReadableUnits((String) sampledFieldsBySize.get(k));
-            assertNotNull(
-                "sampled size missing for field " + k + " in " + sampledFieldsBySize, sampledSize);
             double delta = size * 0.5;
             assertEquals("sampled size of " + k + " is wildly off", size, sampledSize, delta);
           });

diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/ReloadCoreAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/ReloadCoreAPITest.java
@@ -57,7 +57,6 @@ public void setUp() throws Exception {
   public void testValidReloadCoreAPIResponse() throws Exception {
     SolrJerseyResponse response = reloadCoreAPI.reloadCore(coreName, new ReloadCoreRequestBody());
     assertEquals(0, response.responseHeader.status);
-    assertNotNull(response.responseHeader.qTime);
   }
 
   @Test

diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/UnloadCoreAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/UnloadCoreAPITest.java
@@ -56,7 +56,6 @@ public void setUp() throws Exception {
   public void testValidUnloadCoreAPIResponse() throws Exception {
     SolrJerseyResponse response = unloadCoreAPI.unloadCore(coreName, getUnloadCoreRequestBodyObj());
     assertEquals(0, response.responseHeader.status);
-    assertNotNull(response.responseHeader.qTime);
   }
 
   @Test

diff --git a/solr/licenses/error_prone_annotations-2.28.0.jar.sha1 b/solr/licenses/error_prone_annotations-2.28.0.jar.sha1
diff --git a/solr/licenses/error_prone_annotations-2.31.0.jar.sha1 b/solr/licenses/error_prone_annotations-2.31.0.jar.sha1
@@ -0,0 +1 @@
+c3ba307b915d6d506e98ffbb49e6d2d12edad65b
diff --git a/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/Feature.java b/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/Feature.java
@@ -302,7 +302,7 @@ public String toString() {
     }
 
     /** A 'recipe' for computing a feature */
-    public abstract class FeatureScorer extends Scorer {
+    public abstract static class FeatureScorer extends Scorer {
 
       protected final String name;
       private DocInfo docInfo;
@@ -342,7 +342,7 @@ public DocIdSetIterator iterator() {
      * A <code>FeatureScorer</code> that contains a <code>Scorer</code>, which it delegates to where
      * appropriate.
      */
-    public abstract class FilterFeatureScorer extends FeatureScorer {
+    public abstract static class FilterFeatureScorer extends FeatureScorer {
 
       protected final Scorer in;
 
@@ -380,7 +380,7 @@ public float getMaxScore(int upTo) throws IOException {
      * Default FeatureScorer class that returns the score passed in. Can be used as a simple
      * ValueFeature, or to return a default scorer in case an underlying feature's scorer is null.
      */
-    public class ValueFeatureScorer extends FeatureScorer {
+    public static class ValueFeatureScorer extends FeatureScorer {
       float constScore;
 
       public ValueFeatureScorer(FeatureWeight weight, float constScore, DocIdSetIterator itr) {

diff --git a/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java b/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java
@@ -91,7 +91,7 @@ public FeatureScorer scorer(LeafReaderContext context) throws IOException {
       return new OriginalScoreScorer(this, originalScorer);
     }
 
-    public class OriginalScoreScorer extends FilterFeatureScorer {
+    public static class OriginalScoreScorer extends FilterFeatureScorer {
 
       public OriginalScoreScorer(FeatureWeight weight, Scorer originalScorer) {
         super(weight, originalScorer);

diff --git a/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java b/solr/modules/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java
@@ -234,7 +234,7 @@ public FeatureScorer scorer(LeafReaderContext context) throws IOException {
     }
 
     /** Scorer for a SolrFeature */
-    public class SolrFeatureScorer extends FilterFeatureScorer {
+    public static class SolrFeatureScorer extends FilterFeatureScorer {
 
       public SolrFeatureScorer(FeatureWeight weight, Scorer solrScorer) {
         super(weight, solrScorer);

diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
@@ -84,6 +84,9 @@ public class SolrZkClient implements Closeable {
   private Compressor compressor;
   private int minStateByteLenForCompression;
 
+  // Allow method reference to return a reference to a functional interface (Mapwriter),
+  // rather than a reference to a ZkMetrics object
+  @SuppressWarnings("UnnecessaryMethodReference")
   public MapWriter getMetrics() {
     return metrics::writeMap;
   }

diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java
@@ -110,6 +110,9 @@ public void simple() throws IOException {
     assertEquals("b", updateUnmarshalled.getParams().get("a"));
   }
 
+  // Allow method reference to return a reference to a functional interface (Iterable<String>),
+  // rather than a reference to a List<String> object
+  @SuppressWarnings("UnnecessaryMethodReference")
   @Test
   public void testIterable() throws IOException {
     final List<String> values = new ArrayList<>();

diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java b/solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java
@@ -267,7 +267,7 @@ private void updateForFire(WatchedEvent event) {
       }
     }
 
-    private class TestZKDatabase extends ZKDatabase {
+    private static class TestZKDatabase extends ZKDatabase {
 
       private final WatchLimiter limiter;
 

diff --git a/versions.lock b/versions.lock
@@ -46,7 +46,7 @@ com.google.cloud:google-cloud-core-grpc:2.40.0 (1 constraints: 1a1001a6)
 com.google.cloud:google-cloud-core-http:2.40.0 (1 constraints: 1a1001a6)
 com.google.cloud:google-cloud-storage:2.40.1 (2 constraints: cf1cc626)
 com.google.code.gson:gson:2.11.0 (7 constraints: 88614316)
-com.google.errorprone:error_prone_annotations:2.28.0 (15 constraints: a5c51259)
+com.google.errorprone:error_prone_annotations:2.31.0 (15 constraints: a5c51259)
 com.google.guava:failureaccess:1.0.2 (2 constraints: fb19bf37)
 com.google.guava:guava:33.1.0-jre (26 constraints: 0280374a)
 com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (2 constraints: 4b35b0a0)

diff --git a/versions.props b/versions.props
@@ -10,7 +10,7 @@ com.github.ben-manes.caffeine:caffeine=3.1.8
 com.github.spotbugs:*=4.8.6
 com.github.stephenc.jcip:jcip-annotations=1.0-1
 com.google.cloud:google-cloud-bom=0.224.0
-com.google.errorprone:*=2.23.0
+com.google.errorprone:*=2.31.0
 com.google.guava:guava=32.1.3-jre
 com.google.re2j:re2j=1.7
 com.j256.simplemagic:simplemagic=1.17