Only suggest bulk removal operations on sets (#186)

If a collection may contain duplicates, then removing a single occurrence of
each element in some other set is not equivalent to removing all such
occurrences.
This commit is contained in:
Rick Ossendrijver
2022-08-10 14:09:36 +02:00
committed by GitHub
parent c57653dd5b
commit efbde936dc
3 changed files with 23 additions and 19 deletions

View File

@@ -15,6 +15,7 @@ import java.util.List;
import java.util.NavigableSet;
import java.util.Optional;
import java.util.Queue;
import java.util.Set;
import java.util.SortedSet;
import java.util.function.IntFunction;
import java.util.stream.Stream;
@@ -129,31 +130,32 @@ final class CollectionTemplates {
}
}
static final class CollectionRemoveAllFromCollectionBlock<T, S extends T> {
static final class SetRemoveAllCollection<T, S extends T> {
@BeforeTemplate
void before(Collection<T> removeTo, Collection<S> elementsToRemove) {
elementsToRemove.forEach(removeTo::remove);
void before(Set<T> removeFrom, Collection<S> elementsToRemove) {
elementsToRemove.forEach(removeFrom::remove);
}
@BeforeTemplate
void before2(Collection<T> removeTo, Collection<S> elementsToRemove) {
void before2(Set<T> removeFrom, Collection<S> elementsToRemove) {
for (T element : elementsToRemove) {
removeTo.remove(element);
removeFrom.remove(element);
}
}
// XXX: This method is identical to `before2` except for the loop type. Make Refaster smarter so
// that this is supported out of the box.
// that this is supported out of the box. After doing so, also drop the `S extends T` type
// constraint; ideally this check applies to any `S`.
@BeforeTemplate
void before3(Collection<T> removeTo, Collection<S> elementsToRemove) {
void before3(Set<T> removeFrom, Collection<S> elementsToRemove) {
for (S element : elementsToRemove) {
removeTo.remove(element);
removeFrom.remove(element);
}
}
@AfterTemplate
void after(Collection<T> removeTo, Collection<S> elementsToRemove) {
removeTo.removeAll(elementsToRemove);
void after(Set<T> removeFrom, Collection<S> elementsToRemove) {
removeFrom.removeAll(elementsToRemove);
}
}

View File

@@ -6,6 +6,7 @@ import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Optional;
@@ -52,13 +53,13 @@ final class CollectionTemplatesTest implements RefasterTemplateTestCase {
return Iterables.removeAll(new ArrayList<>(), ImmutableSet.of("foo"));
}
void testCollectionRemoveAllFromCollectionBlock() {
ImmutableSet.of("foo").forEach(new ArrayList<>()::remove);
for (Number element : ImmutableSet.of(1)) {
new ArrayList<Number>().remove(element);
void testSetRemoveAllCollection() {
ImmutableSet.of("foo").forEach(new HashSet<>()::remove);
for (Number element : ImmutableList.of(1)) {
new HashSet<Number>().remove(element);
}
for (Integer element : ImmutableSet.of(2)) {
new ArrayList<Number>().remove(element);
new HashSet<Number>().remove(element);
}
}

View File

@@ -6,6 +6,7 @@ import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Optional;
@@ -48,10 +49,10 @@ final class CollectionTemplatesTest implements RefasterTemplateTestCase {
return new ArrayList<>().removeAll(ImmutableSet.of("foo"));
}
void testCollectionRemoveAllFromCollectionBlock() {
new ArrayList<>().removeAll(ImmutableSet.of("foo"));
new ArrayList<Number>().removeAll(ImmutableSet.of(1));
new ArrayList<Number>().removeAll(ImmutableSet.of(2));
void testSetRemoveAllCollection() {
new HashSet<>().removeAll(ImmutableSet.of("foo"));
new HashSet<Number>().removeAll(ImmutableList.of(1));
new HashSet<Number>().removeAll(ImmutableSet.of(2));
}
ArrayList<String> testNewArrayListFromCollection() {