-
Notifications
You must be signed in to change notification settings - Fork 37
/
AssortedRules.java
236 lines (211 loc) · 7.98 KB
/
AssortedRules.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
package tech.picnic.errorprone.refasterrules;
import static com.google.common.base.Preconditions.checkElementIndex;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Sets.toImmutableEnumSet;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import static java.util.Collections.disjoint;
import static java.util.Objects.checkIndex;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
/**
* Assorted Refaster rules that do not (yet) belong in one of the other classes with more topical
* Refaster rules.
*/
@OnlineDocumentation
final class AssortedRules {
private AssortedRules() {}
/** Prefer {@link Objects#checkIndex(int, int)} over the Guava alternative. */
static final class CheckIndex {
@BeforeTemplate
int before(int index, int size) {
return checkElementIndex(index, size);
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
int after(int index, int size) {
return checkIndex(index, size);
}
}
/**
* Prefer {@link Objects#checkIndex(int, int)} over less descriptive or more verbose alternatives.
*
* <p>If a custom error message is desired, consider using Guava's {@link
* com.google.common.base.Preconditions#checkElementIndex(int, int, String)}.
*/
static final class CheckIndexConditional {
@BeforeTemplate
void before(int index, int size) {
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException();
}
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
void after(int index, int size) {
checkIndex(index, size);
}
}
/**
* Use {@link Sets#toImmutableEnumSet()} when possible, as it is more efficient than {@link
* ImmutableSet#toImmutableSet()} and produces a more compact object.
*
* <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving: while the
* original code produces a set that iterates over the elements in encounter order, the
* replacement code iterates over the elements in enum definition order.
*/
// XXX: ^ Consider emitting a comment warning about this fact?
static final class StreamToImmutableEnumSet<T extends Enum<T>> {
@BeforeTemplate
ImmutableSet<T> before(Stream<T> stream) {
return stream.collect(toImmutableSet());
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
ImmutableSet<T> after(Stream<T> stream) {
return stream.collect(toImmutableEnumSet());
}
}
/** Prefer {@link Iterators#getNext(Iterator, Object)} over more contrived alternatives. */
static final class IteratorGetNextOrDefault<T> {
@BeforeTemplate
T before(Iterator<T> iterator, T defaultValue) {
return Refaster.anyOf(
iterator.hasNext() ? iterator.next() : defaultValue,
Streams.stream(iterator).findFirst().orElse(defaultValue),
Streams.stream(iterator).findAny().orElse(defaultValue));
}
@AfterTemplate
@Nullable T after(Iterator<T> iterator, T defaultValue) {
return Iterators.getNext(iterator, defaultValue);
}
}
/** Don't unnecessarily repeat boolean expressions. */
// XXX: This rule captures only the simplest case. `@AlsoNegation` doesn't help. Consider
// contributing a Refaster patch, which handles the negation in the `@BeforeTemplate` more
// intelligently.
static final class LogicalImplication {
@BeforeTemplate
@SuppressWarnings("java:S2589" /* This violation will be rewritten. */)
boolean before(boolean firstTest, boolean secondTest) {
return firstTest || (!firstTest && secondTest);
}
@AfterTemplate
boolean after(boolean firstTest, boolean secondTest) {
return firstTest || secondTest;
}
}
/**
* Prefer {@link Stream#generate(java.util.function.Supplier)} over more contrived alternatives.
*/
static final class UnboundedSingleElementStream<T> {
@BeforeTemplate
Stream<T> before(T object) {
return Streams.stream(Iterables.cycle(object));
}
@AfterTemplate
Stream<T> after(T object) {
return Stream.generate(() -> object);
}
}
/**
* Prefer {@link Collections#disjoint(Collection, Collection)} over more contrived alternatives.
*/
static final class DisjointSets<T> {
@BeforeTemplate
boolean before(Set<T> set1, Set<T> set2) {
return Sets.intersection(set1, set2).isEmpty();
}
@BeforeTemplate
boolean before2(Set<T> set1, Set<T> set2) {
return set1.stream().noneMatch(set2::contains);
}
@AfterTemplate
boolean after(Set<T> set1, Set<T> set2) {
return disjoint(set1, set2);
}
}
/**
* Don't unnecessarily copy collections before passing them to {@link
* Collections#disjoint(Collection, Collection)}.
*/
// XXX: Other copy operations could be elided too, but these are most common after application of
// the `DisjointSets` rule defined above. If we ever introduce a generic "makes a copy" stand-in,
// use it here.
static final class DisjointCollections<T> {
@BeforeTemplate
boolean before(Collection<T> collection1, Collection<T> collection2) {
return Refaster.anyOf(
disjoint(ImmutableSet.copyOf(collection1), collection2),
disjoint(new HashSet<>(collection1), collection2),
disjoint(collection1, ImmutableSet.copyOf(collection2)),
disjoint(collection1, new HashSet<>(collection2)));
}
@AfterTemplate
boolean after(Collection<T> collection1, Collection<T> collection2) {
return disjoint(collection1, collection2);
}
}
/** Prefer {@link Iterables#isEmpty(Iterable)} over more contrived alternatives. */
static final class IterableIsEmpty<T> {
@BeforeTemplate
boolean before(Iterable<T> iterable) {
return !iterable.iterator().hasNext();
}
@AfterTemplate
boolean after(Iterable<T> iterable) {
return Iterables.isEmpty(iterable);
}
}
/** Prefer {@link Splitter#splitToStream(CharSequence)} over less efficient alternatives. */
static final class SplitToStream {
@BeforeTemplate
Stream<String> before(Splitter splitter, CharSequence charSequence) {
return Refaster.anyOf(
Streams.stream(splitter.split(charSequence)),
splitter.splitToList(charSequence).stream());
}
@AfterTemplate
Stream<String> after(Splitter splitter, CharSequence charSequence) {
return splitter.splitToStream(charSequence);
}
}
// /**
// * Don't unnecessarily pass a method reference to {@link Supplier#get()} or wrap this method
// * in a lambda expression.
// */
// // XXX: This rule rewrites both expressions and statements (good), but does not ensure that the
// // actually `anyStatement` accepts a `Supplier<T>`. For example, it will also match if the
// // statement requires a `com.google.common.base.Supplier` rather than a
// // `java.util.function.Supplier`. Investigate how we can improve Refaster matching support.
// abstract static class SupplierAsSupplier<T> {
// @Placeholder
// abstract void anyStatement(Supplier<T> supplier);
//
// @BeforeTemplate
// void before(Supplier<T> supplier) {
// anyStatement(() -> supplier.get());
// }
//
// @AfterTemplate
// void after(Supplier<T> supplier) {
// anyStatement(supplier);
// }
// }
}