IDEA-172417 Dataflow analysis: track that loop is visited if and only if array/collection is non-empty

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

@@ -420,23 +420,52 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     final PsiParameter parameter = statement.getIterationParameter();
     final PsiExpression iteratedValue = statement.getIteratedValue();
 
+    ControlFlow.ControlFlowOffset loopEndOffset = getEndOffset(statement);
+    boolean hasSizeCheck = false;
+
     if (iteratedValue != null) {
       iteratedValue.accept(this);
       addInstruction(new FieldReferenceInstruction(iteratedValue, "Collection iterator or array.length"));
+      DfaValue qualifier = myFactory.createValue(iteratedValue);
+
+      if (qualifier instanceof DfaVariableValue) {
+        PsiType type = iteratedValue.getType();
+        SpecialField length = null;
+        if (type instanceof PsiArrayType) {
+          length = SpecialField.ARRAY_LENGTH;
+        }
+        else if (InheritanceUtil.isInheritor(type, JAVA_UTIL_COLLECTION)) {
+          length = SpecialField.COLLECTION_SIZE;
+        }
+        if (length != null) {
+          addInstruction(new PushInstruction(length.createValue(myFactory, qualifier), null));
+          addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(0, PsiType.INT, null), null));
+          addInstruction(new BinopInstruction(JavaTokenType.EQEQ, iteratedValue, myProject));
+          addInstruction(new ConditionalGotoInstruction(loopEndOffset, false, null));
+          hasSizeCheck = true;
+        }
+      }
     }
 
     ControlFlow.ControlFlowOffset offset = myCurrentFlow.getNextOffset();
     DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(parameter, false);
     addInstruction(new FlushVariableInstruction(dfaVariable));
 
-    pushUnknown();
-    addInstruction(new ConditionalGotoInstruction(getEndOffset(statement), true, null));
+    if (!hasSizeCheck) {
+      pushUnknown();
+      addInstruction(new ConditionalGotoInstruction(loopEndOffset, true, null));
+    }
 
     final PsiStatement body = statement.getBody();
     if (body != null) {
       body.accept(this);
     }
 
+    if (hasSizeCheck) {
+      pushUnknown();
+      addInstruction(new ConditionalGotoInstruction(loopEndOffset, true, null));
+    }
+
     addInstruction(new GotoInstruction(offset));
 
     finishElement(statement);

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

@@ -669,11 +669,24 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
                                        PsiElement psiAnchor,
                                        boolean evaluatesToTrue) {
     if (!skipReportingConstantCondition(visitor, psiAnchor, evaluatesToTrue)) {
-      final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue);
-      String message = InspectionsBundle.message(isAtRHSOfBooleanAnd(psiAnchor) ?
-                                                 "dataflow.message.constant.condition.when.reached" :
-                                                 "dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue));
-      holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix});
+      if (psiAnchor.getParent() instanceof PsiForeachStatement) {
+        // highlighted for-each iterated value means evaluatesToTrue == "collection is always empty"
+        if (!evaluatesToTrue) {
+          // loop on always non-empty collection -- nothing to report
+          return;
+        }
+        boolean array = psiAnchor instanceof PsiExpression && ((PsiExpression)psiAnchor).getType() instanceof PsiArrayType;
+        holder.registerProblem(psiAnchor, array ?
+                                          InspectionsBundle.message("dataflow.message.loop.on.empty.array") :
+                                          InspectionsBundle.message("dataflow.message.loop.on.empty.collection"));
+      }
+      else {
+        final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue);
+        String message = InspectionsBundle.message(isAtRHSOfBooleanAnd(psiAnchor) ?
+                                                   "dataflow.message.constant.condition.when.reached" :
+                                                   "dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue));
+        holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix});
+      }
     }
   }
 

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

@@ -28,6 +28,7 @@ import com.intellij.util.PairFunction;
 import com.intellij.util.containers.*;
 import com.intellij.util.containers.Queue;
 import one.util.streamex.IntStreamEx;
+import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -42,7 +43,7 @@ public class LiveVariablesAnalyzer {
   private final MultiMap<Instruction, Instruction> myForwardMap;
   private final MultiMap<Instruction, Instruction> myBackwardMap;
   @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") private final FactoryMap<PsiElement, List<DfaVariableValue>> myClosureReads = new FactoryMap<PsiElement, List<DfaVariableValue>>() {
-    @Nullable
+    @NotNull
     @Override
     protected List<DfaVariableValue> create(PsiElement closure) {
       final Set<DfaVariableValue> result = ContainerUtil.newLinkedHashSet();
@@ -208,7 +209,11 @@ public class LiveVariablesAnalyzer {
 
     if (ok) {
       for (FinishElementInstruction instruction : toFlush.keySet()) {
-        instruction.getVarsToFlush().addAll(toFlush.get(instruction));
+        Collection<DfaVariableValue> values = toFlush.get(instruction);
+        // Do not flush special values as they could be used implicitly
+        values.removeIf(var -> var.getQualifier() != null &&
+                               StreamEx.of(SpecialField.values()).anyMatch(sf -> sf.isMyAccessor(var.getPsiVariable())));
+        instruction.getVarsToFlush().addAll(values);
       }
     }
   }

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

@@ -0,0 +1,52 @@
+import java.util.Collection;
+import java.util.List;
+
+public class ForEachOverEmptyCollection {
+  void testArray(int[][] arr) {
+    if(arr.length != 0) return;
+    for (int[] ints : <warning descr="Array 'arr' is always empty">arr</warning>) {
+      System.out.println(ints.length);
+    }
+  }
+
+  void testCollection(Collection<?> c) {
+    if(!c.isEmpty()) return;
+    for (Object o : <warning descr="Collection 'c' is always empty">c</warning>) {
+      System.out.println(o);
+    }
+  }
+
+  void testArrayAfter(String[] arr) {
+    int count = 0;
+    boolean hasItem = false;
+    for(String str : arr) {
+      if(str != null) {
+        count++;
+      }
+      hasItem = true;
+    }
+    if(<warning descr="Condition 'arr.length == 0 && count > 0' is always 'false'">arr.length == 0 && <warning descr="Condition 'count > 0' is always 'false' when reached">count > 0</warning></warning>) {
+      // count > 0 means we visited the loop -- impossible
+      System.out.println("Impossible");
+    }
+    if(!hasItem) {
+      // we never visited the loop: array is empty
+      System.out.println(arr[<warning descr="Array index is out of bounds">1</warning>]);
+    }
+  }
+
+  void testCollectionAfter(List<String> list) {
+    boolean hasItem = false;
+    String max = null;
+    for (String s : list) {
+      if(!hasItem || s.compareTo(max) > 0) {
+        max = s;
+      }
+      hasItem = true;
+    }
+    if(!hasItem) {
+      System.out.println(
+        list.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>(<warning descr="Condition 'max == null' is always 'true'">max == null</warning> ? 0 : 1));
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java

@@ -332,6 +332,7 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
 
   public void testAccessingSameArrayElements() { doTest(); }
   public void testArrayLength() { doTest(); }
+  public void testForEachOverEmptyCollection() { doTest(); }
 
   public void testMethodParametersCanChangeNullability() { doTest(); }
 

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

@@ -77,6 +77,8 @@ dataflow.message.cce=Casting <code>{0}</code> to <code>#ref</code> #loc may prod
 dataflow.message.redundant.instanceof=Condition <code>#ref</code> #loc is redundant and can be replaced with <code>!= null</code>
 dataflow.message.constant.condition=Condition <code>#ref</code> #loc is always <code>{0}</code>
 dataflow.message.constant.condition.when.reached=Condition <code>#ref</code> #loc is always <code>{0}</code> when reached
+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.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