From 4935b387daf642ba2dadc5ab6fcb6d4ed796b3e1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 18 Dec 2019 10:47:12 +0700 Subject: [PATCH] MismatchedCollectionQueryUpdate: track queries through impure function arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes IDEA-229050 “Mismatched query and update of collection” misses removeIf as a query method. GitOrigin-RevId: f08e28c1c82f9aa8438df0f3e5bcb880cd9a002e --- .../siyeh/ig/psiutils/SideEffectChecker.java | 4 +- ...atchedCollectionQueryUpdateInspection.java | 44 ++++++++++++++----- .../MismatchedCollectionQueryUpdate.java | 28 ++++++++++++ 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java index 12cbcc22936d..eed444ef7d5f 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java @@ -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); } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java index 9d2b88aea5c8..eaefdadced98 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java @@ -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(); diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java index a4c9b733d710..29f5fa43d76f 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java @@ -604,4 +604,32 @@ class VarArgTest { List list6 = new MyList(data); for(String s : list6) System.out.println(s); } +} +class Idea229050 { + private final Collection collection = new ArrayList<>(); + private final Collection collection2 = new ArrayList<>(); + private final Collection collection3 = 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); + } } \ No newline at end of file