[StatementSwitchToExpressionSwitch] Combine immediately-preceding uni…

…nitialized variable with assignment via assignment switch

PiperOrigin-RevId: 705971103

core/src/main/java/com/google/errorprone/bugpatterns/StatementSwitchToExpressionSwitch.java

@@ -42,7 +42,9 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.SwitchTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher;
 import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
 import com.google.errorprone.util.ASTHelpers;
 import com.google.errorprone.util.ErrorProneComment;
@@ -56,16 +58,22 @@
 import com.sun.source.tree.ExpressionStatementTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.ModifiersTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.SwitchTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.Tree.Kind;
+import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePath;
+import com.sun.source.util.TreePathScanner;
 import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.tree.JCTree.JCAssign;
 import com.sun.tools.javac.tree.JCTree.JCAssignOp;
+import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
 import com.sun.tools.javac.tree.Pretty;
 import java.io.BufferedReader;
 import java.io.CharArrayReader;
@@ -84,6 +92,7 @@
 import javax.lang.model.element.ElementKind;
 
 /** Checks for statement switches that can be expressed as an equivalent expression switch. */
+@SuppressWarnings("PatternMatchingInstanceof")
 @BugPattern(
     severity = WARNING,
     summary = "This statement switch can be converted to an equivalent expression switch")
@@ -101,6 +110,7 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker
   private static final AssignmentSwitchAnalysisResult DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT =
       AssignmentSwitchAnalysisResult.of(
           /* canConvertToAssignmentSwitch= */ false,
+          /* canCombineWithPrecedingVariableDeclaration= */ false,
           /* assignmentTargetOptional= */ Optional.empty(),
           /* assignmentKindOptional= */ Optional.empty(),
           /* assignmentSourceCodeOptional= */ Optional.empty());
@@ -112,6 +122,8 @@ public final class StatementSwitchToExpressionSwitch extends BugChecker
           DEFAULT_ASSIGNMENT_SWITCH_ANALYSIS_RESULT,
           /* groupedWithNextCase= */ ImmutableList.of());
   private static final String EQUALS_STRING = "=";
+  private static final Matcher<ExpressionTree> compileTimeConstExpressionMatcher =
+      CompileTimeConstantExpressionMatcher.instance();
 
   // Tri-state to represent the fall-thru control flow of a particular case of a particular
   // statement switch
@@ -282,11 +294,46 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt
             // The switch must be exhaustive (at compile time)
             && exhaustive;
 
+    List<StatementTree> precedingStatements = getPrecedingStatementsInBlock(switchTree, state);
+    Optional<ExpressionTree> assignmentTarget =
+        assignmentSwitchAnalysisState.assignmentTargetOptional();
+
+    // If present, the variable tree that can be combined with the switch block
+    Optional<VariableTree> combinableVariableTree =
+        precedingStatements.isEmpty()
+            ? Optional.empty()
+            : Optional.of(precedingStatements)
+                // Don't try to combine when multiple variables are declared together
+                .filter(
+                    StatementSwitchToExpressionSwitch
+                        ::precedingTwoStatementsNotInSameVariableDeclaratorList)
+                // Extract the immediately preceding statement
+                .map(statements -> statements.get(statements.size() - 1))
+                // Preceding statement must be a variable declaration
+                .filter(precedingStatement -> precedingStatement instanceof VariableTree)
+                .map(precedingStatement -> (VariableTree) precedingStatement)
+                // Variable must be uninitialized, or initialized with a compile-time constant
+                .filter(
+                    variableTree ->
+                        (variableTree.getInitializer() == null)
+                            || compileTimeConstExpressionMatcher.matches(
+                                variableTree.getInitializer(), state))
+                // If we are reading the initialized value in the switch block, we can't remove it
+                .filter(
+                    variableTree -> noReadsOfVariable(ASTHelpers.getSymbol(variableTree), state))
+                // Type of the variable and the switch assignment must be compatible
+                .filter(
+                    variableTree ->
+                        isCompatibleWithFirstAssignment(assignmentTarget, variableTree));
+    boolean canCombineWithPrecedingVariableDeclaration =
+        canConvertToAssignmentSwitch && combinableVariableTree.isPresent();
+
     return AnalysisResult.of(
         /* canConvertDirectlyToExpressionSwitch= */ allCasesHaveDefiniteControlFlow,
         canConvertToReturnSwitch,
         AssignmentSwitchAnalysisResult.of(
             canConvertToAssignmentSwitch,
+            canCombineWithPrecedingVariableDeclaration,
             assignmentSwitchAnalysisState.assignmentTargetOptional(),
             assignmentSwitchAnalysisState.assignmentExpressionKindOptional(),
             assignmentSwitchAnalysisState
@@ -295,6 +342,47 @@ private static AnalysisResult analyzeSwitchTree(SwitchTree switchTree, VisitorSt
         ImmutableList.copyOf(groupedWithNextCase));
   }
 
+  /**
+   * Returns whether the variable {@code symbol} is read within the scope of the {@code
+   * VisitorState}. (Writes to the variable are ignored.)
+   */
+  private static boolean noReadsOfVariable(VarSymbol symbol, VisitorState state) {
+
+    // MockNotUsedInProduction.java
+    Set<VarSymbol> referencedLocalVariables = new HashSet<>();
+    new TreePathScanner<Void, Void>() {
+
+      @Override
+      public Void visitAssignment(AssignmentTree tree, Void unused) {
+        // Only looks at the right-hand side of the assignment
+        return scan(tree.getExpression(), null);
+      }
+
+      @Override
+      public Void visitMemberSelect(MemberSelectTree memberSelect, Void unused) {
+        handle(memberSelect);
+        return super.visitMemberSelect(memberSelect, null);
+      }
+
+      @Override
+      public Void visitIdentifier(IdentifierTree identifier, Void unused) {
+        handle(identifier);
+        return super.visitIdentifier(identifier, null);
+      }
+
+      private void handle(Tree tree) {
+        var symbol = getSymbol(tree);
+        if (symbol instanceof VarSymbol varSymbol) {
+          referencedLocalVariables.add(varSymbol);
+        }
+      }
+    }.scan(state.getPath(), null);
+
+    System.out.println(
+        String.format("XXX referencedLocalVariables: %s", referencedLocalVariables.toString()));
+    return !referencedLocalVariables.contains(symbol);
+  }
+
   /**
    * Renders the Java source code for a [compound] assignment operator. The parameter must be either
    * an {@code AssignmentTree} or a {@code CompoundAssignmentTree}.
@@ -488,6 +576,19 @@ private static boolean isCompatibleWithFirstAssignment(
     return Objects.equals(assignmentTargetSymbol, caseAssignmentTargetSymbol);
   }
 
+  private static boolean isCompatibleWithFirstAssignment(
+      Optional<ExpressionTree> assignmentTargetOptional, VariableTree variableDefinition) {
+
+    if (assignmentTargetOptional.isEmpty()) {
+      return false;
+    }
+
+    Symbol assignmentTargetSymbol = getSymbol(assignmentTargetOptional.get());
+
+    Symbol definedSymbol = ASTHelpers.getSymbol(variableDefinition);
+    return Objects.equals(assignmentTargetSymbol, definedSymbol);
+  }
+
   /**
    * Determines whether the supplied case's {@code statements} are capable of being mapped to an
    * equivalent expression switch case (without repeating code), returning {@code true} if so.
@@ -740,6 +841,36 @@ private static SuggestedFix convertToReturnSwitch(
     return suggestedFixBuilder.build();
   }
 
+  private static List<StatementTree> getPrecedingStatementsInBlock(
+      SwitchTree switchTree, VisitorState state) {
+
+    List<StatementTree> precedingStatements = new ArrayList<>();
+
+    if (!Matchers.previousStatement(Matchers.<StatementTree>anything())
+        .matches(switchTree, state)) {
+      // No lowest-ancestor block or no preceding statements
+      System.out.println("XXX no preceding statement");
+      return precedingStatements;
+    }
+
+    // Fetch the lowest ancestor statement block
+    TreePath pathToEnclosing = state.findPathToEnclosing(BlockTree.class);
+    // NOMUTANTS--should early return above
+    if (pathToEnclosing != null) {
+      Tree enclosing = pathToEnclosing.getLeaf();
+      if (enclosing instanceof BlockTree blockTree) {
+        // Path from root -> switchTree
+        TreePath rootToSwitchPath = TreePath.getPath(pathToEnclosing, switchTree);
+
+        for (int i = 0; i < findBlockStatementIndex(rootToSwitchPath, blockTree); i++) {
+          precedingStatements.add(blockTree.getStatements().get(i));
+        }
+      }
+    }
+    // Should have returned above
+    return precedingStatements;
+  }
+
   /**
    * Retrieves a list of all statements (if any) following the supplied {@code SwitchTree} in its
    * lowest-ancestor statement block (if any).
@@ -790,6 +921,34 @@ private static int findBlockStatementIndex(TreePath treePath, BlockTree blockTre
     return -1;
   }
 
+  /**
+   * Determines whether the last two preceding statements are not variable declarations within the
+   * same VariableDeclaratorList, for example {@code int x, y;}. VariableDeclaratorLists are defined
+   * in e.g. JLS 21 § 14.4. Precondition: all preceding statements are taken from the same {@code
+   * BlockTree}.
+   */
+  private static boolean precedingTwoStatementsNotInSameVariableDeclaratorList(
+      List<StatementTree> precedingStatements) {
+
+    if (precedingStatements.size() < 2) {
+      return true;
+    }
+
+    StatementTree secondToLastStatement = precedingStatements.get(precedingStatements.size() - 2);
+    StatementTree lastStatement = precedingStatements.get(precedingStatements.size() - 1);
+    if (!(secondToLastStatement instanceof VariableTree)
+        || !(lastStatement instanceof VariableTree)) {
+      return true;
+    }
+
+    VariableTree variableTree1 = (VariableTree) secondToLastStatement;
+    VariableTree variableTree2 = (VariableTree) lastStatement;
+
+    // Start positions will vary if the variable declarations are in the same
+    // VariableDeclaratorList.
+    return getStartPosition(variableTree1) != getStartPosition(variableTree2);
+  }
+
   /**
    * Transforms the supplied statement switch into an assignment switch style of expression switch.
    * In this conversion, each nontrivial statement block is mapped one-to-one to a new expression on
@@ -800,30 +959,94 @@ private static int findBlockStatementIndex(TreePath treePath, BlockTree blockTre
   private static SuggestedFix convertToAssignmentSwitch(
       SwitchTree switchTree, VisitorState state, AnalysisResult analysisResult) {
 
+    List<StatementTree> statementsToDelete = new ArrayList<>();
+    StringBuilder replacementCodeBuilder = new StringBuilder();
+
+    if (analysisResult
+        .assignmentSwitchAnalysisResult()
+        .canCombineWithPrecedingVariableDeclaration()) {
+
+      List<StatementTree> precedingStatements = getPrecedingStatementsInBlock(switchTree, state);
+      VariableTree variableTree =
+          (VariableTree) precedingStatements.get(precedingStatements.size() - 1);
+
+      if (variableTree instanceof JCVariableDecl) {
+
+        JCVariableDecl variableDecl = (JCVariableDecl) variableTree;
+        System.out.println(
+            "XXX JCVariableDecl: --->"
+                + state.getSourceForNode(variableDecl)
+                + " <---"
+                + " name "
+                + variableDecl.name.toString());
+      }
+
+      System.out.println(
+          "XXX variableTree: " + state.getSourceForNode(variableTree) + " which is comprised of:");
+
+      statementsToDelete.add(variableTree);
+
+      ImmutableList<ErrorProneComment> allVariableComments =
+          state.getTokensForNode(variableTree).stream()
+              .flatMap(errorProneToken -> errorProneToken.comments().stream())
+              .collect(toImmutableList());
+
+      Symbol assignmentTargetSymbol = getSymbol(variableTree.getNameExpression());
+      System.out.println("XXX assignmentTargetSymbol in def: " + assignmentTargetSymbol);
+
+      allVariableComments.stream()
+          .filter(comment -> !comment.getText().isEmpty())
+          .forEach(
+              comment -> {
+                //                            + comment.content().length();
+                replacementCodeBuilder.append(comment.getText()).append("\n");
+              });
+
+      ModifiersTree modifiersTree = variableTree.getModifiers();
+      if (!modifiersTree.getAnnotations().isEmpty()) {
+        modifiersTree
+            .getAnnotations()
+            .forEach(
+                annotation -> {
+                  System.out.println("XXX annotation: " + state.getSourceForNode(annotation));
+                  replacementCodeBuilder.append(state.getSourceForNode(annotation)).append("\n");
+                });
+      }
+
+      if (!modifiersTree.getFlags().isEmpty()) {
+
+        modifiersTree
+            .getFlags()
+            .forEach(
+                flag -> {
+                  System.out.println("XXX flag: " + flag);
+                  replacementCodeBuilder.append(flag.toString()).append(" ");
+                });
+      }
+
+      replacementCodeBuilder.append(state.getSourceForNode(variableTree.getType())).append(" ");
+
+      // replacementCodeBuilder.append(variableTree.getName().toString()).append(" = ");
+    }
+
     List<? extends CaseTree> cases = switchTree.getCases();
     ImmutableList<ErrorProneComment> allSwitchComments =
         state.getTokensForNode(switchTree).stream()
             .flatMap(errorProneToken -> errorProneToken.comments().stream())
             .collect(toImmutableList());
 
-    StringBuilder replacementCodeBuilder =
-        new StringBuilder(
-                state.getSourceForNode(
-                    analysisResult
-                        .assignmentSwitchAnalysisResult()
-                        .assignmentTargetOptional()
-                        .get()))
-            .append(" ")
-            // Invariant: always present when a finding exists
-            .append(
-                analysisResult
-                    .assignmentSwitchAnalysisResult()
-                    .assignmentSourceCodeOptional()
-                    .get())
-            .append(" ")
-            .append("switch ")
-            .append(state.getSourceForNode(switchTree.getExpression()))
-            .append(" {");
+    replacementCodeBuilder
+        .append(
+            state.getSourceForNode(
+                analysisResult.assignmentSwitchAnalysisResult().assignmentTargetOptional().get()))
+        .append(" ")
+        // Invariant: always present when a finding exists
+        .append(
+            analysisResult.assignmentSwitchAnalysisResult().assignmentSourceCodeOptional().get())
+        .append(" ")
+        .append("switch ")
+        .append(state.getSourceForNode(switchTree.getExpression()))
+        .append(" {");
 
     StringBuilder groupedCaseCommentsAccumulator = null;
     boolean firstCaseInGroup = true;
@@ -903,7 +1126,10 @@ private static SuggestedFix convertToAssignmentSwitch(
     // Close the switch statement
     replacementCodeBuilder.append("\n};");
 
-    return SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString()).build();
+    SuggestedFix.Builder suggestedFixBuilder =
+        SuggestedFix.builder().replace(switchTree, replacementCodeBuilder.toString());
+    statementsToDelete.forEach(deleteMe -> suggestedFixBuilder.replace(deleteMe, ""));
+    return suggestedFixBuilder.build();
   }
 
   /**
@@ -1258,6 +1484,10 @@ abstract static class AssignmentSwitchAnalysisResult {
     // Whether the statement switch can be converted to an assignment switch
     abstract boolean canConvertToAssignmentSwitch();
 
+    // Whether the assignment switch can be combined with the immediately preceding variable
+    // declaration
+    abstract boolean canCombineWithPrecedingVariableDeclaration();
+
     // Target of the assignment switch, if any
     abstract Optional<ExpressionTree> assignmentTargetOptional();
 
@@ -1269,11 +1499,13 @@ abstract static class AssignmentSwitchAnalysisResult {
 
     static AssignmentSwitchAnalysisResult of(
         boolean canConvertToAssignmentSwitch,
+        boolean canCombineWithPrecedingVariableDeclaration,
         Optional<ExpressionTree> assignmentTargetOptional,
         Optional<Tree.Kind> assignmentKindOptional,
         Optional<String> assignmentSourceCodeOptional) {
       return new AutoValue_StatementSwitchToExpressionSwitch_AssignmentSwitchAnalysisResult(
           canConvertToAssignmentSwitch,
+          canCombineWithPrecedingVariableDeclaration,
           assignmentTargetOptional,
           assignmentKindOptional,
           assignmentSourceCodeOptional);