UnwrapSwitchLabelFix: when only one switch branch is reachable

Fixes IDEA-200651 Analysis for 'switch' statements may determine always truthy conditions on branches in addition to always falsy
Minor refactoring of reporting in DataFlowInspectionBase

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java

@@ -229,6 +229,11 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
     return Collections.emptyList();
   }
 
+  @Nullable
+  protected LocalQuickFix createUnwrapSwitchLabelFix() {
+    return null;
+  }
+
   @Nullable
   protected LocalQuickFix createIntroduceVariableFix(PsiExpression expression) {
     return null;
@@ -253,18 +258,16 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
     ArrayList<Instruction> allProblems = new ArrayList<>();
     allProblems.addAll(trueSet);
     allProblems.addAll(falseSet);
-    allProblems.addAll(visitor.getClassCastExceptionInstructions());
-    allProblems.addAll(ContainerUtil.filter(runner.getInstructions(), instruction1 -> instruction1 instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction1)));
+    StreamEx.of(runner.getInstructions()).select(InstanceofInstruction.class).filter(visitor::isInstanceofRedundant).into(allProblems);
 
     HashSet<PsiElement> reportedAnchors = new HashSet<>();
 
+    reportFailingCasts(holder, visitor, reportedAnchors);
+    reportUnreachableSwitchBranches(trueSet, falseSet, holder);
+
     for (Instruction instruction : allProblems) {
-      if (instruction instanceof TypeCastInstruction &&
-               reportedAnchors.add(((TypeCastInstruction)instruction).getExpression().getCastType())) {
-        reportCastMayFail(holder, (TypeCastInstruction)instruction);
-      }
-      else if (instruction instanceof BranchingInstruction) {
-        handleBranchingInstruction(holder, visitor, trueSet, falseSet, reportedAnchors, (BranchingInstruction)instruction);
+      if (instruction instanceof BranchingInstruction) {
+        handleBranchingInstruction(holder, visitor, trueSet, reportedAnchors, (BranchingInstruction)instruction);
       }
     }
 
@@ -300,6 +303,32 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
     reportPointlessSameArguments(holder, reportedAnchors, visitor);
   }
 
+  private void reportUnreachableSwitchBranches(Set<Instruction> trueSet, Set<Instruction> falseSet, ProblemsHolder holder) {
+    Set<PsiSwitchStatement> coveredSwitches = new HashSet<>();
+    Set<PsiSwitchLabelStatement> trueLabels = StreamEx.of(trueSet).select(BranchingInstruction.class)
+      .map(BranchingInstruction::getPsiAnchor).select(PsiSwitchLabelStatement.class).toSet();
+    Set<PsiSwitchLabelStatement> falseLabels = StreamEx.of(falseSet).select(BranchingInstruction.class)
+      .map(BranchingInstruction::getPsiAnchor).select(PsiSwitchLabelStatement.class).toSet();
+
+    for (PsiSwitchLabelStatement label : trueLabels) {
+      PsiSwitchStatement statement = label.getEnclosingSwitchStatement();
+      if (statement == null) continue;
+      if (!StreamEx.iterate(label, Objects::nonNull, l -> PsiTreeUtil.getPrevSiblingOfType(l, PsiSwitchLabelStatement.class))
+        .skip(1).allMatch(falseLabels::contains)) {
+        continue;
+      }
+      coveredSwitches.add(statement);
+      holder.registerProblem(label, InspectionsBundle.message("dataflow.message.only.switch.label"),
+                             createUnwrapSwitchLabelFix());
+    }
+    for (PsiSwitchLabelStatement label : falseLabels) {
+      if (!coveredSwitches.contains(label.getEnclosingSwitchStatement())) {
+        holder.registerProblem(label, InspectionsBundle.message("dataflow.message.unreachable.switch.label"),
+                               new DeleteSwitchLabelFix(label));
+      }
+    }
+  }
+
   private void reportConstants(ProblemsHolder holder, DataFlowInstructionVisitor visitor, HashSet<PsiElement> reportedAnchors) {
     visitor.getConstantExpressions().forEach((expression, result) -> {
       if (result == ConstantResult.UNKNOWN) return;
@@ -665,19 +694,22 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
     holder.registerProblem(toHighlight, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
   }
 
-  private static void reportCastMayFail(ProblemsHolder holder, TypeCastInstruction instruction) {
-    PsiTypeCastExpression typeCast = instruction.getExpression();
-    PsiExpression operand = typeCast.getOperand();
-    PsiTypeElement castType = typeCast.getCastType();
-    assert castType != null;
-    assert operand != null;
-    holder.registerProblem(castType, InspectionsBundle.message("dataflow.message.cce", operand.getText()));
+  private static void reportFailingCasts(ProblemsHolder holder, DataFlowInstructionVisitor visitor, HashSet<PsiElement> reportedAnchors) {
+    for (TypeCastInstruction instruction : visitor.getClassCastExceptionInstructions()) {
+      if (reportedAnchors.add(instruction.getExpression().getCastType())) {
+        PsiTypeCastExpression typeCast = instruction.getExpression();
+        PsiExpression operand = typeCast.getOperand();
+        PsiTypeElement castType = typeCast.getCastType();
+        assert castType != null;
+        assert operand != null;
+        holder.registerProblem(castType, InspectionsBundle.message("dataflow.message.cce", operand.getText()));
+      }
+    }
   }
 
   private void handleBranchingInstruction(ProblemsHolder holder,
                                           StandardInstructionVisitor visitor,
                                           Set<Instruction> trueSet,
-                                          Set<Instruction> falseSet,
                                           HashSet<PsiElement> reportedAnchors,
                                           BranchingInstruction instruction) {
     PsiElement psiAnchor = instruction.getPsiAnchor();
@@ -691,26 +723,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
         reportConstantBoolean(holder, psiAnchor, reportedAnchors, true);
       }
     }
-    else if (psiAnchor instanceof PsiSwitchLabelStatement) {
-      if (falseSet.contains(instruction)) {
-        holder.registerProblem(psiAnchor,
-                               InspectionsBundle.message("dataflow.message.unreachable.switch.label"),
-                               new DeleteSwitchLabelFix((PsiSwitchLabelStatement)psiAnchor));
-      } else if (trueSet.contains(instruction)) {
-        // If switch branch is always reachable, then all the subsequent branches are unreachable (thus weren't analyzed)
-        PsiSwitchLabelStatement current = (PsiSwitchLabelStatement)psiAnchor;
-        while(true) {
-          current = PsiTreeUtil.getNextSiblingOfType(current, PsiSwitchLabelStatement.class);
-          if (current == null) break;
-          if (!current.isDefaultCase()) {
-            holder.registerProblem(current,
-                                   InspectionsBundle.message("dataflow.message.unreachable.switch.label"),
-                                   new DeleteSwitchLabelFix(current));
-          }
-        }
-      }
-    }
-    else if (psiAnchor != null && !isFlagCheck(psiAnchor)) {
+    else if (psiAnchor != null && !(psiAnchor instanceof PsiSwitchLabelStatement) && !isFlagCheck(psiAnchor)) {
       boolean evaluatesToTrue = trueSet.contains(instruction);
       final PsiElement parent = psiAnchor.getParent();
       if (parent instanceof PsiAssignmentExpression &&

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java

@@ -25,7 +25,7 @@ import static com.intellij.util.ObjectUtils.tryCast;
 final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
   private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.DataFlowInstructionVisitor");
   private final Map<NullabilityProblemKind.NullabilityProblem<?>, StateInfo> myStateInfos = new LinkedHashMap<>();
-  private final Set<Instruction> myCCEInstructions = ContainerUtil.newHashSet();
+  private final Set<TypeCastInstruction> myCCEInstructions = ContainerUtil.newHashSet();
   private final Map<PsiCallExpression, Boolean> myFailingCalls = new HashMap<>();
   private final Map<PsiExpression, ConstantResult> myConstantExpressions = new HashMap<>();
   private final Map<PsiElement, ThreeState> myOfNullableCalls = new HashMap<>();
@@ -148,7 +148,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
     return myMethodReferenceResults;
   }
 
-  Set<Instruction> getClassCastExceptionInstructions() {
+  Set<TypeCastInstruction> getClassCastExceptionInstructions() {
     return myCCEInstructions;
   }
 

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/DeleteSwitchLabelFix.java

@@ -58,6 +58,10 @@ public class DeleteSwitchLabelFix implements LocalQuickFix {
   public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
     PsiSwitchLabelStatement label = PsiTreeUtil.getNonStrictParentOfType(descriptor.getStartElement(), PsiSwitchLabelStatement.class);
     if (label == null) return;
+    deleteLabel(label);
+  }
+
+  public static void deleteLabel(PsiSwitchLabelStatement label) {
     if (shouldRemoveBranch(label)) {
       PsiCodeBlock scope = ObjectUtils.tryCast(label.getParent(), PsiCodeBlock.class);
       if (scope == null) return;

java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java

@@ -0,0 +1,43 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInsight.daemon.impl.quickfix;
+
+import com.intellij.codeInspection.CommonQuickFixBundle;
+import com.intellij.codeInspection.LocalQuickFix;
+import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.codeInspection.dataFlow.fix.DeleteSwitchLabelFix;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiKeyword;
+import com.intellij.psi.PsiSwitchLabelStatement;
+import com.intellij.psi.PsiSwitchStatement;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.util.ObjectUtils;
+import com.siyeh.ig.psiutils.CommentTracker;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.List;
+
+public class UnwrapSwitchLabelFix implements LocalQuickFix {
+  @Nls(capitalization = Nls.Capitalization.Sentence)
+  @NotNull
+  @Override
+  public String getFamilyName() {
+    return CommonQuickFixBundle.message("fix.unwrap.statement", PsiKeyword.SWITCH);
+  }
+
+  @Override
+  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+    PsiSwitchLabelStatement label = ObjectUtils.tryCast(descriptor.getStartElement(), PsiSwitchLabelStatement.class);
+    if (label == null) return;
+    PsiSwitchStatement statement = label.getEnclosingSwitchStatement();
+    if (statement == null) return;
+    List<PsiSwitchLabelStatement> labels = PsiTreeUtil.getChildrenOfTypeAsList(statement.getBody(), PsiSwitchLabelStatement.class);
+    for (PsiSwitchLabelStatement otherLabel : labels) {
+      if (otherLabel != label) {
+        DeleteSwitchLabelFix.deleteLabel(otherLabel);
+      }
+    }
+    new CommentTracker().replaceAndRestoreComments(label, "default:");
+    ConvertSwitchToIfIntention.doProcessIntention(statement); // will not create 'if', just unwrap, because only default label is left
+  }
+}

java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java

@@ -18,6 +18,7 @@ package com.intellij.codeInspection.dataFlow;
 import com.intellij.codeInsight.NullableNotNullDialog;
 import com.intellij.codeInsight.daemon.impl.quickfix.DeleteSideEffectsAwareFix;
 import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix;
+import com.intellij.codeInsight.daemon.impl.quickfix.UnwrapSwitchLabelFix;
 import com.intellij.codeInspection.*;
 import com.intellij.codeInspection.dataFlow.fix.SurroundWithRequireNonNullFix;
 import com.intellij.codeInspection.nullable.NullableStuffInspection;
@@ -36,6 +37,7 @@ import com.siyeh.ig.fixes.IntroduceVariableFix;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import com.siyeh.ig.psiutils.SideEffectChecker;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import javax.swing.*;
 import java.awt.*;
@@ -74,6 +76,12 @@ public class DataFlowInspection extends DataFlowInspectionBase {
     return WrapWithMutableCollectionFix.createFix(violation, holder.isOnTheFly());
   }
 
+  @Nullable
+  @Override
+  protected LocalQuickFix createUnwrapSwitchLabelFix() {
+    return new UnwrapSwitchLabelFix();
+  }
+
   @Override
   protected LocalQuickFix createIntroduceVariableFix(final PsiExpression expression) {
     return new IntroduceVariableFix(true);

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterDeleteWhenOtherIsAlwaysTrue.java

@@ -1,12 +1,8 @@
-// "Remove switch label '"two"'" "true"
+// "Unwrap 'switch' statement" "true"
 class Main {
     static void fff() {
-        switch ("one") {
-            case "one":
-                System.out.println("one");
-                // quick-fix removes body as well
-                System.out.println("two");
-        }
+        System.out.println("one");
+        System.out.println("two");
     }
 
     public static void main(String[] args) {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/afterUnwrap.java

@@ -0,0 +1,15 @@
+// "Unwrap 'switch' statement" "true"
+class Main {
+    static void fff(int x) {
+        if (x == 5) {
+            System.out.println("five-ten-fifteen"); //5
+            System.out.println("six"); //6
+            System.out.println("seven"); //7
+            //other
+        }
+    }
+
+    public static void main(String[] args) {
+        fff();
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeDeleteWhenOtherIsAlwaysTrue.java

@@ -1,10 +1,10 @@
-// "Remove switch label '"two"'" "true"
+// "Unwrap 'switch' statement" "true"
 class Main {
     static void fff() {
         switch ("one") {
-            case "one":
+            case "<caret>one":
                 System.out.println("one");
-            case "<caret>two": // quick-fix removes body as well
+            case "two":
                 System.out.println("two");
         }
     }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel/beforeUnwrap.java

@@ -0,0 +1,22 @@
+// "Unwrap 'switch' statement" "true"
+class Main {
+    static void fff(int x) {
+        if (x == 5) {
+            switch (x) {
+                case 1: System.out.println("one"); //1
+                case 2: System.out.println("two"); //2
+                case 3: System.out.println("three"); //3
+                case 4: System.out.println("four"); //4
+                case 0:case <caret>5:case 10: System.out.println("five-ten-fifteen"); //5
+                case 6: System.out.println("six"); //6
+                case 7: System.out.println("seven"); //7
+                    break;
+                default: System.out.println("and more"); //other
+            }
+        }
+    }
+
+    public static void main(String[] args) {
+        fff();
+    }
+}

java/java-tests/testData/inspection/dataFlow/fixture/AlwaysTrueSwitchLabel.java

@@ -1,39 +1,39 @@
 class Scratch {
   public static void main(String[] args) {
     switch("ping") {
-      case "ping":
+      <warning descr="Switch label 'case \"ping\":' is the only reachable in the whole switch">case "ping":</warning>
         System.out.println("ping");
         break;
-      <warning descr="Switch label 'case \"pong\":' is unreachable">case "pong":</warning>
+      case "pong":
         System.out.println("pong");
         break;
-      <warning descr="Switch label 'case \"simple\":' is unreachable">case "simple":</warning>
+      case "simple":
         System.out.println("simple");
         break;
       default:
         break;
     }
     switch("ping") {
-      <warning descr="Switch label 'case \"pong\":' is unreachable">case "pong":</warning>
+      case "pong":
         System.out.println("pong");
         break;
-      case "ping":
+      <warning descr="Switch label 'case \"ping\":' is the only reachable in the whole switch">case "ping":</warning>
         System.out.println("ping");
         break;
-      <warning descr="Switch label 'case \"simple\":' is unreachable">case "simple":</warning>
+      case "simple":
         System.out.println("simple");
         break;
       default:
         break;
     }
     switch("ping") {
-      <warning descr="Switch label 'case \"pong\":' is unreachable">case "pong":</warning>
+      case "pong":
         System.out.println("pong");
         break;
-      <warning descr="Switch label 'case \"simple\":' is unreachable">case "simple":</warning>
+      case "simple":
         System.out.println("simple");
         break;
-      case "ping":
+      <warning descr="Switch label 'case \"ping\":' is the only reachable in the whole switch">case "ping":</warning>
         System.out.println("ping");
         break;
       default:

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -74,6 +74,7 @@ dataflow.message.constant.condition.when.reached=Condition <code>#ref</code> #lo
 dataflow.message.loop.on.empty.array=Array <code>#ref</code> is always empty
 dataflow.message.loop.on.empty.collection=Collection <code>#ref</code> is always empty
 dataflow.message.unreachable.switch.label=Switch label <code>#ref</code> #loc is unreachable
+dataflow.message.only.switch.label=Switch label <code>#ref</code> #loc is the only reachable in the whole switch
 dataflow.message.pointless.assignment.expression=Condition <code>#ref</code> #loc at the left side of assignment expression is always <code>{0}</code>. Can be simplified
 dataflow.message.passing.null.argument=Passing <code>null</code> argument to parameter annotated as @NotNull
 dataflow.message.passing.nullable.argument=Argument <code>#ref</code> #loc might be null