SONARJAVA-5599 S3024 Arguments to append should not be concatenated

its/autoscan/src/test/resources/autoscan/diffs/diff_S3024.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S3024",
+  "hasTruePositives": true,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}

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

@@ -0,0 +1,14 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java": [
+507,
+508,
+509,
+510,
+511,
+512,
+513,
+514,
+515,
+516
+]
+}

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

@@ -0,0 +1,14 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java": [
+507,
+508,
+509,
+510,
+511,
+512,
+513,
+514,
+515,
+516
+]
+}

its/ruling/src/test/resources/guava/java-S3024.json

@@ -0,0 +1,6 @@
+{
+"com.google.guava:guava:src/com/google/common/collect/Iterators.java": [
+313,
+315
+]
+}

its/ruling/src/test/resources/sonar-server/java-S3024.json

@@ -0,0 +1,18 @@
+{
+"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/debt/DebtModelXMLExporter.java": [
+67,
+71,
+78,
+79,
+81,
+83,
+97,
+114,
+116,
+118,
+120,
+123,
+125,
+127
+]
+}

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

@@ -0,0 +1,99 @@
+package checks;
+
+import java.util.Iterator;
+
+public class StringBufferAndBuilderConcatenationCheckSample {
+  public String appendSimple() {
+    return new StringBuilder()
+      .append("text1")
+      .append("text2")
+      .toString();
+  }
+
+  public String concatThree() {
+    return new StringBuilder()
+      .append("text1" + "text2" + "text3") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf2]]
+//            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+//            fix@qf2 {{Call "append" multiple times.}}
+//            edit@qf2 [[sc=14;ec=43]] {{("text1").append("text2").append("text3")}}
+      .toString();
+  }
+
+  public String concatIntegerNoncompliant() {
+    return new StringBuilder()
+      .append("text" + 1) // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf3]]
+//            ^^^^^^^^^^
+//            fix@qf3 {{Call "append" multiple times.}}
+//            edit@qf3 [[sc=14;ec=26]] {{("text").append(1)}}
+      .toString();
+  }
+
+  public String concatIntegerCompliant() {
+    return new StringBuilder()
+      .append(1 + 1)
+      .toString();
+  }
+
+  public String complexArgumentList() {
+    return new StringBuilder()
+      .append("two " + (1 + 1)) // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf4]]
+//            ^^^^^^^^^^^^^^^^
+//            fix@qf4 {{Call "append" multiple times.}}
+//            edit@qf4 [[sc=14;ec=32]] {{("two ").append(1 + 1)}}
+      .toString();
+  }
+
+  public StringBuilder notChained() {
+    StringBuilder stringBuilder = new StringBuilder();
+    stringBuilder.append("text1" + "text2"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf5]]
+//                       ^^^^^^^^^^^^^^^^^
+//                       fix@qf5 {{Call "append" multiple times.}}
+//                       edit@qf5 [[sc=25;ec=44]] {{("text1").append("text2")}}
+    return stringBuilder;
+  }
+
+  public void parameter(StringBuilder passed) {
+    passed.append("hello " + "hello");  // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf6]]
+//                ^^^^^^^^^^^^^^^^^^
+//                fix@qf6 {{Call "append" multiple times.}}
+//                edit@qf6 [[sc=18;ec=38]] {{("hello ").append("hello")}}
+  }
+
+  public String call(Iterator<String> iter) {
+    return new StringBuilder()
+      .append("<" + iter.next() + "/>") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf7]]
+//            ^^^^^^^^^^^^^^^^^^^^^^^^
+//            fix@qf7 {{Call "append" multiple times.}}
+//            edit@qf7 [[sc=14;ec=40]] {{("<").append(iter.next()).append("/>")}}
+      .toString();
+  }
+
+  public String nestedCall(String s) {
+    return new StringBuilder()
+      .append(s.toLowerCase())
+      .toString();
+  }
+
+  public String stringBufferAppendSimple() {
+    return new StringBuffer()
+      .append("text1")
+      .append("text2")
+      .toString();
+  }
+
+  public String appendWithStartEnd() {
+    // Breaking this up would require changing the second and third argument.
+    return new StringBuilder()
+      .append("text1" + "text2" , 2, 3)
+      .toString();
+  }
+
+  public String stringBufferConcatSimple() {
+    return new StringBuffer()
+      .append("text1" + "text2") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf101]]
+//            ^^^^^^^^^^^^^^^^^
+//            fix@qf101 {{Call "append" multiple times.}}
+//            edit@qf101 [[sc=14;ec=33]] {{("text1").append("text2")}}
+      .toString();
+  }
+}

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

@@ -0,0 +1,108 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2025 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.methods.AbstractMethodDetection;
+import org.sonar.java.model.ExpressionUtils;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
+import org.sonar.plugins.java.api.tree.Arguments;
+import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+/**
+ * Suggest that <code>stringBuilder.append("text1" + "text2")</code> is replaced with
+ * <code>stringBuilder.append("text1").append("text2")</code>.
+ */
+@Rule(key = "S3024")
+public class StringBufferAndBuilderConcatenationCheck extends AbstractMethodDetection {
+
+  private static final String JAVA_LANG_STRING = "java.lang.String";
+
+  @Override
+  protected MethodMatchers getMethodInvocationMatchers() {
+    return MethodMatchers.create()
+      .ofTypes("java.lang.StringBuffer", "java.lang.StringBuilder")
+      .names("append")
+      .addParametersMatcher(JAVA_LANG_STRING)
+      .build();
+  }
+
+  @Override
+  protected void onMethodInvocationFound(MethodInvocationTree mit) {
+    ExpressionTree arg = mit.arguments().get(0);
+
+    getConcatenationTree(arg).ifPresent(binaryExpressionTree ->
+      QuickFixHelper.newIssue(context)
+          .forRule(this)
+          .onTree(arg)
+          .withMessage("Use multiple calls to \"append\" instead of string concatenation.")
+          .withQuickFix(() -> getQuickFix(mit.arguments()))
+          .report()
+    );
+  }
+
+  /**
+   * If the tree represents <code>arg1 + arg2</code>, then return <code>Optional.of(tree)</code>,
+   * otherwise return <code>Optional.empty()</code>.
+   * Using optional, rather than if-then, helps us avoid some problems in coverage by unit tests.
+   */
+  private static Optional<BinaryExpressionTree> getConcatenationTree(ExpressionTree tree) {
+    return Optional.of(tree)
+      .filter(BinaryExpressionTree.class::isInstance)
+      .map(BinaryExpressionTree.class::cast)
+      .filter(bet -> bet.is(Tree.Kind.PLUS));
+  }
+
+  private JavaQuickFix getQuickFix(Arguments arguments) {
+    String replacement = splitExpressionOnPlus(arguments.get(0)).stream()
+      .map(ExpressionUtils::skipParentheses)
+      .map(tree -> "(" + QuickFixHelper.contentForTree(tree, context) + ")")
+      .collect(Collectors.joining(".append"));
+
+    return JavaQuickFix.newQuickFix("Call \"append\" multiple times.")
+      .addTextEdit(JavaTextEdit.replaceTree(arguments, replacement))
+      .build();
+  }
+
+  private static List<ExpressionTree> splitExpressionOnPlus(ExpressionTree tree) {
+    List<ExpressionTree> accumulator = new ArrayList<>();
+    splitExpressionOnPlus(accumulator, tree);
+    return accumulator;
+  }
+
+  private static void splitExpressionOnPlus(List<ExpressionTree> accumulator, ExpressionTree tree) {
+    getConcatenationTree(tree).ifPresentOrElse(
+      bet -> {
+        ExpressionTree left = bet.leftOperand();
+        ExpressionTree right = bet.rightOperand();
+        splitExpressionOnPlus(accumulator, left);
+        accumulator.add(right);
+      },
+      () -> accumulator.add(tree)
+    );
+  }
+}

java-checks/src/test/java/org/sonar/java/checks/StringBufferAndBuilderConcatenationCheckTest.java

@@ -0,0 +1,41 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2025 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+
+class StringBufferAndBuilderConcatenationCheckTest {
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringBufferAndBuilderConcatenationCheckSample.java"))
+      .withCheck(new StringBufferAndBuilderConcatenationCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void test_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringBufferAndBuilderConcatenationCheckSample.java"))
+      .withCheck(new StringBufferAndBuilderConcatenationCheck())
+      .withoutSemantic()
+      .verifyIssues();
+  }
+}