From 94b63d6ff0e6b0a192730f7391dddced596ae3c0 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 5 Aug 2019 06:18:23 +0700 Subject: [PATCH] IDEA-219109 Stream.of(...).anyMatch(Objects::isNull) does not affect nullability analysis GitOrigin-RevId: ce9769eb34c87cf6bc07b0c526f799b9b4f9a324 --- .../dataFlow/ControlFlowAnalyzer.java | 4 ++- .../dataFlow/DfaMemoryState.java | 8 +++++ .../dataFlow/DfaMemoryStateImpl.java | 30 ++++++++++++------- .../dataFlow/StandardInstructionVisitor.java | 16 +++------- .../instructions/BinopInstruction.java | 7 ++++- .../fixture/StreamAnyMatchIsNull.java | 21 +++++++++++++ .../completion/NormalCompletionDfaTest.groovy | 2 +- .../DataFlowInspection8Test.java | 1 + 8 files changed, 63 insertions(+), 26 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/StreamAnyMatchIsNull.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 925a8ac434fd..f451e157fe25 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -954,7 +954,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (caseValue != null && expressionValue != null) { addInstruction(new PushInstruction(expressionValue, null)); caseValue.accept(this); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, PsiType.BOOLEAN)); + addInstruction(new BinopInstruction( + TypeUtils.isJavaLangString(expressionValue.getType()) ? BinopInstruction.STRING_EQUALITY_BY_CONTENT : + JavaTokenType.EQEQ, null, PsiType.BOOLEAN)); } else { pushUnknown(); 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 3d313f9cf807..e024e5a5c924 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 @@ -176,4 +176,12 @@ public interface DfaMemoryState { boolean isEphemeral(); boolean isEmptyStack(); + + /** + * Returns true if two given values should be compared by content, rather than by reference. + * @param dfaLeft left value + * @param dfaRight right value + * @return true if two given values should be compared by content, rather than by reference. + */ + boolean shouldCompareByEquals(DfaValue dfaLeft, DfaValue dfaRight); } 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 f8e45cea0a42..e0b1909f3884 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 @@ -231,12 +231,6 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } } } - else if (DfaUtil.isComparedByEquals(value.getType()) && !DfaUtil.isComparedByEquals(var.getType())) { - // Like Object x = "foo" or Object x = 5; - TypeConstraint typeConstraint = TypeConstraint.empty().withInstanceofValue(myFactory.createDfaType(value.getType())); - DfaFactMap facts = filterFactsOnAssignment(var, getFactMap(value).with(DfaFactType.TYPE_CONSTRAINT, typeConstraint)); - setVariableState(var, createVariableState(var).withFacts(facts)); - } else { setVariableState(var, isNull(value) ? state.withFact(DfaFactType.NULLABILITY, DfaNullability.NULL) : state); DfaRelationValue dfaEqual = myFactory.getRelationFactory().createRelation(var, RelationType.EQ, value); @@ -377,6 +371,15 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return thisToThat; } + @Override + public boolean shouldCompareByEquals(DfaValue dfaLeft, DfaValue dfaRight) { + if (dfaLeft == dfaRight && !(dfaLeft instanceof DfaBoxedValue) && !(dfaLeft instanceof DfaConstValue)) { + return false; + } + return !isNull(dfaLeft) && !isNull(dfaRight) && + DfaUtil.isComparedByEquals(getPsiType(dfaLeft)) && DfaUtil.isComparedByEquals(getPsiType(dfaRight)); + } + private static boolean isSuperValue(DfaValue superValue, DfaValue subValue) { if (superValue == DfaUnknownValue.getInstance() || superValue == subValue) return true; if (superValue instanceof DfaFactMapValue && subValue instanceof DfaFactMapValue) { @@ -1215,10 +1218,13 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } @Nullable - private static PsiType getPsiType(@NotNull DfaValue value) { - if (value instanceof DfaFactMapValue) { - TypeConstraint constraint = ((DfaFactMapValue)value).get(DfaFactType.TYPE_CONSTRAINT); - return constraint == null ? null : constraint.getPsiType(); + private PsiType getPsiType(@NotNull DfaValue value) { + TypeConstraint constraint = getValueFact(value, DfaFactType.TYPE_CONSTRAINT); + if (constraint != null) { + PsiType type = constraint.getPsiType(); + if (type != null) { + return type; + } } return value.getType(); } @@ -1323,7 +1329,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } private static boolean isPrimitive(DfaValue value) { - return value instanceof DfaVariableValue && value.getType() instanceof PsiPrimitiveType; + return value instanceof DfaVariableValue && + value.getType() instanceof PsiPrimitiveType && + !PsiType.VOID.equals(value.getType()); // void is used for temporary variables sometimes } private static boolean preserveConstantDistinction(final Object c1, final Object c2) { 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 a3218fdc854d..d9687c8a48ed 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 @@ -686,7 +686,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { DfaValue dfaLeft = memState.pop(); final IElementType opSign = instruction.getOperationSign(); - RelationType relationType = RelationType.fromElementType(opSign); + RelationType relationType = + RelationType.fromElementType(opSign == BinopInstruction.STRING_EQUALITY_BY_CONTENT ? JavaTokenType.EQEQ : opSign); if (relationType != null) { DfaInstructionState[] states = handleRelationBinop(instruction, runner, memState, dfaRight, dfaLeft, relationType); if (states != null) { @@ -757,8 +758,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { RelationType relationType) { DfaValueFactory factory = runner.getFactory(); if((relationType == RelationType.EQ || relationType == RelationType.NE) && - (dfaLeft != dfaRight || dfaLeft instanceof DfaBoxedValue || dfaLeft instanceof DfaConstValue) && - isComparedByEquals(instruction.getExpression()) && !memState.isNull(dfaLeft) && !memState.isNull(dfaRight)) { + instruction.getOperationSign() != BinopInstruction.STRING_EQUALITY_BY_CONTENT && + memState.shouldCompareByEquals(dfaLeft, dfaRight)) { ArrayList states = new ArrayList<>(2); DfaMemoryState equality = memState.createCopy(); if (equality.applyCondition(factory.createCondition(dfaLeft, RelationType.EQ, dfaRight))) { @@ -801,15 +802,6 @@ public class StandardInstructionVisitor extends InstructionVisitor { return states.toArray(DfaInstructionState.EMPTY_ARRAY); } - private static boolean isComparedByEquals(PsiExpression expression) { - if (expression instanceof PsiBinaryExpression) { - PsiExpression left = ((PsiBinaryExpression)expression).getLOperand(); - PsiExpression right = ((PsiBinaryExpression)expression).getROperand(); - return right != null && (DfaUtil.isComparedByEquals(left.getType()) && DfaUtil.isComparedByEquals(right.getType())); - } - return false; - } - @NotNull private static RelationType[] splitRelation(RelationType relationType) { switch (relationType) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java index f4951c8129b1..4e71cccc5277 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java @@ -32,7 +32,7 @@ import static com.intellij.psi.JavaTokenType.*; public class BinopInstruction extends BranchingInstruction implements ExpressionPushingInstruction { private static final TokenSet ourSignificantOperations = - TokenSet.create(EQEQ, NE, LT, GT, LE, GE, INSTANCEOF_KEYWORD, PLUS, MINUS, AND, OR, XOR, PERC, DIV, ASTERISK, GTGT, GTGTGT, LTLT); + TokenSet.create(EQ, EQEQ, NE, LT, GT, LE, GE, INSTANCEOF_KEYWORD, PLUS, MINUS, AND, OR, XOR, PERC, DIV, ASTERISK, GTGT, GTGTGT, LTLT); /** * A placeholder operation to model string concatenation inside loop: @@ -41,6 +41,11 @@ public class BinopInstruction extends BranchingInstruction implements Expression * so we use special operation for this case. */ public static final IElementType STRING_CONCAT_IN_LOOP = ASTERISK; + /** + * A special operation to express string comparison by content (like equals() method does). + * Used to desugar switch statements + */ + public static final IElementType STRING_EQUALITY_BY_CONTENT = EQ; private final IElementType myOperationSign; private final @Nullable PsiType myResultType; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamAnyMatchIsNull.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamAnyMatchIsNull.java new file mode 100644 index 000000000000..e23bede60ee2 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamAnyMatchIsNull.java @@ -0,0 +1,21 @@ +import java.math.BigInteger; +import java.util.Objects; +import java.util.stream.Stream; +import org.jetbrains.annotations.Nullable; + +class Test { + @Nullable + BigInteger calculate(@Nullable Boolean first, @Nullable BigInteger second, @Nullable BigInteger third) { + if (Stream.of(first, second, third).anyMatch(Objects::isNull)) { + return null; + } + return (first ? second : BigInteger.ZERO).add(third); + } + + void simpler(@Nullable Boolean b) { + Object x = b; + if (x != null) { + System.out.println(b.hashCode()); + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionDfaTest.groovy b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionDfaTest.groovy index d99cfecfca87..f87c5df4dd67 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionDfaTest.groovy +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionDfaTest.groovy @@ -176,7 +176,7 @@ public class Super { private void doTestSecond() { configure() - assert myItems?.length == 0 + assert myItems == null || myItems.length == 0 myFixture.completeBasic() checkResult() } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index ee0da645cd6b..ba36f2b6ad1b 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -179,6 +179,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { setupTypeUseAnnotations("foo", myFixture); doTest(); } + public void testStreamAnyMatchIsNull() { doTest(); } public void testMapGetWithNotNullKeys() { doTestWithCustomAnnotations(); } public void testInferNestedForeachNullability() { doTestWithCustomAnnotations(); }