Errorprone version upgrade (#1096)

Upgrade to ErrorProne 2.4.0

* public constructors on abstract classes are removed
* Javadoc must have meaningfull documentation
* lambdas should not be variables
* Added to the list of confusing inner class names (Entry and Type)
* no assert keyword in tests
* Obsolete JDK classes produce errors now

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

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/clique/CliqueConditions.java

@@ -33,7 +33,6 @@
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableMap;
@@ -126,7 +125,7 @@ public ProposalsConfig removeProposal(final BesuNode node) {
     public Condition build() {
       final Map<Address, Boolean> proposalsAsAddress =
           this.proposals.entrySet().stream()
-              .collect(Collectors.toMap(p -> p.getKey().getAddress(), Entry::getValue));
+              .collect(Collectors.toMap(p -> p.getKey().getAddress(), Map.Entry::getValue));
       return new ExpectProposals(clique, proposalsAsAddress);
     }
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/ibft2/Ibft2Conditions.java

@@ -27,7 +27,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableMap;
@@ -88,7 +87,7 @@ public PendingVotesConfig removeProposal(final BesuNode node) {
     public Condition build() {
       final Map<Address, Boolean> proposalsAsAddress =
           this.proposals.entrySet().stream()
-              .collect(Collectors.toMap(p -> p.getKey().getAddress(), Entry::getValue));
+              .collect(Collectors.toMap(p -> p.getKey().getAddress(), Map.Entry::getValue));
       return new ExpectProposals(ibft, proposalsAsAddress);
     }
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/login/LoginRequestFactory.java

@@ -29,7 +29,6 @@
 public class LoginRequestFactory {
 
   private static final MediaType JSON = MediaType.parse("application/json; charset=utf-8");
-  private static final String AUTHENTICATION_REQUEST = "{\"username\":\"%s\",\"password\":\"%s\"}";
   private static final String LOGIN_PATH = "/login";
 
   private final String contextRootUrl;
@@ -74,7 +73,8 @@ String send(final String username, final String password) throws IOException {
 
   private Request loginRequest(final String username, final String password) {
     final RequestBody requestBody =
-        RequestBody.create(JSON, String.format(AUTHENTICATION_REQUEST, username, password));
+        RequestBody.create(
+            JSON, String.format("{\"username\":\"%s\",\"password\":\"%s\"}", username, password));
     return new Request.Builder().post(requestBody).url(loginUri()).build();
   }
 

besu/src/main/java/org/hyperledger/besu/Runner.java

@@ -271,6 +271,7 @@ private void writePidFile() {
           try {
             pid = Long.toString(ProcessHandle.current().pid());
           } catch (Throwable t) {
+            LOG.error("Error retrieving PID", t);
           }
           try {
             Files.write(

besu/src/main/java/org/hyperledger/besu/cli/options/CLIOptions.java

@@ -30,6 +30,10 @@ public interface CLIOptions<T> {
    */
   T toDomainObject();
 
-  /** @return The list of CLI options corresponding to this class. */
+  /**
+   * Return The list of CLI options corresponding to this class.
+   *
+   * @return The list of CLI options corresponding to this class.
+   */
   List<String> getCLIOptions();
 }

besu/src/main/java/org/hyperledger/besu/cli/subcommands/operator/GenerateBlockchainConfig.java

@@ -194,7 +194,7 @@ private void generateNodeKeypair(final int node) {
    *
    * @param publicKey The public key.
    * @param privateKey The private key. No file is created if privateKey is NULL.
-   * @throws IOException
+   * @throws IOException If the file cannot be written or accessed.
    */
   private void writeKeypair(
       final SECP256K1.PublicKey publicKey, final SECP256K1.PrivateKey privateKey)
@@ -231,7 +231,7 @@ private void createFileAndWrite(final Path directory, final String fileName, fin
   /**
    * Parses the root configuration file and related sub elements.
    *
-   * @throws IOException
+   * @throws IOException If the file cannot be read or accessed.
    */
   private void parseConfig() throws IOException {
     final String configString =
@@ -251,7 +251,7 @@ private void parseConfig() throws IOException {
   /**
    * Checks if the output directory exists.
    *
-   * @throws IOException
+   * @throws IOException If the cannot be accessed or created.
    */
   private void handleOutputDirectory() throws IOException {
     checkNotNull(outputDirectory);
@@ -274,7 +274,7 @@ private void handleOutputDirectory() throws IOException {
    * @param directory The directory to write the file to.
    * @param fileName The name of the output file.
    * @param genesis The genesis content.
-   * @throws IOException
+   * @throws IOException If the genesis file cannot be written or accessed.
    */
   private void writeGenesisFile(
       final File directory, final String fileName, final ObjectNode genesis) throws IOException {

besu/src/test/java/org/hyperledger/besu/chainimport/JsonBlockImporterTest.java

@@ -66,7 +66,7 @@ public abstract class JsonBlockImporterTest {
   protected final GenesisConfigFile genesisConfigFile;
   protected final boolean isEthash;
 
-  public JsonBlockImporterTest(final String consensusEngine) throws IOException {
+  protected JsonBlockImporterTest(final String consensusEngine) throws IOException {
     this.consensusEngine = consensusEngine;
     final String genesisData = getFileContents("genesis.json");
     this.genesisConfigFile = GenesisConfigFile.fromConfig(genesisData);

besu/src/test/java/org/hyperledger/besu/cli/operator/OperatorSubCommandTest.java

@@ -229,7 +229,7 @@ private void runCmdAndCheckOutput(
     final File keysDirectory = new File(expectedKeysPath.toUri());
     assertThat(keysDirectory).exists();
     final File[] nodesKeysFolders = keysDirectory.listFiles();
-    assert nodesKeysFolders != null;
+    assertThat(nodesKeysFolders).isNotNull();
     if (generate) {
       final JsonFactory jsonFactory = new JsonFactory();
       final JsonParser jp = jsonFactory.createParser(configFilePath);

besu/src/test/java/org/hyperledger/besu/util/StringUtilsTest.java

@@ -20,7 +20,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
 import org.junit.Test;
@@ -35,7 +34,7 @@ public void joiningWithLastDelimiter() {
     testCases.put(Arrays.asList("item1", "item2"), "item1 and item2");
     testCases.put(Arrays.asList("item1", "item2", "item3"), "item1, item2 and item3");
 
-    for (Entry<List<String>, String> entry : testCases.entrySet()) {
+    for (Map.Entry<List<String>, String> entry : testCases.entrySet()) {
       String joinedResult =
           entry.getKey().stream()
               .collect(

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/VoteTally.java

@@ -21,17 +21,17 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.NavigableSet;
 import java.util.Optional;
 import java.util.Set;
-import java.util.SortedSet;
 import java.util.TreeSet;
 
 import com.google.common.collect.Maps;
 
 /** Tracks the current list of validators and votes to add or drop validators. */
 public class VoteTally implements ValidatorProvider {
 
-  private final SortedSet<Address> currentValidators;
+  private final NavigableSet<Address> currentValidators;
 
   private final Map<Address, Set<Address>> addVotesBySubject;
   private final Map<Address, Set<Address>> removeVotesBySubject;

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractGetSignerMetricsMethod.java

@@ -41,7 +41,7 @@ public abstract class AbstractGetSignerMetricsMethod {
   private final BlockInterface blockInterface;
   private final BlockchainQueries blockchainQueries;
 
-  public AbstractGetSignerMetricsMethod(
+  protected AbstractGetSignerMetricsMethod(
       final VoteTallyCache voteTallyCache,
       final BlockInterface blockInterface,
       final BlockchainQueries blockchainQueries) {

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/SizeLimitedMap.java

@@ -15,7 +15,7 @@
 package org.hyperledger.besu.consensus.ibft;
 
 import java.util.LinkedHashMap;
-import java.util.Map.Entry;
+import java.util.Map;
 
 /**
  * Map that is limited to a specified size and will evict oldest entries when the size limit is
@@ -29,7 +29,7 @@ public SizeLimitedMap(final int maxEntries) {
   }
 
   @Override
-  protected boolean removeEldestEntry(final Entry<K, V> ignored) {
+  protected boolean removeEldestEntry(final Map.Entry<K, V> ignored) {
     return size() > maxEntries;
   }
 }

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/blockcreation/ProposerSelector.java

@@ -29,7 +29,6 @@
 import java.util.List;
 import java.util.NavigableSet;
 import java.util.Optional;
-import java.util.SortedSet;
 import java.util.TreeSet;
 
 import org.apache.logging.log4j.LogManager;
@@ -111,7 +110,7 @@ private Address handleMissingProposer(
       final Collection<Address> validatorsForRound,
       final ConsensusRoundIdentifier roundIdentifier) {
     final NavigableSet<Address> validatorSet = new TreeSet<>(validatorsForRound);
-    final SortedSet<Address> latterValidators = validatorSet.tailSet(prevBlockProposer, false);
+    final NavigableSet<Address> latterValidators = validatorSet.tailSet(prevBlockProposer, false);
     final Address nextProposer;
     if (latterValidators.isEmpty()) {
       // i.e. prevBlockProposer was at the end of the validator list, so the right validator for

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/headervalidationrules/IbftValidatorsValidationRule.java

@@ -24,7 +24,7 @@
 import org.hyperledger.besu.ethereum.rlp.RLPException;
 
 import java.util.Collection;
-import java.util.SortedSet;
+import java.util.NavigableSet;
 import java.util.TreeSet;
 
 import com.google.common.collect.Iterables;
@@ -50,7 +50,7 @@ public boolean validate(
               .getVoteTallyAfterBlock(parent);
       final IbftExtraData ibftExtraData = IbftExtraData.decode(header);
 
-      final SortedSet<Address> sortedReportedValidators =
+      final NavigableSet<Address> sortedReportedValidators =
           new TreeSet<>(ibftExtraData.getValidators());
 
       if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) {

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/ibftevent/BlockTimerExpiry.java

@@ -15,7 +15,6 @@
 package org.hyperledger.besu.consensus.ibft.ibftevent;
 
 import org.hyperledger.besu.consensus.ibft.ConsensusRoundIdentifier;
-import org.hyperledger.besu.consensus.ibft.ibftevent.IbftEvents.Type;
 
 import java.util.Objects;
 
@@ -35,8 +34,8 @@ public BlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) {
   }
 
   @Override
-  public Type getType() {
-    return Type.BLOCK_TIMER_EXPIRY;
+  public IbftEvents.Type getType() {
+    return IbftEvents.Type.BLOCK_TIMER_EXPIRY;
   }
 
   public ConsensusRoundIdentifier getRoundIndentifier() {

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/ibftevent/NewChainHead.java

@@ -14,7 +14,6 @@
  */
 package org.hyperledger.besu.consensus.ibft.ibftevent;
 
-import org.hyperledger.besu.consensus.ibft.ibftevent.IbftEvents.Type;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 
 import java.util.Objects;
@@ -35,8 +34,8 @@ public NewChainHead(final BlockHeader newChainHeadHeader) {
   }
 
   @Override
-  public Type getType() {
-    return Type.NEW_CHAIN_HEAD;
+  public IbftEvents.Type getType() {
+    return IbftEvents.Type.NEW_CHAIN_HEAD;
   }
 
   @Override

consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftEventQueueTest.java

@@ -18,7 +18,7 @@
 import static org.assertj.core.api.Assertions.assertThatCode;
 
 import org.hyperledger.besu.consensus.ibft.ibftevent.IbftEvent;
-import org.hyperledger.besu.consensus.ibft.ibftevent.IbftEvents.Type;
+import org.hyperledger.besu.consensus.ibft.ibftevent.IbftEvents;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -31,7 +31,7 @@ public class IbftEventQueueTest {
 
   private static class DummyIbftEvent implements IbftEvent {
     @Override
-    public Type getType() {
+    public IbftEvents.Type getType() {
       return null;
     }
   }

consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/ProposerSelectorTest.java

@@ -31,11 +31,11 @@
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 
+import java.util.ArrayList;
 import java.util.Collection;
-import java.util.LinkedList;
+import java.util.List;
 import java.util.Optional;
 
-import com.google.common.collect.Lists;
 import org.junit.Test;
 
 public class ProposerSelectorTest {
@@ -69,9 +69,9 @@ private Blockchain createMockedBlockChainWithHeadOf(
    * @param countHigher The number of validators which have a lower address than localAddr
    * @return A sorted list of validators which matches parameters (including the localAddr).
    */
-  private LinkedList<Address> createValidatorList(
+  private List<Address> createValidatorList(
       final Address localAddr, final int countLower, final int countHigher) {
-    final LinkedList<Address> result = Lists.newLinkedList();
+    final List<Address> result = new ArrayList<>();
 
     // Note: Order of this list is irrelevant, is sorted by value later.
     result.add(localAddr);
@@ -93,7 +93,7 @@ public void roundRobinChangesProposerOnRoundZeroOfNextBlock() {
     final long PREV_BLOCK_NUMBER = 2;
     final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected
 
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 0, 4);
+    final List<Address> validatorList = createValidatorList(localAddr, 0, 4);
     final Blockchain blockchain =
         createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList);
 
@@ -112,7 +112,7 @@ public void lastValidatorInListValidatedPreviousBlockSoFirstIsNextProposer() {
     final long PREV_BLOCK_NUMBER = 2;
     final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected
 
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 4, 0);
+    final List<Address> validatorList = createValidatorList(localAddr, 4, 0);
     final Blockchain blockchain =
         createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList);
 
@@ -132,7 +132,7 @@ public void stickyProposerDoesNotChangeOnRoundZeroOfNextBlock() {
     final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(PREV_BLOCK_NUMBER + 1, 0);
 
     final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 4, 0);
+    final List<Address> validatorList = createValidatorList(localAddr, 4, 0);
     final Blockchain blockchain =
         createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList);
 
@@ -150,7 +150,7 @@ public void stickyProposerChangesOnSubsequentRoundsAtSameBlockHeight() {
 
     final Address localAddr = AddressHelpers.ofValue(10); // arbitrarily selected
 
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 4, 0);
+    final List<Address> validatorList = createValidatorList(localAddr, 4, 0);
     final Blockchain blockchain =
         createMockedBlockChainWithHeadOf(PREV_BLOCK_NUMBER, localAddr, validatorList);
 
@@ -174,7 +174,7 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenSticky() {
 
     // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is
     // removed)
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 2, 2);
+    final List<Address> validatorList = createValidatorList(localAddr, 2, 2);
     validatorList.remove(localAddr);
 
     // Note the signer of the Previous block was not included.
@@ -196,7 +196,7 @@ public void whenProposerSelfRemovesSelectsNextProposerInLineEvenWhenRoundRobin()
 
     // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is
     // removed)
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 2, 2);
+    final List<Address> validatorList = createValidatorList(localAddr, 2, 2);
     validatorList.remove(localAddr);
 
     final Blockchain blockchain =
@@ -217,7 +217,7 @@ public void proposerSelfRemovesAndHasHighestAddressNewProposerIsFirstInList() {
 
     // LocalAddr will be in index 2 - the next proposer will also be in 2 (as prev proposer is
     // removed)
-    final LinkedList<Address> validatorList = createValidatorList(localAddr, 4, 0);
+    final List<Address> validatorList = createValidatorList(localAddr, 4, 0);
     validatorList.remove(localAddr);
 
     // Note the signer of the Previous block was not included.

consensus/ibftlegacy/src/main/java/org/hyperledger/besu/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRule.java

@@ -27,7 +27,7 @@
 
 import java.util.Collection;
 import java.util.List;
-import java.util.SortedSet;
+import java.util.NavigableSet;
 import java.util.TreeSet;
 
 import com.google.common.collect.Iterables;
@@ -77,7 +77,7 @@ public boolean validate(
         }
       }
 
-      final SortedSet<Address> sortedReportedValidators =
+      final NavigableSet<Address> sortedReportedValidators =
           new TreeSet<>(ibftExtraData.getValidators());
 
       if (!Iterables.elementsEqual(ibftExtraData.getValidators(), sortedReportedValidators)) {