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

Report always true/always false condition on boolean returning methods with contracts

Browse Source
This commit is contained in:
Tagir Valeev
2017-04-25 11:25:08 +07:00
parent b6c627382b
commit b0f41a8d62
4 changed files with 66 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java
+53 -9
View File
@@ -346,6 +346,13 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
reportUncheckedOptionalGet(holder, visitor.getOptionalCalls(), visitor.getOptionalQualifiers());
Map<PsiMethodCallExpression, ThreeState> calls = visitor.getBooleanCalls();
calls.forEach((call, state) -> {
if (state != ThreeState.UNSURE && reportedAnchors.add(call)) {
reportConstantCondition(holder, visitor, call, state.toBoolean());
}
});
if (REPORT_CONSTANT_REFERENCE_VALUES) {
reportConstantReferenceValues(holder, visitor, reportedAnchors);
}
@@ -462,7 +469,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
holder.registerProblem(expr, "Passing a non-null argument to <code>Optional</code>",
DfaOptionalSupport.createReplaceOptionalOfNullableWithOfFix());
}
}
}
}
@@ -700,12 +707,12 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
PsiType returnType = method.getReturnType();
// no warnings in void lambdas, where the expression is not returned anyway
if (block instanceof PsiExpression && block.getParent() instanceof PsiLambdaExpression && PsiType.VOID.equals(returnType)) return;
// no warnings for Void methods, where only null can be possibly returned
if (returnType == null || returnType.equalsToText(CommonClassNames.JAVA_LANG_VOID)) return;
for (PsiElement statement : visitor.getProblems(NullabilityProblem.nullableReturn)) {
assert statement instanceof PsiExpression;
assert statement instanceof PsiExpression;
final PsiExpression expr = (PsiExpression)statement;
if (!reportedAnchors.add(expr)) continue;
@@ -962,6 +969,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
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 List<PsiExpression> myOptionalQualifiers = new ArrayList<>();
private boolean myAlwaysReturnsNotNull = true;
@@ -969,7 +977,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
protected void onInstructionProducesCCE(TypeCastInstruction instruction) {
myCCEInstructions.add(instruction);
}
Collection<PsiElement> getProblems(final NullabilityProblem kind) {
return ContainerUtil.filter(myProblems.get(kind), psiElement -> {
StateInfo info = myStateInfos.get(Pair.create(kind, psiElement));
@@ -984,6 +992,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
return myOptionalCalls;
}
Map<PsiMethodCallExpression, ThreeState> getBooleanCalls() {
return myBooleanCalls;
}
List<PsiExpression> getOptionalQualifiers() {
return myOptionalQualifiers;
}
@@ -1010,25 +1022,57 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
else if (DfaOptionalSupport.isOptionalGetMethodName(methodName)) {
ThreeState state = memState.checkOptional(memState.peek());
myOptionalCalls.merge(call, state, (s1, s2) -> s1 == s2 ? s1 : ThreeState.UNSURE);
myOptionalCalls.merge(call, state, ThreeState::merge);
}
}
}
DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState);
if (hasNonTrivialFailingContracts(instruction)) {
boolean allFail = Arrays.stream(states).allMatch(s -> s.getMemoryState().peek() == runner.getFactory().getConstFactory().getContractFail());
DfaConstValue fail = runner.getFactory().getConstFactory().getContractFail();
boolean allFail = Arrays.stream(states).allMatch(s -> s.getMemoryState().peek() == fail);
myFailingCalls.merge(instruction, allFail, Boolean::logicalAnd);
}
handleBooleanCalls(instruction, states);
return states;
}
void handleBooleanCalls(MethodCallInstruction instruction, DfaInstructionState[] states) {
if (!hasNonTrivialBooleanContracts(instruction)) return;
PsiMethod method = instruction.getTargetMethod();
if (method == null || !ControlFlowAnalyzer.isPure(method)) return;
PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class);
if (call == null || myBooleanCalls.get(call) == ThreeState.UNSURE) return;
PsiElement parent = call.getParent();
if (parent instanceof PsiExpressionStatement) return;
if (parent instanceof PsiLambdaExpression &&
PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType((PsiLambdaExpression)parent))) {
return;
}
for (DfaInstructionState s : states) {
DfaValue val = s.getMemoryState().peek();
ThreeState state = ThreeState.UNSURE;
if (val instanceof DfaConstValue) {
Object value = ((DfaConstValue)val).getValue();
if (value instanceof Boolean) {
state = ThreeState.fromBoolean((Boolean)value);
}
}
myBooleanCalls.merge(call, state, ThreeState::merge);
}
}
private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) {
List<MethodContract> contracts = instruction.getContracts();
return !contracts.isEmpty() && contracts.stream().anyMatch(DataFlowInstructionVisitor::isNonTrivialFailingContract);
return !contracts.isEmpty() && contracts.stream().anyMatch(
contract -> contract.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION && !contract.isTrivial());
}
private static boolean isNonTrivialFailingContract(MethodContract contract) {
return contract.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION && !contract.isTrivial();
private static boolean hasNonTrivialBooleanContracts(MethodCallInstruction instruction) {
List<MethodContract> contracts = instruction.getContracts();
return !contracts.isEmpty() && contracts.stream().anyMatch(
contract -> (contract.getReturnValue() == MethodContract.ValueConstraint.FALSE_VALUE ||
contract.getReturnValue() == MethodContract.ValueConstraint.TRUE_VALUE)
&& !contract.isTrivial());
}
@Override
java/java-tests/testData/inspection/dataFlow/fixture/OptionalGetWithoutIsPresent.java
+8 -8
View File
@@ -84,9 +84,9 @@ class OptionalWithoutIsPresent {
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'">((maybe.isPresent()))</warning> && maybe.get() == 1;
boolean c = <warning descr="Condition '(!maybe.isPresent())' is always 'true'">(!maybe.isPresent())</warning> || maybe.get() == 1;
Integer value = <warning descr="Condition '!maybe.isPresent()' is always 'true'">!maybe.isPresent()</warning> ? 0 : maybe.get();
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() {
@@ -95,7 +95,7 @@ class OptionalWithoutIsPresent {
private static void a() {
Optional<String> optional = Optional.empty();
final boolean present = optional.isPresent();
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();
@@ -105,7 +105,7 @@ class OptionalWithoutIsPresent {
private static void b() {
Optional<String> optional = Optional.empty();
final boolean present = optional.isPresent();
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();
@@ -156,7 +156,7 @@ class OptionalWithoutIsPresent {
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>(o3.isPresent());
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());
}
@@ -193,9 +193,9 @@ class OptionalWithoutIsPresent {
public static String demo() {
Optional<String> holder = Optional.empty();
if (<warning descr="Condition '! holder.isPresent()' is always 'true'">! holder.isPresent()</warning>) {
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'">!holder.isPresent()</warning>) {
if (<warning descr="Condition '!holder.isPresent()' is always 'false'">!<warning descr="Condition 'holder.isPresent()' is always 'true'">holder.isPresent()</warning></warning>) {
return null;
}
}
java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java
+1 -1
View File
@@ -7,7 +7,7 @@ class Test {
test = Optional.of("x");
} else {
test = Optional.empty();
if(<warning descr="Condition '!test.isPresent()' is always 'true'">!test.isPresent()</warning>) {
if(<warning descr="Condition '!test.isPresent()' is always 'true'">!<warning descr="Condition 'test.isPresent()' is always 'false'">test.isPresent()</warning></warning>) {
System.out.println("Always");
}
}
platform/util/src/com/intellij/util/ThreeState.java
+4
View File
@@ -26,6 +26,10 @@ public enum ThreeState {
return value ? YES : NO;
}
public ThreeState merge(ThreeState other) {
return this == other ? this : UNSURE;
}
public boolean toBoolean() {
if (this == UNSURE) {
throw new IllegalStateException("Must be or YES, or NO");