OptionalIsPresentInspection: do not warn if map() part can be null

If we cannot determine the non-nullity of map expression, then info
level is used. Fixes IDEA-172609 "Replace Optional.isPresent() checks
with functional-style expressions" is broken.
This commit is contained in:
Tagir Valeev
2017-05-31 17:48:57 +07:00
parent 763953d92a
commit 588d978362
9 changed files with 102 additions and 8 deletions

View File

@@ -74,7 +74,7 @@ public class NullnessUtil {
if (DfaPsiUtil.isFinalField(field)) {
PsiExpression initializer = field.getInitializer();
if (initializer != null) {
return getFieldInitializerNullness(initializer);
return getExpressionNullness(initializer);
}
List<PsiExpression> initializers = DfaPsiUtil.findAllConstructorInitializers(field);
@@ -83,7 +83,7 @@ public class NullnessUtil {
}
for (PsiExpression expression : initializers) {
if (getFieldInitializerNullness(expression) == Nullness.NULLABLE) {
if (getExpressionNullness(expression) == Nullness.NULLABLE) {
return Nullness.NULLABLE;
}
}
@@ -129,13 +129,29 @@ public class NullnessUtil {
return result != PsiSearchHelper.SearchCostResult.TOO_MANY_OCCURRENCES;
}
private static Nullness getFieldInitializerNullness(@NotNull PsiExpression expression) {
public static Nullness getExpressionNullness(@Nullable PsiExpression expression) {
expression = PsiUtil.skipParenthesizedExprDown(expression);
if (expression == null) return Nullness.UNKNOWN;
if (expression.textMatches(PsiKeyword.NULL)) return Nullness.NULLABLE;
if (expression instanceof PsiNewExpression ||
expression instanceof PsiLiteralExpression ||
expression instanceof PsiPolyadicExpression) {
expression instanceof PsiPolyadicExpression ||
expression instanceof PsiFunctionalExpression ||
expression.getType() instanceof PsiPrimitiveType) {
return Nullness.NOT_NULL;
}
if (expression instanceof PsiConditionalExpression) {
PsiExpression thenExpression = ((PsiConditionalExpression)expression).getThenExpression();
PsiExpression elseExpression = ((PsiConditionalExpression)expression).getElseExpression();
if (thenExpression == null || elseExpression == null) return Nullness.UNKNOWN;
Nullness left = getExpressionNullness(thenExpression);
if (left == Nullness.UNKNOWN) return Nullness.UNKNOWN;
Nullness right = getExpressionNullness(elseExpression);
return left == right ? left : Nullness.UNKNOWN;
}
if (expression instanceof PsiTypeCastExpression) {
return getExpressionNullness(((PsiTypeCastExpression)expression).getOperand());
}
if (expression instanceof PsiReferenceExpression) {
PsiElement target = ((PsiReferenceExpression)expression).resolve();
return DfaPsiUtil.getElementNullability(expression.getType(), (PsiModifierListOwner)target);

View File

@@ -16,6 +16,8 @@
package com.intellij.codeInspection;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInspection.dataFlow.Nullness;
import com.intellij.codeInspection.dataFlow.NullnessUtil;
import com.intellij.codeInspection.util.LambdaGenerationUtil;
import com.intellij.codeInspection.util.OptionalUtil;
import com.intellij.openapi.diagnostic.Logger;
@@ -189,8 +191,12 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo
return isOptionalGetCall(e.getParent().getParent(), optionalVariable);
});
if(!hasNoBadRefs) return ProblemType.NONE;
if(hasOptionalReference.get() && lambdaCandidate instanceof PsiExpression) return ProblemType.WARNING;
return ProblemType.INFO;
if (!hasOptionalReference.get() || !(lambdaCandidate instanceof PsiExpression)) return ProblemType.INFO;
PsiExpression expression = (PsiExpression)lambdaCandidate;
if (!PsiType.VOID.equals(expression.getType()) && NullnessUtil.getExpressionNullness(expression) != Nullness.NOT_NULL) {
return ProblemType.INFO;
}
return ProblemType.WARNING;
}
@NotNull

View File

@@ -1,4 +1,4 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import java.util.Arrays;
import java.util.List;

View File

@@ -0,0 +1,13 @@
// "Replace Optional.isPresent() condition with functional style expression" "INFORMATION"
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
public class Main {
public void test(Optional<String> opt, Function<String, Object> onPresent, Supplier<Object> onEmpty) {
// information level: could be semantic change if onPresent returns null
Object o = opt.map(onPresent::apply).orElseGet(onEmpty::get);
}
}

View File

@@ -0,0 +1,14 @@
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import org.jetbrains.annotations.NotNull;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
public class Main {
public void test(Optional<String> opt, Function<String, @NotNull Object> onPresent, Supplier<Object> onEmpty) {
Object o = opt.map(onPresent::apply).orElseGet(onEmpty::get);
}
}

View File

@@ -1,4 +1,4 @@
// "Replace Optional.isPresent() condition with functional style expression" "true"
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import java.util.Arrays;
import java.util.List;

View File

@@ -0,0 +1,13 @@
// "Replace Optional.isPresent() condition with functional style expression" "INFORMATION"
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
public class Main {
public void test(Optional<String> opt, Function<String, Object> onPresent, Supplier<Object> onEmpty) {
// information level: could be semantic change if onPresent returns null
Object o = opt.is<caret>Present() ? onPresent.apply(opt.get()) : onEmpty.get();
}
}

View File

@@ -0,0 +1,14 @@
// "Replace Optional.isPresent() condition with functional style expression" "GENERIC_ERROR_OR_WARNING"
import org.jetbrains.annotations.NotNull;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
public class Main {
public void test(Optional<String> opt, Function<String, @NotNull Object> onPresent, Supplier<Object> onEmpty) {
Object o = opt.i<caret>sPresent() ? onPresent.apply(opt.get()) : onEmpty.get();
}
}

View File

@@ -18,10 +18,28 @@ package com.intellij.java.codeInsight.daemon.quickFix;
import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.OptionalIsPresentInspection;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.testFramework.IdeaTestUtil;
import com.intellij.testFramework.LightProjectDescriptor;
import com.intellij.testFramework.PsiTestUtil;
import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor;
import org.jetbrains.annotations.NotNull;
public class OptionalIsPresentInspectionTest extends LightQuickFixParameterizedTestCase {
private static final DefaultLightProjectDescriptor PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() {
@Override
public Sdk getSdk() {
return PsiTestUtil.addJdkAnnotations(IdeaTestUtil.getMockJdk18());
}
};
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return PROJECT_DESCRIPTOR;
}
@NotNull
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {