diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java index fdadb06056b8..2676f993f5e2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryState.java @@ -52,7 +52,7 @@ public interface DfaMemoryState { boolean checkNotNullable(DfaValue value); - boolean isNotNull(DfaVariableValue dfaVar); + boolean isNotNull(DfaValue dfaVar); @Nullable DfaConstValue getConstantValue(DfaVariableValue value); 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 9aadd449632a..ccf45ae12e6d 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 @@ -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; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index 90a64901c424..fa21da09655d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -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); } diff --git a/java/java-tests/testData/inspection/dataFlow/NullableAssignment/expected.xml b/java/java-tests/testData/inspection/dataFlow/NullableAssignment/expected.xml deleted file mode 100644 index 7c2ceacca676..000000000000 --- a/java/java-tests/testData/inspection/dataFlow/NullableAssignment/expected.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - Npe.java - 12 - Expression 'o' might evaluate to null - - - Npe.java - 13 - Expression 'o' might evaluate to null - - diff --git a/java/java-tests/testData/inspection/dataFlow/NullableAssignment/src/Npe.java b/java/java-tests/testData/inspection/dataFlow/NullableAssignment/src/Npe.java deleted file mode 100644 index eae506186037..000000000000 --- a/java/java-tests/testData/inspection/dataFlow/NullableAssignment/src/Npe.java +++ /dev/null @@ -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; - } -} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AssigningNullableToNotNull.java b/java/java-tests/testData/inspection/dataFlow/fixture/AssigningNullableToNotNull.java new file mode 100644 index 000000000000..b93285792ed4 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AssigningNullableToNotNull.java @@ -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 = o; + @NotNull Object aLocalVariable = o; + } + + void bar2() { + Object o = nullable(); + @NotNull Object aLocalVariable = o; + aField = o; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/EqualsEnumConstant.java b/java/java-tests/testData/inspection/dataFlow/fixture/EqualsEnumConstant.java index 20e7b59bd396..2cb36331e6db 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/EqualsEnumConstant.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/EqualsEnumConstant.java @@ -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 } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java index 6d9423a6fcd9..57a38f4b015f 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java @@ -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(); } diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 7e8aa4a368b6..f4da7ba94d35 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -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(); }