MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-186380 Restore "Optional.get() without Optional.isPresent()" as a separate inspection

Browse Source
This commit is contained in:
Tagir Valeev
2018-02-28 14:08:40 +07:00
parent 947ec0d5b9
commit fa713953e9
14 changed files with 433 additions and 218 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java
+66 -16
View File
@@ -5,27 +5,27 @@ import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction;
import com.intellij.codeInspection.dataFlow.instructions.PushInstruction;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.ide.PowerSaveMode;
import com.intellij.psi.*;
import com.intellij.psi.util.CachedValueProvider;
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiModificationTracker;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.*;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.HashMap;
import java.util.Map;
public class CommonDataflow {
static class DataflowResult {
/**
* Represents the result of dataflow applied to some code fragment (usually a method)
*/
public static class DataflowResult {
private final Map<PsiExpression, DfaFactMap> myFacts = new HashMap<>();
void add(PsiExpression expression, DfaMemoryStateImpl memState) {
void add(PsiExpression expression, DfaMemoryStateImpl memState, DfaValue value) {
DfaFactMap existing = myFacts.get(expression);
if(existing != DfaFactMap.EMPTY) {
DfaValue value = memState.peek();
DfaFactMap newMap = memState.getFactMap(value);
if (!Boolean.FALSE.equals(newMap.get(DfaFactType.CAN_BE_NULL)) && memState.isNotNull(value)) {
newMap = newMap.with(DfaFactType.CAN_BE_NULL, false);
@@ -33,6 +33,33 @@ public class CommonDataflow {
myFacts.put(expression, existing == null ? newMap : existing.union(newMap));
}
}
/**
* Returns true if given expression was visited by dataflow. Note that dataflow usually tracks deparenthesized expressions only,
* so you should deparenthesize it in advance if necessary.
*
* @param expression expression to check
* @return true if given expression was visited by dataflow.
* If false is returned, it's possible that the expression exists in unreachable branch or this expression is not tracked due to
* the dataflow implementation details.
*/
public boolean expressionWasAnalyzed(PsiExpression expression) {
return myFacts.containsKey(expression);
}
/**
* Returns a fact of specific type which is known for given expression or null if fact is not known
*
* @param expression expression to get the fact
* @param type a fact type
* @param <T> resulting type
* @return a fact value or null if fact of given type is not known for given expression
*/
@Nullable
public <T> T getExpressionFact(PsiExpression expression, DfaFactType<T> type) {
DfaFactMap map = this.myFacts.get(expression);
return map == null ? null : map.get(type);
}
}
@Contract("null -> null")
@@ -49,23 +76,43 @@ public class CommonDataflow {
PsiExpression place = instruction.getPlace();
if (place != null && !instruction.isReferenceWrite()) {
for (DfaInstructionState state : states) {
dfr.add(place, (DfaMemoryStateImpl)state.getMemoryState());
DfaMemoryState afterState = state.getMemoryState();
dfr.add(place, (DfaMemoryStateImpl)afterState, instruction.getValue());
}
}
return states;
}
@NotNull
@Override
protected DfaCallArguments popCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState,
boolean contractOnly) {
DfaCallArguments arguments = super.popCall(instruction, runner, memState, contractOnly);
PsiElement context = instruction.getContext();
if (instruction.getMethodType() == MethodCallInstruction.MethodType.REGULAR_METHOD_CALL &&
context instanceof PsiMethodCallExpression) {
PsiExpression qualifier =
PsiUtil.skipParenthesizedExprDown(((PsiMethodCallExpression)context).getMethodExpression().getQualifierExpression());
if (qualifier != null) {
dfr.add(qualifier, (DfaMemoryStateImpl)memState, arguments.myQualifier);
}
}
return arguments;
}
@Override
public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState);
PsiExpression context = ObjectUtils.tryCast(instruction.getContext(), PsiExpression.class);
if (context != null) {
if (context != null && ExpressionUtils.getCallForQualifier(context) == null) {
for (DfaInstructionState state : states) {
DfaValue value = state.getMemoryState().peek();
if(value != fail) {
dfr.add(context, (DfaMemoryStateImpl)state.getMemoryState());
dfr.add(context, (DfaMemoryStateImpl)state.getMemoryState(), state.getMemoryState().peek());
}
}
}
@@ -76,9 +123,13 @@ public class CommonDataflow {
return result == RunnerResult.OK ? dfr : null;
}
private static DataflowResult getDataflowResult(PsiElement context) {
// Disable common dataflow in powersave mode
if(PowerSaveMode.isEnabled()) return null;
/**
* Returns the dataflow result for code fragment which contains given context
* @param context a context to get the dataflow result
* @return the dataflow result or null if dataflow cannot be launched for this context (e.g. we are inside too complex method)
*/
@Nullable
public static DataflowResult getDataflowResult(PsiExpression context) {
PsiMember member = PsiTreeUtil.getParentOfType(context, PsiMember.class);
if(!(member instanceof PsiMethod) && !(member instanceof PsiField) && !(member instanceof PsiClassInitializer)) return null;
PsiElement body = member instanceof PsiMethod ? ((PsiMethod)member).getBody() : member.getContainingClass();
@@ -100,7 +151,6 @@ public class CommonDataflow {
public static <T> T getExpressionFact(PsiExpression expression, DfaFactType<T> type) {
DataflowResult result = getDataflowResult(expression);
if (result == null) return null;
DfaFactMap map = result.myFacts.get(expression);
return map == null ? null : map.get(type);
return result.getExpressionFact(expression, type);
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java
-34
View File
@@ -5,7 +5,6 @@ package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInsight.ExpressionUtil;
import com.intellij.codeInsight.NullableNotNullManager;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInsight.daemon.GroupNames;
import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
import com.intellij.codeInspection.*;
@@ -54,7 +53,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
public boolean REPORT_CONSTANT_REFERENCE_VALUES = true;
public boolean REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER = true;
public boolean REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL = true;
public boolean REPORT_UNCHECKED_OPTIONALS = true;
@Override
public JComponent createOptionsPanel() {
@@ -80,9 +78,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
if (!REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL) {
node.addContent(new Element("option").setAttribute("name", "REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL").setAttribute("value", "false"));
}
if (!REPORT_UNCHECKED_OPTIONALS) {
node.addContent(new Element("option").setAttribute("name", "REPORT_UNCHECKED_OPTIONALS").setAttribute("value", "false"));
}
}
@Override
@@ -262,8 +257,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
reportOptionalOfNullableImprovements(holder, reportedAnchors, visitor.getOfNullableCalls());
reportUncheckedOptionalGet(holder, visitor.getOptionalCalls(), visitor.getOptionalQualifiers());
visitor.getBooleanCalls().forEach((call, state) -> {
if (state != ThreeState.UNSURE && reportedAnchors.add(call)) {
reportConstantCondition(holder, call, state.toBoolean());
@@ -400,33 +393,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
});
}
private void reportUncheckedOptionalGet(ProblemsHolder holder,
Map<PsiMethodCallExpression, ThreeState> calls,
List<PsiExpression> qualifiers) {
if (!REPORT_UNCHECKED_OPTIONALS) return;
for (Map.Entry<PsiMethodCallExpression, ThreeState> entry : calls.entrySet()) {
ThreeState state = entry.getValue();
if (state != ThreeState.UNSURE) continue;
PsiMethodCallExpression call = entry.getKey();
PsiMethod method = call.resolveMethod();
if (method == null) continue;
PsiClass optionalClass = method.getContainingClass();
if (optionalClass == null) continue;
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression());
if (qualifier instanceof PsiMethodCallExpression &&
qualifiers.stream().anyMatch(q -> PsiEquivalenceUtil.areElementsEquivalent(q, qualifier))) {
// Conservatively do not report methodCall().get() cases if methodCall().isPresent() was found in the same method
// without deep correspondence analysis
continue;
}
LocalQuickFix fix = holder.isOnTheFly() ? new SetInspectionOptionFix(this, "REPORT_UNCHECKED_OPTIONALS", InspectionsBundle
.message("inspection.data.flow.turn.off.unchecked.optional.get.quickfix"), false) : null;
holder.registerProblem(getElementToHighlight(call),
InspectionsBundle.message("dataflow.message.optional.get.without.is.present", optionalClass.getName()),
fix);
}
}
private void reportAlwaysReturnsNotNull(ProblemsHolder holder, PsiElement scope) {
if (!(scope.getParent() instanceof PsiMethod)) return;
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java
-27
View File
@@ -5,12 +5,10 @@ import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.ThreeState;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.siyeh.ig.psiutils.TypeUtils;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -23,13 +21,11 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
private final Map<NullabilityProblemKind.NullabilityProblem<?>, StateInfo> myStateInfos = new LinkedHashMap<>();
private final Set<Instruction> myCCEInstructions = ContainerUtil.newHashSet();
private final Map<MethodCallInstruction, Boolean> myFailingCalls = new HashMap<>();
private final Map<PsiMethodCallExpression, ThreeState> myOptionalCalls = new HashMap<>();
private final Map<PsiMethodCallExpression, ThreeState> myBooleanCalls = new HashMap<>();
private final Map<MethodCallInstruction, ThreeState> myOfNullableCalls = new HashMap<>();
private final Map<PsiAssignmentExpression, Pair<PsiType, PsiType>> myArrayStoreProblems = new HashMap<>();
private final Map<PsiMethodReferenceExpression, DfaValue> myMethodReferenceResults = new HashMap<>();
private final Map<PsiArrayAccessExpression, ThreeState> myOutOfBoundsArrayAccesses = new HashMap<>();
private final List<PsiExpression> myOptionalQualifiers = new ArrayList<>();
private final MultiMap<PushInstruction, Object> myPossibleVariableValues = MultiMap.createSet();
private final Set<PsiElement> myReceiverMutabilityViolation = new HashSet<>();
private final Set<PsiElement> myArgumentMutabilityViolation = new HashSet<>();
@@ -87,10 +83,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
return myArrayStoreProblems;
}
Map<PsiMethodCallExpression, ThreeState> getOptionalCalls() {
return myOptionalCalls;
}
Map<MethodCallInstruction, ThreeState> getOfNullableCalls() {
return myOfNullableCalls;
}
@@ -115,10 +107,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals);
}
List<PsiExpression> getOptionalQualifiers() {
return myOptionalQualifiers;
}
Map<PsiCall, List<MethodContract>> getAlwaysFailingCalls() {
return StreamEx.ofKeys(myFailingCalls, v -> v)
.mapToEntry(MethodCallInstruction::getCallExpression, MethodCallInstruction::getContracts).toMap();
@@ -133,21 +121,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class);
if (call != null) {
String methodName = call.getMethodExpression().getReferenceName();
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression());
if (qualifier != null && TypeUtils.isOptional(qualifier.getType())) {
if ("isPresent".equals(methodName) && qualifier instanceof PsiMethodCallExpression) {
myOptionalQualifiers.add(qualifier);
}
else if (DfaOptionalSupport.isOptionalGetMethodName(methodName)) {
Boolean fact = memState.getValueFact(memState.peek(), DfaFactType.OPTIONAL_PRESENCE);
ThreeState state = fact == null ? ThreeState.UNSURE : ThreeState.fromBoolean(fact);
myOptionalCalls.merge(call, state, ThreeState::merge);
}
}
}
if (instruction.matches(DfaOptionalSupport.OPTIONAL_OF_NULLABLE)) {
DfaValue arg = memState.peek();
ThreeState nullArg = memState.isNull(arg) ? ThreeState.YES : memState.isNotNull(arg) ? ThreeState.NO : ThreeState.UNSURE;
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaOptionalSupport.java
+1 -1
View File
@@ -81,7 +81,7 @@ public class DfaOptionalSupport {
return new ReplaceOptionalCallFix("of", false);
}
static boolean isOptionalGetMethodName(String name) {
public static boolean isOptionalGetMethodName(String name) {
return "get".equals(name) || "getAsDouble".equals(name) || "getAsInt".equals(name) || "getAsLong".equals(name);
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
+4 -4
View File
@@ -337,10 +337,10 @@ public class StandardInstructionVisitor extends InstructionVisitor {
}
@NotNull
private DfaCallArguments popCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState,
boolean contractOnly) {
protected DfaCallArguments popCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState,
boolean contractOnly) {
PsiMethod method = instruction.getTargetMethod();
MutationSignature sig = MutationSignature.fromMethod(method);
DfaValue[] argValues = popCallArguments(instruction, runner, memState, contractOnly, sig);
java/java-impl/src/META-INF/JavaPlugin.xml
+5
View File
@@ -531,6 +531,11 @@
groupKey="group.names.code.style.issues" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.OptionalIsPresentInspection"
displayName="Replace Optional.isPresent() checks with functional-style expressions"/>
<localInspection groupPath="Java" language="JAVA" shortName="OptionalGetWithoutIsPresent"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.java18api.OptionalGetWithoutIsPresentInspection"
displayName="Optional.get() is called without isPresent() check"/>
<localInspection groupPath="Java" language="JAVA" shortName="RedundantExplicitClose"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.declaration.redundancy" enabledByDefault="true" level="WARNING"
java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java
-8
View File
@@ -177,7 +177,6 @@ public class DataFlowInspection extends DataFlowInspectionBase {
private final JCheckBox myTreatUnknownMembersAsNullable;
private final JCheckBox myReportNullArguments;
private final JCheckBox myReportNullableMethodsReturningNotNull;
private final JCheckBox myReportUncheckedOptionals;
private OptionsPanel() {
super(new GridBagLayout());
@@ -216,10 +215,6 @@ public class DataFlowInspection extends DataFlowInspectionBase {
"Report nullable methods that always return a non-null value",
REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL, box -> REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL = box.isSelected());
myReportUncheckedOptionals = createCheckBoxWithHTML(
"Report Optional.get() calls without previous isPresent check",
REPORT_UNCHECKED_OPTIONALS, box -> REPORT_UNCHECKED_OPTIONALS = box.isSelected());
gc.insets = JBUI.emptyInsets();
gc.gridy = 0;
add(mySuggestNullables, gc);
@@ -251,9 +246,6 @@ public class DataFlowInspection extends DataFlowInspectionBase {
gc.gridy++;
add(myReportNullableMethodsReturningNotNull, gc);
gc.gridy++;
add(myReportUncheckedOptionals, gc);
}
@Override
java/java-impl/src/com/intellij/codeInspection/java18api/OptionalGetWithoutIsPresentInspection.java
+63
View File
@@ -0,0 +1,63 @@
// Copyright 2000-2018 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.java18api;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool;
import com.intellij.codeInspection.InspectionsBundle;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.codeInspection.dataFlow.CommonDataflow;
import com.intellij.codeInspection.dataFlow.DfaFactType;
import com.intellij.codeInspection.dataFlow.DfaOptionalSupport;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.NotNull;
public class OptionalGetWithoutIsPresentInspection extends AbstractBaseJavaLocalInspectionTool {
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
return new JavaElementVisitor() {
@Override
public void visitMethodCallExpression(PsiMethodCallExpression call) {
PsiElement nameElement = call.getMethodExpression().getReferenceNameElement();
if (nameElement == null) return;
String methodName = nameElement.getText();
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression());
if (qualifier == null) return;
PsiClass optionalClass = PsiUtil.resolveClassInClassTypeOnly(qualifier.getType());
if (optionalClass == null) return;
if (DfaOptionalSupport.isOptionalGetMethodName(methodName) &&
call.getArgumentList().isEmpty() &&
TypeUtils.isOptional(optionalClass)) {
CommonDataflow.DataflowResult result = CommonDataflow.getDataflowResult(qualifier);
if (result != null &&
result.expressionWasAnalyzed(qualifier) &&
result.getExpressionFact(qualifier, DfaFactType.OPTIONAL_PRESENCE) == null &&
!isPresentCallWithSameQualifierExists(qualifier)) {
holder.registerProblem(nameElement,
InspectionsBundle.message("inspection.optional.get.without.is.present.message", optionalClass.getName()));
}
}
}
public boolean isPresentCallWithSameQualifierExists(PsiExpression qualifier) {
// Conservatively skip the results of method calls if there's an isPresent() call with the same qualifier in the method
if (qualifier instanceof PsiMethodCallExpression) {
PsiElement context = PsiTreeUtil.getParentOfType(qualifier, PsiMember.class, PsiLambdaExpression.class);
if (context != null) {
return !PsiTreeUtil.processElements(context, e -> {
if (e == qualifier || !(e instanceof PsiMethodCallExpression)) return true;
PsiMethodCallExpression call = (PsiMethodCallExpression)e;
if (!"isPresent".equals(call.getMethodExpression().getReferenceName()) || !call.getArgumentList().isEmpty()) return true;
PsiExpression isPresentQualifier = call.getMethodExpression().getQualifierExpression();
return isPresentQualifier == null || !PsiEquivalenceUtil.areElementsEquivalent(qualifier, isPresentQualifier);
});
}
}
return false;
}
};
}
}
java/java-impl/src/inspectionDescriptions/OptionalGetWithoutIsPresent.html
+6
View File
@@ -0,0 +1,6 @@
<html>
<body>
<p>Reports when <b>Optional.get()</b> method is called without previous checking that optional is definitely not empty.</p>
<!-- tooltip end -->
</body>
</html>
java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java
+132
View File
@@ -16,4 +16,136 @@ class Test {
System.out.println(test.get());
}
}
void m(Optional<Integer> maybe) {
if (!!!maybe.isPresent()) {
maybe = getIntegerOptional();
}
else {
System.out.println(maybe.get());
maybe = getIntegerOptional();
}
if (maybe.isPresent()) {
maybe = Optional.empty();
System.out.println(maybe.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
boolean b = <warning descr="Condition '((maybe.isPresent()))' is always 'false'">((<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>))</warning> && maybe.get() == 1;
boolean c = <warning descr="Condition '(!maybe.isPresent())' is always 'true'">(!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>)</warning> || maybe.get() == 1;
Integer value = <warning descr="Condition '!maybe.isPresent()' is always 'true'">!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning></warning> ? 0 : maybe.get();
}
Optional<Integer> getIntegerOptional() {
return Math.random() > 0.5 ? Optional.of(1) : Optional.empty();
}
private static void a() {
Optional<String> optional = Optional.empty();
final boolean present = <warning descr="Condition 'optional.isPresent()' is always 'false'">optional.isPresent()</warning>;
// optional = Optional.empty();
if (<warning descr="Condition 'present' is always 'false'">present</warning>) {
final String string = optional.get();
System.out.println(string);
}
}
private static void b() {
Optional<String> optional = Optional.empty();
final boolean present = <warning descr="Condition 'optional.isPresent()' is always 'false'">optional.isPresent()</warning>;
optional = Optional.empty();
if (<warning descr="Condition 'present' is always 'false'">present</warning>) {
final String string = optional.get();
System.out.println(string);
}
}
public void testMultiVars(Optional<String> opt) {
boolean present = opt.isPresent();
boolean absent = !present;
boolean otherAbsent = !!absent;
if(otherAbsent) {
System.out.println(opt.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
} else {
System.out.println(opt.get());
}
}
public static Optional<String> getOptional() {
return Optional.empty();
}
private void checkAsserts1() {
Optional<String> o2 = getOptional();
org.junit.Assert.assertTrue(!o2.isPresent());
System.out.println(o2.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
private void checkAsserts2() {
Optional<String> o3 = Optional.empty();
org.testng.Assert.<warning descr="The call to 'assertTrue' always fails, according to its method contracts">assertTrue</warning>(<warning descr="Condition 'o3.isPresent()' is always 'false'">o3.isPresent()</warning>);
System.out.println(o3.get());
}
private void checkOf(boolean b) {
System.out.println(Optional.of("xyz").get());
Optional<String> test;
if(b) {
test = Optional.empty();
} else {
test = Optional.empty();
}
System.out.println(test.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
public static String demo() {
Optional<String> holder = Optional.empty();
if (<warning descr="Condition '! holder.isPresent()' is always 'true'">! <warning descr="Condition 'holder.isPresent()' is always 'false'">holder.isPresent()</warning></warning>) {
holder = Optional.of("hello world");
if (<warning descr="Condition '!holder.isPresent()' is always 'false'">!<warning descr="Condition 'holder.isPresent()' is always 'true'">holder.isPresent()</warning></warning>) {
return null;
}
}
return holder.get();
}
void order(Optional<String> order, boolean b) {
order.ifPresent(o -> System.out.println(order.get()));
System.out.println(order.orElseGet(() -> order.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>().trim()));
}
void guavaTest(com.google.common.base.Optional<String> opt, String s, String s1) {
System.out.println(opt.get());
if(<warning descr="Condition 'opt.isPresent()' is always 'true'">opt.isPresent()</warning>) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.fromNullable(s);
if(opt.isPresent()) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.of(<warning descr="Argument 's' might be null">s</warning>);
opt = com.google.common.base.Optional.of(s1);
if(<warning descr="Condition 'opt.isPresent()' is always 'true'">opt.isPresent()</warning>) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.absent();
if(<warning descr="Condition 'opt.isPresent()' is always 'false'">opt.isPresent()</warning>) {
System.out.println(opt.get());
}
}
void testThrow2(Optional<String> test) {
test.orElseThrow(RuntimeException::new);
if (<warning descr="Condition 'test.isPresent()' is always 'true'">test.isPresent()</warning>) {
System.out.println("Yes");
}
}
public void testThrowFail(Optional<String> arg) {
if(!arg.isPresent()) {
System.out.println(arg.<warning descr="The call to 'orElseThrow' always fails, according to its method contracts">orElseThrow</warning>(IllegalAccessError::new));
}
String res = Optional.<String>empty().<warning descr="The call to 'orElseThrow' always fails, according to its method contracts">orElseThrow</warning>(RuntimeException::new);
}
}
java/java-tests/testData/inspection/dataFlow/fixture/OptionalGetWithoutIsPresent.java → java/java-tests/testData/inspection/optionalGet/OptionalGet.java
+91 -124
View File
@@ -1,21 +1,30 @@
/*
* 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import java.util.*;
import java.util.stream.Stream;
class OptionalWithoutIsPresent {
class OptionalGet {
private void checkOf(boolean b) {
System.out.println(Optional.of("xyz").get());
Optional<String> test;
if(b) {
test = Optional.of("x");
} else {
test = Optional.of("y");
}
System.out.println(test.get());
if(b) {
test = Optional.of("x");
} else {
test = Optional.empty();
}
System.out.println(test.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
private void checkOfNullable(String value) {
System.out.println(Optional.ofNullable(value).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println(Optional.ofNullable(value+"a").get());
System.out.println(Optional.ofNullable("xyz").get());
}
void testSimple(Optional<String> o, OptionalDouble od, OptionalInt oi, OptionalLong ol) {
System.out.println(o.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
@@ -24,6 +33,27 @@ class OptionalWithoutIsPresent {
System.out.println(od.<warning descr="'OptionalDouble.getAsDouble()' without 'isPresent()' check">getAsDouble</warning>());
}
void testParentheses(Optional<String> o, String value, String value2) {
System.out.println((o).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println((Optional.ofNullable(value)).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println((Optional.of(value)).get());
System.out.println((Optional.ofNullable("foo")).get());
System.out.println((Optional.of("foo")).get());
}
void testTernary(Optional<String> foo, Optional<String> bar, boolean b) {
if(bar.isPresent()) {
if(foo.isPresent()) {
System.out.println((b ? foo : bar).get());
}
System.out.println((b ? foo : bar).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
if(foo.isPresent()) {
System.out.println((b ? foo : bar).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
System.out.println((b ? foo : bar).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
{
System.out.println(getIntegerOptional().<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
@@ -72,32 +102,16 @@ class OptionalWithoutIsPresent {
}
}
void m(Optional<Integer> maybe) {
if (!!!maybe.isPresent()) {
maybe = getIntegerOptional();
}
else {
System.out.println(maybe.get());
maybe = getIntegerOptional();
}
if (maybe.isPresent()) {
maybe = Optional.empty();
System.out.println(maybe.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
boolean b = <warning descr="Condition '((maybe.isPresent()))' is always 'false'">((<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>))</warning> && maybe.get() == 1;
boolean c = <warning descr="Condition '(!maybe.isPresent())' is always 'true'">(!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>)</warning> || maybe.get() == 1;
Integer value = <warning descr="Condition '!maybe.isPresent()' is always 'true'">!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning></warning> ? 0 : maybe.get();
}
Optional<Integer> getIntegerOptional() {
return Math.random() > 0.5 ? Optional.of(1) : Optional.empty();
}
private static void a() {
Optional<String> optional = Optional.empty();
final boolean present = <warning descr="Condition 'optional.isPresent()' is always 'false'">optional.isPresent()</warning>;
final boolean present = optional.isPresent();
// optional = Optional.empty();
if (<warning descr="Condition 'present' is always 'false'">present</warning>) {
if (present) {
// do not warn here as the branch is unreachable
final String string = optional.get();
System.out.println(string);
}
@@ -105,25 +119,15 @@ class OptionalWithoutIsPresent {
private static void b() {
Optional<String> optional = Optional.empty();
final boolean present = <warning descr="Condition 'optional.isPresent()' is always 'false'">optional.isPresent()</warning>;
final boolean present = optional.isPresent();
optional = Optional.empty();
if (<warning descr="Condition 'present' is always 'false'">present</warning>) {
if (present) {
// do not warn here as the branch is unreachable
final String string = optional.get();
System.out.println(string);
}
}
public void testMultiVars(Optional<String> opt) {
boolean present = opt.isPresent();
boolean absent = !present;
boolean otherAbsent = !!absent;
if(otherAbsent) {
System.out.println(opt.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
} else {
System.out.println(opt.get());
}
}
private void checkReassign(Optional<String> a, Optional<String> b) {
if(a.isPresent()) {
b = a;
@@ -148,66 +152,13 @@ class OptionalWithoutIsPresent {
Optional<String> o3 = getOptional();
org.testng.Assert.assertTrue(o3.isPresent());
System.out.println(o3.get());
o2 = getOptional();
org.junit.Assert.assertTrue(!o2.isPresent());
System.out.println(o2.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
private void checkAsserts2() {
Optional<String> o3 = Optional.empty();
org.testng.Assert.<warning descr="The call to 'assertTrue' always fails, according to its method contracts">assertTrue</warning>(<warning descr="Condition 'o3.isPresent()' is always 'false'">o3.isPresent()</warning>);
System.out.println(o3.get());
public Collection<String> findCategories(Long articleId) {
return java.util.stream.Stream.of(Optional.of("asdf")).filter(Optional::isPresent).map(x -> x.get()).collect(
java.util.stream.Collectors.toList()) ;
}
private void checkOf(boolean b) {
System.out.println(Optional.of("xyz").get());
Optional<String> test;
if(b) {
test = Optional.of("x");
} else {
test = Optional.of("y");
}
System.out.println(test.get());
if(b) {
test = Optional.of("x");
} else {
test = Optional.empty();
}
System.out.println(test.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
if(<warning descr="Condition 'b' is always 'true'">b</warning>) {
test = Optional.empty();
} else {
test = Optional.empty();
}
System.out.println(test.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
private void checkOfNullable(String value) {
System.out.println(Optional.ofNullable(value).<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println(Optional.ofNullable(<warning descr="Passing a non-null argument to 'Optional'">value+"a"</warning>).get());
System.out.println(Optional.ofNullable(<warning descr="Passing a non-null argument to 'Optional'">"xyz"</warning>).get());
}
public static String demo() {
Optional<String> holder = Optional.empty();
if (<warning descr="Condition '! holder.isPresent()' is always 'true'">! <warning descr="Condition 'holder.isPresent()' is always 'false'">holder.isPresent()</warning></warning>) {
holder = Optional.of("hello world");
if (<warning descr="Condition '!holder.isPresent()' is always 'false'">!<warning descr="Condition 'holder.isPresent()' is always 'true'">holder.isPresent()</warning></warning>) {
return null;
}
}
return holder.get();
}
//public Collection<String> findCategories(Long articleId) {
// return java.util.stream.Stream.of(Optional.of("asdf")).filter(Optional::isPresent).map(x -> x.get()).collect(
// java.util.stream.Collectors.toList()) ;
//}
public static void main(String[] args) {
Optional<String> stringOpt;
@@ -227,8 +178,10 @@ class OptionalWithoutIsPresent {
}
void order(Optional<String> order, boolean b) {
// here order.get always suceeds
order.ifPresent(o -> System.out.println(order.get()));
System.out.println(order.orElseGet(() -> order.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>().trim()));
// here order.get always fails: tested in normal DFA
System.out.println(order.orElseGet(() -> order.get().trim()));
}
public static void two(Optional<Object> o1,Optional<Object> o2) {
@@ -299,7 +252,7 @@ class OptionalWithoutIsPresent {
}
void shortIf(Optional<String> o) {
if (<warning descr="Condition 'true || o.isPresent()' is always 'true'">true || o.isPresent()</warning>) {
if (true || o.isPresent()) {
o.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>();
}
}
@@ -326,20 +279,20 @@ class OptionalWithoutIsPresent {
void guavaTest(com.google.common.base.Optional<String> opt, String s, String s1) {
System.out.println(opt.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
if(<warning descr="Condition 'opt.isPresent()' is always 'true'">opt.isPresent()</warning>) {
if(opt.isPresent()) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.fromNullable(s);
if(opt.isPresent()) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.of(<warning descr="Argument 's' might be null">s</warning>);
opt = com.google.common.base.Optional.of(s);
opt = com.google.common.base.Optional.of(s1);
if(<warning descr="Condition 'opt.isPresent()' is always 'true'">opt.isPresent()</warning>) {
if(opt.isPresent()) {
System.out.println(opt.get());
}
opt = com.google.common.base.Optional.absent();
if(<warning descr="Condition 'opt.isPresent()' is always 'false'">opt.isPresent()</warning>) {
if(opt.isPresent()) {
System.out.println(opt.get());
}
}
@@ -350,13 +303,6 @@ class OptionalWithoutIsPresent {
System.out.println(o);
}
void testThrow2(Optional<String> test) {
test.orElseThrow(RuntimeException::new);
if (<warning descr="Condition 'test.isPresent()' is always 'true'">test.isPresent()</warning>) {
System.out.println("Yes");
}
}
void testThrowCatch(Optional<String> opt) {
try {
opt.orElseThrow(RuntimeException::new);
@@ -366,13 +312,6 @@ class OptionalWithoutIsPresent {
}
}
public void testThrowFail(Optional<String> arg) {
if(!arg.isPresent()) {
System.out.println(arg.<warning descr="The call to 'orElseThrow' always fails, according to its method contracts">orElseThrow</warning>(IllegalAccessError::new));
}
String res = Optional.<String>empty().<warning descr="The call to 'orElseThrow' always fails, according to its method contracts">orElseThrow</warning>(RuntimeException::new);
}
void testOrElseGet() {
final Optional<String> a = Optional.ofNullable(Math.random() > 0.5 ? null:"");
final Optional<String> b = Optional.ofNullable(Math.random() > 0.5 ? null:"");
@@ -383,4 +322,32 @@ class OptionalWithoutIsPresent {
String result = a.orElseGet(() -> b.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println(result);
}
boolean testBooleanOptional(Optional<Boolean> opt) {
if (opt.isPresent() && !opt.get()) {
return false;
}
return true;
}
void testArrayStream(int[] arr1, int[] arr2) {
if(arr1.length == 0) return;
System.out.println(Arrays.stream(arr1).map(Math::abs).min().getAsInt());
System.out.println(Arrays.stream(arr2).map(Math::abs).min().<warning descr="'OptionalInt.getAsInt()' without 'isPresent()' check">getAsInt</warning>());
}
public String getFirstItem(Collection<String> data) {
return data.stream().findFirst().<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>();
}
public String getFirstItemChecked(Collection<String> data) {
if(data.isEmpty()) throw new IllegalArgumentException("Data should never be empty");
// Non-empty stream: get() is fine
return data.stream().findFirst().get();
}
public String getMax() {
// Non-empty stream: get() is fine
return Stream.of("foo", "bar", "baz").map(String::toUpperCase).max(Comparator.naturalOrder()).get();
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java
+1 -2
View File
@@ -64,8 +64,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
public void testOptionalOfNullable() { doTest(); }
public void testOptionalOrElse() { doTest(); }
public void testOptionalIsPresent() { doTest(); }
public void testOptionalGetWithoutIsPresent() {
public void testOptionalIsPresent() {
myFixture.addClass("package org.junit;" +
"public class Assert {" +
" public static void assertTrue(boolean b) {}" +
java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionTest.java
+62
View File
@@ -0,0 +1,62 @@
// Copyright 2000-2018 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.java.codeInspection;
import com.intellij.JavaTestUtil;
import com.intellij.codeInspection.defUse.DefUseInspection;
import com.intellij.codeInspection.java18api.OptionalGetWithoutIsPresentInspection;
import com.intellij.testFramework.LightProjectDescriptor;
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
import org.jetbrains.annotations.NotNull;
public class OptionalGetWithoutIsPresentInspectionTest extends LightCodeInsightFixtureTestCase {
@Override
protected String getBasePath() {
return JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/optionalGet";
}
public void testOptionalGet() { doTest(); }
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return JAVA_9;
}
private void doTest() {
mockClasses();
myFixture.enableInspections(new OptionalGetWithoutIsPresentInspection());
myFixture.testHighlighting(getTestName(false) + ".java");
}
private void mockClasses() {
myFixture.addClass("package org.junit;" +
"public class Assert {" +
" public static void assertTrue(boolean b) {}" +
"}");
myFixture.addClass("package org.testng;" +
"public class Assert {" +
" public static void assertTrue(boolean b) {}" +
"}");
myFixture.addClass("package com.google.common.base;\n" +
"\n" +
"public interface Supplier<T> { T get();}\n");
myFixture.addClass("package com.google.common.base;\n" +
"\n" +
"public interface Function<F, T> { T apply(F input);}\n");
myFixture.addClass("package com.google.common.base;\n" +
"\n" +
"public abstract class Optional<T> {\n" +
" public static <T> Optional<T> absent() {}\n" +
" public static <T> Optional<T> of(T ref) {}\n" +
" public static <T> Optional<T> fromNullable(T ref) {}\n" +
" public abstract T get();\n" +
" public abstract boolean isPresent();\n" +
" public abstract T orNull();\n" +
" public abstract T or(Supplier<? extends T> supplier);\n" +
" public abstract <V> Optional<V> transform(Function<? super T, V> fn);\n" +
" public abstract T or(T val);\n" +
" public abstract java.util.Optional<T> toJavaUtil();\n" +
"}");
}
}
platform/platform-resources-en/src/messages/InspectionsBundle.properties
+2 -2
View File
@@ -47,7 +47,6 @@ inspection.data.flow.true.asserts.option=Don't report assertions with condition
inspection.data.flow.turn.off.true.asserts.quickfix=Don't report always true assertions
inspection.data.flow.turn.off.constant.references.quickfix=Don't report values which are guaranteed to be constant
inspection.data.flow.turn.off.nullable.returning.notnull.quickfix=Don't report nullable methods which always return not-null value
inspection.data.flow.turn.off.unchecked.optional.get.quickfix=Don't report Optional.get() calls without previous isPresent check
inspection.data.flow.redundant.instanceof.quickfix=Replace with a null check
inspection.data.flow.simplify.boolean.expression.quickfix=Simplify boolean expression
inspection.data.flow.simplify.to.assignment.quickfix.name=Simplify to normal assignment
@@ -89,13 +88,14 @@ dataflow.message.unboxing.method.reference=Use of <code>#ref</code> #loc would n
dataflow.too.complex=Method <code>#ref</code> is too complex to analyze by data flow algorithm
dataflow.too.complex.class=Class initializer is too complex to analyze by data flow algorithm
dataflow.method.fails.with.null.argument=Method will throw an exception when parameter is null
dataflow.message.optional.get.without.is.present=<code>{0}.#ref()</code> without ''isPresent()'' check
dataflow.message.constant.method.reference=Method reference result is always ''{0}''
dataflow.message.array.index.out.of.bounds=Array index is out of bounds
dataflow.message.immutable.modified=Immutable object is modified
dataflow.message.immutable.passed=Immutable object is passed where mutable is expected
dataflow.message.redundant.assignment=Variable is already assigned to this value
inspection.optional.get.without.is.present.message=<code>{0}.#ref()</code> without ''isPresent()'' check
#deprecated
inspection.deprecated.display.name=Deprecated API usage
inspection.marked.for.removal.display.name=Usage of API marked for removal