Enable Error Prone check: AnnotateFormatMethod

core/trino-main/src/main/java/io/trino/Session.java

@@ -279,7 +279,7 @@ public String getPreparedStatementFromExecute(Execute execute)
     public String getPreparedStatement(String name)
     {
         String sql = preparedStatements.get(name);
-        checkCondition(sql != null, NOT_FOUND, "Prepared statement not found: " + name);
+        checkCondition(sql != null, NOT_FOUND, "Prepared statement not found: %s", name);
         return sql;
     }
 

core/trino-main/src/main/java/io/trino/execution/SetColumnTypeTask.java

@@ -42,7 +42,6 @@
 import static io.trino.sql.analyzer.SemanticExceptions.semanticException;
 import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature;
 import static io.trino.type.UnknownType.UNKNOWN;
-import static java.lang.String.format;
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
 
@@ -78,15 +77,18 @@ public ListenableFuture<Void> execute(
         QualifiedObjectName qualifiedObjectName = createQualifiedObjectName(session, statement, statement.getTableName());
         RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, qualifiedObjectName);
         if (redirectionAwareTableHandle.getTableHandle().isEmpty()) {
-            String exceptionMessage = format("Table '%s' does not exist", qualifiedObjectName);
+            String additionalInformation;
             if (metadata.getMaterializedView(session, qualifiedObjectName).isPresent()) {
-                exceptionMessage += ", but a materialized view with that name exists.";
+                additionalInformation = ", but a materialized view with that name exists.";
             }
             else if (metadata.getView(session, qualifiedObjectName).isPresent()) {
-                exceptionMessage += ", but a view with that name exists.";
+                additionalInformation = ", but a view with that name exists.";
+            }
+            else {
+                additionalInformation = "";
             }
             if (!statement.isTableExists()) {
-                throw semanticException(TABLE_NOT_FOUND, statement, exceptionMessage);
+                throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist%s", qualifiedObjectName, additionalInformation);
             }
             return immediateVoidFuture();
         }

core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java

@@ -110,7 +110,7 @@ public ListenableFuture<Void> execute(
             propertyMetadata.decode(objectValue);
         }
         catch (RuntimeException e) {
-            throw semanticException(INVALID_SESSION_PROPERTY, statement, e.getMessage());
+            throw semanticException(INVALID_SESSION_PROPERTY, statement, "Invalid session property value '%s': %s", propertyName.toString(), e.getMessage());
         }
 
         stateMachine.addSetSessionProperties(propertyName.toString(), value);

core/trino-main/src/main/java/io/trino/json/PathEvaluationError.java

@@ -13,6 +13,7 @@
  */
 package io.trino.json;
 
+import com.google.errorprone.annotations.FormatMethod;
 import io.trino.spi.TrinoException;
 
 import static io.trino.spi.StandardErrorCode.PATH_EVALUATION_ERROR;
@@ -44,6 +45,7 @@ public PathEvaluationError(Throwable cause)
      * to the chosen `ON ERROR` option. Non-structural errors (e.g. numeric exceptions)
      * are not suppressed in `lax` or `strict` mode.
      */
+    @FormatMethod
     public static TrinoException structuralError(String format, Object... arguments)
     {
         return new PathEvaluationError("structural error: " + format(format, arguments));

core/trino-main/src/main/java/io/trino/metadata/FunctionManager.java

@@ -15,6 +15,7 @@
 
 import com.google.common.cache.CacheBuilder;
 import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.errorprone.annotations.FormatMethod;
 import io.trino.FeaturesConfig;
 import io.trino.collect.cache.NonEvictableCache;
 import io.trino.connector.CatalogServiceProvider;
@@ -249,6 +250,7 @@ private static void verifyMethodHandleSignature(BoundSignature boundSignature, S
         }
     }
 
+    @FormatMethod
     private static void verifyFunctionSignature(boolean check, String message, Object... args)
     {
         if (!check) {

core/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.java

@@ -125,7 +125,7 @@ public SpecializedSqlScalarFunction specialize(BoundSignature boundSignature, Fu
         ParametricScalarImplementation exactImplementation = implementations.getExactImplementations().get(boundSignature.toSignature());
         if (exactImplementation != null) {
             Optional<SpecializedSqlScalarFunction> scalarFunctionImplementation = exactImplementation.specialize(functionBinding, functionDependencies);
-            checkCondition(scalarFunctionImplementation.isPresent(), FUNCTION_IMPLEMENTATION_ERROR, format("Exact implementation of %s do not match expected java types.", boundSignature.getName()));
+            checkCondition(scalarFunctionImplementation.isPresent(), FUNCTION_IMPLEMENTATION_ERROR, "Exact implementation of %s do not match expected java types.", boundSignature.getName());
             return scalarFunctionImplementation.get();
         }
 

core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java

@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.annotations.FormatMethod;
 import io.airlift.units.DataSize;
 import io.airlift.units.Duration;
 import io.trino.Session.ResourceEstimateBuilder;
@@ -374,6 +375,7 @@ private static ResourceEstimates parseResourceEstimate(ProtocolHeaders protocolH
         return builder.build();
     }
 
+    @FormatMethod
     private static void assertRequest(boolean expression, String format, Object... args)
     {
         if (!expression) {

core/trino-main/src/main/java/io/trino/server/TrinoSystemRequirements.java

@@ -15,6 +15,7 @@
 
 import com.google.common.base.StandardSystemProperty;
 import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.annotations.FormatMethod;
 import com.sun.management.UnixOperatingSystemMXBean;
 import io.airlift.slice.Slice;
 import io.airlift.slice.Slices;
@@ -165,12 +166,14 @@ public static void verifySystemTimeIsReasonable()
         }
     }
 
+    @FormatMethod
     private static void failRequirement(String format, Object... args)
     {
         System.err.println("ERROR: " + format(format, args));
         System.exit(100);
     }
 
+    @FormatMethod
     private static void warnRequirement(String format, Object... args)
     {
         System.err.println("WARNING: " + format(format, args));

core/trino-main/src/main/java/io/trino/server/security/oauth2/NimbusOAuth2Client.java

@@ -378,7 +378,7 @@ private Optional<JWTClaimsSet> queryUserInfo(String accessToken)
         try {
             UserInfoResponse response = httpClient.execute(new UserInfoRequest(userinfoUrl.get(), new BearerAccessToken(accessToken)), UserInfoResponse::parse);
             if (!response.indicatesSuccess()) {
-                LOG.error("Received bad response from userinfo endpoint: " + response.toErrorResponse().getErrorObject());
+                LOG.error("Received bad response from userinfo endpoint: %s", response.toErrorResponse().getErrorObject());
                 return Optional.empty();
             }
             return Optional.of(response.toSuccessResponse().getUserInfo().toJWTClaimsSet());

core/trino-main/src/main/java/io/trino/sql/analyzer/AggregationAnalyzer.java

@@ -14,6 +14,7 @@
 package io.trino.sql.analyzer;
 
 import com.google.common.collect.ImmutableList;
+import com.google.errorprone.annotations.FormatMethod;
 import io.trino.Session;
 import io.trino.metadata.Metadata;
 import io.trino.spi.StandardErrorCode;
@@ -438,8 +439,7 @@ protected Boolean visitFunctionCall(FunctionCall node, Void context)
                 if (node.getFilter().isPresent()) {
                     throw semanticException(FUNCTION_NOT_AGGREGATE,
                             node,
-                            "Filter is only valid for aggregation functions",
-                            node);
+                            "Filter is only valid for aggregation functions");
                 }
                 if (node.getOrderBy().isPresent()) {
                     throw semanticException(FUNCTION_NOT_AGGREGATE, node, "ORDER BY is only valid for aggregation functions");
@@ -787,12 +787,13 @@ private boolean hasOrderByReferencesToOutputColumns(Node node)
         return hasReferencesToScope(node, analysis, orderByScope.get());
     }
 
-    private void verifyNoOrderByReferencesToOutputColumns(Node node, StandardErrorCode errorCode, String errorString)
+    @FormatMethod
+    private void verifyNoOrderByReferencesToOutputColumns(Node node, StandardErrorCode errorCode, String errorString, Object... errorStringArguments)
     {
         getReferencesToScope(node, analysis, orderByScope.get())
                 .findFirst()
                 .ifPresent(expression -> {
-                    throw semanticException(errorCode, expression, errorString);
+                    throw semanticException(errorCode, expression, errorString, errorStringArguments);
                 });
     }
 }

core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java

@@ -22,6 +22,8 @@
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Streams;
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
 import io.trino.Session;
 import io.trino.execution.warnings.WarningCollector;
 import io.trino.metadata.OperatorNotFoundException;
@@ -758,7 +760,7 @@ protected Type visitDereferenceExpression(DereferenceExpression node, StackableA
             for (RowType.Field rowField : rowType.getFields()) {
                 if (fieldName.equalsIgnoreCase(rowField.getName().orElse(null))) {
                     if (foundFieldName) {
-                        throw semanticException(AMBIGUOUS_NAME, field, "Ambiguous row field reference: " + fieldName);
+                        throw semanticException(AMBIGUOUS_NAME, field, "Ambiguous row field reference: %s", fieldName);
                     }
                     foundFieldName = true;
                     rowFieldType = rowField.getType();
@@ -1243,7 +1245,7 @@ protected Type visitFunctionCall(FunctionCall node, StackableAstVisitorContext<C
                 Expression expression = arguments.get(0);
                 Type expressionType = process(expression, context);
                 if (!(expressionType instanceof VarcharType)) {
-                    throw semanticException(TYPE_MISMATCH, node, format("Expected expression of varchar, but '%s' has %s type", expression, expressionType.getDisplayName()));
+                    throw semanticException(TYPE_MISMATCH, node, "Expected expression of varchar, but '%s' has %s type", expression, expressionType.getDisplayName());
                 }
             }
 
@@ -1273,7 +1275,7 @@ protected Type visitFunctionCall(FunctionCall node, StackableAstVisitorContext<C
                 // After optimization, array constructor is rewritten to a function call.
                 // For historic reasons array constructor is allowed to have 254 arguments
                 if (node.getArguments().size() > 254) {
-                    throw semanticException(TOO_MANY_ARGUMENTS, node, "Too many arguments for array constructor", function.getSignature().getName());
+                    throw semanticException(TOO_MANY_ARGUMENTS, node, "Too many arguments for array constructor: %s", function.getSignature().getName());
                 }
             }
             else if (node.getArguments().size() > 127) {
@@ -1491,7 +1493,7 @@ else if (frame.getType() == GROUPS) {
                     }
                 }
                 else {
-                    throw semanticException(NOT_SUPPORTED, frame, "Unsupported frame type: " + frame.getType());
+                    throw semanticException(NOT_SUPPORTED, frame, "Unsupported frame type: %s", frame.getType());
                 }
             }
         }
@@ -2421,7 +2423,7 @@ protected Type visitLambdaExpression(LambdaExpression node, StackableAstVisitorC
 
             if (types.size() != lambdaArguments.size()) {
                 throw semanticException(INVALID_PARAMETER_USAGE, node,
-                        format("Expected a lambda that takes %s argument(s) but got %s", types.size(), lambdaArguments.size()));
+                        "Expected a lambda that takes %s argument(s) but got %s", types.size(), lambdaArguments.size());
             }
 
             ImmutableList.Builder<Field> fields = ImmutableList.builder();
@@ -2557,7 +2559,7 @@ public Type visitJsonValue(JsonValue node, StackableAstVisitorContext<Context> c
                     !isDateTimeType(returnedType) ||
                     returnedType.equals(INTERVAL_DAY_TIME) ||
                     returnedType.equals(INTERVAL_YEAR_MONTH)) {
-                throw semanticException(TYPE_MISMATCH, node, "Invalid return type of function JSON_VALUE: " + node.getReturnedType().get());
+                throw semanticException(TYPE_MISMATCH, node, "Invalid return type of function JSON_VALUE: %s", node.getReturnedType().get());
             }
 
             JsonPathAnalysis pathAnalysis = jsonPathAnalyses.get(NodeRef.of(node));
@@ -2803,7 +2805,7 @@ private ResolvedFunction getInputFunction(Type type, JsonFormat format, Node nod
                     if (isStringType(type)) {
                         yield QualifiedName.of(VARBINARY_TO_JSON);
                     }
-                    throw semanticException(TYPE_MISMATCH, node, format("Cannot read input of type %s as JSON using formatting %s", type, format));
+                    throw semanticException(TYPE_MISMATCH, node, "Cannot read input of type %s as JSON using formatting %s", type, format);
                 }
                 case UTF8 -> QualifiedName.of(VARBINARY_UTF8_TO_JSON);
                 case UTF16 -> QualifiedName.of(VARBINARY_UTF16_TO_JSON);
@@ -2828,23 +2830,23 @@ private ResolvedFunction getOutputFunction(Type type, JsonFormat format, Node no
                     if (isStringType(type)) {
                         yield QualifiedName.of(JSON_TO_VARBINARY);
                     }
-                    throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format));
+                    throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format);
                 }
                 case UTF8 -> {
                     if (!VARBINARY.equals(type)) {
-                        throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format));
+                        throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format);
                     }
                     yield QualifiedName.of(JSON_TO_VARBINARY_UTF8);
                 }
                 case UTF16 -> {
                     if (!VARBINARY.equals(type)) {
-                        throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format));
+                        throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format);
                     }
                     yield QualifiedName.of(JSON_TO_VARBINARY_UTF16);
                 }
                 case UTF32 -> {
                     if (!VARBINARY.equals(type)) {
-                        throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format));
+                        throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format);
                     }
                     yield QualifiedName.of(JSON_TO_VARBINARY_UTF32);
                 }
@@ -3137,7 +3139,8 @@ private void coerceType(StackableAstVisitorContext<Context> context, Expression
             coerceType(expression, actualType, expectedType, message);
         }
 
-        private Type coerceToSingleType(StackableAstVisitorContext<Context> context, Node node, String message, Expression first, Expression second)
+        @FormatMethod
+        private Type coerceToSingleType(StackableAstVisitorContext<Context> context, Node node, @FormatString String message, Expression first, Expression second)
         {
             Type firstType = UNKNOWN;
             if (first != null) {
@@ -3163,7 +3166,9 @@ private Type coerceToSingleType(StackableAstVisitorContext<Context> context, Nod
                 return superType;
             }
 
-            throw semanticException(TYPE_MISMATCH, node, message, firstType, secondType);
+            @SuppressWarnings("FormatStringAnnotation") // Error Prone wants the types of format arguments to be the same as where @FormatString is declared, but we need them to be different
+            TrinoException exception = semanticException(TYPE_MISMATCH, node, message, firstType, secondType);
+            throw exception;
         }
 
         private Type coerceToSingleType(StackableAstVisitorContext<Context> context, String description, List<Expression> expressions)
@@ -3184,12 +3189,12 @@ private Type coerceToSingleType(StackableAstVisitorContext<Context> context, Str
             for (Type type : types) {
                 Optional<Type> newSuperType = typeCoercion.getCommonSuperType(superType, type);
                 if (newSuperType.isEmpty()) {
-                    throw semanticException(TYPE_MISMATCH, Iterables.get(typeExpressions.get(type), 0).getNode(), format(
+                    throw semanticException(TYPE_MISMATCH, Iterables.get(typeExpressions.get(type), 0).getNode(),
                             "%s must be the same type or coercible to a common type. Cannot find common type between %s and %s, all types (without duplicates): %s",
                             description,
                             superType,
                             type,
-                            typeExpressions.keySet()));
+                            typeExpressions.keySet());
                 }
                 superType = newSuperType.get();
             }
@@ -3200,12 +3205,12 @@ private Type coerceToSingleType(StackableAstVisitorContext<Context> context, Str
 
                 if (!type.equals(superType)) {
                     if (!typeCoercion.canCoerce(type, superType)) {
-                        throw semanticException(TYPE_MISMATCH, Iterables.get(coercionCandidates, 0).getNode(), format(
+                        throw semanticException(TYPE_MISMATCH, Iterables.get(coercionCandidates, 0).getNode(),
                                 "%s must be the same type or coercible to a common type. Cannot find common type between %s and %s, all types (without duplicates): %s",
                                 description,
                                 superType,
                                 type,
-                                typeExpressions.keySet()));
+                                typeExpressions.keySet());
                     }
                     addOrReplaceExpressionsCoercion(coercionCandidates, type, superType);
                 }
@@ -3559,7 +3564,7 @@ public static ExpressionAnalyzer createWithoutSubqueries(
                 session,
                 TypeProvider.empty(),
                 parameters,
-                node -> semanticException(errorCode, node, message),
+                node -> semanticException(errorCode, node, "%s", message),
                 warningCollector,
                 isDescribe);
     }