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 58e8414cbe0e..e4c775a4ad53 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 @@ -25,6 +25,7 @@ import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.UnorderedPair; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; +import com.intellij.psi.util.PropertyUtilBase; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; @@ -1035,6 +1036,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (!isNegated) { //Equals if (c1Index.equals(c2Index) || areCompatibleConstants(c1Index, c2Index)) return true; + if (isUnstableValue(dfaLeft) || isUnstableValue(dfaRight)) return true; if (!uniteClasses(c1Index, c2Index)) return false; for (long encodedPair : myDistinctClasses.toArray()) { @@ -1061,6 +1063,24 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return true; } + /** + * Returns true if value represents an "unstable" value. An unstable value is a value of an object type which could be + * a newly object every time it's accessed. Such value is still useful as its nullability is stable + * + * @param value to check. + * @return true if value might be unstable, false otherwise + */ + private boolean isUnstableValue(DfaValue value) { + if (!(value instanceof DfaVariableValue)) return false; + DfaVariableValue var = (DfaVariableValue)value; + PsiModifierListOwner owner = var.getPsiVariable(); + if (!(owner instanceof PsiMethod)) return false; + if (var.getVariableType() instanceof PsiPrimitiveType) return false; + if (PropertyUtilBase.isSimplePropertyGetter((PsiMethod)owner)) return false; + if (isNull(var)) return false; + return true; + } + private static boolean isPrimitive(DfaValue value) { return value instanceof DfaVariableValue && ((DfaVariableValue)value).getVariableType() instanceof PsiPrimitiveType; } 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 1233ce9a969d..233b7f557b58 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 @@ -135,7 +135,8 @@ public class DfaExpressionFactory { if (constValue != null && !maybeUninitializedConstant(constValue, refExpr, var)) return constValue; } - if (ExpressionUtils.isEffectivelyUnqualified(refExpr) || isStaticFinalConstantWithoutInitializationHacks(var)) { + if (ExpressionUtils.isEffectivelyUnqualified(refExpr) || isStaticFinalConstantWithoutInitializationHacks(var) || + (var instanceof PsiMethod && var.hasModifierProperty(PsiModifier.STATIC))) { return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, null); } @@ -199,9 +200,12 @@ public class DfaExpressionFactory { return sf.getCanonicalOwner(null, ((PsiMethod)target).getContainingClass()); } } - if (method.getParameterList().getParametersCount() == 0 && - AnnotationUtil.findAnnotation(method.getContainingClass(), "javax.annotation.concurrent.Immutable") != null) { - return method; + if (method.getParameterList().getParametersCount() == 0) { + if ((ControlFlowAnalyzer.isPure(method) || + AnnotationUtil.findAnnotation(method.getContainingClass(), "javax.annotation.concurrent.Immutable") != null) && + ControlFlowAnalyzer.getMethodCallContracts(method, null).isEmpty()) { + return method; + } } } return null; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CheckFieldInitializers.java b/java/java-tests/testData/inspection/dataFlow/fixture/CheckFieldInitializers.java index e6a87ac88f76..1a7d1846e742 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/CheckFieldInitializers.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/CheckFieldInitializers.java @@ -8,6 +8,9 @@ class Foo { } class Bar { public static final String s = Foo.bar(Foo.foo()); - @NotNull public static Object o = Foo.foo(); + @NotNull public static Object o = Foo.foo(); +} +class Baz { + @NotNull public static Object o = Foo.foo(); } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java b/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java index 952fbfbab8e9..a59c40cc197d 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ComplexInitializer.java @@ -58,7 +58,10 @@ class Constants { static final Object C10 = get(); static final Object C11 = get(); - static Object get() {return new Object();} + static Object get() { + System.out.println(); + return new Object(); + } } class TooComplexInitializer { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/EmptySingletonMap.java b/java/java-tests/testData/inspection/dataFlow/fixture/EmptySingletonMap.java index bd316c1bf5ec..77738cf396f2 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/EmptySingletonMap.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/EmptySingletonMap.java @@ -1,4 +1,5 @@ import java.util.*; +import org.jetbrains.annotations.*; public class EmptySingletonMap { void testEmpty() { @@ -39,4 +40,26 @@ public class EmptySingletonMap { System.out.println("??"); } } + + @Contract(pure = true) + static Map newMap() { + return new HashMap<>(); + } + + void testDoubleEmpty() { + Map m1 = newMap(); + Map m2 = newMap(); + fill(m1, m2); + if(m1.isEmpty() && m2.isEmpty()) { + System.out.println("both empty"); + } + } + + void testNonEqual() { + if(EmptySingletonMap.newMap() == EmptySingletonMap.newMap()) { + System.out.println("who knows"); + } + } + + native void fill(Object m1, Object m2); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/PureNoArgMethodAsVariable.java b/java/java-tests/testData/inspection/dataFlow/fixture/PureNoArgMethodAsVariable.java new file mode 100644 index 000000000000..c04f007be26a --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/PureNoArgMethodAsVariable.java @@ -0,0 +1,43 @@ +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +// IDEA-141547 +public class PureNoArgMethodAsVariable { + + public enum Bar { + A, B; + + @Nullable + @Contract(pure = true) + public String getGroup() { + return this == A ? null : "B"; + } + + @Nullable + @Contract(pure = true) + public String group() { + return this == A ? null : "B"; + } + } + + public void foo(Bar bar) { + if (bar.getGroup() != null && check(bar.getGroup())) { // NO inspection error, OK! + System.out.print("ok"); + } + if (bar.group() != null && check(bar.group())) { // Inspection error, NOT OK! + System.out.print("ok"); + } + } + + void testIntValue(Integer x) { + if(x.intValue() > 5 && + x.intValue() < 0) { + System.out.println("impossible"); + } + } + + public boolean check(@NotNull String string) { + return string.length() > 2; + } +} \ 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 ac1d81992498..9da0613fdf59 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -576,4 +576,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testNullableReturn() { doTest(); } public void testManyBooleans() { doTest(); } + public void testPureNoArgMethodAsVariable() { doTest(); } }