DFA: pure no-args methods are considered as variables

Ref-returning methods are not included into eq-classes, only variable state is tracked for them. Primitive-returning methods are handled like normal variables (their result is considered to be stable)
Fixes IDEA-141547 ConstantConditions inspection reports false positive when using non getter method
This commit is contained in:
Tagir Valeev
2017-12-22 17:59:45 +07:00
parent 71327e0e1c
commit a2ea53a7b0
7 changed files with 103 additions and 6 deletions
@@ -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;
}
@@ -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;
@@ -8,6 +8,9 @@ class Foo {
}
class Bar {
public static final String s = Foo.bar(<warning descr="Argument 'Foo.foo()' might be null">Foo.foo()</warning>);
@NotNull public static Object o = <warning descr="Expression 'Foo.foo()' might evaluate to null but is assigned to a variable that is annotated with @NotNull">Foo.foo()</warning>;
@NotNull public static Object o = Foo.foo();
}
class Baz {
@NotNull public static Object o = <warning descr="Expression 'Foo.foo()' might evaluate to null but is assigned to a variable that is annotated with @NotNull">Foo.foo()</warning>;
}
@@ -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 <weak_warning descr="Class initializer is too complex to analyze by data flow algorithm">TooComplexInitializer</weak_warning> {
@@ -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<String, String> newMap() {
return new HashMap<>();
}
void testDoubleEmpty() {
Map<String, String> m1 = newMap();
Map<String, String> 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);
}
@@ -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(<warning descr="Condition 'x.intValue() > 5 && x.intValue() < 0' is always 'false'">x.intValue() > 5 &&
<warning descr="Condition 'x.intValue() < 0' is always 'false' when reached">x.intValue() < 0</warning></warning>) {
System.out.println("impossible");
}
}
public boolean check(@NotNull String string) {
return string.length() > 2;
}
}
@@ -576,4 +576,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testNullableReturn() { doTest(); }
public void testManyBooleans() { doTest(); }
public void testPureNoArgMethodAsVariable() { doTest(); }
}