Pretty print experiment

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

@@ -6,30 +6,29 @@
 public class CollapsibleIfCandidateCheck {
   private static final Logger LOGGER = Logger.getLogger(CollapsibleIfCandidateCheck.class.getCanonicalName());
 
+  // fix@qf1 {{Merge this if statement with the enclosing one}}
+  // edit@qf1 [[sl=-1;sc=5;el=+3;ec=6]] {{if (file != null && (file.isFile() || file.isDirectory())) {\n      LOGGER.log(Level.INFO, file.getAbsolutePath());\n    }}}
   void testMyFile(File file) {
+    // Noncompliant@+2 [[sc=7;ec=9;quickfixes=qf1]]
     if (file != null) {
-      if (file.isFile() || file.isDirectory()) { // Noncompliant [[sc=7;ec=9;quickfixes=qf1]]
+      if (file.isFile() || file.isDirectory()) {
         LOGGER.log(Level.INFO, file.getAbsolutePath());
       }
-      // fix@qf1 {{Merge this if statement with the enclosing one}}
-      // edit@qf1 [[sl=-1;el=+0;sc=21;ec=11]] {{ && }}
-      // edit@qf1 [[sc=11;ec=11]] {{(}}
-      // edit@qf1 [[sc=46;ec=46]] {{)}}
-      // edit@qf1 [[sl=+8;el=+8;sc=5;ec=6]] {{}}
     }
   }
 
+  // fix@qf2 {{Merge this if statement with the enclosing one}}
+  // edit@qf2 [[sl=-1;sc=5;el=+2;ec=8]] {{if (file != null && (file.isFile() || file.isDirectory())) {\n      LOGGER.log(Level.INFO, file.getAbsolutePath());\n    }}}
   void noBraceOnOuter(File file) {
+    // Noncompliant@+2 [[sc=7;ec=9;quickfixes=qf2]]
     if (file != null)
-      if (file.isFile() || file.isDirectory()) { // Noncompliant [[sc=7;ec=9;quickfixes=qf2]]
+      if (file.isFile() || file.isDirectory()) {
         LOGGER.log(Level.INFO, file.getAbsolutePath());
       }
-    // fix@qf2 {{Merge this if statement with the enclosing one}}
-    // edit@qf2 [[sl=-1;el=+0;sc=21;ec=11]] {{ && }}
-    // edit@qf2 [[sc=11;ec=11]] {{(}}
-    // edit@qf2 [[sc=46;ec=46]] {{)}}
   }
 
+  // TODO update other quickfixes
+
   void noBraceOnInner(File file) {
     if (file != null) {
       if (file.isFile() || file.isDirectory()) LOGGER.log(Level.INFO, file.getAbsolutePath()); // Noncompliant [[sc=7;ec=9;quickfixes=qf3]]

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

@@ -3,12 +3,11 @@
 public class SingleIfInsteadOfPatternMatchGuardCheckSample {
 
   // fix@qf3 {{Merge this "if" statement with the enclosing pattern match guard.}}
-  // edit@qf3 [[sl=+0;el=+2;sc=9;ec=10]] {{System.out.println("two");}}
-  // edit@qf3 [[sl=-2;el=-2;sc=44;ec=44]] {{ && s.length() == 2 }}
+  // edit@qf3 [[sl=-1;el=+3;sc=7;ec=8]] {{case String s when s.startsWith("a") && s.length() == 2 -> {\n        System.out.println("two");\n      }}}
   void conditionsShouldBeMerged(Object o) {
     switch (o) {
+      // Noncompliant@+2 [[sl=+2;el=+4;sc=9;ec=10;quickfixes=qf3]] {{Merge this "if" statement with the enclosing pattern match guard.}}
       case String s when s.startsWith("a") -> {
-        // Noncompliant@+1 [[sl=+1;el=+3;sc=9;ec=10;quickfixes=qf3]] {{Merge this "if" statement with the enclosing pattern match guard.}}
         if (s.length() == 2) {
           System.out.println("two");
         }

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

@@ -24,14 +24,14 @@
 import java.util.Deque;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.prettyprint.FileConfig;
+import org.sonar.java.checks.prettyprint.PrettyPrintStringBuilder;
 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.BaseTreeVisitor;
-import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
 import org.sonar.plugins.java.api.tree.BlockTree;
-import org.sonar.plugins.java.api.tree.ExpressionTree;
 import org.sonar.plugins.java.api.tree.IfStatementTree;
 import org.sonar.plugins.java.api.tree.StatementTree;
 import org.sonar.plugins.java.api.tree.Tree;
@@ -96,29 +96,15 @@ private static boolean hasBodySingleIfStatement(StatementTree thenStatement) {
     return false;
   }
 
-  private static JavaQuickFix computeQuickFix(IfStatementTree innerIf, IfStatementTree outerIf) {
+  private JavaQuickFix computeQuickFix(IfStatementTree innerIf, IfStatementTree outerIf) {
     var quickFixBuilder = JavaQuickFix.newQuickFix("Merge this if statement with the enclosing one");
-    quickFixBuilder.addTextEdit(
-      JavaTextEdit.replaceBetweenTree(outerIf.condition(), false, innerIf.condition(), false, " && "));
-    addParenthesisIfRequired(quickFixBuilder, outerIf.condition());
-    addParenthesisIfRequired(quickFixBuilder, innerIf.condition());
-
-    if (outerIf.thenStatement() instanceof BlockTree outerBlock) {
-      quickFixBuilder.addTextEdit(JavaTextEdit.removeTree(outerBlock.closeBraceToken()));
-    }
+    quickFixBuilder.addTextEdit(JavaTextEdit.replaceTree(
+      outerIf,
+      new PrettyPrintStringBuilder(FileConfig.DEFAULT_FILE_CONFIG, outerIf.firstToken(), false)
+        .add("if (").addBinop(outerIf.condition(), Tree.Kind.CONDITIONAL_AND, innerIf.condition(), context).add(") ")
+        .addTreeContentWithIndentBasedOnLastLine(innerIf.thenStatement(), context)
+        .toString()
+    ));
     return quickFixBuilder.build();
   }
-
-  private static void addParenthesisIfRequired(JavaQuickFix.Builder quickFixBuilder, ExpressionTree expression) {
-    if (isLowerOperatorPrecedenceThanLogicalAnd(expression)) {
-      quickFixBuilder.addTextEdit(JavaTextEdit.insertBeforeTree(expression, "("));
-      quickFixBuilder.addTextEdit(JavaTextEdit.insertAfterTree(expression, ")"));
-    }
-  }
-
-  private static boolean isLowerOperatorPrecedenceThanLogicalAnd(ExpressionTree expression) {
-    return (expression instanceof BinaryExpressionTree binExpression)
-      ? "||".equals(binExpression.operatorToken().text())
-      : expression.is(Tree.Kind.CONDITIONAL_EXPRESSION, Tree.Kind.ASSIGNMENT);
-  }
 }

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

@@ -22,10 +22,13 @@
 import java.util.Deque;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import javax.annotation.Nullable;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.prettyprint.FileConfig;
+import org.sonar.java.checks.prettyprint.PrettyPrintStringBuilder;
 import org.sonar.java.reporting.JavaQuickFix;
 import org.sonar.java.reporting.JavaTextEdit;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
@@ -44,12 +47,13 @@
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.Tree.Kind;
 
+import static org.sonar.java.checks.helpers.QuickFixHelper.contentForTree;
+
 
 @Rule(key = "S6880")
 public class PatternMatchUsingIfCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
 
   private static final String ISSUE_MESSAGE = "Replace the chain of if/else with a switch expression.";
-  private static final int INDENT = 2;
   private static final Set<String> SCRUTINEE_TYPES_FOR_NON_PATTERN_SWITCH = Set.of(
     "byte", "short", "char", "int",
     "java.lang.Byte", "java.lang.Short", "java.lang.Character", "java.lang.Integer"
@@ -200,73 +204,47 @@ private static boolean hasElseIf(IfStatementTree ifStat) {
 
   private JavaQuickFix computeQuickFix(List<Case> cases, IfStatementTree topLevelIfStat) {
     var canLiftReturn = cases.stream().allMatch(caze -> exprWhenReturnLifted(caze) != null);
-    var baseIndent = topLevelIfStat.firstToken().range().start().column() - 1;
-    var sb = new StringBuilder();
+    var pps = new PrettyPrintStringBuilder(FileConfig.DEFAULT_FILE_CONFIG, topLevelIfStat.firstToken(), false);
     if (canLiftReturn) {
-      sb.append("return ");
-    }
-    sb.append("switch (").append(cases.get(0).scrutinee().name()).append(") {\n");
-    for (Case caze : cases) {
-      sb.append(" ".repeat(baseIndent + INDENT));
-      writeCase(caze, sb, baseIndent, canLiftReturn);
-      sb.append("\n");
+      pps.add("return ");
     }
-    sb.append(" ".repeat(baseIndent)).append("}");
+    pps.add("switch (").add(cases.get(0).scrutinee().name()).add(") ")
+      .blockStart()
+      .addWithSep(cases, caze -> writeCase(caze, pps, canLiftReturn), pps::newLine)
+      .blockEnd();
     if (canLiftReturn) {
-      sb.append(";");
+      pps.add(";");
     }
-    var edit = JavaTextEdit.replaceTree(topLevelIfStat, sb.toString());
+    var edit = JavaTextEdit.replaceTree(topLevelIfStat, pps.toString());
     return JavaQuickFix.newQuickFix(ISSUE_MESSAGE).addTextEdit(edit).build();
   }
 
-  private void writeCase(Case caze, StringBuilder sb, int baseIndent, boolean canLiftReturn) {
+  private void writeCase(Case caze, PrettyPrintStringBuilder pps, boolean canLiftReturn) {
     if (caze instanceof PatternMatchCase patternMatchCase) {
-      sb.append("case ").append(QuickFixHelper.contentForTree(patternMatchCase.pattern, context));
+      pps.add("case ").add(contentForTree(patternMatchCase.pattern, context));
       if (!patternMatchCase.guards().isEmpty()) {
         List<ExpressionTree> guards = patternMatchCase.guards();
-        sb.append(" when ");
-        join(guards, " && ", sb);
+        pps.add(" when ").addTreesContentWithSep(guards, " && ", context);
       }
     } else if (caze instanceof EqualityCase equalityCase) {
-      sb.append("case ");
-      join(equalityCase.constants, ", ", sb);
+      pps.add("case ").addTreesContentWithSep(equalityCase.constants, ", ", context);
     } else {
-      sb.append("default");
+      pps.add("default");
     }
-    sb.append(" -> ");
+    pps.add(" -> ");
     if (canLiftReturn) {
-      sb.append(exprWhenReturnLifted(caze));
+      pps.add(Objects.requireNonNull(exprWhenReturnLifted(caze)));
     } else {
-      addIndentedExceptFirstLine(makeBlockCode(caze.body(), baseIndent), sb);
+      makeBlockCode(caze.body(), pps);
     }
   }
 
-  private String makeBlockCode(StatementTree stat, int baseIndent) {
-    var rawCode = QuickFixHelper.contentForTree(stat, context);
+  private void makeBlockCode(StatementTree stat, PrettyPrintStringBuilder pps) {
+    var rawCode = contentForTree(stat, context);
     if (stat instanceof BlockTree) {
-      return rawCode;
+      pps.addWithIndentBasedOnLastLine(rawCode);
     } else {
-      return "{\n" + " ".repeat(baseIndent + INDENT) + rawCode + "\n" + " ".repeat(baseIndent) + "}";
-    }
-  }
-
-  private static void addIndentedExceptFirstLine(String s, StringBuilder sb) {
-    var lines = s.lines().iterator();
-    var indentStr = " ".repeat(INDENT);
-    sb.append(lines.next());
-    while (lines.hasNext()) {
-      sb.append("\n").append(indentStr).append(lines.next());
-    }
-  }
-
-  private void join(List<? extends Tree> elems, String sep, StringBuilder sb) {
-    var iter = elems.iterator();
-    while (iter.hasNext()) {
-      var e = iter.next();
-      sb.append(QuickFixHelper.contentForTree(e, context));
-      if (iter.hasNext()) {
-        sb.append(sep);
-      }
+      pps.blockStart().add(rawCode).blockEnd();
     }
   }
 
@@ -276,9 +254,9 @@ private void join(List<? extends Tree> elems, String sep, StringBuilder sb) {
       stat = block.body().get(0);
     }
     if (stat instanceof ReturnStatementTree returnStat && returnStat.expression() != null) {
-      return QuickFixHelper.contentForTree(returnStat.expression(), context) + ";";
+      return contentForTree(returnStat.expression(), context) + ";";
     } else if (stat instanceof ThrowStatementTree throwStatementTree) {
-      return QuickFixHelper.contentForTree(throwStatementTree, context);
+      return contentForTree(throwStatementTree, context);
     }
     return null;
   }

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

@@ -28,6 +28,8 @@
 import org.sonar.java.cfg.CFG;
 import org.sonar.java.cfg.LiveVariables;
 import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.prettyprint.FileConfig;
+import org.sonar.java.checks.prettyprint.PrettyPrintStringBuilder;
 import org.sonar.java.model.ExpressionUtils;
 import org.sonar.java.reporting.JavaQuickFix;
 import org.sonar.java.reporting.JavaTextEdit;
@@ -40,7 +42,6 @@
 import org.sonar.plugins.java.api.tree.IdentifierTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
-import org.sonar.plugins.java.api.tree.SyntaxToken;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.Tree.Kind;
 import org.sonar.plugins.java.api.tree.VariableTree;
@@ -105,16 +106,13 @@ private List<JavaQuickFix> computeQuickFix(Symbol.VariableSymbol symbol, Variabl
       return Collections.emptyList();
     }
     BlockTree block = methodWhereUsed.block();
-    SyntaxToken openingBrace = block.openBraceToken();
-
-    String padding = generateLeftPadding(block);
     String declarationMinusModifiers = contentForRange(declaration.type().firstToken(), declaration.endToken(), context);
-    String newDeclaration = "\n" + padding + declarationMinusModifiers;
-
+    var pps = new PrettyPrintStringBuilder(FileConfig.DEFAULT_FILE_CONFIG, block.body().get(0).firstToken(), false);
+    pps.newLine().addStripLeading(declarationMinusModifiers);
     return List.of(
       JavaQuickFix.newQuickFix(QUICK_FIX_MESSAGE)
         .addTextEdits(editUsagesWithThis(symbol))
-        .addTextEdit(JavaTextEdit.insertAfterTree(openingBrace, newDeclaration))
+        .addTextEdit(JavaTextEdit.insertAfterTree(block.openBraceToken(), pps.toString()))
         .addTextEdit(JavaTextEdit.removeTree(declaration))
         .build()
     );
@@ -135,11 +133,6 @@ private static boolean wouldRelocationClashWithLocalVariables(Symbol.VariableSym
       .anyMatch(variable -> variable.symbol().name().equals(symbol.name()));
   }
 
-  private static String generateLeftPadding(BlockTree block) {
-    int spacesOnTheLeft = Math.max(0, block.body().get(0).firstToken().range().start().column() - 1);
-    return " ".repeat(spacesOnTheLeft);
-  }
-
   /**
    * Returns edits to transform all usages in the form of this.myVariable to myVariable.
    */

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

@@ -22,6 +22,8 @@
 import java.util.List;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.prettyprint.FileConfig;
+import org.sonar.java.checks.prettyprint.PrettyPrintStringBuilder;
 import org.sonar.java.reporting.JavaQuickFix;
 import org.sonar.java.reporting.JavaTextEdit;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
@@ -34,6 +36,7 @@
 import org.sonar.plugins.java.api.tree.GuardedPatternTree;
 import org.sonar.plugins.java.api.tree.IfStatementTree;
 import org.sonar.plugins.java.api.tree.NullPatternTree;
+import org.sonar.plugins.java.api.tree.PatternTree;
 import org.sonar.plugins.java.api.tree.Tree;
 
 
@@ -63,17 +66,15 @@ public void visitNode(Tree tree) {
       return;
     }
     var caseLabel = caseGroup.labels().get(0);
-    if (isCaseDefaultOrNull(caseLabel)) {
+    var pattern = extractNonDefaultPattern(caseLabel);
+    if (pattern == null) {
       return;
     }
 
-    var caseExpression = caseLabel.expressions().get(0);
-    boolean isGuardedPattern = caseExpression instanceof GuardedPatternTree;
-
     QuickFixHelper.newIssue(context).forRule(this)
       .onTree(ifStatement)
-      .withMessage(isGuardedPattern ? ISSUE_MESSAGE_MERGE : ISSUE_MESSAGE_REPLACE)
-      .withQuickFix(() -> computeQuickFix(ifStatement, caseLabel, isGuardedPattern, context))
+      .withMessage(pattern instanceof GuardedPatternTree ? ISSUE_MESSAGE_MERGE : ISSUE_MESSAGE_REPLACE)
+      .withQuickFix(() -> computeQuickFix(caseGroup, pattern, ifStatement, context))
       .report();
 
   }
@@ -89,29 +90,28 @@ private static IfStatementTree getFirstIfStatementInCaseBody(CaseGroupTree caseG
     return null;
   }
 
-  private static boolean isCaseDefaultOrNull(CaseLabelTree caseLabel) {
-    return caseLabel.expressions().isEmpty() || caseLabel.expressions().get(0) instanceof NullPatternTree;
+  private static PatternTree extractNonDefaultPattern(CaseLabelTree caseLabel) {
+    return (caseLabel.expressions().size() == 1
+      && caseLabel.expressions().get(0) instanceof PatternTree patternTree
+      && !(patternTree instanceof NullPatternTree)
+    ) ?
+      patternTree : null;
   }
 
-  private static JavaQuickFix computeQuickFix(IfStatementTree ifStatement, CaseLabelTree caseLabel, boolean shouldMergeConditions,
-    JavaFileScannerContext context) {
+  private static JavaQuickFix computeQuickFix(CaseGroupTree caseGroup, PatternTree pattern, IfStatementTree ifStatement,
+                                              JavaFileScannerContext context) {
+    var shouldMergeConditions = pattern instanceof GuardedPatternTree;
     var quickFixBuilder = JavaQuickFix.newQuickFix(shouldMergeConditions ? ISSUE_MESSAGE_MERGE : ISSUE_MESSAGE_REPLACE);
-    String replacement;
-    if (ifStatement.thenStatement() instanceof BlockTree block && !block.body().isEmpty()) {
-      var firstToken = QuickFixHelper.nextToken(block.openBraceToken());
-      var lastToken = QuickFixHelper.previousToken(block.closeBraceToken());
-      replacement = QuickFixHelper.contentForRange(firstToken, lastToken, context);
+    var pps = new PrettyPrintStringBuilder(FileConfig.DEFAULT_FILE_CONFIG, caseGroup.firstToken(), false);
+    pps.add("case ");
+    if (pattern instanceof GuardedPatternTree guardedPattern){
+      pps.addTreeContentRaw(guardedPattern.pattern(), context)
+        .add(" when ").addBinop(guardedPattern.expression(), Tree.Kind.CONDITIONAL_AND, ifStatement.condition(), context);
     } else {
-      replacement = QuickFixHelper.contentForTree(ifStatement.thenStatement(), context);
+      pps.addTreeContentRaw(pattern, context).add(" when ").addTreeContentRaw(ifStatement.condition(), context);
     }
-    quickFixBuilder.addTextEdit(
-      JavaTextEdit.replaceTree(ifStatement, replacement)
-    );
-    var replacementStringPrefix = shouldMergeConditions ? " && " : " when ";
-    quickFixBuilder.addTextEdit(
-      JavaTextEdit.insertBeforeTree(caseLabel.colonOrArrowToken(),
-        replacementStringPrefix + QuickFixHelper.contentForTree(ifStatement.condition(), context) + " ")
-    );
+    pps.add(" -> ").addTreeContentWithIndentBasedOnLastLine(ifStatement.thenStatement(), context);
+    quickFixBuilder.addTextEdit(JavaTextEdit.replaceTree(caseGroup, pps.toString()));
     return quickFixBuilder.build();
   }