Optional compared with null: do not warn when subsequent isPresent check is found

IDEA-187578  Inspection to detect when Optional is compared with null
This commit is contained in:
Tagir Valeev
2018-03-09 19:24:57 +07:00
parent 273862e0bb
commit 350eba5000
3 changed files with 76 additions and 1 deletions

View File

@@ -1,14 +1,19 @@
// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.impl.PsiDiamondTypeUtil;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.*;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -82,7 +87,9 @@ public class OptionalAssignedToNullInspection extends AbstractBaseJavaLocalInspe
public void visitBinaryExpression(PsiBinaryExpression binOp) {
if (!WARN_ON_COMPARISON) return;
PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp);
if (value != null && TypeUtils.isOptional(value.getType())) {
if (value != null &&
TypeUtils.isOptional(value.getType()) &&
!hasSubsequentIsPresentCall(value, binOp, JavaTokenType.EQEQ.equals(binOp.getOperationTokenType()))) {
holder.registerProblem(binOp, "Optional value is compared with null",
new ReplaceWithIsPresentFix(),
new SetInspectionOptionFix(OptionalAssignedToNullInspection.this, "WARN_ON_COMPARISON",
@@ -90,6 +97,32 @@ public class OptionalAssignedToNullInspection extends AbstractBaseJavaLocalInspe
}
}
private boolean hasSubsequentIsPresentCall(@NotNull PsiExpression optionalExpression,
@NotNull PsiExpression previousExpression,
boolean negated) {
PsiPolyadicExpression parent =
ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(previousExpression.getParent()), PsiPolyadicExpression.class);
if (parent == null) return false;
IElementType expectedToken = negated ? JavaTokenType.OROR : JavaTokenType.ANDAND;
if (!parent.getOperationTokenType().equals(expectedToken)) return false;
PsiExpression nextExpression =
StreamEx.of(parent.getOperands()).dropWhile(op -> !PsiTreeUtil.isAncestor(op, previousExpression, false))
.skip(1)
.findFirst()
.orElse(null);
nextExpression = PsiUtil.skipParenthesizedExprDown(nextExpression);
if (nextExpression == null) return false;
if (negated) {
if (!BoolUtils.isNegation(nextExpression)) return false;
nextExpression = BoolUtils.getNegated(nextExpression);
}
if (!(nextExpression instanceof PsiMethodCallExpression)) return false;
PsiMethodCallExpression call = (PsiMethodCallExpression)nextExpression;
if (!"isPresent".equals(call.getMethodExpression().getReferenceName()) || !call.getArgumentList().isEmpty()) return false;
PsiExpression qualifier = call.getMethodExpression().getQualifierExpression();
return qualifier != null && PsiEquivalenceUtil.areElementsEquivalent(qualifier, optionalExpression);
}
private void checkNulls(PsiType type, PsiExpression expression, String declaration) {
if (expression != null && TypeUtils.isOptional(type)) {
ExpressionUtils.nonStructuralChildren(expression).filter(ExpressionUtils::isNullLiteral)

View File

@@ -17,4 +17,25 @@ public class Test {
}
}
void test3 (Optional<String> opt) {
// this case is ignored
if(opt == null || !opt.isPresent()) {
System.out.println("null or absent");
}
}
void test4 (Optional<String> opt) {
// this case is ignored as well
if(opt != null && opt.isPresent()) {
System.out.println("present");
}
}
void test5 (Optional<String> opt, Optional<String> opt2) {
// warn: different optionals used
if(opt.isPresent() && opt2.isPresent()) {
System.out.println("present");
}
}
}

View File

@@ -16,4 +16,25 @@ public class Test {
}
}
void test3 (Optional<String> opt) {
// this case is ignored
if(opt == null || !opt.isPresent()) {
System.out.println("null or absent");
}
}
void test4 (Optional<String> opt) {
// this case is ignored as well
if(opt != null && opt.isPresent()) {
System.out.println("present");
}
}
void test5 (Optional<String> opt, Optional<String> opt2) {
// warn: different optionals used
if(opt != null && opt2.isPresent()) {
System.out.println("present");
}
}
}