From a03a5cb0ea12fbd7a8c67101edc611c184c74aae Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 1 Nov 2018 11:38:41 +0700 Subject: [PATCH] Optional presence fact should also report non-null (IDEA-201584) --- .../codeInspection/dataFlow/ContractValue.java | 4 ++-- .../dataFlow/CustomMethodHandlers.java | 4 ++-- .../dataFlow/DfaOptionalSupport.java | 14 ++++++++++++++ .../dataFlow/inliner/OptionalChainInliner.java | 16 ++++++++-------- .../dataFlow/inliner/StreamChainInliner.java | 8 ++++---- .../dataFlow/fixture/StreamInlining.java | 7 +++++++ 6 files changed, 37 insertions(+), 16 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java index 062b1559f238..9b4c82d5940b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java @@ -149,9 +149,9 @@ public abstract class ContractValue { } }; static final IndependentValue OPTIONAL_PRESENT = - new IndependentValue(factory -> factory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, true), "present"); + new IndependentValue(factory -> DfaOptionalSupport.getOptionalValue(factory, true), "present"); static final IndependentValue OPTIONAL_ABSENT = - new IndependentValue(factory -> factory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, false), "empty"); + new IndependentValue(factory -> DfaOptionalSupport.getOptionalValue(factory, false), "empty"); static final IndependentValue ZERO = new IndependentValue(factory -> factory.getInt(0), "0"); private final Function mySupplier; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java index 28df2b2cbbe2..5e6306fc6b85 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java @@ -199,10 +199,10 @@ class CustomMethodHandlers { private static DfaValue ofNullable(DfaValue argument, DfaMemoryState state, DfaValueFactory factory) { if (state.isNull(argument)) { - return factory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, false); + return DfaOptionalSupport.getOptionalValue(factory, false); } if (state.isNotNull(argument)) { - return factory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, true); + return DfaOptionalSupport.getOptionalValue(factory, true); } return null; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java index 7dc4c5bec851..0d75e901334e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java @@ -17,6 +17,8 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; @@ -83,6 +85,18 @@ public class DfaOptionalSupport { return "get".equals(name) || "getAsDouble".equals(name) || "getAsInt".equals(name) || "getAsLong".equals(name); } + /** + * Creates a DfaValue which represents present or absent optional (non-null) + * @param factory a value factory to use + * @param present whether the value should be present + * @return a DfaValue representing an Optional + */ + @NotNull + public static DfaValue getOptionalValue(DfaValueFactory factory, boolean present) { + DfaFactMap facts = DfaFactMap.EMPTY.with(DfaFactType.OPTIONAL_PRESENCE, present).with(DfaFactType.NULLABILITY, DfaNullability.NOT_NULL); + return factory.getFactFactory().createValue(facts); + } + private static class ReplaceOptionalCallFix implements LocalQuickFix { private final String myTargetMethodName; private final boolean myClearArguments; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java index 6b928f3b445b..2e3f1dc0bcf9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java @@ -17,7 +17,7 @@ package com.intellij.codeInspection.dataFlow.inliner; import com.intellij.codeInsight.Nullability; import com.intellij.codeInspection.dataFlow.CFGBuilder; -import com.intellij.codeInspection.dataFlow.DfaFactType; +import com.intellij.codeInspection.dataFlow.DfaOptionalSupport; import com.intellij.codeInspection.dataFlow.NullabilityProblemKind; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; @@ -175,14 +175,14 @@ public class OptionalChainInliner implements CallInliner { DfaValueFactory factFactory = builder.getFactory(); if (pushIntermediateOperationValue(builder, call)) { builder.ifNotNull() - .push(factFactory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, true)) + .push(DfaOptionalSupport.getOptionalValue(factFactory, true)) .elseBranch() - .push(factFactory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, false)) + .push(DfaOptionalSupport.getOptionalValue(factFactory, false)) .end(); return true; } if (OPTIONAL_EMPTY.test(call)) { - builder.push(factFactory.getFactValue(DfaFactType.OPTIONAL_PRESENCE, false)); + builder.push(DfaOptionalSupport.getOptionalValue(factFactory, false)); return true; } return false; @@ -224,7 +224,7 @@ public class OptionalChainInliner implements CallInliner { return true; } } - DfaValue presentOptional = builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, true); + DfaValue presentOptional = DfaOptionalSupport.getOptionalValue(builder.getFactory(), true); builder .pushExpression(expression) .checkNotNull(dereferenceContext, problem) @@ -294,16 +294,16 @@ public class OptionalChainInliner implements CallInliner { .boxUnbox(argument, optionalElementType); if ("of".equals(qualifierCall.getMethodExpression().getReferenceName())) { builder.checkNotNull(argument, NullabilityProblemKind.passingNullableToNotNullParameter) - .push(builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, true), qualifierCall) + .push(DfaOptionalSupport.getOptionalValue(builder.getFactory(), true), qualifierCall) .pop(); } else { builder .dup() .ifNull() - .push(builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, false), qualifierCall) + .push(DfaOptionalSupport.getOptionalValue(builder.getFactory(), false), qualifierCall) .elseBranch() - .push(builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, true), qualifierCall) + .push(DfaOptionalSupport.getOptionalValue(builder.getFactory(), true), qualifierCall) .end() .pop(); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java index 94a30764ae67..d54cfcc23374 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java @@ -262,12 +262,12 @@ public class StreamChainInliner implements CallInliner { @Override protected void pushInitialValue(CFGBuilder builder) { - builder.push(builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, false)); + builder.push(DfaOptionalSupport.getOptionalValue(builder.getFactory(), false)); } @Override void iteration(CFGBuilder builder) { - DfaValue presentOptional = builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, true); + DfaValue presentOptional = DfaOptionalSupport.getOptionalValue(builder.getFactory(), true); if (myFunction != null) { builder.push(myResult) .push(presentOptional) @@ -291,7 +291,7 @@ public class StreamChainInliner implements CallInliner { @Override protected void pushInitialValue(CFGBuilder builder) { - builder.push(builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, false)); + builder.push(DfaOptionalSupport.getOptionalValue(builder.getFactory(), false)); } @Override @@ -303,7 +303,7 @@ public class StreamChainInliner implements CallInliner { @Override void iteration(CFGBuilder builder) { myComparatorModel.invoke(builder); - builder.assignAndPop(myResult, builder.getFactory().getFactValue(DfaFactType.OPTIONAL_PRESENCE, true)); + builder.assignAndPop(myResult, DfaOptionalSupport.getOptionalValue(builder.getFactory(), true)); } @Override diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java index 3ea2df9b4ab6..8198e64e169c 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java @@ -250,4 +250,11 @@ public class StreamInlining { public static void testBoxingExplicit2() { double s = Stream.of(1).mapToDouble(x -> x * x).sum(); } + + void testOptionalNullity(List groups) { + Optional optional = groups.stream().findFirst(); + if (optional != null && optional.isPresent()) { + System.out.println("found"); + } + } }