don't make var nullable if it's not equal to a constant (IDEA-114791)

This commit is contained in:
peter
2013-10-17 21:54:33 +02:00
parent 32d29a0a45
commit eb6455eab9
9 changed files with 74 additions and 76 deletions
@@ -52,7 +52,7 @@ public interface DfaMemoryState {
boolean checkNotNullable(DfaValue value);
boolean isNotNull(DfaVariableValue dfaVar);
boolean isNotNull(DfaValue dfaVar);
@Nullable
DfaConstValue getConstantValue(DfaVariableValue value);
@@ -458,8 +458,11 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
}
@Override
public boolean isNotNull(DfaVariableValue dfaVar) {
if (getVariableState(dfaVar).isNotNull()) {
public boolean isNotNull(DfaValue dfaVar) {
if (dfaVar instanceof DfaVariableValue && getVariableState((DfaVariableValue)dfaVar).isNotNull()) {
return true;
}
if (dfaVar instanceof DfaConstValue && ((DfaConstValue)dfaVar).getValue() != null) {
return true;
}
@@ -575,15 +578,11 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
setVariableState(dfaVar, newState);
return true;
}
return applyCondition(compareToNull(dfaVar, false));
return applyRelation(dfaVar, myFactory.getConstFactory().getNull(), false);
}
boolean wasUnknown = getVariableState(dfaVar).getNullability() == Nullness.UNKNOWN;
if (applyCondition(compareToNull(dfaVar, true))) {
if (applyRelation(dfaVar, myFactory.getConstFactory().getNull(), true)) {
DfaVariableState newState = getVariableState(dfaVar).withInstanceofValue((DfaTypeValue)dfaRight);
if (newState != null) {
if (wasUnknown) {
newState = newState.withNullability(Nullness.UNKNOWN);
}
setVariableState(dfaVar, newState);
return true;
}
@@ -593,10 +592,6 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
return true;
}
if (isNull(dfaRight) && compareVariableWithNull(dfaLeft) || isNull(dfaLeft) && compareVariableWithNull(dfaRight)) {
return isNegated;
}
if (isEffectivelyNaN(dfaLeft) || isEffectivelyNaN(dfaRight)) {
applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight);
return isNegated;
@@ -609,17 +604,15 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
return applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight);
}
private boolean compareVariableWithNull(DfaValue val) {
if (val instanceof DfaVariableValue) {
DfaVariableValue dfaVar = (DfaVariableValue)val;
if (isNotNull(dfaVar)) {
return true;
}
if (!isUnknownState(dfaVar)) {
private void updateVarStateOnComparison(DfaVariableValue dfaVar, DfaValue value) {
if (!isUnknownState(dfaVar)) {
if (isNull(value)) {
setVariableState(dfaVar, getVariableState(dfaVar).withNullability(Nullness.NULLABLE));
} else if (isNotNull(value) && !isNotNull(dfaVar)) {
setVariableState(dfaVar, getVariableState(dfaVar).withNullability(Nullness.UNKNOWN));
applyRelation(dfaVar, myFactory.getConstFactory().getNull(), true);
}
}
return false;
}
private boolean applyEquivalenceRelation(DfaRelationValue dfaRelation, DfaValue dfaLeft, DfaValue dfaRight) {
@@ -627,6 +620,20 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
if (!isNegated && !dfaRelation.isEquality()) {
return true;
}
if (isNull(dfaLeft) && isNotNull(dfaRight) || isNull(dfaRight) && isNotNull(dfaLeft)) {
return isNegated;
}
if (!isNegated) {
if (dfaLeft instanceof DfaVariableValue) {
updateVarStateOnComparison((DfaVariableValue)dfaLeft, dfaRight);
}
if (dfaRight instanceof DfaVariableValue) {
updateVarStateOnComparison((DfaVariableValue)dfaRight, dfaLeft);
}
}
if (!applyRelation(dfaLeft, dfaRight, isNegated)) {
return false;
}
@@ -350,9 +350,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
final DfaMemoryState trueCopy = memState.createCopy();
if (trueCopy.applyCondition(dfaRelation)) {
if (!dfaRelation.isNegated()) {
checkOneOperandNotNull(dfaRight, dfaLeft, factory, trueCopy);
}
if (specialContractTreatment && !dfaRelation.isNegated()) {
trueCopy.markEphemeral();
}
@@ -364,9 +361,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
//noinspection UnnecessaryLocalVariable
DfaMemoryState falseCopy = memState;
if (falseCopy.applyCondition(dfaRelation.createNegated())) {
if (dfaRelation.isNegated()) {
checkOneOperandNotNull(dfaRight, dfaLeft, factory, falseCopy);
}
if (specialContractTreatment && dfaRelation.isNegated()) {
falseCopy.markEphemeral();
}
@@ -439,26 +433,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
return nextInstruction(instruction, runner, memState);
}
private static void checkOneOperandNotNull(DfaValue var1, DfaValue var2, DfaValueFactory factory, DfaMemoryState state) {
DfaValue nowNotNull = isNotNullExpression(var2, state) ? var1 : isNotNullExpression(var1, state) ? var2 : null;
if (nowNotNull != null) {
state.applyCondition(factory.getRelationFactory().createRelation(nowNotNull, factory.getConstFactory().getNull(), JavaTokenType.EQEQ, true));
}
}
private static boolean isNotNullExpression(DfaValue dfa, DfaMemoryState state) {
if (dfa instanceof DfaVariableValue) {
return state.isNotNull((DfaVariableValue)dfa);
}
if (dfa instanceof DfaConstValue) {
Object val = ((DfaConstValue)dfa).getValue();
if (val instanceof PsiEnumConstant) {
return true;
}
}
return false;
}
public boolean isInstanceofRedundant(InstanceofInstruction instruction) {
return !myUsefulInstanceofs.contains(instruction) && !instruction.isConditionConst() && myReachable.contains(instruction);
}
@@ -1,13 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Npe.java</file>
<line>12</line>
<description>Expression 'o' might evaluate to null</description>
</problem>
<problem>
<file>Npe.java</file>
<line>13</line>
<description>Expression 'o' might evaluate to null</description>
</problem>
</problems>
@@ -1,15 +0,0 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class Npe {
@NotNull Object aField;
@Nullable Object nullable() {
return null;
}
void bar() {
Object o = nullable();
aField = o;
@NotNull Object aLocalVariable = o;
}
}
@@ -0,0 +1,21 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class Npe {
@NotNull Object aField;
@Nullable Object nullable() {
return null;
}
void bar() {
Object o = nullable();
aField = <warning descr="Expression 'o' might evaluate to null but is assigned to a variable that is annotated with @NotNull">o</warning>;
@NotNull Object aLocalVariable = o;
}
void bar2() {
Object o = nullable();
@NotNull Object aLocalVariable = <warning descr="Expression 'o' might evaluate to null but is assigned to a variable that is annotated with @NotNull">o</warning>;
aField = o;
}
}
@@ -1,4 +1,8 @@
import org.jetbrains.annotations.*;
import org.jetbrains.annotations.NotNull;
import java.util.List;
class TestIDEAWarn {
void method(@Nullable MyEnum e) {
if (e != MyEnum.foo) {return;}
@@ -9,5 +13,25 @@ class TestIDEAWarn {
System.out.println(e.hashCode());
}
}
void method3(@Nullable MyEnum e) {
if (MyEnum.foo == e) {
System.out.println(e.hashCode());
}
}
void test(List items) {
MyEnum status = calcPodFileStatus();
if (status == MyEnum.foo && items.isEmpty()) {
return;
}
status.toString(); // false NPE warning here
}
@NotNull
private static MyEnum calcPodFileStatus() {
return MyEnum.foo;
}
}
enum MyEnum { foo, bar }
@@ -85,7 +85,6 @@ public class DataFlowInspectionAncientTest extends InspectionTestCase {
public void testNullableProblemThroughCast() { doTest15(); }
public void testNullableThroughVariable() { doTest15(); }
public void testNullableThroughVariableShouldNotBeReported() { doTest15(); }
public void testNullableAssignment() { doTest15(); }
public void testNullableLocalVariable() { doTest15(); }
public void testNotNullLocalVariable() { doTest15(); }
public void testNullableReturn() { doTest15(); }
@@ -110,6 +110,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
public void testAccessorPlusMutator() throws Throwable { doTest(); }
public void testClosureVariableField() throws Throwable { doTest(); }
public void testAssigningNullableToNotNull() throws Throwable { doTest(); }
public void testAssigningUnknownToNullable() throws Throwable { doTest(); }
public void testAssigningClassLiteralToNullable() throws Throwable { doTest(); }