Enable Error Prone check: FormatStringAnnotation

core/trino-main/pom.xml

@@ -212,6 +212,11 @@
             <artifactId>jsr305</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>com.google.errorprone</groupId>
+            <artifactId>error_prone_annotations</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>

core/trino-main/src/main/java/io/trino/operator/Driver.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+import com.google.errorprone.annotations.FormatMethod;
 import io.airlift.log.Logger;
 import io.airlift.units.Duration;
 import io.trino.execution.ScheduledSplit;
@@ -609,6 +610,7 @@ private Optional<ListenableFuture<Void>> getBlockedFuture(Operator operator)
         return Optional.empty();
     }
 
+    @FormatMethod
     private static Throwable addSuppressedException(Throwable inFlightException, Throwable newException, String message, Object... args)
     {
         if (newException instanceof Error) {

core/trino-main/src/main/java/io/trino/operator/WorkProcessorPipelineSourceOperator.java

@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+import com.google.errorprone.annotations.FormatMethod;
 import io.airlift.log.Logger;
 import io.airlift.units.DataSize;
 import io.airlift.units.Duration;
@@ -555,6 +556,7 @@ private void closeOperators(int lastOperatorIndex)
         }
     }
 
+    @FormatMethod
     private static Throwable handleOperatorCloseError(Throwable inFlightException, Throwable newException, String message, Object... args)
     {
         if (newException instanceof Error) {

core/trino-main/src/main/java/io/trino/server/remotetask/RequestErrorTracker.java

@@ -16,6 +16,7 @@
 import com.google.common.collect.ObjectArrays;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import com.google.errorprone.annotations.FormatMethod;
 import io.airlift.event.client.ServiceUnavailableException;
 import io.airlift.log.Logger;
 import io.airlift.units.Duration;
@@ -110,10 +111,10 @@ public void requestFailed(Throwable reason)
         // log failure message
         if (isExpectedError(reason)) {
             // don't print a stack for a known errors
-            log.warn("Error " + jobDescription + " %s: %s: %s", taskId, reason.getMessage(), taskUri);
+            log.warn("Error %s %s: %s: %s", jobDescription, taskId, reason.getMessage(), taskUri);
         }
         else {
-            log.warn(reason, "Error " + jobDescription + " %s: %s", taskId, taskUri);
+            log.warn(reason, "Error %s %s: %s", jobDescription, taskId, taskUri);
         }
 
         // remember the first 10 errors
@@ -138,6 +139,8 @@ public void requestFailed(Throwable reason)
         }
     }
 
+    @FormatMethod
+    @SuppressWarnings("FormatStringAnnotation") // we manipulate the format string and there's no way to make Error Prone accept the result
     static void logError(Throwable t, String format, Object... args)
     {
         if (isExpectedError(t)) {

core/trino-main/src/main/java/io/trino/server/ui/OAuth2WebUiAuthenticationFilter.java

@@ -119,7 +119,7 @@ private Optional<Map<String, Object>> getAccessToken(ContainerRequestContext req
                 return service.convertTokenToClaims(accessToken.get());
             }
             catch (JwtException | IllegalArgumentException e) {
-                LOG.debug("Unable to parse JWT token: " + e.getMessage(), e);
+                LOG.debug(e, "Unable to parse JWT token");
             }
         }
         return Optional.empty();

plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloClient.java

@@ -364,7 +364,7 @@ private void setLocalityGroups(Map<String, Object> tableProperties, AccumuloTabl
         }
 
         Map<String, Set<Text>> localityGroups = localityGroupsBuilder.build();
-        LOG.debug("Setting locality groups: {}", localityGroups);
+        LOG.debug("Setting locality groups: %s", localityGroups);
         tableManager.setLocalityGroups(table.getFullTableName(), localityGroups);
     }
 
@@ -851,7 +851,7 @@ private Optional<String> getTabletLocation(String table, Key key)
             // Swallow this exception so the query does not fail due to being unable
             // to locate the tablet server for the provided Key.
             // This is purely an optimization, but we will want to log the error.
-            LOG.error("Failed to get tablet location, returning dummy location", e);
+            LOG.error(e, "Failed to get tablet location, returning dummy location");
             return Optional.empty();
         }
     }
@@ -882,7 +882,7 @@ private Optional<String> getDefaultTabletLocation(String fulltable)
         catch (Exception e) {
             // Swallow this exception so the query does not fail due to being unable to locate the tablet server for the default tablet.
             // This is purely an optimization, but we will want to log the error.
-            LOG.error("Failed to get tablet location, returning dummy location", e);
+            LOG.error(e, "Failed to get tablet location, returning dummy location");
             return Optional.empty();
         }
     }

plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/index/IndexLookup.java

@@ -270,7 +270,7 @@ private boolean getRangesWithMetrics(
         // of rows
         long numEntries = indexRanges.size();
         double ratio = (double) numEntries / (double) numRows;
-        LOG.debug("Use of index would scan %d of %d rows, ratio %s. Threshold %2f, Using for table? %b", numEntries, numRows, ratio, threshold, ratio < threshold, table);
+        LOG.debug("Use of index would scan %d of %d rows, ratio %s. Threshold %2f, Using for table %s? %b", numEntries, numRows, ratio, threshold, table, ratio < threshold);
 
         // If the percentage of scanned rows, the ratio, less than the configured threshold
         if (ratio < threshold) {

plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java

@@ -128,8 +128,7 @@ private static void copyTable(
                 break;
         }
 
-        LOG.info("Running import for %s", target, sql);
-        LOG.info("%s", sql);
+        LOG.info("Running import for %s%n%s", target, sql);
         long start = System.nanoTime();
         long rows = queryRunner.execute(session, sql).getUpdateCount().getAsLong();
         LOG.info("Imported %s rows for %s in %s", rows, target, nanosSince(start));

plugin/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ScanQueryPageSource.java

@@ -316,7 +316,7 @@ public void close()
                 }
                 catch (Exception e) {
                     // ignore
-                    LOG.debug("Error clearing scroll", e);
+                    LOG.debug(e, "Error clearing scroll");
                 }
             }
         }

plugin/trino-hive/pom.xml

@@ -169,6 +169,11 @@
             <optional>true</optional>
         </dependency>
 
+        <dependency>
+            <groupId>com.google.errorprone</groupId>
+            <artifactId>error_prone_annotations</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>

plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.errorprone.annotations.FormatMethod;
 import io.airlift.log.Logger;
 import io.airlift.units.Duration;
 import io.trino.plugin.hive.HdfsEnvironment;
@@ -2308,6 +2309,7 @@ private static boolean isSameOrParent(Path parent, Path child)
         return parent.equals(child);
     }
 
+    @FormatMethod
     private void logCleanupFailure(String format, Object... args)
     {
         if (throwOnCleanupFailure) {
@@ -2316,6 +2318,7 @@ private void logCleanupFailure(String format, Object... args)
         log.warn(format, args);
     }
 
+    @FormatMethod
     private void logCleanupFailure(Throwable t, String format, Object... args)
     {
         if (throwOnCleanupFailure) {

plugin/trino-hive/src/test/java/io/trino/plugin/hive/parquet/write/TestDataWritableWriter.java

@@ -84,7 +84,7 @@ public void write(ParquetHiveRecord record)
             }
             catch (RuntimeException e) {
                 String errorMessage = "Parquet record is malformed: " + e.getMessage();
-                log.error(errorMessage, e);
+                log.error(e, errorMessage);
                 throw new RuntimeException(errorMessage, e);
             }
             recordConsumer.endMessage();

plugin/trino-kafka/src/test/java/io/trino/plugin/kafka/KafkaQueryRunner.java

@@ -145,7 +145,7 @@ public void postInit(DistributedQueryRunner queryRunner)
                 long start = System.nanoTime();
                 log.info("Running import for %s", table.getTableName());
                 queryRunner.execute(format("INSERT INTO %1$s SELECT * FROM tpch.tiny.%1$s", table.getTableName()));
-                log.info("Imported %s in %s", 0, table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit());
+                log.info("Imported %s in %s", table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit());
             }
             log.info("Loading complete in %s", nanosSince(startTime).toString(SECONDS));
         }

plugin/trino-kinesis/src/main/java/io/trino/plugin/kinesis/s3config/S3TableConfigClient.java

@@ -198,7 +198,7 @@ private void updateTablesFromS3()
                     log.info("Put table description into the map from %s", summary.getKey());
                 }
                 catch (IOException iox) {
-                    log.error("Problem reading input stream from object.", iox);
+                    log.error(iox, "Problem reading input stream from object.");
                     throw new RuntimeException(iox);
                 }
             }

plugin/trino-kudu/src/main/java/io/trino/plugin/kudu/KuduClientSession.java

@@ -231,7 +231,7 @@ public KuduTable openTable(SchemaTableName schemaTableName)
             return client.openTable(rawName);
         }
         catch (KuduException e) {
-            log.debug("Error on doOpenTable: " + e, e);
+            log.debug(e, "Error on doOpenTable");
             if (!listSchemaNames().contains(schemaTableName.getSchemaName())) {
                 throw new SchemaNotFoundException(schemaTableName.getSchemaName());
             }

pom.xml

@@ -1721,6 +1721,7 @@
                                     -Xep:EqualsIncompatibleType:ERROR
                                     -Xep:FallThrough:ERROR
                                     -Xep:FormatString:ERROR
+                                    -Xep:FormatStringAnnotation:ERROR
                                     -Xep:GetClassOnAnnotation:ERROR
                                     -Xep:GetClassOnClass:ERROR
                                     -Xep:IdentityBinaryExpression:ERROR

testing/trino-testing/src/main/java/io/trino/testing/datatype/DataTypeTest.java

@@ -104,7 +104,7 @@ private void queryWithWhere(QueryRunner trinoExecutor, Session session, List<Typ
             checkResults(expectedTypes, expectedResults, filteredRows);
         }
         catch (RuntimeException e) {
-            log.error("Exception caught during query with merged WHERE clause, querying one column at a time", e);
+            log.error(e, "Exception caught during query with merged WHERE clause, querying one column at a time");
             debugTypes(trinoExecutor, session, expectedTypes, expectedResults, testTable);
         }
     }