IgnoreResultOfCallInspection: report Optional.get() under isPresent() (IDEA-205785)

This commit is contained in:
Tagir Valeev
2019-01-23 15:32:37 +07:00
parent b78984a1e1
commit 11ab5b4153
3 changed files with 39 additions and 6 deletions
@@ -28,6 +28,7 @@ public class CommonDataflow {
@Nullable Set<Object> myPossibleValues = Collections.emptySet();
// null = top; empty = bottom
@Nullable Set<Object> myNotValues = null;
boolean myMayFailByContract = false;
DataflowPoint() {}
@@ -35,6 +36,7 @@ public class CommonDataflow {
myFacts = other.myFacts;
myPossibleValues = other.myPossibleValues;
myNotValues = other.myNotValues == null || other.myNotValues.isEmpty() ? other.myNotValues : new THashSet<>(other.myNotValues);
myMayFailByContract = other.myMayFailByContract;
}
void addNotValues(DfaMemoryStateImpl memState, DfaValue value) {
@@ -109,6 +111,10 @@ public class CommonDataflow {
void add(PsiExpression expression, DfaMemoryStateImpl memState, DfaValue value) {
DataflowPoint point = myData.computeIfAbsent(expression, e -> new DataflowPoint());
if (DfaConstValue.isContractFail(value)) {
point.myMayFailByContract = true;
return;
}
if (point.myFacts != DfaFactMap.EMPTY) {
PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
if (parent instanceof PsiConditionalExpression &&
@@ -147,6 +153,18 @@ public class CommonDataflow {
return myData.containsKey(expression);
}
/**
* Returns true if given call cannot fail according to its contracts
* (e.g. {@code Optional.get()} executed under {@code Optional.isPresent()}).
*
* @param call call to check
* @return true if it cannot fail by contract; false if unknown or can fail
*/
public boolean cannotFailByContract(PsiCallExpression call) {
DataflowPoint point = myData.get(call);
return point != null && !point.myMayFailByContract;
}
/**
* Returns a fact of specific type which is known for given expression or null if fact is not known
*
@@ -289,7 +307,7 @@ public class CommonDataflow {
@NotNull PsiExpression expression,
@Nullable TextRange range,
@NotNull DfaMemoryState state) {
if (range == null && !DfaConstValue.isContractFail(value)) {
if (range == null) {
// Do not track instructions which cover part of expression
myResult.add(expression, (DfaMemoryStateImpl)state, value);
}
@@ -16,6 +16,7 @@
package com.siyeh.ig.bugs;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInspection.dataFlow.CommonDataflow;
import com.intellij.codeInspection.dataFlow.ContractReturnValue;
import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.codeInspection.dataFlow.MethodContract;
@@ -201,7 +202,7 @@ public class IgnoreResultOfCallInspection extends BaseInspection {
if (isKnownExceptionalSideEffectCaught(call)) return;
if (isPureMethod(method)) {
if (isPureMethod(method, call)) {
registerMethodCallOrRefError(call, aClass);
return;
}
@@ -272,12 +273,14 @@ public class IgnoreResultOfCallInspection extends BaseInspection {
return false;
}
private boolean isPureMethod(PsiMethod method) {
private boolean isPureMethod(PsiMethod method, PsiExpression call) {
final boolean honorInferred = Registry.is("ide.ignore.call.result.inspection.honor.inferred.pure");
if (!honorInferred && !JavaMethodContractUtil.hasExplicitContractAnnotation(method)) return false;
return JavaMethodContractUtil.isPure(method) &&
!SideEffectChecker.mayHaveExceptionalSideEffect(method) &&
!hasTrivialReturnValue(method);
if (!JavaMethodContractUtil.isPure(method) || hasTrivialReturnValue(method)) return false;
if (!SideEffectChecker.mayHaveExceptionalSideEffect(method)) return true;
if (!(call instanceof PsiCallExpression)) return false;
CommonDataflow.DataflowResult result = CommonDataflow.getDataflowResult(call);
return result != null && result.cannotFailByContract((PsiCallExpression)call);
}
private boolean hasTrivialReturnValue(PsiMethod method) {
@@ -348,6 +348,7 @@ public static int atLeast(int min, int actual, String varName) {
}
void testInForExpressionList() {
//noinspection StatementWithEmptyBody
doTest """class X {
void test(String s) {
for(int i=0; i<10; i++, s./*Result of 'String.trim()' is ignored*/trim/**/()) {}
@@ -356,12 +357,23 @@ public static int atLeast(int min, int actual, String varName) {
}
void testInSwitchExpression() {
//noinspection SwitchStatementWithTooFewBranches
doTest """class X {
String test(String s) {
return switch(s) {
default -> s.trim();
};
}
}"""
}
void testOptionalGet() {
//noinspection ALL
doTest """class X {
void test(java.util.Optional<String> opt) {
opt.get();
if (opt.isPresent()) opt./*Result of 'Optional.get()' is ignored*/get/**/();
}
}"""
}
}