From 0800dca8a83685abfcafa5c4be98991cc32567f2 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 19 Mar 2018 11:09:22 +0700 Subject: [PATCH] IDEA-188301 Map index range to array value in DFA --- .../codeInspection/dataFlow/DfaFactMap.java | 16 +++++++ .../dataFlow/DfaMemoryStateImpl.java | 7 +-- .../dataFlow/StandardInstructionVisitor.java | 13 ++---- .../dataFlow/value/DfaExpressionFactory.java | 46 ++++++++++++++++++- .../dataFlow/value/DfaValue.java | 13 ++++++ .../dataFlow/fixture/AdvancedArrayAccess.java | 19 ++++++++ .../dataFlow/fixture/February31.java | 2 +- .../siyeh/ig/psiutils/ExpressionUtils.java | 18 +++++++- 8 files changed, 115 insertions(+), 19 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactMap.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactMap.java index 106feef9d186..21455ea3a393 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactMap.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactMap.java @@ -15,6 +15,7 @@ */ package com.intellij.codeInspection.dataFlow; +import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.util.keyFMap.KeyFMap; import one.util.streamex.StreamEx; @@ -195,6 +196,21 @@ public final class DfaFactMap { return map.with(factType, factType.calcFromVariable(value)); } + /** + * Derives facts which might be known from given DfaValue without knowing the particular memory state + * + * @param value a value to derive facts from + * @return map of facts derived from the value + */ + @NotNull + public static DfaFactMap fromDfaValue(DfaValue value) { + return StreamEx.of(DfaFactType.getTypes()).foldLeft(EMPTY, (map, type) -> updateMap(map, type, value)); + } + + private static DfaFactMap updateMap(DfaFactMap map, DfaFactType factType, DfaValue value) { + return map.with(factType, factType.fromDfaValue(value)); + } + @FunctionalInterface public interface FactMapper { R apply(DfaFactType factType, T factValue); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index a330e7dc48c0..cab64cafb28d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -1143,12 +1143,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } value = resolveVariableValue((DfaVariableValue)value); } - DfaValue finalValue = value; - return StreamEx.of(DfaFactType.getTypes()).foldLeft(DfaFactMap.EMPTY, (map, type) -> updateMap(map, type, finalValue)); - } - - private static DfaFactMap updateMap(DfaFactMap map, DfaFactType factType, DfaValue value) { - return map.with(factType, factType.fromDfaValue(value)); + return DfaFactMap.fromDfaValue(value); } void setVariableState(DfaVariableValue dfaVar, DfaVariableState state) { 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 4d2755f7cfe8..11c5988a9e56 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 @@ -162,15 +162,10 @@ public class StandardInstructionVisitor extends InstructionVisitor { DfaValue result = instruction.getValue(); LongRangeSet rangeSet = memState.getValueFact(index, DfaFactType.RANGE); - if (rangeSet != null && !rangeSet.isEmpty() && rangeSet.min() == rangeSet.max()) { - long longIdx = rangeSet.min(); - if(longIdx >= 0 && longIdx <= Integer.MAX_VALUE) { - int intIdx = (int)longIdx; - DfaValue arrayElementValue = runner.getFactory().getExpressionFactory().getArrayElementValue(array, intIdx); - if (arrayElementValue != null) { - result = arrayElementValue; - } - } + DfaValue arrayElementValue = + runner.getFactory().getExpressionFactory().getArrayElementValue(array, rangeSet == null ? LongRangeSet.all() : rangeSet); + if (arrayElementValue != DfaUnknownValue.getInstance()) { + result = arrayElementValue; } memState.push(result); return nextInstruction(instruction, runner, memState); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java index 836d9c4d22e9..879c9ec3bfd2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaExpressionFactory.java @@ -18,6 +18,7 @@ package com.intellij.codeInspection.dataFlow.value; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.ExpressionUtil; import com.intellij.codeInspection.dataFlow.*; +import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.Conditions; @@ -31,6 +32,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.ExpressionUtils; +import one.util.streamex.LongStreamEx; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -231,6 +233,48 @@ public class DfaExpressionFactory { return null; } + @NotNull + private DfaValue getAdvancedExpressionDfaValue(@Nullable PsiExpression expression) { + if (expression == null) return DfaUnknownValue.getInstance(); + DfaValue value = getExpressionDfaValue(expression); + if (value != null) { + return value; + } + if (expression instanceof PsiConditionalExpression) { + return getAdvancedExpressionDfaValue(((PsiConditionalExpression)expression).getThenExpression()).union( + getAdvancedExpressionDfaValue(((PsiConditionalExpression)expression).getElseExpression())); + } + PsiType type = expression.getType(); + if (type instanceof PsiPrimitiveType) return DfaUnknownValue.getInstance(); + return myFactory.createTypeValue(type, NullnessUtil.getExpressionNullness(expression)); + } + + @NotNull + public DfaValue getArrayElementValue(DfaValue array, LongRangeSet indexSet) { + if (!(array instanceof DfaVariableValue)) return DfaUnknownValue.getInstance(); + if (indexSet.isEmpty()) return DfaUnknownValue.getInstance(); + long min = indexSet.min(); + long max = indexSet.max(); + if (min == max && min >= 0 && min < Integer.MAX_VALUE) { + DfaValue value = getArrayElementValue(array, (int)min); + return value == null ? DfaUnknownValue.getInstance() : value; + } + DfaVariableValue arrayDfaVar = (DfaVariableValue)array; + PsiModifierListOwner arrayPsiVar = arrayDfaVar.getPsiVariable(); + if (!(arrayPsiVar instanceof PsiVariable)) return DfaUnknownValue.getInstance(); + PsiExpression[] elements = ExpressionUtils.getConstantArrayElements((PsiVariable)arrayPsiVar); + if (elements == null || elements.length == 0) return DfaUnknownValue.getInstance(); + indexSet = indexSet.intersect(LongRangeSet.range(0, elements.length - 1)); + if (indexSet.isEmpty() || indexSet.max() - indexSet.min() > 100) return DfaUnknownValue.getInstance(); + return LongStreamEx.of(indexSet.stream()) + .mapToObj(idx -> getAdvancedExpressionDfaValue(elements[(int)idx])) + .prefix(DfaValue::union) + .takeWhileInclusive(value -> value != DfaUnknownValue.getInstance()) + .reduce((a, b) -> b) + .orElse(DfaUnknownValue.getInstance()); + } + + @Nullable public DfaValue getArrayElementValue(DfaValue array, int index) { if (!(array instanceof DfaVariableValue)) return null; DfaVariableValue arrayDfaVar = (DfaVariableValue)array; @@ -241,7 +285,7 @@ public class DfaExpressionFactory { if (arrayPsiVar instanceof PsiVariable) { PsiExpression constantArrayElement = ExpressionUtils.getConstantArrayElement((PsiVariable)arrayPsiVar, index); if (constantArrayElement != null) { - return getExpressionDfaValue(constantArrayElement); + return getAdvancedExpressionDfaValue(constantArrayElement); } } PsiVariable indexVariable = getArrayIndexVariable(arrayPsiVar, index); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValue.java index c1b8d3ed5ccc..b42eb55dea90 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValue.java @@ -15,6 +15,8 @@ */ package com.intellij.codeInspection.dataFlow.value; +import com.intellij.codeInspection.dataFlow.DfaFactMap; + public abstract class DfaValue { private final int myID; protected final DfaValueFactory myFactory; @@ -32,6 +34,17 @@ public abstract class DfaValue { return myID; } + /** + * Produces a value which describes a union of this value and other value + * + * @param other other value to unite with + * @return a union value. Any particular runtime value which satisfies this value or other value, satisfies also the returned value. + */ + public DfaValue union(DfaValue other) { + if(this == other) return this; + return myFactory.getFactFactory().createValue(DfaFactMap.fromDfaValue(this).union(DfaFactMap.fromDfaValue(other))); + } + public DfaValue createNegated() { return DfaUnknownValue.getInstance(); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java index e8031adef2b2..d1d49f459853 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java @@ -3,6 +3,7 @@ import java.util.*; class AdvancedArrayAccess { private static final int[] LENGTH = {0, 10, 20, 30}; private static final int[] LENGTH2 = {0, 10, 20, 30}; + private static final String[] ARRAY = new String[] {"xyz".toLowerCase(Locale.ENGLISH)}; void testInstanceOf() { Object[] arr = {new String("foo"), new Integer(0)}; @@ -17,6 +18,24 @@ class AdvancedArrayAccess { } } + void testAny(int i) { + if(LENGTH[i] > 30) { + System.out.println("Impossible"); + } + if(LENGTH[i] > 20) { + System.out.println("Possible"); + } + if(i < 3 && LENGTH[i] > 20) { + System.out.println("Impossible"); + } + } + + void testCall(int i) { + if(ARRAY[i] == null) { + System.out.println("Impossible"); + } + } + int[] getLengths() { // array is still read-only return LENGTH.clone(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/February31.java b/java/java-tests/testData/inspection/dataFlow/fixture/February31.java index 935df06e899e..f729b353d42c 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/February31.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/February31.java @@ -17,7 +17,7 @@ public class February31 { if (time.month == 2 && IsLeapYear(time.year)) { return time.month <= kDaysInMonth[time.month] + 1; } else { - return time.month <= kDaysInMonth[time.month]; + return time.month <= kDaysInMonth[time.month]; } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index 99a352c74300..681230403791 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -1182,17 +1182,31 @@ public class ExpressionUtils { @Nullable public static PsiExpression getConstantArrayElement(PsiVariable array, int index) { if (index < 0) return null; + PsiExpression[] elements = getConstantArrayElements(array); + if (elements == null || index >= elements.length) return null; + return elements[index]; + } + + /** + * Returns an array of expressions which represent all array elements if array is known to be never modified + * after initialization. + * + * @param array an array variable + * @return an array or null if array could be modified after initialization + * (empty array means that the initializer is known to be an empty array). + */ + @Nullable + public static PsiExpression[] getConstantArrayElements(PsiVariable array) { PsiExpression initializer = array.getInitializer(); if (initializer instanceof PsiNewExpression) initializer = ((PsiNewExpression)initializer).getArrayInitializer(); if (!(initializer instanceof PsiArrayInitializerExpression)) return null; PsiExpression[] initializers = ((PsiArrayInitializerExpression)initializer).getInitializers(); - if (index >= initializers.length) return null; if (array instanceof PsiField && !(array.hasModifierProperty(PsiModifier.PRIVATE) && array.hasModifierProperty(PsiModifier.STATIC))) { return null; } Boolean isConstantArray = CachedValuesManager.getCachedValue(array, () -> CachedValueProvider.Result .create(isConstantArray(array), PsiModificationTracker.MODIFICATION_COUNT)); - return Boolean.TRUE.equals(isConstantArray) ? initializers[index] : null; + return Boolean.TRUE.equals(isConstantArray) ? initializers : null; } private static boolean isConstantArray(PsiVariable array) {