[java-dfa] Introduce ephemeral values (possible only if separate compilation happens)

Now we visit switch default branch, even if it's reachable only on added enum constant
This fixes IDEA-227734 Data flow inspection doesn't report return null inside a switch for @NotNull annotated method
Also if-chains and switch are processed more uniformly: if we replace exhaustive switch statement with the equivalent if, we won't get possible NPE anymore

GitOrigin-RevId: c090b9e76a31a4626b518461b1b4ffce8cd3a7aa
This commit is contained in:
Tagir Valeev
2020-10-06 09:54:49 +00:00
committed by intellij-monorepo-bot
parent cf665c2269
commit 75d6d70bfe
10 changed files with 250 additions and 35 deletions
@@ -886,7 +886,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
private void processSwitch(@NotNull PsiSwitchBlock switchBlock) {
PsiExpression selector = PsiUtil.skipParenthesizedExprDown(switchBlock.getExpression());
Set<PsiEnumConstant> enumValues = null;
DfaVariableValue expressionValue = null;
boolean syntheticVar = true;
if (selector != null) {
@@ -907,17 +906,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
selector.accept(this);
generateBoxingUnboxingInstructionFor(selector, targetType);
final PsiClass psiClass = PsiUtil.resolveClassInClassTypeOnly(targetType);
if (psiClass != null) {
if (psiClass.isEnum()) {
enumValues = new HashSet<>();
for (PsiField f : psiClass.getFields()) {
if (f instanceof PsiEnumConstant) {
enumValues.add((PsiEnumConstant)f);
}
}
}
}
if (syntheticVar) {
addInstruction(new AssignInstruction(null, null));
}
@@ -928,7 +916,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
if (body != null) {
PsiStatement[] statements = body.getStatements();
ControlFlowOffset offset = null;
ControlFlowOffset offset;
PsiSwitchLabelStatementBase defaultLabel = null;
for (PsiStatement statement : statements) {
if (statement instanceof PsiSwitchLabelStatementBase) {
@@ -943,14 +931,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
if (values != null) {
for (PsiExpression caseValue : values.getExpressions()) {
boolean enumConstant = false;
if (enumValues != null && caseValue instanceof PsiReferenceExpression) {
PsiEnumConstant target = ObjectUtils.tryCast(((PsiReferenceExpression)caseValue).resolve(), PsiEnumConstant.class);
if (target != null) {
enumValues.remove(target);
enumConstant = true;
}
}
boolean enumConstant = caseValue instanceof PsiReferenceExpression &&
((PsiReferenceExpression)caseValue).resolve() instanceof PsiEnumConstant;
if (caseValue != null && expressionValue != null && (enumConstant || PsiUtil.isConstantExpression(caseValue))) {
addInstruction(new PushInstruction(expressionValue, null));
@@ -974,10 +956,15 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
}
if (offset == null || enumValues == null || !enumValues.isEmpty()) {
offset = defaultLabel != null ? getStartOffset(defaultLabel) : getEndOffset(body);
} // else default label goes to the last switch label making it always true
addInstruction(new GotoInstruction(offset));
if (defaultLabel != null) {
addInstruction(new GotoInstruction(getStartOffset(defaultLabel)));
}
else if (switchBlock instanceof PsiSwitchExpression) {
throwException(myExceptionCache.get("java.lang.IncompatibleClassChangeError"), null);
}
else {
addInstruction(new GotoInstruction(getEndOffset(body)));
}
body.accept(this);
}
@@ -2107,7 +2094,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
* @return true if some inliner may add constraints on the precise type of given expression
*/
public static boolean inlinerMayInferPreciseType(PsiExpression expression) {
return Arrays.stream(INLINERS).anyMatch(inliner -> inliner.mayInferPreciseType(expression));
return ContainerUtil.exists(INLINERS, inliner -> inliner.mayInferPreciseType(expression));
}
private static final class Synthetic implements VariableDescriptor {
@@ -141,12 +141,20 @@ public interface DfaMemoryState {
boolean isNotNull(DfaValue dfaVar);
/**
* Ephemeral means a state that was created when considering a method contract and checking if one of its arguments is null.
* With explicit null check, that would result in any non-annotated variable being treated as nullable and producing possible NPE warnings later.
* With contracts, we don't want this. So the state where this variable is null is marked ephemeral and no NPE warnings are issued for such states.
* Mark this state as ephemeral. See {@link #isEphemeral()} for details.
*/
void markEphemeral();
/**
* Ephemeral means a state that could be unreachable under normal program execution. Examples of ephemeral states include:
* <ul>
* <li>State created by method contract processing that checks for null (otherwise, if argument has unknown nullity, this would make it nullable)</li>
* <li>State that appears on VM exception path (e.g. catching NPE or CCE)</li>
* <li>State that appears when {@linkplain com.intellij.codeInspection.dataFlow.types.DfEphemeralReferenceType an ephemeral value} is stored on the stack</li>
* </ul>
* The "unsound" warnings (e.g. possible NPE) are not reported if they happen only in ephemeral states, and there's a non-ephemeral state
* where the same problem doesn't happen.
*/
boolean isEphemeral();
boolean isEmptyStack();
@@ -1183,6 +1183,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
} else {
myVariableTypes.put(dfaVar, type);
}
if (type instanceof DfEphemeralReferenceType) {
markEphemeral();
}
myCachedHash = null;
}
@@ -0,0 +1,92 @@
// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection.dataFlow.types;
import com.intellij.codeInspection.dataFlow.*;
import org.jetbrains.annotations.NotNull;
import java.util.Set;
/**
* A reference value that is impossible with currently available source code but
* could appear if separate compilation takes place. A common example of ephemeral value
* is a constant of enum type that doesn't equal to any existing constant.
* <p>
* When ephemeral value is stored in the memory state variable we assume that the whole
* memory state is ephemeral.
*
* @see DfaMemoryState#isEphemeral()
*/
public class DfEphemeralReferenceType implements DfReferenceType {
private final TypeConstraint myTypeConstraint;
DfEphemeralReferenceType(TypeConstraint constraint) {
myTypeConstraint = constraint;
}
@Override
public @NotNull DfaNullability getNullability() {
return DfaNullability.NOT_NULL;
}
@Override
public @NotNull TypeConstraint getConstraint() {
return myTypeConstraint;
}
@Override
public @NotNull DfReferenceType dropNullability() {
return this;
}
@Override
public @NotNull DfReferenceType dropTypeConstraint() {
return DfTypes.NOT_NULL_OBJECT;
}
@Override
public boolean isSuperType(@NotNull DfType other) {
if (other == DfTypes.BOTTOM) return true;
if (other instanceof DfEphemeralReferenceType) {
return myTypeConstraint.isSuperConstraintOf(((DfEphemeralReferenceType)other).myTypeConstraint);
}
return false;
}
@Override
public @NotNull DfType join(@NotNull DfType other) {
if (other == DfTypes.BOTTOM) return this;
if (other == DfTypes.TOP || !(other instanceof DfReferenceType)) return DfTypes.TOP;
TypeConstraint otherConstraint = ((DfReferenceType)other).getConstraint();
TypeConstraint constraint = myTypeConstraint.join(otherConstraint);
if (other instanceof DfEphemeralReferenceType) {
return constraint == myTypeConstraint ? this :
constraint == otherConstraint ? other :
constraint == TypeConstraints.TOP ? DfTypes.NOT_NULL_OBJECT :
new DfEphemeralReferenceType(constraint);
}
Set<Object> notValues = other instanceof DfGenericObjectType ? ((DfGenericObjectType)other).getNotValues() : Set.of();
return new DfGenericObjectType(notValues, constraint, ((DfReferenceType)other).getNullability(),
Mutability.UNKNOWN, null, DfTypes.BOTTOM, false);
}
@Override
public @NotNull DfType meet(@NotNull DfType other) {
if (other == DfTypes.TOP) return this;
if (other == DfTypes.BOTTOM) return other;
if (other instanceof DfEphemeralReferenceType ||
other instanceof DfGenericObjectType) {
TypeConstraint otherConstraint = ((DfReferenceType)other).getConstraint();
TypeConstraint constraint = myTypeConstraint.meet(otherConstraint);
return constraint == myTypeConstraint ? this :
constraint == otherConstraint ? other :
constraint == TypeConstraints.BOTTOM ? DfTypes.BOTTOM :
new DfEphemeralReferenceType(constraint);
}
return DfTypes.BOTTOM;
}
@Override
public String toString() {
return "ephemeral " + getConstraint().toString();
}
}
@@ -2,7 +2,11 @@
package com.intellij.codeInspection.dataFlow.types;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.java.JavaBundle;import gnu.trove.THashSet;
import com.intellij.java.JavaBundle;
import com.intellij.psi.JavaPsiFacade;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiEnumConstant;
import gnu.trove.THashSet;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -182,7 +186,7 @@ class DfGenericObjectType extends DfAntiConstantType<Object> implements DfRefere
if (isSuperType(other)) return this;
if (other.isSuperType(this)) return other;
if (!(other instanceof DfReferenceType)) return TOP;
if (other instanceof DfNullConstantType) {
if (other instanceof DfNullConstantType || other instanceof DfEphemeralReferenceType) {
return other.join(this);
}
DfReferenceType type = (DfReferenceType)other;
@@ -207,7 +211,7 @@ class DfGenericObjectType extends DfAntiConstantType<Object> implements DfRefere
@NotNull
@Override
public DfType meet(@NotNull DfType other) {
if (other instanceof DfConstantType) {
if (other instanceof DfConstantType || other instanceof DfEphemeralReferenceType) {
return other.meet(this);
}
if (isSuperType(other)) return other;
@@ -245,11 +249,36 @@ class DfGenericObjectType extends DfAntiConstantType<Object> implements DfRefere
} else if (!myNotValues.containsAll(otherNotValues)) {
notValues = new THashSet<>(myNotValues);
notValues.addAll(otherNotValues);
DfEphemeralReferenceType ephemeralValue = checkEphemeral(constraint, notValues);
if (ephemeralValue != null) {
return ephemeralValue;
}
}
}
return new DfGenericObjectType(notValues, constraint, nullability, mutability, sf, sfType, locality);
}
private static DfEphemeralReferenceType checkEphemeral(TypeConstraint constraint, Set<Object> notValues) {
if (notValues.isEmpty()) return null;
Object value = notValues.iterator().next();
if (!(value instanceof PsiEnumConstant)) return null;
PsiClass enumClass = ((PsiEnumConstant)value).getContainingClass();
if (enumClass == null) return null;
TypeConstraint enumType = TypeConstraints.instanceOf(
JavaPsiFacade.getElementFactory(enumClass.getProject()).createType(enumClass));
if (!enumType.equals(constraint)) return null;
Set<PsiEnumConstant> allEnumConstants = StreamEx.of(enumClass.getFields()).select(PsiEnumConstant.class).toSet();
if (notValues.size() != allEnumConstants.size()) return null;
for (Object notValue : notValues) {
if (!(notValue instanceof PsiEnumConstant)) return null;
if (!allEnumConstants.remove(notValue)) return null;
}
if (allEnumConstants.isEmpty()) {
return new DfEphemeralReferenceType(constraint);
}
return null;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
@@ -33,6 +33,7 @@ public class DfReferenceConstantType extends DfConstantType<Object> implements D
@Override
public DfType meet(@NotNull DfType other) {
if (other.isSuperType(this)) return this;
if (other instanceof DfEphemeralReferenceType) return BOTTOM;
if (other instanceof DfGenericObjectType) {
DfReferenceType type = ((DfReferenceType)other).dropMutability();
if (type.isSuperType(this)) return this;
@@ -96,7 +97,7 @@ public class DfReferenceConstantType extends DfConstantType<Object> implements D
@NotNull
@Override
public DfType join(@NotNull DfType other) {
if (other instanceof DfGenericObjectType) {
if (other instanceof DfGenericObjectType || other instanceof DfEphemeralReferenceType) {
return other.join(this);
}
if (isSuperType(other)) return this;
@@ -0,0 +1,38 @@
import org.jetbrains.annotations.*;
// IDEA-227734
class Test {
enum MyEnum{ A, B, C }
@NotNull
String doesNotShowWarning(@NotNull MyEnum myEnum){
switch(myEnum){
case A:
case B:
case C:
return "YES";
default: return <warning descr="'null' is returned by the method declared as @NotNull">null</warning>;
}
}
@NotNull
String doesNotShowWarning2(@NotNull final MyEnum myEnum){
switch(myEnum){
case A:
case B:
case C:
return "YES";
}
return <warning descr="'null' is returned by the method declared as @NotNull">null</warning>;
}
@NotNull
String showsWarning(@NotNull MyEnum myEnum){
switch(myEnum){
case A:
case B:
return "YES";
default: return <warning descr="'null' is returned by the method declared as @NotNull">null</warning>;
}
}
}
@@ -0,0 +1,37 @@
import org.jetbrains.annotations.*;
class Test {
enum MyEnum {A, B, C}
void test(MyEnum x) {
String s = null;
if (x == MyEnum.A) {
s = "A";
}
else if (x == MyEnum.B) {
s = "B";
}
else if (x == MyEnum.C) {
s = "C";
}
System.out.println(s.trim());
}
void test2(MyEnum x) {
String s = null;
if (x == MyEnum.A) {
s = "A";
}
else if (x == MyEnum.B) {
s = "B";
}
else if (x == MyEnum.C) {
s = "C";
}
if (s == null) {
System.out.println("Incompatible class change!");
return;
}
System.out.println(s.trim());
}
}
@@ -24,7 +24,8 @@ public class SwitchExpressionsJava12 {
case B -> 2;
};
if (i == 0) {} // default and case C is missing: we assume that any other result is also possible
// default and case C is missing: we assume that IncompatibleClassChangeError is still thrown
if (<warning descr="Condition 'i == 0' is always 'false'">i == 0</warning>) {}
int i1 = switch(x) {
case A -> 1;
@@ -34,6 +35,23 @@ public class SwitchExpressionsJava12 {
if (<warning descr="Condition 'i1 == 0' is always 'false'">i1 == 0</warning>) {} // exhaustive
}
static void testEnumAndCatch(X x) {
int i1 = 0;
try {
i1 = switch(x) {
case A -> 1;
case B -> 2;
case C -> 3;
};
}
catch (IncompatibleClassChangeError ex) {
if (<warning descr="Condition 'i1 == 0' is always 'true'">i1 == 0</warning>) {}
if (<warning descr="Condition 'x == X.A' is always 'false'">x == X.A</warning>) {}
if (<warning descr="Condition 'x == X.B' is always 'false'">x == X.B</warning>) {}
if (<warning descr="Condition 'x == X.C' is always 'false'">x == X.C</warning>) {}
}
}
static void testBoxed(int i) {
Integer j = switch(i) {
@@ -86,6 +86,8 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testNotEqualsDoesntImplyNotNullity() { doTest(); }
public void testEqualsEnumConstant() { doTest(); }
public void testSwitchEnumConstant() { doTest(); }
public void testEphemeralDefaultCaseVisited() { doTest(); }
public void testEphemeralInIfChain() { doTest(); }
public void testIncompleteSwitchEnum() { doTest(); }
public void testEnumConstantNotNull() { doTest(); }
public void testCheckEnumConstantConstructor() { doTest(); }