Upgrade Errorprone (#3851)

* Upgrade Errorprone

Upgrade errorprone to 2.13.1.  Like all errorprone upgrades there are
new checks requiring code changes.

* Unused methods now cause compilation errors
* fields must be static and final
* Effectively constant booleans must now be returned as true/false.
* longs should not auto-cast to double.
* turn off errorprone javadocs

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/OpenTelemetryAcceptanceTest.java

@@ -60,7 +60,7 @@ public class OpenTelemetryAcceptanceTest extends AcceptanceTestBase {
 
   private static final class FakeCollector extends TraceServiceGrpc.TraceServiceImplBase {
     private final List<ResourceSpans> receivedSpans = new ArrayList<>();
-    private Status returnedStatus = Status.OK;
+    private final Status returnedStatus = Status.OK;
 
     @Override
     public void export(
@@ -82,16 +82,12 @@ public class OpenTelemetryAcceptanceTest extends AcceptanceTestBase {
     List<ResourceSpans> getReceivedSpans() {
       return receivedSpans;
     }
-
-    void setReturnedStatus(final Status returnedStatus) {
-      this.returnedStatus = returnedStatus;
-    }
   }
 
   private static final class FakeMetricsCollector
       extends MetricsServiceGrpc.MetricsServiceImplBase {
     private final List<ResourceMetrics> receivedMetrics = new ArrayList<>();
-    private Status returnedStatus = Status.OK;
+    private final Status returnedStatus = Status.OK;
 
     @Override
     public void export(
@@ -114,10 +110,6 @@ public class OpenTelemetryAcceptanceTest extends AcceptanceTestBase {
     List<ResourceMetrics> getReceivedMetrics() {
       return receivedMetrics;
     }
-
-    void setReturnedStatus(final Status returnedStatus) {
-      this.returnedStatus = returnedStatus;
-    }
   }
 
   private final FakeMetricsCollector fakeMetricsCollector = new FakeMetricsCollector();

besu/src/main/java/org/hyperledger/besu/cli/subcommands/PublicKeySubCommand.java

@@ -154,6 +154,7 @@ public class PublicKeySubCommand implements Runnable {
                 + SignatureAlgorithmType.DEFAULT_EC_CURVE_NAME
                 + ")",
         arity = "0..1")
+    @SuppressWarnings("FieldCanBeFinal")
     protected String ecCurve = null;
 
     @Spec private final CommandSpec spec = null;

config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java

@@ -21,7 +21,6 @@ import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
 
 import java.math.BigInteger;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -80,11 +79,6 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions {
     return new TransitionsConfigOptions(transitionsNode.get());
   }
 
-  private JsonGenesisConfigOptions(
-      final ObjectNode maybeConfig, final TransitionsConfigOptions transitionsConfig) {
-    this(maybeConfig, Collections.emptyMap(), transitionsConfig);
-  }
-
   JsonGenesisConfigOptions(
       final ObjectNode maybeConfig,
       final Map<String, String> configOverrides,

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/statemachine/IbftRound.java

@@ -183,7 +183,7 @@ public class IbftRound {
         commitSeal = createCommitSeal(block);
       } catch (final SecurityModuleException e) {
         LOG.warn("Failed to construct commit seal; {}", e.getMessage());
-        return blockAccepted;
+        return true;
       }
 
       // There are times handling a proposed block is enough to enter prepared.
@@ -202,7 +202,7 @@ public class IbftRound {
         roundState.addCommitMessage(localCommitMessage);
       } catch (final SecurityModuleException e) {
         LOG.warn("Failed to create signed Commit message; {}", e.getMessage());
-        return blockAccepted;
+        return true;
       }
 
       // It is possible sufficient commit seals are now available and the block should be imported

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java

@@ -200,7 +200,7 @@ public class QbftRound {
         commitSeal = createCommitSeal(block);
       } catch (final SecurityModuleException e) {
         LOG.warn("Failed to construct commit seal; {}", e.getMessage());
-        return blockAccepted;
+        return true;
       }
 
       // There are times handling a proposed block is enough to enter prepared.
@@ -219,7 +219,7 @@ public class QbftRound {
         roundState.addCommitMessage(localCommitMessage);
       } catch (final SecurityModuleException e) {
         LOG.warn("Failed to create signed Commit message; {}", e.getMessage());
-        return blockAccepted;
+        return true;
       }
 
       // It is possible sufficient commit seals are now available and the block should be imported

errorprone-checks/build.gradle

@@ -45,3 +45,29 @@ dependencies {
 }
 
 test { testLogging { showStandardStreams = true } }
+
+
+tasks.withType(JavaCompile) {
+  options.compilerArgs += [
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED',
+    '--add-exports',
+    'jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED'
+  ]
+}
+
+javadoc {
+  enabled = false
+}

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/BannedMethod.java

@@ -33,7 +33,6 @@ import com.sun.source.tree.MethodInvocationTree;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "BannedMethod",
     summary = "Some methods should not be used, make sure that doesn't happen.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/DoNotCreateSecureRandomDirectly.java

@@ -30,7 +30,6 @@ import com.sun.tools.javac.code.Symbol;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "DoNotCreateSecureRandomDirectly",
     summary = "Do not create SecureRandom directly.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/DoNotInvokeMessageDigestDirectly.java

@@ -26,7 +26,6 @@ import com.sun.source.tree.MethodInvocationTree;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "DoNotInvokeMessageDigestDirectly",
     summary = "Do not invoke MessageDigest.getInstance directly.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/DoNotReturnNullOptionals.java

@@ -36,7 +36,6 @@ import com.sun.source.tree.Tree;
 
 @AutoService(BugChecker.class) // the service descriptor
 @BugPattern(
-    name = "DoNotReturnNullOptionals",
     summary = "Do not return null optionals.",
     severity = SUGGESTION,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/ExperimentalCliOptionMustBeCorrectlyDisplayed.java

@@ -34,7 +34,6 @@ import com.sun.tools.javac.tree.JCTree;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "ExperimentalCliOptionMustBeCorrectlyDisplayed",
     summary = "Experimental options must be hidden and not present in the BesuCommand class.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/MethodInputParametersMustBeFinal.java

@@ -32,7 +32,6 @@ import com.sun.source.tree.VariableTree;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "MethodInputParametersMustBeFinal",
     summary = "Method input parameters must be final.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/PrivateStaticFinalLoggers.java

@@ -41,7 +41,6 @@ import com.sun.tools.javac.code.Type;
 
 @AutoService(BugChecker.class)
 @BugPattern(
-    name = "PrivateStaticFinalLoggers",
     summary = "Logger classes should be private, static, and final.",
     severity = WARNING,
     linkType = BugPattern.LinkType.NONE)

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueriesTest.java

@@ -569,7 +569,6 @@ public class BlockchainQueriesTest {
     final List<BlockData> blockData;
     final WorldStateArchive worldStateArchive;
     final BlockchainQueries blockchainQueries;
-    final EthScheduler scheduler;
 
     private BlockchainWithData(
         final MutableBlockchain blockchain,
@@ -579,7 +578,6 @@ public class BlockchainQueriesTest {
       this.blockchain = blockchain;
       this.blockData = blockData;
       this.worldStateArchive = worldStateArchive;
-      this.scheduler = scheduler;
       this.blockchainQueries = new BlockchainQueries(blockchain, worldStateArchive, scheduler);
     }
   }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolScheduleBuilder.java

@@ -55,10 +55,6 @@ public class ProtocolScheduleBuilder {
     public ProtocolSpecBuilder getBuilder() {
       return builder;
     }
-
-    public Function<ProtocolSpecBuilder, ProtocolSpecBuilder> getModifier() {
-      return modifier;
-    }
   }
 
   private static final Logger LOG = LoggerFactory.getLogger(ProtocolScheduleBuilder.class);

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/feemarket/BaseFeeMarketBaseFeeTest.java

@@ -89,6 +89,7 @@ public class BaseFeeMarketBaseFeeTest {
         .isEqualTo(expectedBaseFee);
   }
 
+  @SuppressWarnings("unused")
   private static class BaseFeeMarketBaseFeeTestCase {
 
     private Wei parentBaseFee;
@@ -96,35 +97,6 @@ public class BaseFeeMarketBaseFeeTest {
     private long parentTargetGasUsed;
     private Wei expectedBaseFee;
 
-    public BaseFeeMarketBaseFeeTestCase() {}
-
-    public BaseFeeMarketBaseFeeTestCase(
-        final Wei parentBaseFee,
-        final long parentGasUsed,
-        final long parentTargetGasUsed,
-        final Wei expectedBaseFee) {
-      this.parentBaseFee = parentBaseFee;
-      this.parentGasUsed = parentGasUsed;
-      this.parentTargetGasUsed = parentTargetGasUsed;
-      this.expectedBaseFee = expectedBaseFee;
-    }
-
-    public Wei getParentBaseFee() {
-      return parentBaseFee;
-    }
-
-    public long getParentGasUsed() {
-      return parentGasUsed;
-    }
-
-    public long getParentTargetGasUsed() {
-      return parentTargetGasUsed;
-    }
-
-    public Wei getExpectedBaseFee() {
-      return expectedBaseFee;
-    }
-
     public void setParentBaseFee(final Wei parentBaseFee) {
       this.parentBaseFee = parentBaseFee;
     }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DetermineCommonAncestorTaskTest.java

@@ -164,7 +164,7 @@ public class DetermineCommonAncestorTaskTest {
     final long range = maximumPossibleCommonAncestorNumber - minimumPossibleCommonAncestorNumber;
     final int skipInterval =
         DetermineCommonAncestorTask.calculateSkipInterval(range, headerRequestSize);
-    final int count = DetermineCommonAncestorTask.calculateCount(range, skipInterval);
+    final int count = DetermineCommonAncestorTask.calculateCount((double) range, skipInterval);
 
     assertThat(count).isEqualTo(11);
     assertThat(skipInterval).isEqualTo(9);

ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/AllowlistPersistor.java

@@ -73,8 +73,7 @@ public class AllowlistPersistor {
             ? configItems.get(allowlistType)
             : Collections.emptyList();
 
-    boolean listsMatch = existingValues.containsAll(checkLists);
+    if (!existingValues.containsAll(checkLists)) {
-    if (!listsMatch) {
       debugLambda(
           LOG,
           "\n LISTS DO NOT MATCH configFile::",
@@ -83,7 +82,7 @@ public class AllowlistPersistor {
       debugLambda(LOG, "\nLISTS DO NOT MATCH in-memory ::", checkLists::toString);
       throw new AllowlistFileSyncException();
     }
-    return listsMatch;
+    return true;
   }
 
   public boolean verifyConfigFileMatchesState(

ethereum/trie/src/main/java/org/hyperledger/besu/ethereum/trie/TrieNodeDecoder.java

@@ -126,11 +126,6 @@ public class TrieNodeDecoder {
           .ifPresent(currentNodes::add);
     }
 
-    public static BreadthFirstIterator create(
-        final NodeLoader nodeLoader, final Bytes32 rootHash, final int maxDepth) {
-      return new BreadthFirstIterator(nodeLoader, rootHash, maxDepth);
-    }
-
     @Override
     public boolean hasNext() {
       return !currentNodes.isEmpty() && currentDepth <= maxDepth;

gradle/versions.gradle

@@ -32,7 +32,7 @@ dependencyManagement {
       entry'dagger'
     }
 
-    dependencySet(group: 'com.google.errorprone', version: '2.10.0') {
+    dependencySet(group: 'com.google.errorprone', version: '2.13.1') {
       entry 'error_prone_annotation'
       entry 'error_prone_check_api'
       entry 'error_prone_core'

metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java

@@ -277,7 +277,7 @@ public class OpenTelemetrySystem implements ObservableMetricsSystem {
             resultLongObserver -> {
               for (int i = 0; i < garbageCollectors.size(); i++) {
                 resultLongObserver.observe(
-                    garbageCollectors.get(i).getCollectionTime(), labelSets.get(i));
+                    (double) garbageCollectors.get(i).getCollectionTime(), labelSets.get(i));
               }
             });
     final AttributeKey<String> typeKey = AttributeKey.stringKey(TYPE_LABEL_KEY);

metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryTimer.java

@@ -51,7 +51,7 @@ public class OpenTelemetryTimer implements LabelledMetric<OperationTimer> {
         meter
             .gaugeBuilder(metricName)
             .setDescription(help)
-            .buildWithCallback((measurement) -> measurement.observe(elapsed, labels));
+            .buildWithCallback((measurement) -> measurement.observe((double) elapsed, labels));
         return elapsed / 1e9;
       };
     };

metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusCounter.java

@@ -44,7 +44,7 @@ class PrometheusCounter implements LabelledMetric<Counter> {
 
     @Override
     public void inc(final long amount) {
-      counter.inc(amount);
+      counter.inc((double) amount);
     }
   }
 }