From ecb8fba0357fedff5aa06cafcdc0f06ce0cdbfdb Mon Sep 17 00:00:00 2001
From: Tagir Valeev <Tagir.Valeev@jetbrains.com>
Date: Thu, 23 Jul 2020 10:46:24 +0700
Subject: [PATCH] Check array length > 0 on array access even if index is
 unknown

Fixes IDEA-246390 DFA skips the warning "Array index is out of bounds" in for-loop

GitOrigin-RevId: 828e7353ad7220472818b10f4d4c51c5a8eeba64
---
 .../dataFlow/StandardInstructionVisitor.java  | 28 ++++++++++---------
 .../dataFlow/fixture/ArrayLength.java         |  5 ++++
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
index a1bf0bdbb11a..6d5c81082687 100644
--- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
@@ -164,19 +164,7 @@ public class StandardInstructionVisitor extends InstructionVisitor {
     PsiArrayAccessExpression arrayExpression = instruction.getExpression();
     DfaValue index = memState.pop();
     DfaValue array = memState.pop();
-    boolean alwaysOutOfBounds = false;
-    DfaValueFactory factory = runner.getFactory();
-    if (!DfaTypeValue.isUnknown(index)) {
-      DfaCondition indexNonNegative = index.cond(RelationType.GE, factory.getInt(0));
-      if (!memState.applyCondition(indexNonNegative)) {
-        alwaysOutOfBounds = true;
-      }
-      DfaValue dfaLength = SpecialField.ARRAY_LENGTH.createValue(factory, array);
-      DfaCondition indexLessThanLength = index.cond(RelationType.LT, dfaLength);
-      if (!memState.applyCondition(indexLessThanLength)) {
-        alwaysOutOfBounds = true;
-      }
-    }
+    boolean alwaysOutOfBounds = !applyBoundsCheck(memState, array, index);
     processArrayAccess(arrayExpression, alwaysOutOfBounds);
     if (alwaysOutOfBounds) {
       DfaControlTransferValue transfer = instruction.getOutOfBoundsExceptionTransfer();
@@ -207,6 +195,20 @@ public class StandardInstructionVisitor extends InstructionVisitor {
     return nextInstruction(instruction, runner, memState);
   }
 
+  private static boolean applyBoundsCheck(@NotNull DfaMemoryState memState,
+                                          @NotNull DfaValue array,
+                                          @NotNull DfaValue index) {
+    DfaValueFactory factory = index.getFactory();
+    DfaValue length = SpecialField.ARRAY_LENGTH.createValue(factory, array);
+    DfaCondition lengthMoreThanZero = length.cond(RelationType.GT, factory.getInt(0));
+    if (!memState.applyCondition(lengthMoreThanZero)) return false;
+    DfaCondition indexNonNegative = index.cond(RelationType.GE, factory.getInt(0));
+    if (!memState.applyCondition(indexNonNegative)) return false;
+    DfaCondition indexLessThanLength = index.cond(RelationType.LT, length);
+    if (!memState.applyCondition(indexLessThanLength)) return false;
+    return true;
+  }
+
   protected void processArrayAccess(PsiArrayAccessExpression expression, boolean alwaysOutOfBounds) {
 
   }
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayLength.java b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayLength.java
index b95f39a8e716..0d17ddadbe45 100644
--- a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayLength.java
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayLength.java
@@ -1,6 +1,11 @@
 import java.util.Arrays;
 
 public final class ArrayLength {
+  void testArrayLengthNonZeroUnknownIndex(int[] arr, int a, int b, int c) {
+    arr[a+b+c] = 0;
+    if (<warning descr="Condition 'arr.length == 0' is always 'false'">arr.length == 0</warning>) {}
+  }
+
   public static void diff(String[] args, String[] args2) {
     String[] arr = new String[args2.length - args.length];
     if (arr.length > 0) {}