MismatchedCollectionQueryUpdate: track queries through impure function arguments

Fixes IDEA-229050 “Mismatched query and update of collection” misses removeIf as a query method.

GitOrigin-RevId: f08e28c1c82f9aa8438df0f3e5bcb880cd9a002e
This commit is contained in:
Tagir Valeev
2019-12-18 04:01:25 +00:00
committed by intellij-monorepo-bot
parent 7d92e68b21
commit 4935b387da
3 changed files with 65 additions and 11 deletions
@@ -226,7 +226,9 @@ public class SideEffectChecker {
@Override
public void visitReturnStatement(PsiReturnStatement statement) {
if (addSideEffect(statement)) return;
if (!(myStartElement.getParent() instanceof PsiParameterListOwner)) {
if (addSideEffect(statement)) return;
}
super.visitReturnStatement(statement);
}
@@ -16,6 +16,7 @@
package com.siyeh.ig.bugs;
import com.intellij.codeInsight.daemon.impl.UnusedSymbolUtil;
import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.codeInspection.dataFlow.Mutability;
import com.intellij.codeInspection.ui.ListTable;
import com.intellij.codeInspection.ui.ListWrappingTableModel;
@@ -32,10 +33,7 @@ import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.CollectionUtils;
import com.siyeh.ig.psiutils.ConstructionUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.SideEffectChecker;
import com.siyeh.ig.psiutils.*;
import com.siyeh.ig.ui.ExternalizableStringSet;
import com.siyeh.ig.ui.UiUtils;
import one.util.streamex.StreamEx;
@@ -48,6 +46,7 @@ import java.awt.*;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static com.siyeh.ig.psiutils.ClassUtils.isImmutable;
@@ -78,7 +77,7 @@ public class MismatchedCollectionQueryUpdateInspection
public final ExternalizableStringSet queryNames =
new ExternalizableStringSet(
"contains", "copyInto", "equals", "forEach", "get", "hashCode", "iterator", "parallelStream", "propertyNames",
"replaceAll", "save", "size", "store", "stream", "toArray", "toString", "write");
"save", "size", "store", "stream", "toArray", "toString", "write");
@SuppressWarnings("PublicField")
public final ExternalizableStringSet updateNames =
new ExternalizableStringSet("add", "clear", "insert", "load", "merge", "offer", "poll", "pop", "push", "put", "remove", "replace",
@@ -255,12 +254,12 @@ public class MismatchedCollectionQueryUpdateInspection
makeUpdated();
}
final PsiMethod method = ObjectUtils.tryCast(expression.resolve(), PsiMethod.class);
if (method == null ||
PsiType.VOID.equals(method.getReturnType()) ||
PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(expression))) {
return;
if (method != null &&
(!PsiType.VOID.equals(method.getReturnType()) &&
!PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(expression)) ||
Stream.of(method.getParameterList().getParameters()).anyMatch(p -> LambdaUtil.isFunctionalType(p.getType())))) {
makeQueried();
}
makeQueried();
}
private boolean processCollectionMethods(PsiMethodCallExpression call, PsiExpression arg) {
@@ -298,6 +297,17 @@ public class MismatchedCollectionQueryUpdateInspection
if (!voidContext) {
makeQueried();
}
else {
for (PsiExpression arg : call.getArgumentList().getExpressions()) {
PsiParameter parameter = MethodCallUtils.getParameterForArgument(arg);
if (parameter != null && LambdaUtil.isFunctionalType(parameter.getType())) {
if (ExpressionUtils.nonStructuralChildren(arg).anyMatch(e -> mayHaveSideEffect(e))) {
makeQueried();
break;
}
}
}
}
}
if (!queryQualifier && !updateQualifier) {
if (!isQueryMethod(call)) {
@@ -307,6 +317,20 @@ public class MismatchedCollectionQueryUpdateInspection
}
}
private boolean mayHaveSideEffect(PsiExpression fn) {
if (fn instanceof PsiLambdaExpression) {
PsiElement body = ((PsiLambdaExpression)fn).getBody();
if (body != null) {
return SideEffectChecker.mayHaveSideEffects(body, x -> false);
}
}
if (fn instanceof PsiMethodReferenceExpression) {
PsiElement target = ((PsiMethodReferenceExpression)fn).resolve();
return !(target instanceof PsiMethod) || !JavaMethodContractUtil.isPure((PsiMethod)target);
}
return true;
}
private PsiExpression findEffectiveReference(PsiExpression expression) {
while (true) {
PsiElement parent = expression.getParent();
@@ -604,4 +604,32 @@ class VarArgTest {
List<String> list6 = new MyList(data);
for(String s : list6) System.out.println(s);
}
}
class Idea229050 {
private final Collection<String> collection = new ArrayList<>();
private final Collection<String> <warning descr="Contents of collection 'collection2' are updated, but never queried">collection2</warning> = new ArrayList<>();
private final Collection<String> <warning descr="Contents of collection 'collection3' are updated, but never queried">collection3</warning> = new ArrayList<>();
public void add(String value) {
collection.add(value);
collection2.add(value);
collection3.add(value);
}
public void printAndRemoveLongStrings() {
collection.removeIf(s -> {
if (s.length() > 10) {
System.out.println(s);
return true;
} else {
return false;
}
});
collection2.removeIf(s -> {
if (s.length() > 10) {
return true;
} else {
return false;
}
});
collection3.removeIf(Objects::isNull);
}
}