diff --git a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties index 52ec9302dcab..6c3bf90cbb7e 100644 --- a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties +++ b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties @@ -123,7 +123,9 @@ mismatched.read.write.array.display.name=Mismatched read and write of array mismatched.read.write.array.problem.descriptor.write.not.read=Contents of array #ref are written to, but never read #loc mismatched.read.write.array.problem.descriptor.read.not.write=Contents of array #ref are read, but never written to #loc mismatched.update.collection.display.name=Mismatched query and update of collection -mismatched.update.collection.problem.descriptor.updated.not.queried=Contents of collection #ref are updated, but never queried #loc +mismatched.update.collection.problem.description.no.effect.updates=Update operations on empty collection #ref have no effect #loc +mismatched.update.collection.problem.description.updated.not.queried=Contents of collection #ref are updated, but never queried #loc +mismatched.update.collection.problem.description.queried.empty=Contents of empty collection #ref are queried, but it's never populated #loc mismatched.update.collection.problem.description.queried.not.updated=Contents of collection #ref are queried, but never updated #loc rename.quickfix=Rename renameto.quickfix=Rename to ''{0}'' diff --git a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/CollectionUtils.java b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/CollectionUtils.java index adcc397b6cea..79d1192d9487 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/CollectionUtils.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/CollectionUtils.java @@ -20,7 +20,6 @@ import com.intellij.psi.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; -import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.callMatcher.CallMatcher; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; @@ -45,7 +44,8 @@ public final class CollectionUtils { CallMatcher.anyOf( CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "keySet", "values", "entrySet").parameterCount(0), CallMatcher.instanceCall("java.util.NavigableMap", "descendingKeySet", "descendingMap", "navigableKeySet").parameterCount(0), - CallMatcher.instanceCall("java.util.NavigableSet", "descendingSet").parameterCount(0) + CallMatcher.instanceCall("java.util.NavigableSet", "descendingSet").parameterCount(0), + CallMatcher.instanceCall("java.util.SequencedCollection", "reversed").parameterCount(0) ); /** diff --git a/java/java-impl/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java b/java/java-impl/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java index 773ae99b6443..3b7d26ae0150 100644 --- a/java/java-impl/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java +++ b/java/java-impl/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspection.java @@ -41,7 +41,6 @@ import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -58,6 +57,9 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect "synchronizedSortedMap", "synchronizedSortedSet"); private static final CallMatcher DERIVED = CallMatcher.anyOf( CollectionUtils.DERIVED_COLLECTION, + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "iterator").parameterCount(0), + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_LIST, "listIterator").parameterCount(0), + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_LIST, "listIterator").parameterTypes("int"), CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_LIST, "subList"), CallMatcher.instanceCall("java.util.SortedMap", "headMap", "tailMap", "subMap"), CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_SORTED_SET, "headSet", "tailSet", "subSet")); @@ -71,6 +73,19 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect private static final @NonNls Set COLLECTIONS_UPDATES = Set.of("addAll", "fill", "copy", "replaceAll", "sort"); private static final Set COLLECTIONS_ALL = StreamEx.of(COLLECTIONS_QUERIES).append(COLLECTIONS_UPDATES).toImmutableSet(); + private static final @NotNull CallMatcher QUERY_ITERATOR_METHODS = + CallMatcher.anyOf( + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_ITERATOR, "hasNext", "next", "forEachRemaining"), + CallMatcher.instanceCall("java.util.ListIterator", "hasPrevious", "previous", "nextIndex", "previousIndex") + ); + private static final @NotNull CallMatcher NO_UPDATE_FOR_EMPTY = + CallMatcher.anyOf( + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_ITERATOR, "remove").parameterCount(0), + CallMatcher.instanceCall("java.util.ListIterator", "set").parameterCount(1), + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "remove", "clear", "removeAll", "retainAll", "removeIf"), + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_LIST, "sort", "replaceAll"), + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "replaceAll", "replace", "remove") + ); private static final Set defaultQueryNames = Set.of("contains", "copyInto", "equals", "forEach", "get", "hashCode", "indexOf", "iterator", "lastIndexOf", "parallelStream", "peek", "propertyNames", "save", "size", "store", "stream", "toArray", "toString", "write"); @@ -86,6 +101,8 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect @SuppressWarnings("PublicField") public final ExternalizableStringSet ignoredClasses = new ExternalizableStringSet(); + private final MismatchedQueryUpdateRunner myRunner = new MismatchedQueryUpdateRunner(queryNames, updateNames, ignoredClasses); + @Override public @NotNull OptPane getOptionsPane() { return pane( @@ -107,13 +124,7 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect @Override public @NotNull String buildErrorString(Object... infos) { - final boolean updated = ((Boolean)infos[0]).booleanValue(); - if (updated) { - return InspectionGadgetsBundle.message("mismatched.update.collection.problem.descriptor.updated.not.queried"); - } - else { - return InspectionGadgetsBundle.message("mismatched.update.collection.problem.description.queried.not.updated"); - } + return (String)infos[0]; } @Override @@ -131,46 +142,84 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect return new MismatchedCollectionQueryUpdateVisitor(); } - private static QueryUpdateInfo getCollectionQueryUpdateInfo(@Nullable PsiVariable variable, - PsiElement context, - Set queryNames, - Set updateNames) { - return getCollectionQueryUpdateInfo(variable, context, queryNames, updateNames, Set.of()); - } - private static QueryUpdateInfo getCollectionQueryUpdateInfo(@Nullable PsiVariable variable, - PsiElement context, - Set queryNames, - Set updateNames, - Set ignored) { - QueryUpdateInfo info = new QueryUpdateInfo(); - Visitor visitor = new Visitor(variable, queryNames, updateNames, info, ignored); - context.accept(visitor); - return info; + private record MismatchedQueryUpdateRunner( + @NotNull Set queryNames, + @NotNull Set updateNames, + @NotNull Set ignoredClasses + ) { + @NotNull QueryUpdateInfo compute(@Nullable PsiVariable variable, + @Nullable PsiElement context, + @NotNull Set ignored) { + if (context == null) return QueryUpdateInfo.UNKNOWN; + if (variable != null && !checkVariable(variable)) { + return QueryUpdateInfo.UNKNOWN; + } + Visitor visitor = new Visitor(variable, queryNames, updateNames, ignored); + context.accept(visitor); + return visitor.getInfo(); + } + + @NotNull QueryUpdateInfo compute(@NotNull PsiVariable variable, @NotNull Set ignored) { + final PsiElement context = variable instanceof PsiField ? PsiUtil.getTopLevelClass(variable) : + PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + return compute(variable, context, ignored); + } + + @NotNull QueryUpdateInfo processSingleRef(@NotNull PsiExpression ref) { + Visitor visitor = new Visitor(null, queryNames, updateNames, Set.of()); + visitor.process(ref); + return visitor.getInfo(); + } + + private boolean checkVariable(@NotNull PsiVariable variable) { + if (variable instanceof PsiField 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 false; + } + } + } + else if (!(variable instanceof PsiLocalVariable)) { + return false; + } + final PsiType type = variable.getType(); + if (!CollectionUtils.isCollectionClassOrInterface(type)) { + return false; + } + return !ContainerUtil.exists(ignoredClasses, className -> InheritanceUtil.isInheritor(type, className)); + } } private static class Visitor extends JavaRecursiveElementWalkingVisitor { - final PsiVariable myVariable; - final Set myQueryNames; - final Set myUpdateNames; - final QueryUpdateInfo myInfo; - final Set myIgnored = new HashSet<>(); + final @Nullable PsiVariable myVariable; + final @NotNull Set myDerivedVariables = new HashSet<>(); + final @NotNull Set myQueryNames; + final @NotNull Set myUpdateNames; + final @NotNull Set myIgnored = new HashSet<>(); + boolean myQueried; + boolean myUpdated; + boolean myUpdatedForNonEmpty; + boolean myKnownEmpty; private Visitor(@Nullable PsiVariable variable, @NotNull Set queryNames, @NotNull Set updateNames, - @NotNull QueryUpdateInfo info) { + @NotNull Set ignored) { myVariable = variable; myQueryNames = queryNames; myUpdateNames = updateNames; - myInfo = info; - } - private Visitor(@Nullable PsiVariable variable, - @NotNull Set queryNames, - @NotNull Set updateNames, - @NotNull QueryUpdateInfo info, - @NotNull Set ignored) { - this(variable, queryNames, updateNames, info); myIgnored.addAll(ignored); + if (variable != null) { + myKnownEmpty = definitelyEmptyCollection(variable.getInitializer()); + } + } + + private static boolean definitelyEmptyCollection(@Nullable PsiExpression initializer) { + return initializer == null || ExpressionUtils.nonStructuralChildren(initializer) + .allMatch(e -> ConstructionUtils.isEmptyCollectionInitializer(e)); } @Override @@ -181,8 +230,13 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect makeUpdated(); makeQueried(); } - } else if (ref.isReferenceTo(myVariable) && !myIgnored.contains(ref)) { - process(findEffectiveReference(ref)); + } + else { + if (ref.resolve() instanceof PsiVariable target) { + if ((target.equals(myVariable) || myDerivedVariables.contains(target)) && !myIgnored.contains(ref)) { + process(ref); + } + } } } @@ -190,25 +244,37 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect public void visitThisExpression(@NotNull PsiThisExpression expression) { super.visitThisExpression(expression); if (myVariable == null) { - process(findEffectiveReference(expression)); + process(expression); } } private void makeUpdated() { - myInfo.updated = true; - if (myInfo.queried) { + myUpdated = true; + if (myQueried) { stopWalking(); } } + private void makeUpdatedForNonEmpty() { + myUpdatedForNonEmpty = true; + } + private void makeQueried() { - myInfo.queried = true; - if (myInfo.updated) { + myQueried = true; + if (myUpdated) { stopWalking(); } } + @NotNull QueryUpdateInfo getInfo() { + return new QueryUpdateInfo(myQueried, myUpdated, myUpdatedForNonEmpty, myKnownEmpty); + } + private void process(PsiExpression reference) { + doProcess(findEffectiveReference(reference)); + } + + private void doProcess(PsiExpression reference) { PsiMethodCallExpression qualifiedCall = ExpressionUtils.getCallForQualifier(reference); if (qualifiedCall != null) { processQualifiedCall(qualifiedCall); @@ -228,6 +294,10 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect } return; } + if (myVariable != null && parent instanceof PsiLocalVariable derived && !VariableAccessUtils.variableIsAssigned(derived)) { + myDerivedVariables.add(derived); + return; + } if (parent instanceof PsiMethodReferenceExpression methodReference) { processQualifiedMethodReference(methodReference); return; @@ -237,11 +307,12 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect return; } if (parent instanceof PsiAssignmentExpression assignment && assignment.getLExpression() == reference) { - process(findEffectiveReference(assignment)); + process(assignment); PsiExpression rValue = assignment.getRExpression(); if (rValue == null) return; if (ExpressionUtils.nonStructuralChildren(rValue) .allMatch(MismatchedCollectionQueryUpdateInspection::isEmptyCollectionInitializer)) { + myKnownEmpty = myKnownEmpty && definitelyEmptyCollection(rValue); return; } if (ExpressionUtils.nonStructuralChildren(rValue) @@ -312,6 +383,14 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect private void processQualifiedCall(PsiMethodCallExpression call) { boolean voidContext = ExpressionUtils.isVoidContext(call); String name = call.getMethodExpression().getReferenceName(); + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier != null && InheritanceUtil.isInheritor(qualifier.getType(), CommonClassNames.JAVA_UTIL_ITERATOR)) { + makeQueried(); + if (!QUERY_ITERATOR_METHODS.test(call)) { + makeUpdatedCall(call); + } + return; + } boolean queryQualifier = isQueryUpdateMethodName(name, myQueryNames); boolean updateQualifier = isQueryUpdateMethodName(name, myUpdateNames); if (queryQualifier && @@ -319,7 +398,7 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect makeQueried(); } if (updateQualifier) { - makeUpdated(); + makeUpdatedCall(call); if (!voidContext) { makeQueried(); } @@ -356,6 +435,15 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect } } + private void makeUpdatedCall(PsiMethodCallExpression call) { + if (NO_UPDATE_FOR_EMPTY.test(call)) { + makeUpdatedForNonEmpty(); + } + else { + makeUpdated(); + } + } + private static boolean mayHaveSideEffect(PsiExpression fn) { if (fn instanceof PsiLambdaExpression) { PsiElement body = ((PsiLambdaExpression)fn).getBody(); @@ -474,51 +562,55 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect return immutable && !SideEffectChecker.mayHaveSideEffects(call); } - private static class QueryUpdateInfo { - boolean updated; - boolean queried; + private record QueryUpdateInfo(boolean queried, boolean updated, boolean updatedForNonEmpty, boolean knownEmpty) { + static final QueryUpdateInfo UNKNOWN = new QueryUpdateInfo(true, true, true, false); } private class MismatchedCollectionQueryUpdateVisitor extends BaseInspectionVisitor { - private void register(PsiVariable variable, boolean written) { + private void check(@NotNull PsiVariable variable) { + QueryUpdateInfo info = myRunner.compute(variable, Set.of()); + final boolean written = info.updated || updatedViaInitializer(variable); + final boolean read = info.queried || queriedViaInitializer(variable); + if (!written && info.updatedForNonEmpty && info.knownEmpty) { + registerVariableError(variable, + InspectionGadgetsBundle.message("mismatched.update.collection.problem.description.no.effect.updates")); + return; + } + if (read == written) { + return; + } + String message = written ? InspectionGadgetsBundle.message("mismatched.update.collection.problem.description.updated.not.queried") : + info.knownEmpty ? InspectionGadgetsBundle.message( + "mismatched.update.collection.problem.description.queried.empty") : + InspectionGadgetsBundle.message("mismatched.update.collection.problem.description.queried.not.updated"); if (written) { PsiExpression initializer = variable.getInitializer(); if (initializer != null) { List expressions = ExpressionUtils.nonStructuralChildren(initializer).toList(); if (!ContainerUtil.and(expressions, MismatchedCollectionQueryUpdateInspection::isCollectionInitializer)) { expressions.stream().filter(MismatchedCollectionQueryUpdateInspection::isCollectionInitializer) - .forEach(emptyCollection -> registerError(emptyCollection, Boolean.TRUE)); + .forEach(emptyCollection -> registerError(emptyCollection, message)); return; } } } - registerVariableError(variable, written); + registerVariableError(variable, message); } @Override public void visitField(@NotNull PsiField field) { super.visitField(field); - 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)) { + if (UnusedSymbolUtil.isImplicitWrite(field) || UnusedSymbolUtil.isImplicitRead(field)) { // Even implicit read of the mutable collection field may cause collection change return; } - register(field, written); + check(field); } @Override public void visitLocalVariable(@NotNull PsiLocalVariable variable) { super.visitLocalVariable(variable); - QueryUpdateInfo info = getCollectionQueryUpdateInfo(variable, updateNames, queryNames, ignoredClasses, Set.of()); - if (info == null) return; - final boolean written = info.updated || updatedViaInitializer(variable); - final boolean read = info.queried || queriedViaInitializer(variable); - if (read != written) { - register(variable, written); - } + check(variable); } private boolean updatedViaInitializer(PsiVariable variable) { @@ -531,7 +623,7 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect if (initializer instanceof PsiNewExpression newExpression) { final PsiAnonymousClass anonymousClass = newExpression.getAnonymousClass(); if (anonymousClass != null) { - if (getCollectionQueryUpdateInfo(null, anonymousClass, queryNames, updateNames).updated) { + if (myRunner.compute(null, anonymousClass, Set.of()).updated) { return true; } final ThisPassedAsArgumentVisitor visitor = new ThisPassedAsArgumentVisitor(); @@ -552,48 +644,6 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect } } - 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, - Set ignored) { - final PsiCodeBlock codeBlock = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); - if (!checkVariable(variable, codeBlock, ignoredClasses)) { - return null; - } - return getCollectionQueryUpdateInfo(variable, codeBlock, queryNames, updateNames, ignored); - } - /** * @param collectCall a method call returning a list (e.g. {@code Stream.of(1, 2, 3).collect(Collectors.toList())}) * @return {@code true} if it's known that the result of {@code collectCall} is never updated; @@ -602,10 +652,10 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect @Contract("null -> false") public static boolean isUnmodified(@Nullable PsiMethodCallExpression collectCall) { if (collectCall == null) return false; - final PsiExpression effectiveReference = findEffectiveReference(collectCall); - QueryUpdateInfo info = new QueryUpdateInfo(); - new Visitor(null, defaultQueryNames, defaultUpdateNames, info).process(effectiveReference); - if (!info.updated) return true; + final PsiExpression effectiveReference = findEffectiveReference(collectCall); + MismatchedQueryUpdateRunner runner = new MismatchedQueryUpdateRunner(defaultQueryNames, defaultUpdateNames, Set.of()); + QueryUpdateInfo info = runner.processSingleRef(collectCall); + if (!info.updated && !info.updatedForNonEmpty) return true; PsiElement parent = effectiveReference.getParent(); Set ignored = new HashSet<>(); if (parent instanceof PsiAssignmentExpression assignmentExpression) { @@ -614,12 +664,10 @@ public final class MismatchedCollectionQueryUpdateInspection extends BaseInspect parent = referenceExpression.resolve(); } } - 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(), ignored); + if (parent instanceof PsiVariable variable) { + info = runner.compute(variable, variable instanceof PsiField ? Set.of() : ignored); + return !info.updated && !info.updatedForNonEmpty; } - return info != null && !info.updated; + return false; } } diff --git a/java/java-tests/testData/ig/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java b/java/java-tests/testData/ig/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java index 5493e6bce224..b50111e37391 100644 --- a/java/java-tests/testData/ig/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java +++ b/java/java-tests/testData/ig/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java @@ -22,8 +22,8 @@ import java.util.concurrent.*; public class MismatchedCollectionQueryUpdate { private Set foo = new HashSet(); - private Set foo2 = new HashSet(); - private Set bar ; + private Set foo2 = new HashSet(); + private Set bar ; private Set bar2 = new HashSet(foo2); private Set bal ; private Set injected = new HashSet<>(); @@ -86,7 +86,7 @@ public class MismatchedCollectionQueryUpdate { } Object[] testRemoveFromAnotherCollection(List> list) { - List l = new ArrayList<>(); + List l = new ArrayList<>(); list.remove(l); process(list); return l.toArray(); @@ -95,7 +95,7 @@ public class MismatchedCollectionQueryUpdate { native void process(List> list); void testPureMethod() { - List list = new ArrayList<>(); + List list = new ArrayList<>(); if(hasNull(list)) { System.out.println("has nulls!"); } @@ -134,13 +134,13 @@ public class MismatchedCollectionQueryUpdate { } boolean testSeparateInitialization() { - List list; + List list; list = new ArrayList<>(); return list.isEmpty(); } void testKeySet() { - Map map = new HashMap<>(); + Map map = new HashMap<>(); for(String s : map.keySet()) { System.out.println(s); } @@ -163,18 +163,40 @@ public class MismatchedCollectionQueryUpdate { System.out.println(i.next()); } + void testListIterator2() { + List test = new ArrayList(); + ListIterator i = test.listIterator(0); + System.out.println(i.next()); + } + + void testListIterator3() { + List test = new ArrayList(); + ListIterator iter = test.listIterator(); + while(iter.hasNext()) { + iter.next(); + iter.set("foo"); + } + } + void testIterator() { - List test = new ArrayList(); + List test = new ArrayList(); Iterator i = test.iterator(); while(i.hasNext()) { if(i.next() == null) { - // Normally iterator cannot add, remove only. If collection is always empty (not updated in any other way), - // this is useless anyways i.remove(); } } } + void derivedWrite() { + List list = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + list.add(String.valueOf(i)); + } + List subList = list.subList(0, 10); + subList.add("Hi!"); + } + void add(TestPrivateClass tpc) { tpc.field.add("foo"); } @@ -300,8 +322,8 @@ public class MismatchedCollectionQueryUpdate { } void methodArgument() { - List foos = new ArrayList<>(); - List bars = new ArrayList<>(); + List foos = new ArrayList<>(); + List bars = new ArrayList<>(); foos.removeAll( bars ); @@ -310,6 +332,18 @@ public class MismatchedCollectionQueryUpdate { m(other); } + private final List noAdd = new ArrayList<>(); + void updateEmptyAndQuery() { + noAdd.remove("xyz"); + if (noAdd.indexOf("a") > 10) { + noAdd.remove("b"); + } + } + + void sortEmpty() { + noAdd.sort(Comparator.naturalOrder()); + } + void m(List l) {} void initializationInsideMethodCall() { @@ -347,7 +381,7 @@ public class MismatchedCollectionQueryUpdate { public void foofoo() { - final Map anotherMap = new HashMap(); + final Map anotherMap = new HashMap(); final SortedMap map = new TreeMap(anotherMap); final Iterator it = map.keySet().iterator(); while(it.hasNext()){ @@ -444,12 +478,12 @@ class CollectionsUser { } void c() { - List l = new ArrayList(); + List l = new ArrayList(); final int frequency = Collections.frequency(l, "one"); } List d() { - List l = new ArrayList(); + List l = new ArrayList(); return Collections.unmodifiableList(l); } @@ -573,7 +607,7 @@ class SomeList extends ArrayList { class UnmodifiableTernaryTest { private final List myList = new ArrayList<>(); - private final List myList2 = new ArrayList<>(); + private final List myList2 = new ArrayList<>(); void add() { myList.add("foo"); @@ -609,7 +643,7 @@ class InLambdaTest { class AssertTest { void test() { - List list = new ArrayList<>(); + List list = new ArrayList<>(); assert list.isEmpty() : list; } } diff --git a/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java b/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java index 0d16ed734bcd..e3b6aa9082fa 100644 --- a/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java +++ b/java/java-tests/testData/inspection/simplifyStreamApiCallChains/CollectCollectorsToList.java @@ -164,6 +164,29 @@ class TernaryTest { } } +// IDEA-356315 +class MissingIteratorRemoveAnalysis { + public void problem() { + final List strings = Stream.of("anything", "at", "all") + .collect(Collectors.toList()); + + for(Iterator iterator = strings.iterator(); iterator.hasNext(); ) { + String string = iterator.next(); + iterator.remove(); + } + } + + public void noRemove() { + final List strings = Stream.of("anything", "at", "all") + .collect(Collectors.toList()); + + for(Iterator iterator = strings.iterator(); iterator.hasNext(); ) { + String string = iterator.next(); + System.out.println(string); + } + } +} + //class TODO() { // // void testMustHave1(Stream stream) {