SONARPY-1042: Fix the quick-fix for S3923 (AllBranchesAreIdenticalCheck) (#1153)

Author: marco-bearzi-sonarsource

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

@@ -54,7 +54,8 @@ public void initialize(Context context) {
   }
 
   private static void handleIfStatement(IfStatement ifStmt, SubscriptionContext ctx) {
-    if (ifStmt.elseBranch() == null) {
+    ElseClause elseBranch = ifStmt.elseBranch();
+    if (elseBranch == null) {
       return;
     }
     StatementList body = ifStmt.body();
@@ -64,13 +65,13 @@ private static void handleIfStatement(IfStatement ifStmt, SubscriptionContext ct
         return;
       }
     }
-    if (!CheckUtils.areEquivalent(body, ifStmt.elseBranch().body())) {
+    if (!CheckUtils.areEquivalent(body, elseBranch.body())) {
       return;
     }
     PreciseIssue issue = ctx.addIssue(ifStmt.keyword(), "Remove this if statement or edit its code blocks so that they're not all the same.");
     issue.secondary(issueLocation(ifStmt.body()));
     ifStmt.elifBranches().forEach(e -> issue.secondary(issueLocation(e.body())));
-    issue.secondary(issueLocation(ifStmt.elseBranch().body()));
+    issue.secondary(issueLocation(elseBranch.body()));
     createQuickFix((IssueWithQuickFix) issue, ifStmt, ifStmt.body().statements());
   }
 
@@ -126,8 +127,9 @@ private static void createQuickFixConditional(IssueWithQuickFix issue, Tree tree
     List<Tree> children = tree.children();
     Token lastTokenOfFirst = children.get(0).lastToken();
     Token lastTokenOfConditional = children.get(children.size() - 1).lastToken();
-    PythonTextEdit edit = new PythonTextEdit("", lastTokenOfFirst.line(), lastTokenOfFirst.column(),
-      lastTokenOfConditional.line(), lastTokenOfConditional.column());
+    // Keep the first statement and remove the rest
+    PythonTextEdit edit = new PythonTextEdit("", lastTokenOfFirst.line(), lastTokenOfFirst.column() + lastTokenOfFirst.value().length(),
+      lastTokenOfConditional.line(), lastTokenOfConditional.column() + lastTokenOfConditional.value().length());
 
     PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Remove the if statement")
       .addTextEdit(edit)
@@ -168,8 +170,8 @@ private static void createQuickFix(IssueWithQuickFix issue, Tree tree, List<Stat
     // Remove else branch
     elseBranch.ifPresent(branch -> quickFixBuilder.addTextEdit(PythonTextEdit.remove(branch)));
 
-    // Remove the indent on the else line
-    if (ifStatement.elifBranches().isEmpty()) {
+    // Remove the indent on the else line if it doesn't start at column 0
+    if (ifStatement.elifBranches().isEmpty() && elseBranch.map(b -> b.firstToken().column()).orElse(0) != 0) {
       lineElseBranch.ifPresent(lineElse -> quickFixBuilder.addTextEdit(editIndentAtLine(lineElse)));
     }
 

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

@@ -26,7 +26,7 @@
 
 public class AllBranchesAreIdenticalCheckTest {
 
-  private PythonCheck check = new AllBranchesAreIdenticalCheck();
+  private final PythonCheck check = new AllBranchesAreIdenticalCheck();
   @Test
   public void test() {
     PythonCheckVerifier.verify("src/test/resources/checks/allBranchesAreIdentical.py", check);
@@ -113,6 +113,10 @@ public void multiple_conditional_statements(){
     String codeWithIssue = "a = 1 if x else 1 if y else 1 if z else 1";
     String codeFixed = "a = 1";
     PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
+
+    codeWithIssue = "a = (1 if x else 1) if cond else 1";
+    codeFixed = "a = (1 if x else 1)";  // SONARPY-1047
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
   }
 
   @Test
@@ -175,4 +179,32 @@ public void test_complex_condition(){
     String codeFixed = "a = do_something(a, b, c, do_something_else(d))";
     PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
   }
+
+  @Test
+  public void test_remove(){
+    String codeWithIssue = ""+
+      "if b == 0:\n" +
+      "    doSomething()\n" +
+      "else:\n" +
+      "    doSomething()\n" +
+      "\n" +  // blank line should not be removed: SONARPY-1048
+      "a = 1";
+    String codeFixed = ""+
+      "doSomething()\n" +
+      "a = 1";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
+
+    codeWithIssue = ""+
+      "if b == 0:\n" +
+      "    doSomething()\n" +
+      "else:\n" +
+      "    doSomething()\n" +
+      "\n" +
+      "\n" +
+      "a = 1";
+    codeFixed = ""+
+      "doSomething()\n" +
+      "a = 1";
+    PythonQuickFixVerifier.verify(check, codeWithIssue, codeFixed);
+  }
 }