Add ErrorProne Plugin for Gradle (#81)

* Specify Locale.ROOT in toUpperCase method * Fix string format usage * Avoid double brace initialization in StorageCredentialCacheTest * Add ErrorProne Plugin for Gradle
apache · Aug 6, 2024 · 64bb35c · 64bb35c
1 parent ecea59a
commit 64bb35c
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 34 deletions.
diff --git a/build.gradle b/build.gradle
@@ -29,6 +29,7 @@ plugins {
     id "idea"
     id "eclipse"
     id "org.jetbrains.gradle.plugin.idea-ext" version "1.1.8"
+    id "net.ltgt.errorprone" version "4.0.1"
 }
 
 allprojects {
@@ -50,10 +51,14 @@ subprojects {
     apply plugin: "com.diffplug.spotless"
     apply plugin: "jacoco-report-aggregation"
     apply plugin: "groovy"
+    apply plugin: "net.ltgt.errorprone"
 
     tasks.withType(JavaCompile).configureEach {
         options.compilerArgs << "-Xlint:unchecked"
         options.compilerArgs << "-Xlint:deprecation"
+        options.errorprone.disableAllWarnings = true
+        options.errorprone.disableWarningsInGeneratedCode = true
+        options.errorprone.error("StringCaseLocaleUsage")
 
         // TODO Disabled until the code is only Java 11/17, see #76
         // options.release = 17
@@ -76,6 +81,7 @@ subprojects {
         testImplementation(libs.assertj.core)
         testImplementation(libs.mockito.core)
 
+        errorprone("com.google.errorprone:error_prone_core:${libs.errorprone.get().version}")
         testRuntimeOnly("org.junit.platform:junit-platform-launcher")
     }
 

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -33,6 +33,7 @@ commons-codec1 = { module = "commons-codec:commons-codec", version = "1.17.0" }
 commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.14.0" }
 dropwizard-bom = { module = "io.dropwizard:dropwizard-bom", version = "4.0.7" }
 eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.3" }
+errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.29.2" }
 google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.40.1" }
 guava = { module = "com.google.guava:guava", version = "33.2.1-jre" }
 h2 = { module = "com.h2database:h2", version = "2.2.224" }

diff --git a/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java
@@ -138,8 +138,8 @@ public Map<String, String> getOrGenerateSubScopeCreds(
               .addKeyValue("errorMessage", scopedCredentialsResult.getExtraInformation())
               .log("Failed to get subscoped credentials");
           throw new UnprocessableEntityException(
-              "Failed to get subscoped credentials: "
-                  + scopedCredentialsResult.getExtraInformation());
+              "Failed to get subscoped credentials: %s",
+              scopedCredentialsResult.getExtraInformation());
         };
     return cache.get(key, loader).convertToMapOfString();
   }

diff --git a/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java
@@ -15,6 +15,7 @@
  */
 package io.polaris.core.storage.cache;
 
+import com.google.common.collect.ImmutableMap;
 import io.polaris.core.PolarisCallContext;
 import io.polaris.core.PolarisDefaultDiagServiceImpl;
 import io.polaris.core.PolarisDiagnostics;
@@ -382,32 +383,29 @@ private static List<PolarisMetaStoreManager.ScopedCredentialsResult> getFakeScop
               : String.valueOf(Long.MAX_VALUE);
       res.add(
           new PolarisMetaStoreManager.ScopedCredentialsResult(
-              new EnumMap<>(PolarisCredentialProperty.class) {
-                {
-                  put(PolarisCredentialProperty.AWS_KEY_ID, "key_id_" + finalI);
-                  put(PolarisCredentialProperty.AWS_SECRET_KEY, "key_secret_" + finalI);
-                  put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime);
-                }
-              }));
+              new EnumMap<>(
+                  ImmutableMap.<PolarisCredentialProperty, String>builder()
+                      .put(PolarisCredentialProperty.AWS_KEY_ID, "key_id_" + finalI)
+                      .put(PolarisCredentialProperty.AWS_SECRET_KEY, "key_secret_" + finalI)
+                      .put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime)
+                      .buildOrThrow())));
       if (res.size() == number) return res;
       res.add(
           new PolarisMetaStoreManager.ScopedCredentialsResult(
-              new EnumMap<>(PolarisCredentialProperty.class) {
-                {
-                  put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_" + finalI);
-                  put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "account_host");
-                  put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime);
-                }
-              }));
+              new EnumMap<>(
+                  ImmutableMap.<PolarisCredentialProperty, String>builder()
+                      .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_" + finalI)
+                      .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "account_host")
+                      .put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime)
+                      .buildOrThrow())));
       if (res.size() == number) return res;
       res.add(
           new PolarisMetaStoreManager.ScopedCredentialsResult(
-              new EnumMap<>(PolarisCredentialProperty.class) {
-                {
-                  put(PolarisCredentialProperty.GCS_ACCESS_TOKEN, "gcs_token_" + finalI);
-                  put(PolarisCredentialProperty.GCS_ACCESS_TOKEN_EXPIRES_AT, expireTime);
-                }
-              }));
+              new EnumMap<>(
+                  ImmutableMap.<PolarisCredentialProperty, String>builder()
+                      .put(PolarisCredentialProperty.GCS_ACCESS_TOKEN, "gcs_token_" + finalI)
+                      .put(PolarisCredentialProperty.GCS_ACCESS_TOKEN_EXPIRES_AT, expireTime)
+                      .buildOrThrow())));
     }
     return res;
   }

diff --git a/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java b/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java
@@ -584,13 +584,12 @@ public void deleteCatalog(String name) {
     if (!dropEntityResult.isSuccess()) {
       if (dropEntityResult.failedBecauseNotEmpty()) {
         throw new BadRequestException(
-            String.format("Catalog '%s' cannot be dropped, it is not empty", entity.getName()));
+            "Catalog '%s' cannot be dropped, it is not empty", entity.getName());
       } else {
         throw new BadRequestException(
-            String.format(
-                "Catalog '%s' cannot be dropped, concurrent modification detected. Please try "
-                    + "again",
-                entity.getName()));
+            "Catalog '%s' cannot be dropped, concurrent modification detected. Please try "
+                + "again",
+            entity.getName());
       }
     }
   }
@@ -701,7 +700,7 @@ private void validateUpdateCatalogDiffOrThrow(
             .orElseThrow(
                 () ->
                     new CommitFailedException(
-                        "Concurrent modification on Catalog '%s'; retry later"));
+                        "Concurrent modification on Catalog '%s'; retry later", name));
     return returnedEntity;
   }
 
@@ -826,7 +825,7 @@ public void deletePrincipal(String name) {
             .orElseThrow(
                 () ->
                     new CommitFailedException(
-                        "Concurrent modification on Principal '%s'; retry later"));
+                        "Concurrent modification on Principal '%s'; retry later", name));
     return returnedEntity;
   }
 
@@ -1004,7 +1003,7 @@ public void deletePrincipalRole(String name) {
             .orElseThrow(
                 () ->
                     new CommitFailedException(
-                        "Concurrent modification on PrincipalRole '%s'; retry later"));
+                        "Concurrent modification on PrincipalRole '%s'; retry later", name));
     return returnedEntity;
   }
 
@@ -1142,7 +1141,7 @@ public void deleteCatalogRole(String catalogName, String name) {
             .orElseThrow(
                 () ->
                     new CommitFailedException(
-                        "Concurrent modification on CatalogRole '%s'; retry later"));
+                        "Concurrent modification on CatalogRole '%s'; retry later", name));
     return returnedEntity;
   }
 

diff --git a/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java
@@ -1570,7 +1570,7 @@ private void renameTableLike(
             if (existingEntitySubType == null) {
               // this code path is unexpected
               throw new AlreadyExistsException(
-                  "Cannot rename %s to %s. Object %s already exists", from, to);
+                  "Cannot rename %s to %s. Object already exists", from, to);
             } else if (existingEntitySubType == PolarisEntitySubType.TABLE) {
               throw new AlreadyExistsException(
                   "Cannot rename %s to %s. Table already exists", from, to);

diff --git a/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java b/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java
@@ -459,7 +459,7 @@ public Response getConfig(String warehouse, SecurityContext securityContext) {
             CallContext.getCurrentContext(), authenticatedPrincipal, warehouse);
     ResolverStatus resolverStatus = resolver.resolveAll();
     if (!resolverStatus.getStatus().equals(ResolverStatus.StatusEnum.SUCCESS)) {
-      throw new NotFoundException("Unable to find warehouse " + warehouse);
+      throw new NotFoundException("Unable to find warehouse %s", warehouse);
     }
     EntityCacheEntry resolvedReferenceCatalog = resolver.getResolvedReferenceCatalog();
     Map<String, String> properties =

diff --git a/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java b/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java
@@ -16,6 +16,7 @@
 package io.polaris.service.types;
 
 import java.util.Arrays;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -80,7 +81,7 @@ public static Optional<NotificationType> lookupByName(String name) {
     }
 
     for (NotificationType NotificationType : NotificationType.values()) {
-      if (name.toUpperCase().equals(NotificationType.name())) {
+      if (name.toUpperCase(Locale.ROOT).equals(NotificationType.name())) {
         return Optional.of(NotificationType);
       }
     }