Skip to content

Commit e1baa20

Browse files
Stephan202Stephan202
committed
Extend CollectionRules Refaster rule collection
Primarily by introducing rules for APIs introduced between Java 17 and Java 21.
1 parent 16c61b1 commit e1baa20

File tree

3 files changed

+231
-5
lines changed

3 files changed

+231
-5
lines changed

‎error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java‎

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.common.collect.ImmutableList;
55
import com.google.common.collect.Iterables;
66
import com.google.common.collect.Lists;
7+
import com.google.common.collect.Streams;
78
import com.google.errorprone.refaster.Refaster;
89
import com.google.errorprone.refaster.annotation.AfterTemplate;
910
import com.google.errorprone.refaster.annotation.AlsoNegation;
@@ -17,6 +18,7 @@
1718
import java.util.NavigableSet;
1819
import java.util.Optional;
1920
import java.util.Queue;
21+
import java.util.SequencedCollection;
2022
import java.util.Set;
2123
import java.util.SortedSet;
2224
import java.util.function.Consumer;
@@ -399,7 +401,7 @@ Optional<T> before(Collection<T> collection) {
399401

400402
@BeforeTemplate
401403
Optional<T> before(List<T> collection) {
402-
return collection.isEmpty() ? Optional.empty() : Optional.of(collection.get(0));
404+
return collection.isEmpty() ? Optional.empty() : Optional.of(collection.getFirst());
403405
}
404406

405407
@BeforeTemplate
@@ -484,6 +486,143 @@ void after(Collection<T> collection, Consumer<? super T> consumer) {
484486
}
485487
}
486488

489+
/** Prefer {@code collection.iterator().next()} over more contrived alternatives. */
490+
static final class CollectionIteratorNext<S, T extends S> {
491+
@BeforeTemplate
492+
S before(Collection<T> collection) {
493+
return collection.stream().findFirst().orElseThrow();
494+
}
495+
496+
@AfterTemplate
497+
S after(Collection<T> collection) {
498+
return collection.iterator().next();
499+
}
500+
}
501+
502+
/** Prefer {@link SequencedCollection#getFirst()} over less idiomatic alternatives. */
503+
static final class SequencedCollectionGetFirst<S, T extends S> {
504+
@BeforeTemplate
505+
S before(SequencedCollection<T> collection) {
506+
return collection.iterator().next();
507+
}
508+
509+
@BeforeTemplate
510+
S before(List<T> collection) {
511+
return collection.get(0);
512+
}
513+
514+
@AfterTemplate
515+
S after(SequencedCollection<T> collection) {
516+
return collection.getFirst();
517+
}
518+
}
519+
520+
/** Prefer {@link SequencedCollection#getLast()} over less idiomatic alternatives. */
521+
static final class SequencedCollectionGetLast<S, T extends S> {
522+
@BeforeTemplate
523+
S before(SequencedCollection<T> collection) {
524+
return Refaster.anyOf(
525+
collection.reversed().getFirst(), Streams.findLast(collection.stream()).orElseThrow());
526+
}
527+
528+
@BeforeTemplate
529+
S before(List<T> collection) {
530+
return collection.get(collection.size() - 1);
531+
}
532+
533+
@AfterTemplate
534+
S after(SequencedCollection<T> collection) {
535+
return collection.getLast();
536+
}
537+
}
538+
539+
/** Prefer {@link List#addFirst(Object)} over less idiomatic alternatives. */
540+
static final class ListAddFirst<S, T extends S> {
541+
@BeforeTemplate
542+
void before(List<S> list, T element) {
543+
list.add(0, element);
544+
}
545+
546+
@AfterTemplate
547+
void after(List<S> list, T element) {
548+
list.addFirst(element);
549+
}
550+
}
551+
552+
/** Prefer {@link List#add(Object)} over less idiomatic alternatives. */
553+
static final class ListAdd<S, T extends S> {
554+
@BeforeTemplate
555+
void before(List<S> list, T element) {
556+
list.addLast(element);
557+
}
558+
559+
@BeforeTemplate
560+
void before2(List<S> list, T element) {
561+
list.add(list.size(), element);
562+
}
563+
564+
@AfterTemplate
565+
void after(List<S> list, T element) {
566+
list.add(element);
567+
}
568+
}
569+
570+
/** Prefer {@link List#removeFirst()}} over less idiomatic alternatives. */
571+
// XXX: This rule changes the exception thrown for empty lists from `IndexOutOfBoundsException` to
572+
// `NoSuchElementException`.
573+
static final class ListRemoveFirst<S, T extends S> {
574+
@BeforeTemplate
575+
S before(List<T> list) {
576+
return list.remove(0);
577+
}
578+
579+
@AfterTemplate
580+
S after(List<T> list) {
581+
return list.removeFirst();
582+
}
583+
}
584+
585+
/** Prefer {@link List#removeLast()}} over less idiomatic alternatives. */
586+
// XXX: This rule changes the exception thrown for empty lists from `IndexOutOfBoundsException` to
587+
// `NoSuchElementException`.
588+
static final class ListRemoveLast<S, T extends S> {
589+
@BeforeTemplate
590+
S before(List<T> list) {
591+
return list.remove(list.size() - 1);
592+
}
593+
594+
@AfterTemplate
595+
S after(List<T> list) {
596+
return list.removeLast();
597+
}
598+
}
599+
600+
/** Prefer {@link SortedSet#first()} over more verbose alternatives. */
601+
static final class SortedSetFirst<S, T extends S> {
602+
@BeforeTemplate
603+
S before(SortedSet<T> set) {
604+
return set.getFirst();
605+
}
606+
607+
@AfterTemplate
608+
S after(SortedSet<T> set) {
609+
return set.first();
610+
}
611+
}
612+
613+
/** Prefer {@link SortedSet#last()} over more verbose alternatives. */
614+
static final class SortedSetLast<S, T extends S> {
615+
@BeforeTemplate
616+
S before(SortedSet<T> set) {
617+
return set.getLast();
618+
}
619+
620+
@AfterTemplate
621+
S after(SortedSet<T> set) {
622+
return set.last();
623+
}
624+
}
625+
487626
// XXX: collection.stream().noneMatch(e -> e.equals(other))
488627
// ^ This is !collection.contains(other). Do we already rewrite variations on this?
489628
}

‎error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.common.collect.ImmutableSortedSet;
66
import com.google.common.collect.Iterables;
77
import com.google.common.collect.Lists;
8+
import com.google.common.collect.Streams;
89
import java.util.ArrayList;
910
import java.util.Arrays;
1011
import java.util.HashSet;
@@ -19,7 +20,7 @@
1920
final class CollectionRulesTest implements RefasterRuleCollectionTestCase {
2021
@Override
2122
public ImmutableSet<Object> elidedTypesAndStaticImports() {
22-
return ImmutableSet.of(Iterables.class, Lists.class);
23+
return ImmutableSet.of(Iterables.class, Lists.class, Streams.class);
2324
}
2425

2526
ImmutableSet<Boolean> testCollectionIsEmpty() {
@@ -130,14 +131,18 @@ ImmutableSet<Optional<Integer>> testOptionalFirstCollectionElement() {
130131
ImmutableSet.of(1).isEmpty()
131132
? Optional.empty()
132133
: Optional.of(ImmutableSet.of(1).iterator().next()),
133-
ImmutableList.of(2).isEmpty() ? Optional.empty() : Optional.of(ImmutableList.of(2).get(0)),
134+
ImmutableList.of(2).isEmpty()
135+
? Optional.empty()
136+
: Optional.of(ImmutableList.of(2).getFirst()),
134137
ImmutableSortedSet.of(3).isEmpty()
135138
? Optional.empty()
136139
: Optional.of(ImmutableSortedSet.of(3).first()),
137140
!ImmutableSet.of(1).isEmpty()
138141
? Optional.of(ImmutableSet.of(1).iterator().next())
139142
: Optional.empty(),
140-
!ImmutableList.of(2).isEmpty() ? Optional.of(ImmutableList.of(2).get(0)) : Optional.empty(),
143+
!ImmutableList.of(2).isEmpty()
144+
? Optional.of(ImmutableList.of(2).getFirst())
145+
: Optional.empty(),
141146
!ImmutableSortedSet.of(3).isEmpty()
142147
? Optional.of(ImmutableSortedSet.of(3).first())
143148
: Optional.empty());
@@ -207,4 +212,45 @@ ImmutableSet<Optional<String>> testRemoveOptionalFirstQueueElement() {
207212
void testCollectionForEach() {
208213
ImmutableSet.of(1).stream().forEach(String::valueOf);
209214
}
215+
216+
String testCollectionIteratorNext() {
217+
return ImmutableSet.of("foo").stream().findFirst().orElseThrow();
218+
}
219+
220+
ImmutableSet<String> testSequencedCollectionGetFirst() {
221+
return ImmutableSet.of(
222+
ImmutableList.of("foo").iterator().next(), ImmutableList.of("bar").get(0));
223+
}
224+
225+
ImmutableSet<String> testSequencedCollectionGetLast() {
226+
return ImmutableSet.of(
227+
ImmutableList.of("foo").reversed().getFirst(),
228+
Streams.findLast(ImmutableList.of("bar").stream()).orElseThrow(),
229+
ImmutableList.of("baz").get(ImmutableList.of("baz").size() - 1));
230+
}
231+
232+
void testListAddFirst() {
233+
new ArrayList<String>().add(0, "foo");
234+
}
235+
236+
void testListAdd() {
237+
new ArrayList<String>(0).addLast("bar");
238+
new ArrayList<String>(1).add(new ArrayList<String>(1).size(), "qux");
239+
}
240+
241+
String testListRemoveFirst() {
242+
return new ArrayList<String>().remove(0);
243+
}
244+
245+
String testListRemoveLast() {
246+
return new ArrayList<String>().remove(new ArrayList<String>().size() - 1);
247+
}
248+
249+
String testSortedSetFirst() {
250+
return ImmutableSortedSet.of("foo").getFirst();
251+
}
252+
253+
String testSortedSetLast() {
254+
return ImmutableSortedSet.of("foo").getLast();
255+
}
210256
}

‎error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.common.collect.ImmutableSortedSet;
66
import com.google.common.collect.Iterables;
77
import com.google.common.collect.Lists;
8+
import com.google.common.collect.Streams;
89
import java.util.ArrayList;
910
import java.util.Arrays;
1011
import java.util.HashSet;
@@ -19,7 +20,7 @@
1920
final class CollectionRulesTest implements RefasterRuleCollectionTestCase {
2021
@Override
2122
public ImmutableSet<Object> elidedTypesAndStaticImports() {
22-
return ImmutableSet.of(Iterables.class, Lists.class);
23+
return ImmutableSet.of(Iterables.class, Lists.class, Streams.class);
2324
}
2425

2526
ImmutableSet<Boolean> testCollectionIsEmpty() {
@@ -156,4 +157,44 @@ ImmutableSet<Optional<String>> testRemoveOptionalFirstQueueElement() {
156157
void testCollectionForEach() {
157158
ImmutableSet.of(1).forEach(String::valueOf);
158159
}
160+
161+
String testCollectionIteratorNext() {
162+
return ImmutableSet.of("foo").iterator().next();
163+
}
164+
165+
ImmutableSet<String> testSequencedCollectionGetFirst() {
166+
return ImmutableSet.of(ImmutableList.of("foo").getFirst(), ImmutableList.of("bar").getFirst());
167+
}
168+
169+
ImmutableSet<String> testSequencedCollectionGetLast() {
170+
return ImmutableSet.of(
171+
ImmutableList.of("foo").getLast(),
172+
ImmutableList.of("bar").getLast(),
173+
ImmutableList.of("baz").getLast());
174+
}
175+
176+
void testListAddFirst() {
177+
new ArrayList<String>().addFirst("foo");
178+
}
179+
180+
void testListAdd() {
181+
new ArrayList<String>(0).add("bar");
182+
new ArrayList<String>(1).add("qux");
183+
}
184+
185+
String testListRemoveFirst() {
186+
return new ArrayList<String>().removeFirst();
187+
}
188+
189+
String testListRemoveLast() {
190+
return new ArrayList<String>().removeLast();
191+
}
192+
193+
String testSortedSetFirst() {
194+
return ImmutableSortedSet.of("foo").first();
195+
}
196+
197+
String testSortedSetLast() {
198+
return ImmutableSortedSet.of("foo").last();
199+
}
159200
}

0 commit comments

Comments
 (0)