SONARJAVA-7070: Implemented rule S9413 Redundant "String.format" should be removed when native formatting is available

its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9413.json

@@ -0,0 +1,16 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java": [
+1974
+],
+"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java": [
+368
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java": [
+714
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Server.java": [
+420,
+440,
+480
+]
+}

its/ruling/src/test/resources/expected/eclipse-jetty/java-S9413.json

@@ -0,0 +1,16 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java": [
+1974
+],
+"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java": [
+368
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java": [
+714
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Server.java": [
+420,
+440,
+480
+]
+}

java-checks-test-sources/default/src/main/java/checks/RedundantStringFormatCheckLoggingSample.java

@@ -0,0 +1,62 @@
+package checks;
+
+import java.util.Formattable;
+import java.util.Formatter;
+import java.util.Locale;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.Marker;
+
+class RedundantStringFormatCheckLoggingSample {
+
+  private static final Logger LOG = LoggerFactory.getLogger(RedundantStringFormatCheckLoggingSample.class);
+  private static final org.apache.logging.log4j.Logger LOG4J = LogManager.getLogger();
+  private static final java.util.logging.Logger JUL = java.util.logging.Logger.getLogger("jul");
+  private static final String FORMAT = "User %s";
+
+  void slf4j(String username, String role, String id, Exception e, Marker marker, int count, double ratio) {
+    LOG.info(String.format("Activated user '%s' with role '%s'", username, role)); // Noncompliant {{Use the logger's built-in "{}" formatting instead of "String.format".}}
+//           ^^^^^^^^^^^^^
+    LOG.error(String.format("Failed to process %s", id), e); // Noncompliant
+    LOG.warn(marker, String.format("Retry count %d", count)); // Noncompliant
+    LOG.debug(String.format("Progress 100%% for %s", id)); // Noncompliant
+
+    LOG.info("Activated user '{}' with role '{}'", username, role);
+    LOG.info("Ratio: {}", String.format("%.2f", ratio));
+    LOG.info(String.format("Ratio %.2f", ratio));
+    LOG.info(String.format(Locale.FRANCE, "User %s", username));
+    LOG.info(String.format(FORMAT, username));
+    LOG.info(String.format("User %s", username).trim());
+    String message = String.format("User %s", username);
+    LOG.info(message);
+  }
+
+  void formattable(FormattableId id, Formattable formattable, Object value) {
+    LOG.info(String.format("Value: %s", id));
+    LOG.info(String.format("Value: %s", formattable));
+    LOG4J.info(String.format("Values: %s %s", value, id));
+    LOG.info(String.format("Value: %s", value)); // Noncompliant
+  }
+
+  static class FormattableId implements Formattable {
+    @Override
+    public void formatTo(Formatter formatter, int flags, int width, int precision) {
+      formatter.format("ID");
+    }
+  }
+
+  void log4j(String id, int count) {
+    LOG4J.debug(String.format("Loaded %s", id)); // Noncompliant
+    LOG4J.fatal(String.format("Crashed after %d attempts", count)); // Noncompliant
+    LOG4J.log(Level.INFO, String.format("Loaded %s", id)); // Noncompliant
+
+    LOG4J.debug("Loaded {}", id);
+    LOG4J.info(String.format("Id %x", count));
+  }
+
+  void jul(String id) {
+    JUL.info(String.format("Loaded %s", id));
+  }
+}

java-checks-test-sources/default/src/main/java/checks/RedundantStringFormatCheckSample.java

@@ -0,0 +1,109 @@
+package checks;
+
+import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.Formattable;
+import java.util.Formatter;
+import java.util.List;
+import java.util.Locale;
+
+class RedundantStringFormatCheckSample {
+
+  private static final String ROW_FORMAT = "%s: %d";
+
+  void printing(int a, int b, int sum, double ratio, long count, PrintWriter writer, int pageNum, String title) {
+    System.out.println(String.format("Result: %d + %d = %d", a, b, sum)); // Noncompliant {{Use "printf" instead of "String.format".}} [[quickfixes=qf1]]
+//                     ^^^^^^^^^^^^^
+    // fix@qf1 {{Replace with "printf"}}
+    // edit@qf1 [[sc=59;ec=59]] {{%n}}
+    // edit@qf1 [[sc=16;ec=23]] {{printf}}
+    // edit@qf1 [[sc=24;ec=38]] {{}}
+    // edit@qf1 [[sc=71;ec=72]] {{}}
+    System.err.print(String.format("%.2f", ratio)); // Noncompliant [[quickfixes=qf2]]
+    // fix@qf2 {{Replace with "printf"}}
+    // edit@qf2 [[sc=16;ec=21]] {{printf}}
+    // edit@qf2 [[sc=22;ec=36]] {{}}
+    // edit@qf2 [[sc=49;ec=50]] {{}}
+    System.out.println(String.format(Locale.US, "%,d", count)); // Noncompliant [[quickfixes=qf3]]
+    // fix@qf3 {{Replace with "printf"}}
+    // edit@qf3 [[sc=53;ec=53]] {{%n}}
+    // edit@qf3 [[sc=16;ec=23]] {{printf}}
+    // edit@qf3 [[sc=24;ec=38]] {{}}
+    // edit@qf3 [[sc=61;ec=62]] {{}}
+    writer.println(String.format("Page %d: %s", pageNum, title)); // Noncompliant
+    writer.print(String.format(ROW_FORMAT, title, pageNum)); // Noncompliant [[quickfixes=qf4]]
+    // fix@qf4 {{Replace with "printf"}}
+    // edit@qf4 [[sc=12;ec=17]] {{printf}}
+    // edit@qf4 [[sc=18;ec=32]] {{}}
+    // edit@qf4 [[sc=58;ec=59]] {{}}
+    System.out.println((String.format("Total: %d", sum))); // Noncompliant [[quickfixes=qf5]]
+    // fix@qf5 {{Replace with "printf"}}
+    // edit@qf5 [[sc=49;ec=49]] {{%n}}
+    // edit@qf5 [[sc=16;ec=23]] {{printf}}
+    // edit@qf5 [[sc=24;ec=39]] {{}}
+    // edit@qf5 [[sc=55;ec=57]] {{}}
+    System.out.println(String.format(ROW_FORMAT, title, pageNum)); // Noncompliant [[quickfixes=!]]
+    // Noncompliant@+1 [[quickfixes=!]]
+    writer.println(String.format("""
+      %s""", title));
+    System.out.println(String.format("Id %s", new FormattableId())); // Noncompliant
+
+    System.out.printf("Result: %d + %d = %d%n", a, b, sum);
+    System.out.println("Result: " + sum);
+    System.out.println(String.format("Result: %d", sum).trim());
+    System.out.println("Total " + String.format("%.2f", ratio));
+  }
+
+  String builder(String name, int age) {
+    StringBuilder sb = new StringBuilder("{");
+    sb.append(String.format("name: %s, ", name));
+    new StringBuffer().append(String.format("age: %d", age));
+    sb.append("name: ").append(name);
+    return sb.toString();
+  }
+
+  void validate(int age, String name, Exception cause) throws ValidationException {
+    if (age < 0) {
+      throw new IllegalArgumentException(String.format("Age cannot be negative: %d", age));
+    }
+    if (name == null) {
+      throw new RuntimeException(String.format("Missing name for age %d", age), cause);
+    }
+    if (name.isEmpty()) {
+      throw new ValidationException(String.format("Empty name %s", name));
+    }
+    if (name.length() > 10) {
+      throw new IllegalStateException(String.format("%-10s|", name));
+    }
+    if (age > 150) {
+      throw new IllegalArgumentException("Age too high: " + age);
+    }
+  }
+
+  List<String> unrelatedTargets(String name, int age) {
+    List<String> lines = new ArrayList<>();
+    lines.add(String.format("name=%s", name));
+    Object[] values = new Object[]{String.format("age=%d", age)};
+    String message = String.format("name=%s", name);
+    System.out.println(message);
+    new Wrapper(String.format("name=%s", name));
+    lines.add("name=%s".formatted(name));
+    return lines;
+  }
+
+  static class ValidationException extends Exception {
+    ValidationException(String message) {
+      super(String.format("Validation failed: %s", message));
+    }
+  }
+
+  record Wrapper(String value) {
+  }
+
+  static class FormattableId implements Formattable {
+    @Override
+    public void formatTo(Formatter formatter, int flags, int width, int precision) {
+      formatter.format("ID");
+    }
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/RedundantStringFormatCheck.java

@@ -0,0 +1,163 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import java.util.List;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.LoggingMatchers;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.model.ExpressionUtils;
+import org.sonar.java.model.LiteralUtils;
+import org.sonar.java.reporting.AnalyzerMessage;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
+import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.LiteralTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S9413")
+public class RedundantStringFormatCheck extends IssuableSubscriptionVisitor {
+
+  private static final String STRING = "java.lang.String";
+
+  private static final MethodMatchers STRING_FORMAT = MethodMatchers.create()
+    .ofTypes(STRING)
+    .names("format")
+    .withAnyParameters()
+    .build();
+
+  private static final MethodMatchers PRINT_METHODS = MethodMatchers.create()
+    .ofSubTypes("java.io.PrintStream", "java.io.PrintWriter")
+    .names("print", "println")
+    .addParametersMatcher(STRING)
+    .build();
+
+  private static final MethodMatchers LOG_METHODS = MethodMatchers.or(LoggingMatchers.SLF4J_LOG_METHODS, LoggingMatchers.LOG4J_LOG_METHODS);
+
+  private static final String PRINTF_MESSAGE = "Use \"printf\" instead of \"String.format\".";
+  private static final String LOGGING_MESSAGE = "Use the logger's built-in \"{}\" formatting instead of \"String.format\".";
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.METHOD_INVOCATION);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    MethodInvocationTree formatCall = (MethodInvocationTree) tree;
+    if (!STRING_FORMAT.matches(formatCall)) {
+      return;
+    }
+    Tree argument = outermostParentheses(formatCall);
+    Tree arguments = argument.parent();
+    if (!arguments.is(Tree.Kind.ARGUMENTS)) {
+      return;
+    }
+    if (!(arguments.parent() instanceof MethodInvocationTree call) || !isMessageArgument(call.methodSymbol(), call.arguments().indexOf(argument))) {
+      return;
+    }
+    if (PRINT_METHODS.matches(call)) {
+      QuickFixHelper.newIssue(context)
+        .forRule(this)
+        .onTree(formatCall.methodSelect())
+        .withMessage(PRINTF_MESSAGE)
+        .withQuickFixes(() -> printfQuickFix(call, argument, formatCall))
+        .report();
+    } else if (LOG_METHODS.matches(call) && hasSimpleLiteralFormat(formatCall) && hasNoFormattableArgument(formatCall)) {
+      reportIssue(formatCall.methodSelect(), LOGGING_MESSAGE);
+    }
+  }
+
+  private static Tree outermostParentheses(Tree tree) {
+    Tree result = tree;
+    while (result.parent().is(Tree.Kind.PARENTHESIZED_EXPRESSION)) {
+      result = result.parent();
+    }
+    return result;
+  }
+
+  /**
+   * Builds a quick fix turning {@code print(String.format(args))} into {@code printf(args)}.
+   * For {@code println}, a {@code %n} is appended to the format, which is only possible when the format is a string literal.
+   */
+  private static List<JavaQuickFix> printfQuickFix(MethodInvocationTree printCall, Tree argument, MethodInvocationTree formatCall) {
+    List<ExpressionTree> formatArguments = formatCall.arguments();
+    boolean isPrintln = "println".equals(printCall.methodSymbol().name());
+    JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"printf\"");
+    if (isPrintln) {
+      ExpressionTree format = formatArguments.get(formatArguments.get(0).symbolType().is("java.util.Locale") ? 1 : 0);
+      if (!format.is(Tree.Kind.STRING_LITERAL)) {
+        return List.of();
+      }
+      AnalyzerMessage.TextSpan formatSpan = AnalyzerMessage.textSpanFor(format);
+      builder.addTextEdit(JavaTextEdit.insertAtPosition(formatSpan.endLine, formatSpan.endCharacter - 1, "%n"));
+    }
+    return List.of(builder
+      .addTextEdit(JavaTextEdit.replaceTree(ExpressionUtils.methodName(printCall), "printf"))
+      .addTextEdit(JavaTextEdit.removeTextSpan(AnalyzerMessage.textSpanBetween(argument, true, formatArguments.get(0), false)))
+      .addTextEdit(JavaTextEdit.removeTextSpan(AnalyzerMessage.textSpanBetween(formatArguments.get(formatArguments.size() - 1), false, argument, true)))
+      .build());
+  }
+
+  /**
+   * An argument is the message argument when its declared parameter is the first parameter of type {@code String}.
+   */
+  private static boolean isMessageArgument(Symbol.MethodSymbol symbol, int index) {
+    if (symbol.isUnknown()) {
+      return false;
+    }
+    List<Type> parameterTypes = symbol.parameterTypes();
+    if (index < 0 || index >= parameterTypes.size() || !parameterTypes.get(index).is(STRING)) {
+      return false;
+    }
+    return parameterTypes.subList(0, index).stream().noneMatch(type -> type.is(STRING));
+  }
+
+  private static boolean hasSimpleLiteralFormat(MethodInvocationTree formatCall) {
+    List<ExpressionTree> arguments = formatCall.arguments();
+    if (arguments.isEmpty() || !arguments.get(0).is(Tree.Kind.STRING_LITERAL)) {
+      return false;
+    }
+    return hasOnlySimpleConversions(LiteralUtils.trimQuotes(((LiteralTree) arguments.get(0)).value()));
+  }
+
+  /**
+   * {@code %s} calls {@code formatTo} on {@link java.util.Formattable} values instead of {@code toString}, which "{}" placeholders would not preserve.
+   */
+  private static boolean hasNoFormattableArgument(MethodInvocationTree formatCall) {
+    return formatCall.arguments().stream().noneMatch(arg -> arg.symbolType().isSubtypeOf("java.util.Formattable"));
+  }
+
+  /**
+   * Returns true when every {@code %} sequence of the format string is {@code %s}, {@code %d} or {@code %%}.
+   */
+  private static boolean hasOnlySimpleConversions(String format) {
+    int index = format.indexOf('%');
+    while (index >= 0) {
+      if (index + 1 >= format.length() || "sd%".indexOf(format.charAt(index + 1)) < 0) {
+        return false;
+      }
+      index = format.indexOf('%', index + 2);
+    }
+    return true;
+  }
+}