build - Refactor Besu custom error prone dependency (hyperledger#6692)

Move Besu custom error-prone checks into its own repository and use it as an external dependency. This allows to move to a newer version of Google errorprone checks as well while cleaning up build.gradle file. Key changes resulted due to this change: * String toLowerCase and toUpperCase to use Locale.ROOT as argument * Use interface such as List,Map or NavigatableMap instead of concrete class where appropriate. * Simplify StringBuilder to plain String * Suppress warnings where appropriate. ----- Signed-off-by: Usman Saleem <usman@usmans.info> Signed-off-by: amsmota <antonio.mota@citi.com>
Citi · Apr 16, 2024 · d452127 · d452127
1 parent cc81a34
commit d452127
Show file tree

Hide file tree

Showing 109 changed files with 325 additions and 2,011 deletions.
diff --git a/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java b/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java
@@ -62,6 +62,7 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
@@ -431,7 +432,9 @@ public NodeRequests nodeRequests() {
           getGenesisConfig()
               .map(
                   gc ->
-                      gc.toLowerCase().contains("ibft") ? ConsensusType.IBFT2 : ConsensusType.QBFT)
+                      gc.toLowerCase(Locale.ROOT).contains("ibft")
+                          ? ConsensusType.IBFT2
+                          : ConsensusType.QBFT)
               .orElse(ConsensusType.IBFT2);
 
       nodeRequests =

diff --git a/...org/hyperledger/besu/tests/acceptance/dsl/transaction/DeploySmartContractTransaction.java b/...org/hyperledger/besu/tests/acceptance/dsl/transaction/DeploySmartContractTransaction.java
@@ -20,6 +20,7 @@
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.stream.Collectors;
 
 import org.web3j.crypto.Credentials;
@@ -83,7 +84,7 @@ && parameterTypesAreEqual(i.getParameterTypes(), parameterObjects))
 
   @SuppressWarnings("rawtypes")
   private boolean parameterTypesAreEqual(
-      final Class<?>[] expectedTypes, final ArrayList<Object> actualObjects) {
+      final Class<?>[] expectedTypes, final List<Object> actualObjects) {
     if (expectedTypes.length != actualObjects.size()) {
       return false;
     }

diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
@@ -133,6 +133,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -800,7 +801,7 @@ public Runner build() {
               metricsSystem,
               supportedCapabilities,
               jsonRpcConfiguration.getRpcApis().stream()
-                  .filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
+                  .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
                   .collect(Collectors.toList()),
               filterManager,
               accountLocalConfigPermissioningController,
@@ -938,7 +939,7 @@ public Runner build() {
               metricsSystem,
               supportedCapabilities,
               webSocketConfiguration.getRpcApis().stream()
-                  .filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
+                  .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
                   .collect(Collectors.toList()),
               filterManager,
               accountLocalConfigPermissioningController,
@@ -1021,7 +1022,7 @@ public Runner build() {
               metricsSystem,
               supportedCapabilities,
               jsonRpcIpcConfiguration.getEnabledApis().stream()
-                  .filter(apiGroup -> !apiGroup.toLowerCase().startsWith("engine"))
+                  .filter(apiGroup -> !apiGroup.toLowerCase(Locale.ROOT).startsWith("engine"))
                   .collect(Collectors.toList()),
               filterManager,
               accountLocalConfigPermissioningController,

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -2602,10 +2602,7 @@ private void instantiateSignatureAlgorithmFactory() {
       SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
     } catch (final IllegalArgumentException e) {
       throw new CommandLine.InitializationException(
-          new StringBuilder()
-              .append("Invalid genesis file configuration for ecCurve. ")
-              .append(e.getMessage())
-              .toString());
+          "Invalid genesis file configuration for ecCurve. " + e.getMessage());
     }
   }
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java b/besu/src/main/java/org/hyperledger/besu/cli/config/NetworkName.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.cli.config;
 
 import java.math.BigInteger;
+import java.util.Locale;
 import java.util.Optional;
 
 import org.apache.commons.lang3.StringUtils;
@@ -91,7 +92,7 @@ public boolean canSnapSync() {
    * @return the string
    */
   public String normalize() {
-    return StringUtils.capitalize(name().toLowerCase());
+    return StringUtils.capitalize(name().toLowerCase(Locale.ROOT));
   }
 
   /**

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/config/ProfileName.java b/besu/src/main/java/org/hyperledger/besu/cli/config/ProfileName.java
@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.cli.config;
 
+import java.util.Locale;
+
 import org.apache.commons.lang3.StringUtils;
 
 /** Enum for profile names. Each profile corresponds to a configuration file. */
@@ -51,6 +53,6 @@ public String getConfigFile() {
 
   @Override
   public String toString() {
-    return StringUtils.capitalize(name().replaceAll("_", " ").toLowerCase());
+    return StringUtils.capitalize(name().replaceAll("_", " ").toLowerCase(Locale.ROOT));
   }
 }
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/MetricCategoryConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/MetricCategoryConverter.java
@@ -18,6 +18,7 @@
 
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -54,7 +55,7 @@ public <T extends Enum<T> & MetricCategory> void addCategories(final Class<T> ca
    * @param metricCategory the metric category
    */
   public void addRegistryCategory(final MetricCategory metricCategory) {
-    metricCategories.put(metricCategory.getName().toUpperCase(), metricCategory);
+    metricCategories.put(metricCategory.getName().toUpperCase(Locale.ROOT), metricCategory);
   }
 
   /**

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/DataStorageOptions.java
@@ -30,6 +30,7 @@
 import org.hyperledger.besu.plugin.services.storage.DataStorageFormat;
 
 import java.util.List;
+import java.util.Locale;
 
 import org.apache.commons.lang3.StringUtils;
 import picocli.CommandLine;
@@ -193,6 +194,6 @@ public List<String> getCLIOptions() {
    * @return the normalized string
    */
   public String normalizeDataStorageFormat() {
-    return StringUtils.capitalize(dataStorageFormat.toString().toLowerCase());
+    return StringUtils.capitalize(dataStorageFormat.toString().toLowerCase(Locale.ROOT));
   }
 }
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/LoggingLevelOption.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/LoggingLevelOption.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.cli.options.stable;
 
+import java.util.Locale;
 import java.util.Set;
 
 import picocli.CommandLine;
@@ -52,8 +53,8 @@ public void setLogLevel(final String logLevel) {
     if ("FATAL".equalsIgnoreCase(logLevel)) {
       System.out.println("FATAL level is deprecated");
       this.logLevel = "ERROR";
-    } else if (ACCEPTED_VALUES.contains(logLevel.toUpperCase())) {
-      this.logLevel = logLevel.toUpperCase();
+    } else if (ACCEPTED_VALUES.contains(logLevel.toUpperCase(Locale.ROOT))) {
+      this.logLevel = logLevel.toUpperCase(Locale.ROOT);
     } else {
       throw new CommandLine.ParameterException(
           spec.commandLine(), "Unknown logging value: " + logLevel);

diff --git a/...src/main/java/org/hyperledger/besu/cli/subcommands/operator/GenerateBlockchainConfig.java b/...src/main/java/org/hyperledger/besu/cli/subcommands/operator/GenerateBlockchainConfig.java
@@ -178,11 +178,9 @@ private void importPublicKey(final JsonNode publicKeyJson) {
 
       if (!SIGNATURE_ALGORITHM.get().isValidPublicKey(publicKey)) {
         throw new IllegalArgumentException(
-            new StringBuilder()
-                .append(publicKeyText)
-                .append(" is not a valid public key for elliptic curve ")
-                .append(SIGNATURE_ALGORITHM.get().getCurveName())
-                .toString());
+            publicKeyText
+                + " is not a valid public key for elliptic curve "
+                + SIGNATURE_ALGORITHM.get().getCurveName());
       }
 
       writeKeypair(publicKey, null);
@@ -297,10 +295,7 @@ private void processEcCurve() {
       SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
     } catch (IllegalArgumentException e) {
       throw new IllegalArgumentException(
-          new StringBuilder()
-              .append("Invalid parameter for ecCurve in genesis config: ")
-              .append(e.getMessage())
-              .toString());
+          "Invalid parameter for ecCurve in genesis config: " + e.getMessage());
     }
   }
 

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java
@@ -249,6 +249,7 @@ protected Synchronizer createSynchronizer(
     return sync;
   }
 
+  @SuppressWarnings("UnusedVariable")
   private void initTransitionWatcher(
       final ProtocolContext protocolContext, final TransitionCoordinator composedCoordinator) {
 

diff --git a/besu/src/main/java/org/hyperledger/besu/services/RpcEndpointServiceImpl.java b/besu/src/main/java/org/hyperledger/besu/services/RpcEndpointServiceImpl.java
@@ -24,6 +24,7 @@
 
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -58,7 +59,10 @@ public <T> void registerRPCEndpoint(
                 namespaces.stream()
                     .anyMatch(
                         namespace ->
-                            entry.getKey().toUpperCase().startsWith(namespace.toUpperCase())))
+                            entry
+                                .getKey()
+                                .toUpperCase(Locale.ROOT)
+                                .startsWith(namespace.toUpperCase(Locale.ROOT))))
         .map(entry -> new PluginJsonRpcMethod(entry.getKey(), entry.getValue()))
         .collect(Collectors.toMap(PluginJsonRpcMethod::getName, e -> e));
   }
@@ -71,6 +75,7 @@ public <T> void registerRPCEndpoint(
    */
   public boolean hasNamespace(final String namespace) {
     return rpcMethods.keySet().stream()
-        .anyMatch(key -> key.toUpperCase().startsWith(namespace.toUpperCase()));
+        .anyMatch(
+            key -> key.toUpperCase(Locale.ROOT).startsWith(namespace.toUpperCase(Locale.ROOT)));
   }
 }
diff --git a/build.gradle b/build.gradle
@@ -29,7 +29,7 @@ plugins {
   id 'com.jfrog.artifactory' version '5.1.11'
   id 'io.spring.dependency-management' version '1.1.4'
   id 'me.champeau.jmh' version '0.7.2' apply false
-  id 'net.ltgt.errorprone' version '3.0.1'
+  id 'net.ltgt.errorprone' version '3.1.0'
   id 'maven-publish'
   id 'org.sonarqube' version '4.4.1.3373'
 }
@@ -132,12 +132,12 @@ allprojects {
   }
 
   task sourcesJar(type: Jar, dependsOn: classes) {
-    classifier = 'sources'
+    archiveClassifier = 'sources'
     from sourceSets.main.allSource
   }
 
   task javadocJar(type: Jar, dependsOn: javadoc) {
-    classifier = 'javadoc'
+    archiveClassifier = 'javadoc'
     from javadoc.outputDirectory
   }
 
@@ -160,6 +160,7 @@ allprojects {
       url 'https://splunk.jfrog.io/splunk/ext-releases-local'
       content { includeGroupByRegex('com\\.splunk\\..*') }
     }
+
     mavenCentral()
 
     // ethereum execution spec tests fixtures. Exclusively for ethereum submodule to run ref tests
@@ -181,6 +182,8 @@ allprojects {
   dependencies {
     components.all(BouncyCastleCapability)
     errorprone 'com.google.errorprone:error_prone_core'
+    // https://github.com/hyperledger/besu-errorprone-checks/
+    errorprone "org.hyperledger.besu:besu-errorprone-checks"
   }
 
   configurations.all {
@@ -216,7 +219,7 @@ allprojects {
     format 'sol', { target '**/*.sol' }
   }
 
-  tasks.withType(JavaCompile) {
+  tasks.withType(JavaCompile).configureEach {
     options.compilerArgs += [
       '-Xlint:unchecked',
       '-Xlint:cast',
@@ -229,8 +232,8 @@ allprojects {
     ]
 
     options.errorprone {
-      excludedPaths = '.*/(generated/*.*|.*ReferenceTest_.*|build/.*/annotation-output/.*)'
-
+      excludedPaths = '.*/generated/*.*'
+      disableWarningsInGeneratedCode = true
       // Our equals need to be symmetric, this checker doesn't respect that.
       check('EqualsGetClass', CheckSeverity.OFF)
       // We like to use futures with no return values.
@@ -292,7 +295,7 @@ allprojects {
    *
    */
   test {
-    jvmArgs = [
+    jvmArgs += [
       '-Xmx4g',
       '-XX:-UseGCOverheadLimit',
       // Mockito and jackson-databind do some strange reflection during tests.
@@ -397,7 +400,7 @@ subprojects {
 
     task testSupportJar(type: Jar) {
       archiveBaseName = "${project.name}-support-test"
-      classifier = 'test-support'
+      archiveClassifier = 'test-support'
       from sourceSets.testSupport.output
     }
   }
@@ -997,7 +1000,7 @@ task checkSpdxHeader(type: CheckSpdxHeader) {
 
 jacocoTestReport {
   reports {
-    xml.enabled true
+    xml.required = true
   }
 }
 
@@ -1008,25 +1011,12 @@ task jacocoRootReport(type: org.gradle.testing.jacoco.tasks.JacocoReport) {
   executionData.from fileTree(dir: '.', includes: ['**/jacoco/*.exec'])
   reports {
     xml.required = true
-    xml.enabled = true
     csv.required = true
     html.destination file("build/reports/jacocoHtml")
   }
   onlyIf = { true }
 }
 
-configurations { annotationProcessor }
-
-// Prevent errorprone-checks being dependent upon errorprone-checks!
-// However, ensure all subprojects comply with the custom rules.
-configure(subprojects.findAll { it.name != 'errorprone-checks' }) {
-  dependencies { annotationProcessor project(":errorprone-checks") }
-
-  tasks.withType(JavaCompile) {
-    options.annotationProcessorPath = configurations.annotationProcessor
-  }
-}
-
 // http://label-schema.org/rc1/
 // using the RFC3339 format "2016-04-12T23:20:50.52Z"
 def buildTime() {
@@ -1107,9 +1097,11 @@ tasks.register("verifyDistributions") {
 }
 
 dependencies {
+  errorprone 'com.google.errorprone:error_prone_core'
+  // https://github.com/hyperledger/besu-errorprone-checks/
+  errorprone 'org.hyperledger.besu:besu-errorprone-checks'
   implementation project(':besu')
   implementation project(':ethereum:evmtool')
-  errorprone 'com.google.errorprone:error_prone_core'
 }
 
 @CompileStatic

diff --git a/config/src/main/java/org/hyperledger/besu/config/BftFork.java b/config/src/main/java/org/hyperledger/besu/config/BftFork.java
@@ -18,6 +18,7 @@
 
 import java.math.BigInteger;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.OptionalInt;
 
@@ -101,7 +102,7 @@ public Optional<BigInteger> getBlockRewardWei() {
       return Optional.empty();
     }
     final String weiStr = configFileContent.get();
-    if (weiStr.toLowerCase().startsWith("0x")) {
+    if (weiStr.toLowerCase(Locale.ROOT).startsWith("0x")) {
       return Optional.of(new BigInteger(1, Bytes.fromHexStringLenient(weiStr).toArrayUnsafe()));
     }
     return Optional.of(new BigInteger(weiStr));

diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonBftConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/JsonBftConfigOptions.java
@@ -17,6 +17,7 @@
 import org.hyperledger.besu.datatypes.Address;
 
 import java.math.BigInteger;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 
@@ -125,7 +126,7 @@ public BigInteger getBlockRewardWei() {
       return BigInteger.ZERO;
     }
     final String weiStr = configFileContent.get();
-    if (weiStr.toLowerCase().startsWith("0x")) {
+    if (weiStr.toLowerCase(Locale.ROOT).startsWith("0x")) {
       return new BigInteger(1, Bytes.fromHexStringLenient(weiStr).toArrayUnsafe());
     }
     return new BigInteger(weiStr);