IDEA-153922 "Optional.get() without isPresent" check when used in ifPresent's consumer

This commit is contained in:
Tagir Valeev
2017-02-13 17:48:12 +07:00
parent b4ab7f98b2
commit 0e3cb89ae2
5 changed files with 87 additions and 61 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2015 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -26,7 +26,6 @@ package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.progress.ProgressManager;
@@ -286,14 +285,14 @@ public class DataFlowRunner {
for (PsiMethod method : nestedClass.getMethods()) {
PsiCodeBlock body = method.getBody();
if (body != null) {
myNestedClosures.putValue(body, createClosureState(state));
myNestedClosures.putValue(body, state.createClosureState());
}
}
for (PsiClassInitializer initializer : nestedClass.getInitializers()) {
myNestedClosures.putValue(initializer.getBody(), createClosureState(state));
myNestedClosures.putValue(initializer.getBody(), state.createClosureState());
}
for (PsiField field : nestedClass.getFields()) {
myNestedClosures.putValue(field, createClosureState(state));
myNestedClosures.putValue(field, state.createClosureState());
}
}
@@ -301,7 +300,7 @@ public class DataFlowRunner {
DfaMemoryState state = instructionState.getMemoryState();
PsiElement body = expr.getBody();
if (body != null) {
myNestedClosures.putValue(body, createClosureState(state));
myNestedClosures.putValue(body, state.createClosureState());
}
}
@@ -359,16 +358,4 @@ public class DataFlowRunner {
return Pair.create(trueSet, falseSet);
}
@NotNull
private static DfaMemoryStateImpl createClosureState(@NotNull DfaMemoryState memState) {
DfaMemoryStateImpl copy = (DfaMemoryStateImpl)memState.createCopy();
copy.flushFields();
Set<DfaVariableValue> vars = new HashSet<>(copy.getVariableStates().keySet());
for (DfaVariableValue value : vars) {
copy.flushDependencies(value);
}
copy.emptyStack();
return copy;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2015 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -34,6 +34,9 @@ public interface DfaMemoryState {
@NotNull
DfaMemoryState createCopy();
@NotNull
DfaMemoryStateImpl createClosureState();
DfaValue pop();
DfaValue peek();
void push(@NotNull DfaValue value);
@@ -53,6 +53,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
// dfa value id -> indices in myEqClasses list of the classes which contain the id (or wrapped)
private final TIntObjectHashMap<int[]> myIdToEqClassesIndices;
private final Stack<DfaValue> myStack;
// Closures which correspond to the stack top (do not track other closures for now)
private final List<DfaMemoryState> myStackTopClosures = new ArrayList<>();
private final TLongHashSet myDistinctClasses;
private final LinkedHashMap<DfaVariableValue,DfaVariableState> myVariableStates;
private final Map<DfaVariableValue,DfaVariableState> myDefaultVariableStates;
@@ -106,6 +108,24 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
return new DfaMemoryStateImpl(this);
}
@NotNull
@Override
public DfaMemoryStateImpl createClosureState() {
DfaMemoryStateImpl copy = createCopy();
copy.flushFields();
Set<DfaVariableValue> vars = new HashSet<>(copy.getVariableStates().keySet());
for (DfaVariableValue value : vars) {
copy.flushDependencies(value);
}
copy.emptyStack();
myStackTopClosures.add(copy);
return copy;
}
List<DfaMemoryState> getStackTopClosures() {
return new ArrayList<>(myStackTopClosures);
}
public boolean equals(Object obj) {
if (obj == this) return true;
if (!(obj instanceof DfaMemoryStateImpl)) return false;
@@ -221,6 +241,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
@Override
public DfaValue pop() {
myCachedHash = null;
myStackTopClosures.clear();
return myStack.pop();
}
@@ -232,12 +253,14 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
@Override
public void push(@NotNull DfaValue value) {
myCachedHash = null;
myStackTopClosures.clear();
myStack.push(value);
}
@Override
public void emptyStack() {
myCachedHash = null;
myStackTopClosures.clear();
while (!myStack.isEmpty() && !(myStack.peek() instanceof DfaControlTransferValue)) {
myStack.pop();
}
@@ -1116,7 +1139,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
@Override
public boolean execute(int id, int[] set) {
DfaValue value = myFactory.getValue(id);
s.append(value + " -> " + Arrays.toString(set) + ", ");
s.append(value).append(" -> ").append(Arrays.toString(set)).append(", ");
return true;
}
});
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2009 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -42,6 +42,9 @@ import static com.intellij.psi.JavaTokenType.*;
* @author peter
*/
public class StandardInstructionVisitor extends InstructionVisitor {
private static final Set<String> OPTIONAL_METHOD_NAMES =
ContainerUtil.set("isPresent", "of", "ofNullable", "fromNullable", "empty", "absent",
"or", "orElseGet", "ifPresent", "map", "flatMap", "filter", "transform");
private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.StandardInstructionVisitor");
private static final Object ANY_VALUE = new Object();
private final Set<BinopInstruction> myReachable = new THashSet<>();
@@ -180,13 +183,13 @@ public class StandardInstructionVisitor extends InstructionVisitor {
@Override
public DfaInstructionState[] visitMethodCall(final MethodCallInstruction instruction, final DataFlowRunner runner, final DfaMemoryState memState) {
DfaValue[] argValues = popCallArguments(instruction, runner, memState);
final DfaValue qualifier = popQualifier(instruction, runner, memState);
Set<DfaMemoryState> finalStates = ContainerUtil.newLinkedHashSet();
finalStates.addAll(handleOptionalMethods(instruction, runner, memState, qualifier, argValues));
finalStates.addAll(handleOptionalMethods(instruction, runner, memState));
if (finalStates.isEmpty()) {
DfaValue[] argValues = popCallArguments(instruction, runner, memState);
final DfaValue qualifier = popQualifier(instruction, runner, memState);
LinkedHashSet<DfaMemoryState> currentStates = ContainerUtil.newLinkedHashSet(memState);
if (argValues != null) {
for (MethodContract contract : instruction.getContracts()) {
@@ -219,54 +222,63 @@ public class StandardInstructionVisitor extends InstructionVisitor {
}
@NotNull
private static List<DfaMemoryState> handleOptionalMethods(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState,
DfaValue qualifierValue, @Nullable DfaValue[] argValues) {
private List<DfaMemoryState> handleOptionalMethods(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class);
if (call == null) return Collections.emptyList();
String methodName = call.getMethodExpression().getReferenceName();
if ("isPresent".equals(methodName)) {
PsiMethod method = call.resolveMethod();
if (method != null && TypeUtils.isOptional(method.getContainingClass())) {
ThreeState state = memState.checkOptional(qualifierValue);
if (methodName == null || !OPTIONAL_METHOD_NAMES.contains(methodName)) return Collections.emptyList();
PsiMethod method = call.resolveMethod();
if (method == null || !TypeUtils.isOptional(method.getContainingClass())) return Collections.emptyList();
List<DfaMemoryState> closures = ((DfaMemoryStateImpl)memState).getStackTopClosures();
DfaValue[] argValues = popCallArguments(instruction, runner, memState);
final DfaValue qualifier = popQualifier(instruction, runner, memState);
switch (methodName) {
case "isPresent": {
ThreeState state = memState.checkOptional(qualifier);
DfaConstValue.Factory constFactory = runner.getFactory().getConstFactory();
if (state == ThreeState.UNSURE) {
DfaMemoryState falseState = memState.createCopy();
memState.push(constFactory.getTrue());
memState.applyIsPresentCheck(true, qualifierValue);
memState.applyIsPresentCheck(true, qualifier);
falseState.push(constFactory.getFalse());
falseState.applyIsPresentCheck(false, qualifierValue);
falseState.applyIsPresentCheck(false, qualifier);
return Arrays.asList(memState, falseState);
}
else {
DfaValue result = state == ThreeState.YES ? constFactory.getTrue() : constFactory.getFalse();
memState.push(result);
return Collections.singletonList(memState);
memState.push(state == ThreeState.YES ? constFactory.getTrue() : constFactory.getFalse());
}
break;
}
}
if ("of".equals(methodName)) {
PsiMethod method = call.resolveMethod();
if (method != null && TypeUtils.isOptional(method.getContainingClass())) {
memState.push(runner.getFactory().getOptionalFactory().getOptional(true));
return Collections.singletonList(memState);
}
}
if (DfaOptionalSupport.resolveOfNullable(call) != null) {
if (argValues != null && argValues.length == 1 && memState.isNotNull(argValues[0])) {
memState.push(runner.getFactory().getOptionalFactory().getOptional(true));
return Collections.singletonList(memState);
}
}
if ("empty".equals(methodName) || "absent".equals(methodName)) {
PsiMethod method = call.resolveMethod();
if (method != null && TypeUtils.isOptional(method.getContainingClass())) {
case "of":
case "ofNullable":
case "fromNullable":
if ("of".equals(methodName) || (argValues != null && argValues.length == 1 && memState.isNotNull(argValues[0]))) {
memState.push(runner.getFactory().getOptionalFactory().getOptional(true));
} else {
memState.push(getMethodResultValue(instruction, qualifier, runner.getFactory()));
}
break;
case "empty":
case "absent":
memState.push(runner.getFactory().getOptionalFactory().getOptional(false));
return Collections.singletonList(memState);
}
break;
case "filter":
case "flatMap":
case "ifPresent":
case "map":
case "or":
case "orElseGet":
case "transform":
for (DfaMemoryState closure : closures) {
closure.applyIsPresentCheck(!methodName.startsWith("or"), qualifier);
}
memState.push(getMethodResultValue(instruction, qualifier, runner.getFactory()));
break;
default:
memState.push(getMethodResultValue(instruction, qualifier, runner.getFactory()));
break;
}
return Collections.emptyList();
return Collections.singletonList(memState);
}
@Nullable
@@ -226,9 +226,10 @@ class OptionalWithoutIsPresent {
return Optional.empty();
}
//void order(Optional<String> order) {
// order.ifPresent(o -> System.out.println(order.get()));
//}
void order(Optional<String> order, boolean b) {
order.ifPresent(o -> System.out.println(order.get()));
System.out.println(order.orElseGet(() -> order.<warning descr="'Optional.get()' will definitely fail as Optional is empty here">get</warning>().trim()));
}
public static void two(Optional<Object> o1,Optional<Object> o2) {
if (!o1.isPresent() && !o2.isPresent()) {