SONARPY-1024: Fix Quickfix for sets, parameters and improve format (ImplicitStringConcatenation) (#1149)

Author: marco-bearzi-sonarsource

python-checks/src/main/java/org/sonar/python/checks/ImplicitStringConcatenationCheck.java

@@ -30,7 +30,7 @@
 import org.sonar.python.quickfix.IssueWithQuickFix;
 import org.sonar.python.quickfix.PythonQuickFix;
 
-import static org.sonar.python.quickfix.PythonTextEdit.insertAfter;
+import static org.sonar.python.quickfix.PythonTextEdit.replaceRange;
 
 @Rule(key = "S5799")
 public class ImplicitStringConcatenationCheck extends PythonSubscriptionCheck {
@@ -67,12 +67,12 @@ private static void checkStringLiteral(StringLiteral stringLiteral, Subscription
         continue;
       }
       if (current.firstToken().line() == previous.firstToken().line()) {
-        createQuickFix(ctx.addIssue(previous.firstToken(), MESSAGE_SINGLE_LINE).secondary(current.firstToken(), null), previous);
+        createQuickFix(ctx.addIssue(previous.firstToken(), MESSAGE_SINGLE_LINE).secondary(current.firstToken(), null), previous, current);
         // Only raise 1 issue per string literal
         return;
       }
       if ((isWithinCollection(stringLiteral) && !isException(previous, current))) {
-        createQuickFix(ctx.addIssue(previous.firstToken(), MESSAGE_MULTIPLE_LINES).secondary(current.firstToken(), null), previous);
+        createQuickFix(ctx.addIssue(previous.firstToken(), MESSAGE_MULTIPLE_LINES).secondary(current.firstToken(), null), previous, current);
         return;
       }
     }
@@ -94,26 +94,31 @@ private static boolean haveSameQuotes(StringElement first, StringElement second)
       first.value().charAt(first.value().length() - 1) == second.value().charAt(second.value().length() - 1);
   }
 
-  private static boolean isInFunctionOrArrayOrTupleOrExpression(StringElement token) {
+  private static boolean isInFunctionOrArrayOrTupleOrExpressionOrSet(StringElement token) {
     Tree t = token;
     while (t.parent().is(Tree.Kind.STRING_LITERAL)) {
       t = t.parent();
     }
-    return t.parent().is(Tree.Kind.ARG_LIST, Tree.Kind.EXPRESSION_LIST, Tree.Kind.PARAMETER_LIST, Tree.Kind.TUPLE);
+    Tree parent = t.parent();
+
+    return parent.is(Tree.Kind.EXPRESSION_LIST, Tree.Kind.PLUS, Tree.Kind.REGULAR_ARGUMENT,
+      Tree.Kind.SET_LITERAL, Tree.Kind.TUPLE);
   }
 
-  private static void createQuickFix(PreciseIssue issueRaised, StringElement stringElement) {
+  private static void createQuickFix(PreciseIssue issueRaised, StringElement start, StringElement end) {
     IssueWithQuickFix issue = (IssueWithQuickFix) issueRaised;
+    String textStart = start.value();
+    String textEnd = end.value();
 
-    if (isInFunctionOrArrayOrTupleOrExpression(stringElement)) {
+    if (isInFunctionOrArrayOrTupleOrExpressionOrSet(start)) {
       PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Add the comma between string or byte tokens.")
-        .addTextEdit(insertAfter(stringElement, ","))
+        .addTextEdit(replaceRange(start, end, textStart + ", " + textEnd))
         .build();
       issue.addQuickFix(quickFix);
     }
 
     PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Make the addition sign between string or byte tokens explicit.")
-      .addTextEdit(insertAfter(stringElement, "+"))
+      .addTextEdit(replaceRange(start, end, textStart + " + " + textEnd))
       .build();
     issue.addQuickFix(quickFix);
   }

python-checks/src/test/java/org/sonar/python/checks/ImplicitStringConcatenationCheckTest.java

@@ -36,12 +36,12 @@ public void test() {
   @Test
   public void simple_expression_quickfix() {
     String codeWithIssue = "a = '1' '2'";
-    String codeFixed1 = "a = '1'+ '2'";
+    String codeFixed1 = "a = '1' + '2'";
     PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1);
 
     codeWithIssue = "a = ['1' '2']";
     codeFixed1 = "a = ['1', '2']";
-    String codeFixed2 = "a = ['1'+ '2']";
+    String codeFixed2 = "a = ['1' + '2']";
     PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
   }
 
@@ -52,7 +52,51 @@ public void complex_expression_quickfix() {
     String codeFixed1 = "def a():\n" +
       "    b = ['1', '2']\n";
     String codeFixed2 = "def a():\n" +
-      "    b = ['1'+ '2']\n";
+      "    b = ['1' + '2']\n";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
+  }
+
+  @Test
+  public void concat_expression() {
+    String codeWithIssue = "def a():\n" +
+      "    b = ['1'+'2' '3'+'4']\n";
+    String codeFixed1 = "def a():\n" +
+      "    b = ['1'+'2', '3'+'4']\n";
+    String codeFixed2 = "def a():\n" +
+      "    b = ['1'+'2' + '3'+'4']\n";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
+  }
+
+  @Test
+  public void sets() {
+    String codeWithIssue = "def a():\n" +
+      "    {'1' '2', '3'}\n";
+    String codeFixed1 = "def a():\n" +
+      "    {'1', '2', '3'}\n";
+    String codeFixed2 = "def a():\n" +
+      "    {'1' + '2', '3'}\n";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
+  }
+
+  @Test
+  public void parameters() {
+    String codeWithIssue = "def a():\n" +
+      "    print('1' '2', '3')\n";
+    String codeFixed1 = "def a():\n" +
+      "    print('1', '2', '3')\n";
+    String codeFixed2 = "def a():\n" +
+      "    print('1' + '2', '3')\n";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
+  }
+
+  @Test
+  public void parameters2() {
+    String codeWithIssue = "def a():\n" +
+      "    foo('1' '2', '3')\n";
+    String codeFixed1 = "def a():\n" +
+      "    foo('1', '2', '3')\n";
+    String codeFixed2 = "def a():\n" +
+      "    foo('1' + '2', '3')\n";
     PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed1, codeFixed2);
   }