Introduce EagerStringFormatting check (#1139)

This new check flags code that can be simplified and/or optimized by
deferring certain string formatting operations.

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java

@@ -0,0 +1,325 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.BugPattern.StandardTags.PERFORMANCE;
+import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
+import static com.google.errorprone.matchers.Matchers.staticMethod;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.joining;
+import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
+
+import com.google.auto.service.AutoService;
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
+import com.google.common.base.VerifyException;
+import com.google.common.collect.ImmutableList;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.annotations.Var;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
+import java.util.Collections;
+import java.util.Formattable;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Stream;
+import tech.picnic.errorprone.utils.SourceCode;
+
+/**
+ * A {@link BugChecker} that flags {@link String#format} and {@link String#formatted} invocations
+ * that can be omitted by delegating to another format method.
+ */
+// XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see
+// https://www.slf4j.org/faq.html#paramException. That should be documented.
+// XXX: Some of the `Matcher`s defined here are also declared by the `Slf4jLogStatement` and
+// `RedundantStringConversion` checks. Look into deduplicating them.
+// XXX: Should we also simplify e.g. `LOG.error(String.join("sep", arg1, arg2), throwable)`? Perhaps
+// that's too obscure.
+// XXX: This check currently only flags string format expressions that are a direct argument to
+// another format-capable method invocation. Indirect cases, such as where the result is assigned to
+// a variable, are currently not covered.
+@AutoService(BugChecker.class)
+@BugPattern(
+    summary = "String formatting can be deferred",
+    link = BUG_PATTERNS_BASE_URL + "EagerStringFormatting",
+    linkType = CUSTOM,
+    severity = WARNING,
+    tags = {PERFORMANCE, SIMPLIFICATION})
+public final class EagerStringFormatting extends BugChecker implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> FORMATTABLE = isSubtypeOf(Formattable.class);
+  private static final Matcher<ExpressionTree> LOCALE = isSubtypeOf(Locale.class);
+  private static final Matcher<ExpressionTree> SLF4J_MARKER = isSubtypeOf("org.slf4j.Marker");
+  private static final Matcher<ExpressionTree> THROWABLE = isSubtypeOf(Throwable.class);
+  private static final Matcher<ExpressionTree> REQUIRE_NON_NULL_INVOCATION =
+      staticMethod().onClass(Objects.class.getCanonicalName()).named("requireNonNull");
+  private static final Matcher<ExpressionTree> GUAVA_GUARD_INVOCATION =
+      anyOf(
+          staticMethod()
+              .onClass(Preconditions.class.getCanonicalName())
+              .namedAnyOf("checkArgument", "checkNotNull", "checkState"),
+          staticMethod()
+              .onClass(Verify.class.getCanonicalName())
+              .namedAnyOf("verify", "verifyNotNull"));
+  private static final Matcher<ExpressionTree> SLF4J_LOGGER_INVOCATION =
+      instanceMethod()
+          .onDescendantOf("org.slf4j.Logger")
+          .namedAnyOf("trace", "debug", "info", "warn", "error");
+  private static final Matcher<ExpressionTree> STATIC_FORMAT_STRING =
+      staticMethod().onClass(String.class.getCanonicalName()).named("format");
+  private static final Matcher<ExpressionTree> INSTANCE_FORMAT_STRING =
+      instanceMethod().onDescendantOf(String.class.getCanonicalName()).named("formatted");
+  private static final String MESSAGE_NEVER_NULL_ARGUMENT =
+      "String formatting never yields `null` expression";
+
+  /** Instantiates a new {@link EagerStringFormatting} instance. */
+  public EagerStringFormatting() {}
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    Tree parent = state.getPath().getParentPath().getLeaf();
+    if (!(parent instanceof MethodInvocationTree methodInvocation)) {
+      /*
+       * Fast path: this isn't a method invocation whose result is an argument to another method
+       * invocation.
+       */
+      // XXX: This logic assumes that the string format operation isn't redundantly wrapped in
+      // parentheses. Similar assumptions likely exist throughout the code base. Investigate how to
+      // structurally cover such cases.
+      return Description.NO_MATCH;
+    }
+
+    return StringFormatExpression.tryCreate(tree, state)
+        .map(expr -> analyzeFormatStringContext(expr, methodInvocation, state))
+        .orElse(Description.NO_MATCH);
+  }
+
+  private Description analyzeFormatStringContext(
+      StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
+    if (REQUIRE_NON_NULL_INVOCATION.matches(context, state)) {
+      return analyzeRequireNonNullStringFormatContext(stringFormat, context);
+    }
+
+    if (GUAVA_GUARD_INVOCATION.matches(context, state)) {
+      return analyzeGuavaGuardStringFormatContext(stringFormat, context, state);
+    }
+
+    if (SLF4J_LOGGER_INVOCATION.matches(context, state)) {
+      return analyzeSlf4jLoggerStringFormatContext(stringFormat, context, state);
+    }
+
+    /*
+     * The string formatting operation does not appear to happen in a context that admits of
+     * simplification or optimization.
+     */
+    return Description.NO_MATCH;
+  }
+
+  private Description analyzeRequireNonNullStringFormatContext(
+      StringFormatExpression stringFormat, MethodInvocationTree context) {
+    List<? extends ExpressionTree> arguments = context.getArguments();
+    if (arguments.size() != 2 || arguments.get(0).equals(stringFormat.expression())) {
+      /* Vacuous validation that string formatting doesn't yield `null`. */
+      return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
+    }
+
+    if (stringFormat.arguments().stream()
+        .anyMatch(EagerStringFormatting::isNonFinalLocalVariable)) {
+      /*
+       * The format operation depends on a variable that isn't final or effectively final; moving
+       * it into a lambda expression would cause a compilation error.
+       */
+      return buildDescription(context)
+          .setMessage(message() + " (but this requires introducing an effectively final variable)")
+          .build();
+    }
+
+    /* Suggest that the string formatting is deferred. */
+    return describeMatch(context, SuggestedFix.prefixWith(stringFormat.expression(), "() -> "));
+  }
+
+  private Description analyzeGuavaGuardStringFormatContext(
+      StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
+    List<? extends ExpressionTree> arguments = context.getArguments();
+    if (arguments.get(0).equals(stringFormat.expression())) {
+      /*
+       * Vacuous `checkNotNull` or `verifyNotNull` validation that string formatting doesn't yield
+       * `null`.
+       */
+      return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
+    }
+
+    if (stringFormat.simplifiableFormatString().isEmpty() || arguments.size() > 2) {
+      /*
+       * The format string cannot be simplified, or the format string produces a format string
+       * itself, or its result is the input to another format operation. These are complex cases
+       * that we'll only flag.
+       */
+      return createSimplificationSuggestion(context, "Guava");
+    }
+
+    return describeMatch(context, stringFormat.suggestFlattening("%s", state));
+  }
+
+  private Description analyzeSlf4jLoggerStringFormatContext(
+      StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
+    if (stringFormat.simplifiableFormatString().isEmpty()) {
+      /* We can't simplify this case; only flag it. */
+      return createSimplificationSuggestion(context, "SLF4J");
+    }
+
+    List<? extends ExpressionTree> arguments = context.getArguments();
+    int leftOffset = SLF4J_MARKER.matches(arguments.get(0), state) ? 1 : 0;
+    int rightOffset = THROWABLE.matches(arguments.get(arguments.size() - 1), state) ? 1 : 0;
+    if (arguments.size() != leftOffset + 1 + rightOffset) {
+      /*
+       * The format string produces a format string itself, or its result is the input to another
+       * format operation. This is a complex case that we'll only flag.
+       */
+      return createSimplificationSuggestion(context, "SLF4J");
+    }
+
+    return describeMatch(context, stringFormat.suggestFlattening("{}", state));
+  }
+
+  private static boolean isNonFinalLocalVariable(Tree tree) {
+    Symbol symbol = ASTHelpers.getSymbol(tree);
+    return symbol instanceof VarSymbol
+        && symbol.owner instanceof MethodSymbol
+        && !ASTHelpers.isConsideredFinal(symbol);
+  }
+
+  private Description createSimplificationSuggestion(MethodInvocationTree context, String library) {
+    return buildDescription(context)
+        .setMessage(
+            "%s (assuming that %s's simplified formatting support suffices)"
+                .formatted(message(), library))
+        .build();
+  }
+
+  /** Description of a string format expression. */
+  @AutoValue
+  abstract static class StringFormatExpression {
+    /** The full string format expression. */
+    abstract MethodInvocationTree expression();
+
+    /** The format string expression. */
+    abstract Tree formatString();
+
+    /** The string format arguments to be plugged into its format string. */
+    abstract ImmutableList<ExpressionTree> arguments();
+
+    /**
+     * The constant format string, if it contains only {@code %s} placeholders, and the number of
+     * said placeholders matches the number of format arguments.
+     */
+    abstract Optional<String> simplifiableFormatString();
+
+    private SuggestedFix suggestFlattening(String newPlaceholder, VisitorState state) {
+      return SuggestedFix.replace(
+          expression(),
+          Stream.concat(
+                  Stream.of(deriveFormatStringExpression(newPlaceholder, state)),
+                  arguments().stream().map(arg -> SourceCode.treeToString(arg, state)))
+              .collect(joining(", ")));
+    }
+
+    private String deriveFormatStringExpression(String newPlaceholder, VisitorState state) {
+      String formatString =
+          String.format(
+              simplifiableFormatString()
+                  .orElseThrow(() -> new VerifyException("Format string cannot be simplified")),
+              Collections.nCopies(arguments().size(), newPlaceholder).toArray());
+
+      /*
+       * If the suggested replacement format string is the same as the original, then use the
+       * expression's existing source code representation. This way string constant references are
+       * not unnecessarily replaced.
+       */
+      return formatString.equals(ASTHelpers.constValue(formatString(), String.class))
+          ? SourceCode.treeToString(formatString(), state)
+          : SourceCode.toStringConstantExpression(formatString, state);
+    }
+
+    private static Optional<StringFormatExpression> tryCreate(
+        MethodInvocationTree tree, VisitorState state) {
+      if (INSTANCE_FORMAT_STRING.matches(tree, state)) {
+        return Optional.of(
+            create(
+                tree,
+                requireNonNull(ASTHelpers.getReceiver(tree), "Receiver unexpectedly absent"),
+                ImmutableList.copyOf(tree.getArguments()),
+                state));
+      }
+
+      if (STATIC_FORMAT_STRING.matches(tree, state)) {
+        List<? extends ExpressionTree> arguments = tree.getArguments();
+        int argOffset = LOCALE.matches(arguments.get(0), state) ? 1 : 0;
+        return Optional.of(
+            create(
+                tree,
+                arguments.get(argOffset),
+                ImmutableList.copyOf(arguments.subList(argOffset + 1, arguments.size())),
+                state));
+      }
+
+      return Optional.empty();
+    }
+
+    private static StringFormatExpression create(
+        MethodInvocationTree expression,
+        Tree formatString,
+        ImmutableList<ExpressionTree> arguments,
+        VisitorState state) {
+      return new AutoValue_EagerStringFormatting_StringFormatExpression(
+          expression,
+          formatString,
+          arguments,
+          Optional.ofNullable(ASTHelpers.constValue(formatString, String.class))
+              .filter(template -> isSimplifiable(template, arguments, state)));
+    }
+
+    private static boolean isSimplifiable(
+        String formatString, ImmutableList<ExpressionTree> arguments, VisitorState state) {
+      if (arguments.stream().anyMatch(arg -> FORMATTABLE.matches(arg, state))) {
+        /* `Formattable` arguments can have arbitrary format semantics. */
+        return false;
+      }
+
+      @Var int placeholderCount = 0;
+      for (int p = formatString.indexOf('%'); p != -1; p = formatString.indexOf('%', p + 2)) {
+        if (p == formatString.length() - 1) {
+          /* Malformed format string with trailing `%`. */
+          return false;
+        }
+
+        char modifier = formatString.charAt(p + 1);
+        if (modifier == 's') {
+          placeholderCount++;
+        } else if (modifier != '%') {
+          /* Only `%s` and `%%` (a literal `%`) are supported. */
+          return false;
+        }
+      }
+
+      return placeholderCount == arguments.size();
+    }
+  }
+}

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java

@@ -28,8 +28,6 @@ import tech.picnic.errorprone.utils.SourceCode;
 /** A {@link BugChecker} that flags SLF4J usages that are likely to be in error. */
 // XXX: The special-casing of Throwable applies only to SLF4J 1.6.0+; see
 // https://www.slf4j.org/faq.html#paramException. That should be documented.
-// XXX: Also simplify `LOG.error(String.format("Something %s", arg), throwable)`.
-// XXX: Also simplify `LOG.error(String.join("sep", arg1, arg2), throwable)`? Perhaps too obscure.
 // XXX: Write a similar checker for Spring RestTemplates, String.format and friends, Guava
 // preconditions, ...
 @AutoService(BugChecker.class)

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/EagerStringFormattingTest.java

@@ -0,0 +1,286 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+final class EagerStringFormattingTest {
+  @Test
+  void identification() {
+    CompilationTestHelper.newInstance(EagerStringFormatting.class, getClass())
+        .expectErrorMessage("DEFER", m -> m.contains("String formatting can be deferred\n"))
+        .expectErrorMessage(
+            "DEFER_EXTRA_VARIABLE",
+            m ->
+                m.contains(
+                    "String formatting can be deferred (but this requires introducing an effectively final variable)"))
+        .expectErrorMessage(
+            "DEFER_SIMPLIFIED_GUAVA",
+            m ->
+                m.contains(
+                    "String formatting can be deferred (assuming that Guava's simplified formatting support suffices)"))
+        .expectErrorMessage(
+            "DEFER_SIMPLIFIED_SLF4J",
+            m ->
+                m.contains(
+                    "String formatting can be deferred (assuming that SLF4J's simplified formatting support suffices)"))
+        .expectErrorMessage(
+            "VACUOUS", m -> m.contains("String formatting never yields `null` expression"))
+        .addSourceLines(
+            "A.java",
+            "import static com.google.common.base.Preconditions.checkArgument;",
+            "import static com.google.common.base.Preconditions.checkNotNull;",
+            "import static com.google.common.base.Preconditions.checkState;",
+            "import static com.google.common.base.Verify.verify;",
+            "import static com.google.common.base.Verify.verifyNotNull;",
+            "import static java.util.Objects.requireNonNull;",
+            "",
+            "import java.util.Formattable;",
+            "import java.util.Locale;",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
+            "import org.slf4j.Marker;",
+            "",
+            "class A {",
+            "  private static final Logger LOG = LoggerFactory.getLogger(A.class);",
+            "",
+            "  private int nonFinalField = 0;",
+            "",
+            "  void m() {",
+            "    Formattable formattable = (formatter, flags, width, precision) -> {};",
+            "    int effectivelyFinalLocal = 0;",
+            "    /* A local variable that is also not effectively final. */",
+            "    int nonFinalLocal = 0;",
+            "    nonFinalLocal = 1;",
+            "",
+            "    String.format(\"%s\", \"foo\");",
+            "    String.format(Locale.US, \"%s\", \"foo\");",
+            "    \"%s\".formatted(\"foo\");",
+            "    String.format(\"%s\", \"foo\", \"bar\");",
+            "    String.format(\"%s %s\", \"foo\", \"bar\");",
+            "    String.format(\"%s %s %%\", \"foo\", \"bar\");",
+            "",
+            "    System.out.println(String.format(\"%s\", nonFinalLocal));",
+            "",
+            "    requireNonNull(\"never-null\");",
+            "    requireNonNull(\"never-null\", () -> String.format(\"Format string: %s\", nonFinalField));",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    requireNonNull(String.format(\"Never-null format string: %s\", nonFinalField));",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    requireNonNull(\"Never-null format string: %s\".formatted(nonFinalField), \"message\");",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    requireNonNull(",
+            "        String.format(\"Never-null format string\"), String.format(\"Malformed format string: %\"));",
+            "    // BUG: Diagnostic matches: DEFER_EXTRA_VARIABLE",
+            "    requireNonNull(\"never-null\", String.format(\"Format string: %s\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    requireNonNull(\"never-null\", String.format(\"Format string: %s\", effectivelyFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    requireNonNull(",
+            "        \"never-null\",",
+            "        String.format(",
+            "            \"Custom format string: %s, %d, %s\", getClass(), nonFinalField, \"string-constant\"));",
+            "",
+            "    checkArgument(true);",
+            "    checkNotNull(\"never-null\");",
+            "    checkState(false);",
+            "    verify(true);",
+            "    verifyNotNull(\"never-null\");",
+            "    checkArgument(false, \"Without format string\");",
+            "    checkNotNull(\"never-null\", \"Without format string\");",
+            "    checkState(true, \"Without format string\");",
+            "    verify(false, \"Without format string\");",
+            "    verifyNotNull(\"never-null\", \"Without format string\");",
+            "    checkArgument(true, \"With format string: %s\", nonFinalLocal);",
+            "    checkNotNull(\"never-null\", \"With format string: %s\", nonFinalLocal);",
+            "    checkState(false, \"With format string: %s\", nonFinalLocal);",
+            "    verify(true, \"With format string: %s\", nonFinalLocal);",
+            "    verifyNotNull(\"never-null\", \"With format string: %s\", nonFinalLocal);",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    checkNotNull(String.format(\"Never-null format string: %s\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    verifyNotNull(\"Never-null format string: %s\".formatted(nonFinalLocal), \"message\");",
+            "    // BUG: Diagnostic matches: VACUOUS",
+            "    checkNotNull(",
+            "        String.format(\"Never-null format string\"), String.format(\"Malformed format string: %\"));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkArgument(true, String.format(toString()));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkNotNull(\"never-null\", toString().formatted());",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkState(true, String.format(\"Custom format string: %d\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    verify(true, \"Mismatched format string:\".formatted(nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    verifyNotNull(\"never-null\", \"Mismatched format string: %d\".formatted());",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkArgument(true, String.format(\"Malformed format string: %\"));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkNotNull(\"never-null\", \"Format string with `Formattable`: %s\".formatted(formattable));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    checkState(true, String.format(\"Generated format string: %%s\"), nonFinalLocal);",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_GUAVA",
+            "    verify(",
+            "        true,",
+            "        \"Format string with format string argument: %s\",",
+            "        String.format(\"Format string argument: %s\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    verifyNotNull(",
+            "        \"never-null\", String.format(\"Format string: %s, %s\", nonFinalLocal, nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    checkArgument(true, \"Format string: %s%%\".formatted(nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    checkNotNull(",
+            "        \"never-null\", String.format(Locale.US, \"Format string with locale: %s\", nonFinalLocal));",
+            "",
+            "    LOG.trace(\"Without format string\");",
+            "    LOG.debug(\"With format string: {}\", nonFinalLocal);",
+            "    LOG.info((Marker) null, \"With marker\");",
+            "    LOG.warn((Marker) null, \"With marker and format string: {}\", nonFinalLocal);",
+            "    LOG.error(\"With throwable\", new RuntimeException());",
+            "    LOG.trace(\"With throwable and format string: {}\", nonFinalLocal, new RuntimeException());",
+            "    LOG.debug((Marker) null, \"With marker and throwable\", new RuntimeException());",
+            "    LOG.info(",
+            "        (Marker) null,",
+            "        \"With marker, throwable and format string: {}\",",
+            "        nonFinalLocal,",
+            "        new RuntimeException());",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.warn(String.format(toString()));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.error(toString().formatted());",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.trace(String.format(\"Custom format string: %d\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.debug(\"Mismatched format string:\".formatted(nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.info(\"Mismatched format string %d:\".formatted());",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.warn(String.format(\"Malformed format string: %\"));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.error(\"Format string with `Formattable`: %s\".formatted(formattable));",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.trace(String.format(\"Generated format string: {}\"), nonFinalLocal);",
+            "    // BUG: Diagnostic matches: DEFER_SIMPLIFIED_SLF4J",
+            "    LOG.debug(",
+            "        \"Format string with format string argument: {}\",",
+            "        String.format(\"Format string argument: %s\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.info(String.format(\"Vacuous format string %%\"));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.warn(String.format(\"With format string: %s, %s\", nonFinalLocal, nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.error(String.format(Locale.ROOT, \"With vacuous localized format string %%\"));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.trace((Marker) null, String.format(\"With marker and format string: %s\", nonFinalLocal));",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.debug(",
+            "        String.format(\"With throwable and format string: %s\", nonFinalLocal),",
+            "        new RuntimeException());",
+            "    // BUG: Diagnostic matches: DEFER",
+            "    LOG.info(",
+            "        (Marker) null,",
+            "        String.format(\"With marker, throwable and format string: %s\", nonFinalLocal),",
+            "        new RuntimeException());",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacement() {
+    BugCheckerRefactoringTestHelper.newInstance(EagerStringFormatting.class, getClass())
+        .addInputLines(
+            "A.java",
+            "import static com.google.common.base.Preconditions.checkArgument;",
+            "import static com.google.common.base.Preconditions.checkNotNull;",
+            "import static com.google.common.base.Preconditions.checkState;",
+            "import static com.google.common.base.Verify.verify;",
+            "import static com.google.common.base.Verify.verifyNotNull;",
+            "import static java.util.Objects.requireNonNull;",
+            "",
+            "import java.util.Locale;",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
+            "import org.slf4j.Marker;",
+            "",
+            "class A {",
+            "  private static final Logger LOG = LoggerFactory.getLogger(A.class);",
+            "  private static final String GUAVA_COMPATIBLE_PATTERN = \"with-only-%s-placeholder\";",
+            "  private static final String GUAVA_INCOMPATIBLE_PATTERN = \"with-%%-marker\";",
+            "",
+            "  void m() {",
+            "    requireNonNull(\"never-null\", String.format(\"Format string: %s\", 0));",
+            "",
+            "    checkArgument(true, String.format(\"Vacuous format string %%\"));",
+            "    checkNotNull(\"never-null\", \"Format string: %s %s%%\".formatted(1, 2));",
+            "    checkState(false, String.format(Locale.US, \"Format string with locale: %s\", 3));",
+            "    verify(true, GUAVA_COMPATIBLE_PATTERN.formatted(4));",
+            "    verifyNotNull(\"never-null\", String.format(Locale.ENGLISH, GUAVA_COMPATIBLE_PATTERN, 5));",
+            "    checkArgument(false, GUAVA_INCOMPATIBLE_PATTERN.formatted());",
+            "    checkNotNull(\"never-null\", String.format(GUAVA_INCOMPATIBLE_PATTERN));",
+            "",
+            "    LOG.trace(\"Vacuous format string %%\".formatted());",
+            "    LOG.debug(String.format(\"With format string: %s, %s%%\", 6, 7));",
+            "    LOG.info(String.format(Locale.ROOT, \"With vacuous localized format string %%\"));",
+            "    LOG.warn((Marker) null, \"With marker and format string: %s\".formatted(8));",
+            "    LOG.error(",
+            "        String.format(Locale.US, \"With throwable and format string: %s, %s\", 9, 10),",
+            "        new RuntimeException());",
+            "    LOG.trace(",
+            "        (Marker) null,",
+            "        \"With marker, throwable and format string: %s\".formatted(11),",
+            "        new RuntimeException());",
+            "    LOG.debug(GUAVA_COMPATIBLE_PATTERN.formatted(12));",
+            "    LOG.info(String.format(Locale.ENGLISH, GUAVA_COMPATIBLE_PATTERN, 13));",
+            "    LOG.warn(GUAVA_INCOMPATIBLE_PATTERN.formatted());",
+            "    LOG.error(String.format(GUAVA_INCOMPATIBLE_PATTERN));",
+            "  }",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import static com.google.common.base.Preconditions.checkArgument;",
+            "import static com.google.common.base.Preconditions.checkNotNull;",
+            "import static com.google.common.base.Preconditions.checkState;",
+            "import static com.google.common.base.Verify.verify;",
+            "import static com.google.common.base.Verify.verifyNotNull;",
+            "import static java.util.Objects.requireNonNull;",
+            "",
+            "import java.util.Locale;",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
+            "import org.slf4j.Marker;",
+            "",
+            "class A {",
+            "  private static final Logger LOG = LoggerFactory.getLogger(A.class);",
+            "  private static final String GUAVA_COMPATIBLE_PATTERN = \"with-only-%s-placeholder\";",
+            "  private static final String GUAVA_INCOMPATIBLE_PATTERN = \"with-%%-marker\";",
+            "",
+            "  void m() {",
+            "    requireNonNull(\"never-null\", () -> String.format(\"Format string: %s\", 0));",
+            "",
+            "    checkArgument(true, \"Vacuous format string %\");",
+            "    checkNotNull(\"never-null\", \"Format string: %s %s%\", 1, 2);",
+            "    checkState(false, \"Format string with locale: %s\", 3);",
+            "    verify(true, GUAVA_COMPATIBLE_PATTERN, 4);",
+            "    verifyNotNull(\"never-null\", GUAVA_COMPATIBLE_PATTERN, 5);",
+            "    checkArgument(false, \"with-%-marker\");",
+            "    checkNotNull(\"never-null\", \"with-%-marker\");",
+            "",
+            "    LOG.trace(\"Vacuous format string %\");",
+            "    LOG.debug(\"With format string: {}, {}%\", 6, 7);",
+            "    LOG.info(\"With vacuous localized format string %\");",
+            "    LOG.warn((Marker) null, \"With marker and format string: {}\", 8);",
+            "    LOG.error(\"With throwable and format string: {}, {}\", 9, 10, new RuntimeException());",
+            "    LOG.trace(",
+            "        (Marker) null, \"With marker, throwable and format string: {}\", 11, new RuntimeException());",
+            "    LOG.debug(\"with-only-{}-placeholder\", 12);",
+            "    LOG.info(\"with-only-{}-placeholder\", 13);",
+            "    LOG.warn(\"with-%-marker\");",
+            "    LOG.error(\"with-%-marker\");",
+            "  }",
+            "}")
+        .doTest(TestMode.TEXT_MATCH);
+  }
+}

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java

@@ -63,9 +63,11 @@ public final class SourceCode {
    *     found.
    * @return A non-{@code null} string.
    * @apiNote This method differs from {@link com.sun.tools.javac.util.Constants#format(Object)} in
-   *     that it does not superfluously escape single quote characters. It is different from {@link
+   *     that it does not superfluously escape single quote characters (the latter only does the
+   *     "clean thing" starting from JDK 23). It is different from {@link
    *     VisitorState#getConstantExpression(Object)} in that it is more performant and accepts any
    *     {@link CharSequence} instance.
+   * @see <a href="https://bugs.openjdk.org/browse/JDK-8325078">JDK-8325078</a>
    */
   // XXX: Drop this method if https://github.com/google/error-prone/pull/4586 is merged and released
   // with the proposed `CharSequence` compatibility change.

integration-tests/checkstyle-expected-changes.patch

@@ -15402,7 +15402,7 @@
 -    if (name == null) {
 -      throw new IllegalArgumentException(String.format(Locale.ROOT, TOKEN_ID_EXCEPTION_FORMAT, id));
 -    }
-+    checkArgument(name != null, String.format(Locale.ROOT, TOKEN_ID_EXCEPTION_FORMAT, id));
++    checkArgument(name != null, TOKEN_ID_EXCEPTION_FORMAT, id);
      return name;
    }
  
@@ -15414,11 +15414,11 @@
 -      throw new IllegalArgumentException(
 -          String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name));
 -    }
-+    checkArgument(id != null, String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name));
++    checkArgument(id != null, TOKEN_NAME_EXCEPTION_FORMAT, name);
      return id;
    }
  
-@@ -165,10 +163,9 @@ public final class TokenUtil {
+@@ -165,10 +163,7 @@ public final class TokenUtil {
     * @throws IllegalArgumentException when name is unknown
     */
    public static String getShortDescription(String name) {
@@ -15426,13 +15426,11 @@
 -      throw new IllegalArgumentException(
 -          String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name));
 -    }
-+    checkArgument(
++    checkArgument(TOKEN_NAME_TO_VALUE.containsKey(name), TOKEN_NAME_EXCEPTION_FORMAT, name);
-+        TOKEN_NAME_TO_VALUE.containsKey(name),
-+        String.format(Locale.ROOT, TOKEN_NAME_EXCEPTION_FORMAT, name));
  
      final String tokenTypes = "com.puppycrawl.tools.checkstyle.api.tokentypes";
      final ResourceBundle bundle = ResourceBundle.getBundle(tokenTypes, Locale.ROOT);
-@@ -344,7 +341,7 @@ public final class TokenUtil {
+@@ -344,7 +339,7 @@ public final class TokenUtil {
    public static BitSet asBitSet(String... tokens) {
      return Arrays.stream(tokens)
          .map(String::trim)

integration-tests/checkstyle-expected-warnings.txt

@@ -1,3 +1,5 @@
+src/it/java/com/google/checkstyle/test/base/AbstractIndentationTestSupport.java:[100,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices)
+src/it/java/com/google/checkstyle/test/base/AbstractIndentationTestSupport.java:[85,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices)
 src/it/java/com/google/checkstyle/test/chapter7javadoc/rule734nonrequiredjavadoc/NonRequiredJavadocTest.java:[33,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
 src/it/java/com/sun/checkstyle/test/chapter5comments/rule52documentationcomments/InvalidJavadocPositionTest.java:[35,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
 src/it/java/org/checkstyle/suppressionxpathfilter/XpathRegressionAbbreviationAsWordInNameTest.java:[116,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `enum` is not a valid identifier)
@@ -105,6 +107,8 @@ src/test/java/com/puppycrawl/tools/checkstyle/checks/design/InterfaceIsTypeCheck
 src/test/java/com/puppycrawl/tools/checkstyle/checks/design/MutableExceptionCheckTest.java:[54,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
 src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java:[100,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `null` is not a valid identifier)
 src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheckTest.java:[81,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
+src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java:[69,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices)
+src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java:[80,21] [EagerStringFormatting] String formatting can be deferred (assuming that Guava's simplified formatting support suffices)
 src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/InvalidJavadocPositionCheckTest.java:[59,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
 src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocContentLocationCheckTest.java:[57,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `default` is not a valid identifier)
 src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocContentLocationCheckTest.java:[75,8] [JUnitMethodDeclaration] This method's name should not redundantly start with `test` (but note that `package` is not a valid identifier)

integration-tests/prometheus-java-client-expected-changes.patch

@@ -5968,7 +5968,7 @@
  import static io.prometheus.metrics.instrumentation.dropwizard5.labels.MapperConfig.METRIC_GLOB_REGEX;
  
  import java.util.HashMap;
-@@ -32,10 +33,9 @@ class GraphiteNamePattern {
+@@ -32,10 +33,11 @@ class GraphiteNamePattern {
     * @param pattern The glob style pattern to be used.
     */
    GraphiteNamePattern(final String pattern) throws IllegalArgumentException {
@@ -5978,11 +5978,13 @@
 -    }
 +    checkArgument(
 +        VALIDATION_PATTERN.matcher(pattern).matches(),
-+        String.format("Provided pattern [%s] does not matches [%s]", pattern, METRIC_GLOB_REGEX));
++        "Provided pattern [%s] does not matches [%s]",
++        pattern,
++        METRIC_GLOB_REGEX);
      initializePattern(pattern);
    }
  
-@@ -84,7 +84,7 @@ class GraphiteNamePattern {
+@@ -84,7 +86,7 @@ class GraphiteNamePattern {
        escapedPattern.append("([^.]*)").append(quoted);
      }
  
@@ -6001,7 +6003,7 @@
  import java.util.HashMap;
  import java.util.Map;
  import java.util.regex.Pattern;
-@@ -106,21 +108,19 @@ public final class MapperConfig {
+@@ -106,21 +108,21 @@ public final class MapperConfig {
    }
  
    private void validateMatch(final String match) {
@@ -6013,9 +6015,9 @@
 -    }
 +    checkArgument(
 +        MATCH_EXPRESSION_PATTERN.matcher(match).matches(),
-+        String.format(
++        "Match expression [%s] does not match required pattern %s",
-+            "Match expression [%s] does not match required pattern %s",
++        match,
-+            match, MATCH_EXPRESSION_PATTERN));
++        MATCH_EXPRESSION_PATTERN);
    }
  
    private void validateLabels(final Map<String, String> labels) {
@@ -6027,11 +6029,13 @@
 -        }
 +        checkArgument(
 +            LABEL_PATTERN.matcher(key).matches(),
-+            String.format("Label [%s] does not match required pattern %s", match, LABEL_PATTERN));
++            "Label [%s] does not match required pattern %s",
++            match,
++            LABEL_PATTERN);
        }
      }
    }
-@@ -149,7 +149,6 @@ public final class MapperConfig {
+@@ -149,7 +151,6 @@ public final class MapperConfig {
    public int hashCode() {
      int result = match != null ? match.hashCode() : 0;
      result = 31 * result + (name != null ? name.hashCode() : 0);