DFA: make distinct static fields initialized with new objects

This commit is contained in:
Tagir Valeev
2017-09-05 14:06:19 +07:00
parent 9ff2300440
commit 7ceb89985e
6 changed files with 146 additions and 13 deletions
@@ -830,9 +830,23 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
private void updateVarStateOnComparison(@NotNull DfaVariableValue dfaVar, DfaValue value) {
if (!isUnknownState(dfaVar)) {
if (value instanceof DfaConstValue && ((DfaConstValue)value).getValue() == null) {
setVariableState(dfaVar, getVariableState(dfaVar).withFact(DfaFactType.CAN_BE_NULL, true));
} else if (isNotNull(value) && !isNotNull(dfaVar)) {
if (value instanceof DfaConstValue) {
Object constValue = ((DfaConstValue)value).getValue();
if (constValue == null) {
setVariableState(dfaVar, getVariableState(dfaVar).withFact(DfaFactType.CAN_BE_NULL, true));
return;
}
if (constValue instanceof PsiVariable) {
DfaValue typeValue = myFactory.createTypeValue(((PsiVariable)constValue).getType(), Nullness.NOT_NULL);
if (typeValue instanceof DfaTypeValue) {
DfaVariableState state = getVariableState(dfaVar).withInstanceofValue((DfaTypeValue)typeValue);
if (state != null) {
setVariableState(dfaVar, state);
}
}
}
}
if (isNotNull(value) && !isNotNull(dfaVar)) {
setVariableState(dfaVar, getVariableState(dfaVar).withoutFact(DfaFactType.CAN_BE_NULL));
applyRelation(dfaVar, myFactory.getConstFactory().getNull(), true);
}
@@ -981,8 +995,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
}
private static boolean preserveConstantDistinction(final Object c1, final Object c2) {
return c1 == null && c2 instanceof PsiEnumConstant ||
c2 == null && c1 instanceof PsiEnumConstant;
return c1 == null && c2 instanceof PsiVariable ||
c2 == null && c1 instanceof PsiVariable;
}
private boolean areCompatibleConstants(int i1, int i2) {
@@ -16,9 +16,12 @@
package com.intellij.codeInspection.dataFlow.value;
import com.intellij.lang.jvm.JvmModifier;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -62,10 +65,13 @@ public class DfaConstValue extends DfaValue {
DfaConstValue unboxed = createFromValue(boo, PsiType.BOOLEAN, variable);
return myFactory.getBoxedFactory().createBoxed(unboxed);
}
PsiExpression initializer = variable.getInitializer();
PsiExpression initializer = PsiUtil.skipParenthesizedExprDown(variable.getInitializer());
if (initializer instanceof PsiLiteralExpression && initializer.textMatches(PsiKeyword.NULL)) {
return dfaNull;
}
if (variable instanceof PsiField && variable.hasModifier(JvmModifier.STATIC) && ExpressionUtils.isNewObject(initializer)) {
return createFromValue(variable, type, variable);
}
return null;
}
return createFromValue(value, type, variable);
@@ -29,6 +29,7 @@ import com.intellij.psi.*;
import com.intellij.psi.impl.JavaConstantExpressionEvaluator;
import com.intellij.psi.impl.light.LightVariableBuilder;
import com.intellij.psi.util.PropertyUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
@@ -75,11 +76,11 @@ public class DfaExpressionFactory {
if (expression instanceof PsiArrayAccessExpression) {
PsiExpression arrayExpression = ((PsiArrayAccessExpression)expression).getArrayExpression();
DfaValue qualifier = getExpressionDfaValue(arrayExpression);
if (qualifier instanceof DfaVariableValue) {
DfaVariableValue qualifier = getQualifierVariable(arrayExpression);
if (qualifier != null) {
PsiVariable indexVar = getArrayIndexVariable(((PsiArrayAccessExpression)expression).getIndexExpression());
if (indexVar != null) {
return myFactory.getVarFactory().createVariableValue(indexVar, expression.getType(), false, (DfaVariableValue)qualifier);
return myFactory.getVarFactory().createVariableValue(indexVar, expression.getType(), false, qualifier);
}
}
PsiType type = expression.getType();
@@ -132,16 +133,16 @@ public class DfaExpressionFactory {
if (!var.hasModifierProperty(PsiModifier.VOLATILE)) {
if (var instanceof PsiVariable && var.hasModifierProperty(PsiModifier.FINAL) && !PsiUtil.isAccessedForWriting(refExpr)) {
DfaValue constValue = myFactory.getConstFactory().create((PsiVariable)var);
if (constValue != null) return constValue;
if (constValue != null && !maybeUninitializedConstant(constValue, refExpr, var)) return constValue;
}
if (DfaValueFactory.isEffectivelyUnqualified(refExpr) || isStaticFinalConstantWithoutInitializationHacks(var)) {
return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, null);
}
DfaValue qualifierValue = getExpressionDfaValue(refExpr.getQualifierExpression());
if (qualifierValue instanceof DfaVariableValue) {
return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, (DfaVariableValue)qualifierValue);
DfaVariableValue qualifier = getQualifierVariable(refExpr.getQualifierExpression());
if (qualifier != null) {
return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, qualifier);
}
}
@@ -149,6 +150,32 @@ public class DfaExpressionFactory {
return myFactory.createTypeValue(type, DfaPsiUtil.getElementNullability(type, var));
}
private DfaVariableValue getQualifierVariable(PsiExpression qualifierExpression) {
DfaValue qualifierValue = getExpressionDfaValue(qualifierExpression);
DfaVariableValue qualifier = null;
if (qualifierValue instanceof DfaVariableValue) {
qualifier = (DfaVariableValue)qualifierValue;
}
else if (qualifierValue instanceof DfaConstValue) {
Object constValue = ((DfaConstValue)qualifierValue).getValue();
if (constValue instanceof PsiVariable) {
qualifier = myFactory.getVarFactory().createVariableValue((PsiVariable)constValue, false);
}
}
return qualifier;
}
private static boolean maybeUninitializedConstant(DfaValue constValue,
@NotNull PsiReferenceExpression refExpr,
PsiModifierListOwner var) {
// If static final field is referred from the same or inner/nested class,
// we consider that it might be uninitialized yet as some class initializers may call its methods or
// even instantiate objects of this class and call their methods
if(!(constValue instanceof DfaConstValue) || ((DfaConstValue)constValue).getValue() != var) return false;
if(!(var instanceof PsiField) || var instanceof PsiEnumConstant) return false;
return PsiTreeUtil.getTopmostParentOfType(refExpr, PsiClass.class) == PsiTreeUtil.getTopmostParentOfType(var, PsiClass.class);
}
private static boolean isStaticFinalConstantWithoutInitializationHacks(PsiModifierListOwner var) {
return (var instanceof PsiField && var.hasModifierProperty(PsiModifier.FINAL) && var.hasModifierProperty(PsiModifier.STATIC)) &&
!DfaUtil.hasInitializationHacks((PsiField)var);
@@ -0,0 +1,75 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
class ConstantHolder {
static final ConstantHolder X = new ConstantHolder();
static final ConstantHolder Y = new ConstantHolder();
static final Object[] ARRAY = new Object[10];
static final Object[] ARRAY2 = new Object[10];
ConstantHolder() {
if(X == null) {
System.out.println("X is initializing");
}
}
interface Foo {
ConstantHolder A = new ConstantHolder();
ConstantHolder B = new ConstantHolder();
}
@Nullable String str;
}
class TestNewObjects {
void test(ConstantHolder ti) {
if(<warning descr="Condition 'ConstantHolder.X == ConstantHolder.Y' is always 'false'">ConstantHolder.X == ConstantHolder.Y</warning>) {
System.out.println("Impossible");
}
if(<warning descr="Condition 'ConstantHolder.Foo.A != ConstantHolder.Foo.B' is always 'true'">ConstantHolder.Foo.A != ConstantHolder.Foo.B</warning>) {
System.out.println("Always");
}
if(<warning descr="Condition 'ti == ConstantHolder.X && ti == ConstantHolder.Y' is always 'false'">ti == ConstantHolder.X && <warning descr="Condition 'ti == ConstantHolder.Y' is always 'false' when reached">ti == ConstantHolder.Y</warning></warning>) {
System.out.println("Impossible");
}
if(<warning descr="Condition 'ti != ConstantHolder.X || ti != ConstantHolder.Foo.A' is always 'true'">ti != ConstantHolder.X || <warning descr="Condition 'ti != ConstantHolder.Foo.A' is always 'true' when reached">ti != ConstantHolder.Foo.A</warning></warning>) {
System.out.println("Always");
}
if(ConstantHolder.X.str != null && ConstantHolder.X.str.isEmpty()) {
System.out.println("ok");
}
if(ConstantHolder.X.str != null && ConstantHolder.Y.str.<warning descr="Method invocation 'isEmpty' may produce 'java.lang.NullPointerException'">isEmpty</warning>()) {
System.out.println("possible NPE");
}
}
Object getObject() {
return new Object();
}
void testTypes(boolean b) {
Object x = b ? getObject() : ConstantHolder.X;
if(x instanceof ConstantHolder && b) {
System.out.println("true");
}
if(!(x instanceof ConstantHolder) && <warning descr="Condition 'b' is always 'true' when reached">b</warning>) {
System.out.println("false");
}
}
void testArray() {
if(<warning descr="Condition 'ConstantHolder.ARRAY == ConstantHolder.ARRAY2' is always 'false'">ConstantHolder.ARRAY == ConstantHolder.ARRAY2</warning>) {
System.out.println("Impossible");
}
ConstantHolder.ARRAY[0] = Math.random() > 0.5 ? null : "foo";
if(ConstantHolder.ARRAY[0] != null) {
System.out.println(ConstantHolder.ARRAY[0].hashCode());
}
if(ConstantHolder.ARRAY2[0] != null) {
System.out.println(ConstantHolder.ARRAY[0].<warning descr="Method invocation 'hashCode' may produce 'java.lang.NullPointerException'">hashCode</warning>());
}
}
}
@@ -535,4 +535,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
}
public void testEmptySingletonMap() {doTest();}
public void testStaticFieldsWithNewObjects() { doTest(); }
}
@@ -1078,4 +1078,14 @@ public class ExpressionUtils {
});
return result.get();
}
/**
* @param expression expression to test
* @return true if the expression return value is a new object which is guaranteed to be distinct from any other object created
* in the program.
*/
@Contract("null -> false")
public static boolean isNewObject(@Nullable PsiExpression expression) {
return expression != null && nonStructuralChildren(expression).allMatch(PsiNewExpression.class::isInstance);
}
}