Skip to content

Commit 6ea1b7c

Browse files
Stephan202Stephan202
committed
More new rules
1 parent 9d1e29a commit 6ea1b7c

File tree

3 files changed

+198
-18
lines changed

3 files changed

+198
-18
lines changed

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

Lines changed: 121 additions & 8 deletions
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;
@@ -484,29 +486,140 @@ void after(Collection<T> collection, Consumer<? super T> consumer) {
484486
}
485487
}
486488

487-
/** Prefer {@link List#getFirst()} over less idiomatic alternatives. */
488-
static final class ListGetFirst<T> {
489+
/** Prefer {@code collection.iterator().next()} over more contrived alternatives. */
490+
static final class CollectionIteratorNext<T> {
491+
@BeforeTemplate
492+
T before(Collection<T> collection) {
493+
return collection.stream().findFirst().orElseThrow();
494+
}
495+
496+
@AfterTemplate
497+
T 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<T> {
504+
@BeforeTemplate
505+
T before(SequencedCollection<T> collection) {
506+
return collection.iterator().next();
507+
}
508+
509+
@BeforeTemplate
510+
T before(List<T> collection) {
511+
return collection.get(0);
512+
}
513+
514+
@AfterTemplate
515+
T 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<T> {
522+
@BeforeTemplate
523+
T before(SequencedCollection<T> collection) {
524+
return Refaster.anyOf(
525+
collection.reversed().getFirst(), Streams.findLast(collection.stream()).orElseThrow());
526+
}
527+
528+
@BeforeTemplate
529+
T before(List<T> collection) {
530+
return collection.get(collection.size() - 1);
531+
}
532+
533+
@AfterTemplate
534+
T 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<T> {
489574
@BeforeTemplate
490575
T before(List<T> list) {
491-
return list.get(0);
576+
return list.remove(0);
492577
}
493578

494579
@AfterTemplate
495580
T after(List<T> list) {
496-
return list.getFirst();
581+
return list.removeFirst();
497582
}
498583
}
499584

500-
/** Prefer {@link List#getLast()} over less idiomatic alternatives. */
501-
static final class ListGetLast<T> {
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<T> {
502589
@BeforeTemplate
503590
T before(List<T> list) {
504-
return list.get(list.size() - 1);
591+
return list.remove(list.size() - 1);
505592
}
506593

507594
@AfterTemplate
508595
T after(List<T> list) {
509-
return list.getLast();
596+
return list.removeLast();
597+
}
598+
}
599+
600+
/** Prefer {@link SortedSet#first()} over more verbose alternatives. */
601+
static final class SortedSetFirst<T> {
602+
@BeforeTemplate
603+
T before(SortedSet<T> set) {
604+
return set.getFirst();
605+
}
606+
607+
@AfterTemplate
608+
T 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<T> {
615+
@BeforeTemplate
616+
T before(SortedSet<T> set) {
617+
return set.getLast();
618+
}
619+
620+
@AfterTemplate
621+
T after(SortedSet<T> set) {
622+
return set.last();
510623
}
511624
}
512625

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

Lines changed: 39 additions & 5 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() {
@@ -212,11 +213,44 @@ void testCollectionForEach() {
212213
ImmutableSet.of(1).stream().forEach(String::valueOf);
213214
}
214215

215-
String testListGetFirst() {
216-
return ImmutableList.of("foo").get(0);
216+
String testCollectionIteratorNext() {
217+
return ImmutableSet.of("foo").stream().findFirst().orElseThrow();
217218
}
218219

219-
String testListGetLast() {
220-
return ImmutableList.of("foo").get(ImmutableList.of("foo").size() - 1);
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();
221255
}
222256
}

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

Lines changed: 38 additions & 5 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() {
@@ -157,11 +158,43 @@ void testCollectionForEach() {
157158
ImmutableSet.of(1).forEach(String::valueOf);
158159
}
159160

160-
String testListGetFirst() {
161-
return ImmutableList.of("foo").getFirst();
161+
String testCollectionIteratorNext() {
162+
return ImmutableSet.of("foo").iterator().next();
162163
}
163164

164-
String testListGetLast() {
165-
return ImmutableList.of("foo").getLast();
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();
166199
}
167200
}

0 commit comments

Comments
 (0)