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 a074d58d29ed..32ab1d07182d 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 @@ -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) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java index 99ad4f7e0ea5..e03279918323 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java @@ -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); 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 5653a342ad03..e8b7d0a3d1b5 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 @@ -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); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StaticFieldsWithNewObjects.java b/java/java-tests/testData/inspection/dataFlow/fixture/StaticFieldsWithNewObjects.java new file mode 100644 index 000000000000..89b25fe5a596 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StaticFieldsWithNewObjects.java @@ -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(ConstantHolder.X == ConstantHolder.Y) { + System.out.println("Impossible"); + } + if(ConstantHolder.Foo.A != ConstantHolder.Foo.B) { + System.out.println("Always"); + } + if(ti == ConstantHolder.X && ti == ConstantHolder.Y) { + System.out.println("Impossible"); + } + if(ti != ConstantHolder.X || ti != ConstantHolder.Foo.A) { + 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.isEmpty()) { + 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) && b) { + System.out.println("false"); + } + } + + void testArray() { + if(ConstantHolder.ARRAY == ConstantHolder.ARRAY2) { + 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].hashCode()); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 05697b4e3cf6..622faa144912 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -535,4 +535,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { } public void testEmptySingletonMap() {doTest();} + public void testStaticFieldsWithNewObjects() { doTest(); } } 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 15e365cc033e..73c704253ede 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -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); + } } \ No newline at end of file