SONARJAVA-4502 Add quickfix for S1153 (#4538)

SonarSource · Nov 16, 2023 · ac75ee7 · ac75ee7
1 parent abe5248
commit ac75ee7
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 9 deletions.
diff --git a/java-checks-test-sources/src/main/java/checks/ConcatenationWithStringValueOfCheck.java b/java-checks-test-sources/src/main/java/checks/ConcatenationWithStringValueOfCheck.java
@@ -35,5 +35,46 @@ private void f(int buf, ConcatenationWithStringValueOfCheck a) {
     o = 42 - 2;
   }
 
+  private void quickfixes() {
+    var num = 99;
+    var a = "" + String.valueOf(num) + " balloons"; // Noncompliant [[sc=18;ec=37;quickfixes=qf1]] {{Directly append the argument of String.valueOf().}}
+    // fix@qf1 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf1 [[sc=18;ec=37]] {{num}}
+
+    a = "" + String.valueOf(num + num); // Noncompliant [[sc=14;ec=39;quickfixes=qf2]]
+    // fix@qf2 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf2 [[sc=14;ec=39]] {{(num + num)}}
+
+    a = "" + String.valueOf(num - num); // Noncompliant [[sc=14;ec=39;quickfixes=qf3]]
+    // fix@qf3 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf3 [[sc=14;ec=39]] {{(num - num)}}
+
+    a = "" + String.valueOf(-num); // Noncompliant [[sc=14;ec=34;quickfixes=qf4]]
+    // fix@qf4 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf4 [[sc=14;ec=34]] {{-num}}
+
+    // Noncompliant@+1 [[sc=14;sl=+1;ec=6;el=+4;quickfixes=qfml]]
+    a = "" + String.valueOf(2 +
+      3 * 4 -
+      5 / 6
+    ) + 7;
+
+    // fix@qfml {{Remove redundant String.valueOf() wrapping}}
+    // edit@qfml [[sc=14;ec=6;el=+3]] {{(2 +\n      3 * 4 -\n      5 / 6)}}
+
+    var world = "world";
+    var b = "Hello, " + String.valueOf(world + world); // Noncompliant [[sc=25;ec=54;quickfixes=qf5]]
+    // fix@qf5 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf5 [[sc=25;ec=54]] {{world + world}}
+
+    b = "Hello, " + String.valueOf(world.charAt(2)) + "!"; // Noncompliant [[sc=21;ec=52;quickfixes=qf6]]
+    // fix@qf6 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf6 [[sc=21;ec=52]] {{world.charAt(2)}}
+
+    b = "Hello, " + String.valueOf((world.charAt(2))) + "!"; // Noncompliant [[sc=21;ec=54;quickfixes=qf7]]
+    // fix@qf7 {{Remove redundant String.valueOf() wrapping}}
+    // edit@qf7 [[sc=21;ec=54]] {{(world.charAt(2))}}
+  }
+
   int bar(int i) { return 0; }
 }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/ConcatenationWithStringValueOfCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ConcatenationWithStringValueOfCheck.java
@@ -19,7 +19,12 @@
  */
 package org.sonar.java.checks;
 
+import java.util.HashSet;
+import java.util.Set;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.tree.Arguments;
@@ -31,9 +36,6 @@
 import org.sonar.plugins.java.api.tree.MethodInvocationTree;
 import org.sonar.plugins.java.api.tree.Tree.Kind;
 
-import java.util.HashSet;
-import java.util.Set;
-
 @Rule(key = "S1153")
 public class ConcatenationWithStringValueOfCheck extends BaseTreeVisitor implements JavaFileScanner {
 
@@ -52,14 +54,14 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
       return;
     }
 
-    Set<ExpressionTree> valueOfTrees = new HashSet<>();
+    Set<MethodInvocationTree> valueOfTrees = new HashSet<>();
     boolean flagIssue = false;
     ExpressionTree current = tree;
     while (current.is(Kind.PLUS)) {
       BinaryExpressionTree binOp = (BinaryExpressionTree) current;
       scan(binOp.rightOperand());
       if (isStringValueOf(binOp.rightOperand())) {
-        valueOfTrees.add(binOp.rightOperand());
+        valueOfTrees.add((MethodInvocationTree) binOp.rightOperand());
       }
       flagIssue |= binOp.leftOperand().is(Kind.STRING_LITERAL);
       if (!valueOfTrees.isEmpty()) {
@@ -69,13 +71,31 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
     }
 
     if (flagIssue) {
-      for (ExpressionTree valueOfTree : valueOfTrees) {
-        context.reportIssue(this, valueOfTree, "Directly append the argument of String.valueOf().");
+      for (MethodInvocationTree valueOfTree : valueOfTrees) {
+        QuickFixHelper.newIssue(context)
+          .forRule(this)
+          .onTree(valueOfTree)
+          .withMessage("Directly append the argument of String.valueOf().")
+          .withQuickFix(() -> createQuickFix(valueOfTree))
+          .report();
       }
     }
     scan(current);
   }
 
+  private JavaQuickFix createQuickFix(MethodInvocationTree invocationTree) {
+    ExpressionTree argumentTree = invocationTree.arguments().get(0);
+    String replacement = QuickFixHelper.contentForTree(argumentTree, context);
+
+    if (argumentTree instanceof BinaryExpressionTree && !argumentTree.symbolType().is("java.lang.String")) {
+      replacement = "(" + replacement + ")";
+    }
+
+    return JavaQuickFix.newQuickFix("Remove redundant String.valueOf() wrapping")
+      .addTextEdit(JavaTextEdit.replaceTree(invocationTree, replacement))
+      .build();
+  }
+
   private static boolean isStringValueOf(ExpressionTree tree) {
     return tree.is(Kind.METHOD_INVOCATION) && isStringValueOf((MethodInvocationTree) tree);
   }

diff --git a/java-checks/src/test/java/org/sonar/java/checks/ConcatenationWithStringValueOfCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/ConcatenationWithStringValueOfCheckTest.java
@@ -21,17 +21,18 @@
 
 import org.junit.jupiter.api.Test;
 import org.sonar.java.checks.verifier.CheckVerifier;
+import org.sonar.java.checks.verifier.internal.InternalCheckVerifier;
 
 import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
 
 class ConcatenationWithStringValueOfCheckTest {
 
   @Test
   void test() {
-    CheckVerifier.newVerifier()
+    InternalCheckVerifier.newInstance()
       .onFile(mainCodeSourcesPath("checks/ConcatenationWithStringValueOfCheck.java"))
       .withCheck(new ConcatenationWithStringValueOfCheck())
+      .withQuickFixes()
       .verifyIssues();
   }
-
 }