diff --git a/java/java-impl/src/com/intellij/codeInspection/CollectCollectorsToListUtil.java b/java/java-impl/src/com/intellij/codeInspection/CollectCollectorsToListUtil.java deleted file mode 100644 index 8b3b1f75b664..000000000000 --- a/java/java-impl/src/com/intellij/codeInspection/CollectCollectorsToListUtil.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * Copyright 2003-2021 Dave Griffith, Bas Leijdekkers - * - * 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. - */ -package com.intellij.codeInspection; - -import com.intellij.psi.*; -import com.intellij.psi.tree.IElementType; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; -import com.intellij.util.ArrayUtil; -import com.intellij.util.ObjectUtils; -import com.intellij.util.containers.ContainerUtil; -import com.siyeh.ig.bugs.MismatchedCollectionQueryUpdateInspection; -import com.siyeh.ig.callMatcher.CallMatcher; -import com.siyeh.ig.psiutils.ExpressionUtils; -import com.siyeh.ig.ui.ExternalizableStringSet; -import one.util.streamex.StreamEx; -import org.jetbrains.annotations.NonNls; - -import java.util.Arrays; -import java.util.Set; - -public class CollectCollectorsToListUtil { - - private static final CallMatcher COLLECTION_SAFE_ARGUMENT_METHODS = - CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "addAll", "removeAll", "containsAll", "remove"); - private static final @NonNls Set COLLECTIONS_QUERIES = - ContainerUtil.set("binarySearch", "disjoint", "indexOfSubList", "lastIndexOfSubList", "max", "min"); - private static final @NonNls Set COLLECTIONS_UPDATES = ContainerUtil.set("addAll", "fill", "copy", "replaceAll", "sort"); - private static final Set COLLECTIONS_ALL = StreamEx.of(COLLECTIONS_QUERIES).append(COLLECTIONS_UPDATES).toImmutableSet(); - private static final ExternalizableStringSet queryNames = - new ExternalizableStringSet( - "contains", "copyInto", "equals", "forEach", "get", "hashCode", "iterator", "parallelStream", "peek", "propertyNames", - "save", "size", "store", "stream", "toArray", "toString", "write"); - private static final ExternalizableStringSet updateNames = - new ExternalizableStringSet("add", "clear", "insert", "load", "merge", "offer", "poll", "pop", "push", "put", "remove", "replace", - "retain", "set", "take"); - - public static boolean isUnmodified(PsiMethodCallExpression methodCall) { - final PsiExpression effectiveReference = MismatchedCollectionQueryUpdateInspection.findEffectiveReference(methodCall); - if (!process(effectiveReference)) return true; - final PsiElement parent = effectiveReference.getParent(); - if (parent instanceof PsiLocalVariable || parent instanceof PsiField) { - final PsiElement context = - parent instanceof PsiLocalVariable ? PsiTreeUtil.getParentOfType(parent, PsiCodeBlock.class) : PsiUtil.getTopLevelClass(parent); - if (context == null) return false; - MismatchedCollectionQueryUpdateInspection.QueryUpdateInfo info = - MismatchedCollectionQueryUpdateInspection.getCollectionQueryUpdateInfo((PsiVariable)parent, context, queryNames, updateNames); - return !info.updated; - } - return false; - } - - private static boolean process(PsiExpression reference) { - PsiMethodCallExpression qualifiedCall = ExpressionUtils.getCallForQualifier(reference); - if (qualifiedCall != null) { - return isUpdateMethodName(qualifiedCall.getMethodExpression().getReferenceName()); - } - PsiElement parent = reference.getParent(); - if (parent instanceof PsiExpressionList) { - PsiExpressionList args = (PsiExpressionList)parent; - PsiCallExpression surroundingCall = ObjectUtils.tryCast(args.getParent(), PsiCallExpression.class); - if (surroundingCall != null) { - if (surroundingCall instanceof PsiMethodCallExpression) { - PsiMethodCallExpression call = (PsiMethodCallExpression)surroundingCall; - PsiExpressionList expressionList = call.getArgumentList(); - String name = call.getMethodExpression().getReferenceName(); - if (COLLECTIONS_ALL.contains(name) && MismatchedCollectionQueryUpdateInspection.isCollectionsClassMethod(call)) { - if (COLLECTIONS_QUERIES.contains(name) && !(call.getParent() instanceof PsiExpressionStatement)) { - return false; - } - if (COLLECTIONS_UPDATES.contains(name)) { - return ArrayUtil.indexOf(expressionList.getExpressions(), reference) == 0; - } - } - } - return !MismatchedCollectionQueryUpdateInspection.isQueryMethod(surroundingCall) && - !COLLECTION_SAFE_ARGUMENT_METHODS.matches(surroundingCall); - } - } - if (parent instanceof PsiMethodReferenceExpression) { - final String methodName = ((PsiMethodReferenceExpression)parent).getReferenceName(); - return (isUpdateMethodName(methodName)); - } - if (parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == reference) { - return false; - } - if (parent instanceof PsiPolyadicExpression) { - IElementType tokenType = ((PsiPolyadicExpression)parent).getOperationTokenType(); - if (Arrays.asList(JavaTokenType.PLUS, JavaTokenType.EQEQ, JavaTokenType.NE).contains(tokenType)) { - return false; - } - } - if (parent instanceof PsiAssertStatement && ((PsiAssertStatement)parent).getAssertDescription() == reference) return false; - if (parent instanceof PsiInstanceOfExpression || parent instanceof PsiSynchronizedStatement) return false; - return true; - } - - private static boolean isUpdateMethodName(String methodName) { - return MismatchedCollectionQueryUpdateInspection.isQueryUpdateMethodName(methodName, updateNames); - } -} diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index a29e90c7fa8e..ace7aa2d3f58 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -1,4 +1,4 @@ -// Copyright 2000-2020 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. +// Copyright 2000-2021 JetBrains s.r.o. and contributors. 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; import com.intellij.codeInsight.ExpressionUtil; @@ -29,6 +29,7 @@ import com.intellij.psi.util.*; import com.intellij.refactoring.util.LambdaRefactoringUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; +import com.siyeh.ig.bugs.MismatchedCollectionQueryUpdateInspection; import com.siyeh.ig.callMatcher.CallHandler; import com.siyeh.ig.callMatcher.CallMapper; import com.siyeh.ig.callMatcher.CallMatcher; @@ -185,11 +186,6 @@ public class SimplifyStreamApiCallChainsInspection extends AbstractBaseJavaLocal if(parameter instanceof PsiMethodCallExpression) { PsiMethodCallExpression collectorCall = (PsiMethodCallExpression)parameter; ReplaceCollectorFix fix = ReplaceCollectorFix.COLLECTOR_TO_FIX_MAPPER.mapFirst(collectorCall); - PsiMethodCallExpression qualifier = getQualifierMethodCall(methodCall); - if (fix == null && !COLLECTION_STREAM.test(qualifier) && CollectCollectorsToListUtil.isUnmodified(methodCall)) { - fix = CallHandler.of(collectorMatcher("toList", 0).withLanguageLevelAtLeast(LanguageLevel.JDK_16), - call -> new ReplaceCollectorFix("toList", "toList()", false)).apply(collectorCall); - } if (fix != null) { TextRange range = methodCall.getTextRange(); PsiElement nameElement = methodCall.getMethodExpression().getReferenceNameElement(); @@ -202,6 +198,7 @@ public class SimplifyStreamApiCallChainsInspection extends AbstractBaseJavaLocal if(!(PsiUtil.resolveClassInClassTypeOnly(methodCall.getType()) instanceof PsiTypeParameter)) { String replacement = SimplifyCollectionCreationFix.COLLECTOR_TO_CLASS_MAPPER.mapFirst(collectorCall); if (replacement != null) { + PsiMethodCallExpression qualifier = getQualifierMethodCall(methodCall); if (COLLECTION_STREAM.test(qualifier)) { PsiElement startElement = qualifier.getMethodExpression().getReferenceNameElement(); if (startElement != null) { @@ -552,7 +549,13 @@ public class SimplifyStreamApiCallChainsInspection extends AbstractBaseJavaLocal handler("summingLong", 1, "mapToLong({0}).sum()", false), handler("summingDouble", 1, "mapToDouble({0}).sum()", false), CallHandler.of(collectorMatcher("toUnmodifiableList", 0).withLanguageLevelAtLeast(LanguageLevel.JDK_16), - call -> new ReplaceCollectorFix("toUnmodifiableList", "toList()", false))); + call -> new ReplaceCollectorFix("toUnmodifiableList", "toList()", false)), + CallHandler.of(collectorMatcher("toList", 0).withLanguageLevelAtLeast(LanguageLevel.JDK_16), call -> { + PsiMethodCallExpression collectCall = PsiTreeUtil.getParentOfType(call, PsiMethodCallExpression.class); + return MismatchedCollectionQueryUpdateInspection.isUnmodified(collectCall) + ? new ReplaceCollectorFix("toList", "toList()", false) + : null; + })); private final String myCollector; private final String myStreamSequence; diff --git a/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java b/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java index e73b22cf5e14..399c942f2441 100644 --- a/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java +++ b/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java @@ -4,8 +4,9 @@ import java.util.stream.Stream; class Test { - List list1 = ((((List) ((true ? ((Stream.of(1, 2, 3).collect(Collectors.toList()))) : Arrays.asList(1, 2)))))); - List list2 = ((((List) ((true ? ((Stream.of(1, 2, 3).collect(Collectors.toList()))) : Arrays.asList(1, 2)))))); + private List list1 = ((((List) ((true ? ((Stream.of(1, 2, 3).collect(Collectors.toList()))) : Arrays.asList(1, 2)))))); + private List list2 = ((((List) ((true ? ((Stream.of(1, 2, 3).collect(Collectors.toList()))) : Arrays.asList(1, 2)))))); + public List list3 = ((((List) ((true ? ((Stream.of(1, 2, 3).collect(Collectors.toList()))) : Arrays.asList(1, 2)))))); void foo1() { List list = Stream.of(1, 2, 3).collect(Collectors.toList()); @@ -214,4 +215,11 @@ class TernaryTest { // List subList = stream.collect(Collectors.toList()).subList(0, 3); // subList.forEach(System.out::println); // } + +// void test7(Stream stream) { +// List list = stream.collect(Collectors.toList()); +// list.forEach(System.out::println); +// list = new ArrayList<>(); +// list.add("foo"); +// } //} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToList.java b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToList.java index 81ea9abe9a18..902819c59086 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToList.java +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToList.java @@ -1,11 +1,10 @@ -// "Replace with 'java.util.ArrayList' constructor" "true" +// "Replace 'collect(toList())' with 'toList()'" "true" -import java.util.ArrayList; import java.util.List; import java.util.stream.*; class Test { public static void test(List s) { - new ArrayList<>(s).contains("abc"); + s.stream().toList().contains("abc"); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToListOtherType.java b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToListOtherType.java deleted file mode 100644 index 8ee47516fa34..000000000000 --- a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToListOtherType.java +++ /dev/null @@ -1,11 +0,0 @@ -// "Replace with 'java.util.ArrayList' constructor" "true" - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.*; - -class Test { - public static void test(List s) { - new ArrayList(s).contains("abc"); - } -} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToSetOtherType.java b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToSetOtherType.java new file mode 100644 index 000000000000..4339379b7815 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterStreamToSetOtherType.java @@ -0,0 +1,11 @@ +// "Replace with 'java.util.HashSet' constructor" "true" + +import java.util.HashSet; +import java.util.List; +import java.util.stream.*; + +class Test { + public static void test(List s) { + new HashSet(s).contains("abc"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToList.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToList.java index dcf6c386e8e6..50b3f9b40a1b 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToList.java +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToList.java @@ -1,4 +1,4 @@ -// "Replace with 'java.util.ArrayList' constructor" "true" +// "Replace 'collect(toList())' with 'toList()'" "true" import java.util.List; import java.util.stream.*; diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToListOtherType.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToListOtherType.java deleted file mode 100644 index 10edac2d1b3e..000000000000 --- a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToListOtherType.java +++ /dev/null @@ -1,10 +0,0 @@ -// "Replace with 'java.util.ArrayList' constructor" "true" - -import java.util.List; -import java.util.stream.*; - -class Test { - public static void test(List s) { - s.stream().collect(Collectors.toList()).contains("abc"); - } -} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToSetOtherType.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToSetOtherType.java new file mode 100644 index 000000000000..cd955602fedc --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeStreamToSetOtherType.java @@ -0,0 +1,10 @@ +// "Replace with 'java.util.HashSet' constructor" "true" + +import java.util.List; +import java.util.stream.*; + +class Test { + public static void test(List s) { + s.stream().collect(Collectors.toSet()).contains("abc"); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java index df2bd6a0f016..ef99a21e1303 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java @@ -45,14 +45,14 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.awt.*; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.stream.Collectors; import static com.siyeh.ig.psiutils.ClassUtils.isImmutable; -public class MismatchedCollectionQueryUpdateInspection - extends BaseInspection { +public class MismatchedCollectionQueryUpdateInspection extends BaseInspection { private static final CallMatcher TRANSFORMED = CallMatcher.staticCall( CommonClassNames.JAVA_UTIL_COLLECTIONS, "asLifoQueue", "checkedCollection", "checkedList", "checkedMap", "checkedNavigableMap", @@ -74,15 +74,15 @@ public class MismatchedCollectionQueryUpdateInspection private static final @NonNls Set COLLECTIONS_UPDATES = ContainerUtil.set("addAll", "fill", "copy", "replaceAll", "sort"); private static final Set COLLECTIONS_ALL = StreamEx.of(COLLECTIONS_QUERIES).append(COLLECTIONS_UPDATES).toImmutableSet(); + private static final Set defaultQueryNames = + Set.of("contains", "copyInto", "equals", "forEach", "get", "hashCode", "iterator", "parallelStream", "peek", "propertyNames", "save", + "size", "store", "stream", "toArray", "toString", "write"); + private static final Set defaultUpdateNames = + Set.of("add", "clear", "insert", "load", "merge", "offer", "poll", "pop", "push", "put", "remove", "replace", "retain", "set", "take"); @SuppressWarnings("PublicField") - public final ExternalizableStringSet queryNames = - new ExternalizableStringSet( - "contains", "copyInto", "equals", "forEach", "get", "hashCode", "iterator", "parallelStream", "peek", "propertyNames", - "save", "size", "store", "stream", "toArray", "toString", "write"); + public final ExternalizableStringSet queryNames = new ExternalizableStringSet(defaultQueryNames.toArray(String[]::new)); @SuppressWarnings("PublicField") - public final ExternalizableStringSet updateNames = - new ExternalizableStringSet("add", "clear", "insert", "load", "merge", "offer", "poll", "pop", "push", "put", "remove", "replace", - "retain", "set", "take"); + public final ExternalizableStringSet updateNames = new ExternalizableStringSet(defaultUpdateNames.toArray(String[]::new)); @SuppressWarnings("PublicField") public final ExternalizableStringSet ignoredClasses = new ExternalizableStringSet(); @@ -154,217 +154,233 @@ public class MismatchedCollectionQueryUpdateInspection return new MismatchedCollectionQueryUpdateVisitor(); } - public static QueryUpdateInfo getCollectionQueryUpdateInfo(@Nullable PsiVariable variable, PsiElement context, - Set queryNames, Set updateNames) { + private static QueryUpdateInfo getCollectionQueryUpdateInfo(@Nullable PsiVariable variable, + PsiElement context, + Set queryNames, + Set updateNames) { QueryUpdateInfo info = new QueryUpdateInfo(); - class Visitor extends JavaRecursiveElementWalkingVisitor { - @Override - public void visitReferenceExpression(PsiReferenceExpression ref) { - super.visitReferenceExpression(ref); - if (variable == null) { - if (ref.getQualifierExpression() == null) { - makeUpdated(); - makeQueried(); - } - } else if (ref.isReferenceTo(variable)) { - process(findEffectiveReference(ref)); - } - } - - @Override - public void visitThisExpression(PsiThisExpression expression) { - super.visitThisExpression(expression); - if (variable == null) { - process(findEffectiveReference(expression)); - } - } - - private void makeUpdated() { - info.updated = true; - if (info.queried) { - stopWalking(); - } - } - - private void makeQueried() { - info.queried = true; - if (info.updated) { - stopWalking(); - } - } - - public void process(PsiExpression reference) { - PsiMethodCallExpression qualifiedCall = ExpressionUtils.getCallForQualifier(reference); - if (qualifiedCall != null) { - processQualifiedCall(qualifiedCall); - return; - } - PsiElement parent = reference.getParent(); - PsiElement grandParent = skipAssigmentExprUp(parent); - if (parent instanceof PsiExpressionList || - (parent instanceof PsiAssignmentExpression && grandParent instanceof PsiExpressionList)) { - PsiExpressionList args = (PsiExpressionList)(parent instanceof PsiExpressionList ? parent : grandParent); - PsiCallExpression surroundingCall = ObjectUtils.tryCast(args.getParent(), PsiCallExpression.class); - if (surroundingCall != null) { - if (surroundingCall instanceof PsiMethodCallExpression && - processCollectionMethods((PsiMethodCallExpression)surroundingCall, reference)) { - return; - } - makeQueried(); - if (!isQueryMethod(surroundingCall) && !COLLECTION_SAFE_ARGUMENT_METHODS.matches(surroundingCall)) { - makeUpdated(); - } - return; - } - } - if (parent instanceof PsiMethodReferenceExpression) { - processQualifiedMethodReference(((PsiMethodReferenceExpression)parent)); - return; - } - if (parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == reference) { - makeQueried(); - return; - } - if (parent instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)parent).getLExpression() == reference) { - PsiExpression rValue = ((PsiAssignmentExpression)parent).getRExpression(); - if (rValue == null) return; - if (ExpressionUtils.nonStructuralChildren(rValue) - .allMatch(MismatchedCollectionQueryUpdateInspection::isEmptyCollectionInitializer)) { - return; - } - if (ExpressionUtils.nonStructuralChildren(rValue) - .allMatch(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer)) { - makeUpdated(); - return; - } - } - if (parent instanceof PsiPolyadicExpression) { - IElementType tokenType = ((PsiPolyadicExpression)parent).getOperationTokenType(); - if (tokenType.equals(JavaTokenType.PLUS)) { - // String concatenation - makeQueried(); - return; - } - if (tokenType.equals(JavaTokenType.EQEQ) || tokenType.equals(JavaTokenType.NE)) { - return; - } - } - if (parent instanceof PsiAssertStatement && ((PsiAssertStatement)parent).getAssertDescription() == reference) { - makeQueried(); - return; - } - if (parent instanceof PsiInstanceOfExpression || parent instanceof PsiSynchronizedStatement) return; - // Any other reference - makeUpdated(); - makeQueried(); - } - - private void processQualifiedMethodReference(PsiMethodReferenceExpression expression) { - final String methodName = expression.getReferenceName(); - if (isQueryUpdateMethodName(methodName, queryNames)) { - makeQueried(); - } - if (isQueryUpdateMethodName(methodName, updateNames)) { - makeUpdated(); - } - final PsiMethod method = ObjectUtils.tryCast(expression.resolve(), PsiMethod.class); - if (method != null && - (!PsiType.VOID.equals(method.getReturnType()) && - !PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(expression)) || - ContainerUtil.or(method.getParameterList().getParameters(), p -> LambdaUtil.isFunctionalType(p.getType())))) { - makeQueried(); - } - } - - private boolean processCollectionMethods(PsiMethodCallExpression call, PsiExpression arg) { - PsiExpressionList expressionList = call.getArgumentList(); - String name = call.getMethodExpression().getReferenceName(); - if (!COLLECTIONS_ALL.contains(name) || !isCollectionsClassMethod(call)) return false; - if (COLLECTIONS_QUERIES.contains(name) && !(call.getParent() instanceof PsiExpressionStatement)) { - makeQueried(); - return true; - } - if (COLLECTIONS_UPDATES.contains(name)) { - int index = ArrayUtil.indexOf(expressionList.getExpressions(), arg); - if (index == 0) { - makeUpdated(); - } - else { - makeQueried(); - } - return true; - } - return false; - } - - private void processQualifiedCall(PsiMethodCallExpression call) { - boolean voidContext = ExpressionUtils.isVoidContext(call); - String name = call.getMethodExpression().getReferenceName(); - boolean queryQualifier = isQueryUpdateMethodName(name, queryNames); - boolean updateQualifier = isQueryUpdateMethodName(name, updateNames); - if (queryQualifier && - (!voidContext || PsiType.VOID.equals(call.getType()) || "toArray".equals(name) && !call.getArgumentList().isEmpty())) { - makeQueried(); - } - if (updateQualifier) { - makeUpdated(); - 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 (call.getArgumentList().getExpressionCount() == 2 && - ("poll".equals(name) || "pollFirst".equals(name) || "pollLast".equals(name)) && - TypeUtils.variableHasTypeOrSubtype(variable, "java.util.concurrent.BlockingQueue")) { - // poll(timeout, unit) on a blocking queue/dequeue may be considered querying, even if the result is not used, - // because the thread will be blocked until a value is received (or a timeout happens). - makeQueried(); - } - else if (("take".equals(name) || "takeFirst".equals(name) || "takeLast".equals(name)) && - TypeUtils.variableHasTypeOrSubtype(variable, "java.util.concurrent.BlockingQueue")) { - // take() on a blocking queue/dequeue may be considered querying, even if the result is not used. - // because the thread will be blocked until a value is received. - makeQueried(); - } - } - } - if (!queryQualifier && !updateQualifier) { - if (!isQueryMethod(call)) { - makeUpdated(); - } - makeQueried(); - } - } - - 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; - } - - } - Visitor visitor = new Visitor(); + Visitor visitor = new Visitor(variable, queryNames, updateNames, info); context.accept(visitor); return info; } - public static PsiExpression findEffectiveReference(PsiExpression expression) { + private static class Visitor extends JavaRecursiveElementWalkingVisitor { + final PsiVariable myVariable; + final Set myQueryNames; + final Set myUpdateNames; + final QueryUpdateInfo myInfo; + + private Visitor(@Nullable PsiVariable variable, Set queryNames, Set updateNames, QueryUpdateInfo info) { + myVariable = variable; + myQueryNames = queryNames; + myUpdateNames = updateNames; + myInfo = info; + } + + @Override + public void visitReferenceExpression(PsiReferenceExpression ref) { + super.visitReferenceExpression(ref); + if (myVariable == null) { + if (ref.getQualifierExpression() == null) { + makeUpdated(myInfo); + makeQueried(myInfo); + } + } else if (ref.isReferenceTo(myVariable)) { + process(findEffectiveReference(ref)); + } + } + + @Override + public void visitThisExpression(PsiThisExpression expression) { + super.visitThisExpression(expression); + if (myVariable == null) { + process(findEffectiveReference(expression)); + } + } + + private void makeUpdated(QueryUpdateInfo info) { + info.updated = true; + if (info.queried) { + stopWalking(); + } + } + + private void makeQueried(QueryUpdateInfo info) { + info.queried = true; + if (info.updated) { + stopWalking(); + } + } + + private void process(PsiExpression reference) { + PsiMethodCallExpression qualifiedCall = ExpressionUtils.getCallForQualifier(reference); + if (qualifiedCall != null) { + processQualifiedCall(qualifiedCall, myQueryNames, myUpdateNames); + return; + } + PsiElement parent = reference.getParent(); + PsiElement grandParent = skipAssigmentExprUp(parent); + if (parent instanceof PsiExpressionList || + (parent instanceof PsiAssignmentExpression && grandParent instanceof PsiExpressionList)) { + PsiExpressionList args = (PsiExpressionList)(parent instanceof PsiExpressionList ? parent : grandParent); + PsiCallExpression surroundingCall = ObjectUtils.tryCast(args.getParent(), PsiCallExpression.class); + if (surroundingCall != null) { + if (surroundingCall instanceof PsiMethodCallExpression && + processCollectionMethods((PsiMethodCallExpression)surroundingCall, reference)) { + return; + } + makeQueried(myInfo); + if (!isQueryMethod(surroundingCall) && !COLLECTION_SAFE_ARGUMENT_METHODS.matches(surroundingCall)) { + makeUpdated(myInfo); + } + return; + } + } + if (parent instanceof PsiMethodReferenceExpression) { + processQualifiedMethodReference(((PsiMethodReferenceExpression)parent), myQueryNames, myUpdateNames); + return; + } + if (parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == reference) { + makeQueried(myInfo); + return; + } + if (parent instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)parent).getLExpression() == reference) { + PsiExpression rValue = ((PsiAssignmentExpression)parent).getRExpression(); + if (rValue == null) return; + if (ExpressionUtils.nonStructuralChildren(rValue) + .allMatch(MismatchedCollectionQueryUpdateInspection::isEmptyCollectionInitializer)) { + return; + } + if (ExpressionUtils.nonStructuralChildren(rValue) + .allMatch(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer)) { + makeUpdated(myInfo); + return; + } + } + if (parent instanceof PsiPolyadicExpression) { + IElementType tokenType = ((PsiPolyadicExpression)parent).getOperationTokenType(); + if (tokenType.equals(JavaTokenType.PLUS)) { + // String concatenation + makeQueried(myInfo); + return; + } + if (tokenType.equals(JavaTokenType.EQEQ) || tokenType.equals(JavaTokenType.NE)) { + return; + } + } + if (parent instanceof PsiAssertStatement && ((PsiAssertStatement)parent).getAssertDescription() == reference) { + makeQueried(myInfo); + return; + } + if (parent instanceof PsiInstanceOfExpression || parent instanceof PsiSynchronizedStatement) return; + // Any other reference + makeUpdated(myInfo); + makeQueried(myInfo); + } + + private void processQualifiedMethodReference(PsiMethodReferenceExpression expression, + Set queryNames, + Set updateNames) { + final String methodName = expression.getReferenceName(); + if (isQueryUpdateMethodName(methodName, queryNames)) { + makeQueried(myInfo); + } + if (isQueryUpdateMethodName(methodName, updateNames)) { + makeUpdated(myInfo); + } + final PsiMethod method = ObjectUtils.tryCast(expression.resolve(), PsiMethod.class); + if (method != null && + (!PsiType.VOID.equals(method.getReturnType()) && + !PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType(expression)) || + ContainerUtil.or(method.getParameterList().getParameters(), p -> LambdaUtil.isFunctionalType(p.getType())))) { + makeQueried(myInfo); + } + } + + private boolean processCollectionMethods(PsiMethodCallExpression call, PsiExpression arg) { + PsiExpressionList expressionList = call.getArgumentList(); + String name = call.getMethodExpression().getReferenceName(); + if (!COLLECTIONS_ALL.contains(name) || !isCollectionsClassMethod(call)) return false; + if (COLLECTIONS_QUERIES.contains(name) && !(call.getParent() instanceof PsiExpressionStatement)) { + makeQueried(myInfo); + return true; + } + if (COLLECTIONS_UPDATES.contains(name)) { + int index = ArrayUtil.indexOf(expressionList.getExpressions(), arg); + if (index == 0) { + makeUpdated(myInfo); + } + else { + makeQueried(myInfo); + } + return true; + } + return false; + } + + private void processQualifiedCall(PsiMethodCallExpression call, Set queryNames, Set updateNames) { + boolean voidContext = ExpressionUtils.isVoidContext(call); + String name = call.getMethodExpression().getReferenceName(); + boolean queryQualifier = isQueryUpdateMethodName(name, queryNames); + boolean updateQualifier = isQueryUpdateMethodName(name, updateNames); + if (queryQualifier && + (!voidContext || PsiType.VOID.equals(call.getType()) || "toArray".equals(name) && !call.getArgumentList().isEmpty())) { + makeQueried(myInfo); + } + if (updateQualifier) { + makeUpdated(myInfo); + if (!voidContext) { + makeQueried(myInfo); + } + 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(myInfo); + break; + } + } + } + if (call.getArgumentList().getExpressionCount() == 2 && + ("poll".equals(name) || "pollFirst".equals(name) || "pollLast".equals(name)) && + TypeUtils.variableHasTypeOrSubtype(myVariable, "java.util.concurrent.BlockingQueue")) { + // poll(timeout, unit) on a blocking queue/dequeue may be considered querying, even if the result is not used, + // because the thread will be blocked until a value is received (or a timeout happens). + makeQueried(myInfo); + } + else if (("take".equals(name) || "takeFirst".equals(name) || "takeLast".equals(name)) && + TypeUtils.variableHasTypeOrSubtype(myVariable, "java.util.concurrent.BlockingQueue")) { + // take() on a blocking queue/dequeue may be considered querying, even if the result is not used. + // because the thread will be blocked until a value is received. + makeQueried(myInfo); + } + } + } + if (!queryQualifier && !updateQualifier) { + if (!isQueryMethod(call)) { + makeUpdated(myInfo); + } + makeQueried(myInfo); + } + } + + private static 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 static PsiExpression findEffectiveReference(PsiExpression expression) { while (true) { PsiElement parent = expression.getParent(); if (parent instanceof PsiParenthesizedExpression || parent instanceof PsiTypeCastExpression || @@ -391,7 +407,7 @@ public class MismatchedCollectionQueryUpdateInspection return expression; } - static boolean isEmptyCollectionInitializer(PsiExpression initializer) { + private static boolean isEmptyCollectionInitializer(PsiExpression initializer) { if (!(initializer instanceof PsiNewExpression)) { return ConstructionUtils.isEmptyCollectionInitializer(initializer); } @@ -419,11 +435,11 @@ public class MismatchedCollectionQueryUpdateInspection return true; } - static boolean isCollectionInitializer(PsiExpression initializer) { + private static boolean isCollectionInitializer(PsiExpression initializer) { return isEmptyCollectionInitializer(initializer) || ConstructionUtils.isPrepopulatedCollectionInitializer(initializer); } - public static boolean isQueryUpdateMethodName(String methodName, Set myNames) { + private static boolean isQueryUpdateMethodName(String methodName, Set myNames) { if (methodName == null) { return false; } @@ -438,7 +454,7 @@ public class MismatchedCollectionQueryUpdateInspection return false; } - public static boolean isCollectionsClassMethod(PsiMethodCallExpression call) { + private static boolean isCollectionsClassMethod(PsiMethodCallExpression call) { final PsiMethod method = call.resolveMethod(); if (method == null) return false; final PsiClass aClass = method.getContainingClass(); @@ -447,7 +463,7 @@ public class MismatchedCollectionQueryUpdateInspection return CommonClassNames.JAVA_UTIL_COLLECTIONS.equals(qualifiedName); } - public static boolean isQueryMethod(@NotNull PsiCallExpression call) { + private static boolean isQueryMethod(@NotNull PsiCallExpression call) { PsiType type = call.getType(); boolean immutable = isImmutable(type); // If pure method returns mutable object, then it's possible that further mutation of that object will modify the original collection @@ -476,9 +492,9 @@ public class MismatchedCollectionQueryUpdateInspection return parent; } - public static class QueryUpdateInfo { - public boolean updated; - public boolean queried; + private static class QueryUpdateInfo { + boolean updated; + boolean queried; } private class MismatchedCollectionQueryUpdateVisitor extends BaseInspectionVisitor { @@ -487,9 +503,9 @@ public class MismatchedCollectionQueryUpdateInspection PsiExpression initializer = variable.getInitializer(); if (initializer != null) { List expressions = ExpressionUtils.nonStructuralChildren(initializer).collect(Collectors.toList()); - if (!expressions.stream().allMatch(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer)) { + if (!ContainerUtil.and(expressions, MismatchedCollectionQueryUpdateInspection::isCollectionInitializer)) { expressions.stream().filter(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer) - .forEach(emptyCollection -> registerError(emptyCollection, Boolean.TRUE)); + .forEach(emptyCollection -> registerError(emptyCollection, Boolean.TRUE)); return; } } @@ -500,19 +516,8 @@ public class MismatchedCollectionQueryUpdateInspection @Override public void visitField(@NotNull PsiField field) { super.visitField(field); - if (!field.hasModifierProperty(PsiModifier.PRIVATE)) { - PsiClass aClass = field.getContainingClass(); - if (aClass == null || !aClass.hasModifierProperty(PsiModifier.PRIVATE) || field.hasModifierProperty(PsiModifier.PUBLIC)) { - // Public field within private class can be written/read via reflection even without setAccessible hacks - // so we don't analyze such fields to reduce false-positives - return; - } - } - final PsiClass containingClass = PsiUtil.getTopLevelClass(field); - if (!checkVariable(field, containingClass)) { - return; - } - QueryUpdateInfo info = getCollectionQueryUpdateInfo(field, containingClass, queryNames, updateNames); + QueryUpdateInfo info = getCollectionQueryUpdateInfo(field, updateNames, queryNames, ignoredClasses); + if (info == null) return; final boolean written = info.updated || updatedViaInitializer(field); final boolean read = info.queried || queriedViaInitializer(field); if (read == written || UnusedSymbolUtil.isImplicitWrite(field) || UnusedSymbolUtil.isImplicitRead(field)) { @@ -525,11 +530,8 @@ public class MismatchedCollectionQueryUpdateInspection @Override public void visitLocalVariable(@NotNull PsiLocalVariable variable) { super.visitLocalVariable(variable); - final PsiCodeBlock codeBlock = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); - if (!checkVariable(variable, codeBlock)) { - return; - } - QueryUpdateInfo info = getCollectionQueryUpdateInfo(variable, codeBlock, queryNames, updateNames); + QueryUpdateInfo info = getCollectionQueryUpdateInfo(variable, updateNames, queryNames, ignoredClasses); + if (info == null) return; final boolean written = info.updated || updatedViaInitializer(variable); final boolean read = info.queried || queriedViaInitializer(variable); if (read != written) { @@ -537,17 +539,6 @@ public class MismatchedCollectionQueryUpdateInspection } } - public boolean checkVariable(PsiVariable variable, PsiElement context) { - if (context == null) { - return false; - } - final PsiType type = variable.getType(); - if (!CollectionUtils.isCollectionClassOrInterface(type)) { - return false; - } - return !ContainerUtil.exists(ignoredClasses, className -> InheritanceUtil.isInheritor(type, className)); - } - private boolean updatedViaInitializer(PsiVariable variable) { final PsiExpression initializer = variable.getInitializer(); if (initializer != null && @@ -579,4 +570,60 @@ public class MismatchedCollectionQueryUpdateInspection .noneMatch(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer); } } + + private static boolean checkVariable(PsiVariable variable, PsiElement context, Set ignoredClasses) { + if (context == null) { + return false; + } + final PsiType type = variable.getType(); + if (!CollectionUtils.isCollectionClassOrInterface(type)) { + return false; + } + return !ContainerUtil.exists(ignoredClasses, className -> InheritanceUtil.isInheritor(type, className)); + } + + private static QueryUpdateInfo getCollectionQueryUpdateInfo(@NotNull PsiField field, + Set updateNames, + Set queryNames, + Set ignoredClasses) { + if (!field.hasModifierProperty(PsiModifier.PRIVATE)) { + PsiClass aClass = field.getContainingClass(); + if (aClass == null || !aClass.hasModifierProperty(PsiModifier.PRIVATE) || field.hasModifierProperty(PsiModifier.PUBLIC)) { + // Public field within private class can be written/read via reflection even without setAccessible hacks, + // so we don't analyze such fields to reduce false-positives + return null; + } + } + final PsiClass containingClass = PsiUtil.getTopLevelClass(field); + if (!checkVariable(field, containingClass, ignoredClasses)) { + return null; + } + return getCollectionQueryUpdateInfo(field, containingClass, queryNames, updateNames); + } + + private static QueryUpdateInfo getCollectionQueryUpdateInfo(@NotNull PsiLocalVariable variable, + Set updateNames, + Set queryNames, + Set ignoredClasses) { + final PsiCodeBlock codeBlock = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + if (!checkVariable(variable, codeBlock, ignoredClasses)) { + return null; + } + return getCollectionQueryUpdateInfo(variable, codeBlock, queryNames, updateNames); + } + + public static boolean isUnmodified(PsiMethodCallExpression methodCall) { + final PsiExpression effectiveReference = findEffectiveReference(methodCall); + QueryUpdateInfo info = new QueryUpdateInfo(); + new Visitor(null, defaultQueryNames, defaultUpdateNames, info).process(effectiveReference); + if (!info.updated) return true; + final PsiElement parent = effectiveReference.getParent(); + if (!(parent instanceof PsiLocalVariable || parent instanceof PsiField)) return false; + if (parent instanceof PsiField) { + info = getCollectionQueryUpdateInfo((PsiField)parent, defaultUpdateNames, defaultQueryNames, Collections.emptySet()); + } else { + info = getCollectionQueryUpdateInfo((PsiLocalVariable)parent, defaultUpdateNames, defaultQueryNames, Collections.emptySet()); + } + return info != null && !info.updated; + } }