Skip to content

Commit 1ba080b

Browse files
graememorganError Prone Team
authored andcommitted
Switch to a sealed interface of records rather than an AutoOneof in ConstantExpressions.
PiperOrigin-RevId: 827944832
1 parent f12e7e3 commit 1ba080b

File tree

2 files changed

+76
-134
lines changed

2 files changed

+76
-134
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/OptionalNotPresent.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@
2929
import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
3030
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
3131
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression;
32-
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression.ConstantExpressionKind;
33-
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.PureMethodInvocation;
32+
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression.PureMethodInvocation;
3433
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.Truthiness;
3534
import com.google.errorprone.matchers.Description;
3635
import com.google.errorprone.matchers.Matcher;
@@ -148,8 +147,7 @@ private void checkForEmptiness(
148147

149148
private Stream<PureMethodInvocation> getMethodInvocations(Multiset<ConstantExpression> truths) {
150149
return truths.stream()
151-
.filter(truth -> truth.kind().equals(ConstantExpressionKind.PURE_METHOD))
152-
.map(ConstantExpression::pureMethod);
150+
.flatMap(ce -> ce instanceof PureMethodInvocation pmi ? Stream.of(pmi) : Stream.empty());
153151
}
154152
}
155153
}

core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ConstantExpressions.java

Lines changed: 74 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,11 @@
3636
import static java.util.stream.Collectors.joining;
3737
import static javax.lang.model.element.Modifier.ABSTRACT;
3838

39-
import com.google.auto.value.AutoOneOf;
40-
import com.google.auto.value.AutoValue;
4139
import com.google.common.collect.ImmutableList;
4240
import com.google.common.collect.ImmutableSet;
4341
import com.google.errorprone.ErrorProneFlags;
4442
import com.google.errorprone.VisitorState;
4543
import com.google.errorprone.annotations.Immutable;
46-
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression.ConstantExpressionKind;
4744
import com.google.errorprone.matchers.Matcher;
4845
import com.google.errorprone.matchers.Matchers;
4946
import com.google.errorprone.suppliers.Supplier;
@@ -102,18 +99,9 @@ public static ConstantExpressions fromFlags(ErrorProneFlags flags) {
10299
}
103100

104101
/** Represents sets of things known to be true and false if a boolean statement evaluated true. */
105-
@AutoValue
106-
public abstract static class Truthiness {
107-
public abstract ImmutableSet<ConstantExpression> requiredTrue();
108-
109-
public abstract ImmutableSet<ConstantExpression> requiredFalse();
110-
111-
private static Truthiness create(
112-
Iterable<ConstantExpression> requiredTrue, Iterable<ConstantExpression> requiredFalse) {
113-
return new AutoValue_ConstantExpressions_Truthiness(
114-
ImmutableSet.copyOf(requiredTrue), ImmutableSet.copyOf(requiredFalse));
115-
}
116-
}
102+
public record Truthiness(
103+
ImmutableSet<ConstantExpression> requiredTrue,
104+
ImmutableSet<ConstantExpression> requiredFalse) {}
117105

118106
/**
119107
* Scans an {@link ExpressionTree} to find anything guaranteed to be false or true if this
@@ -193,61 +181,86 @@ private void add(ConstantExpression e) {
193181
}.visit(tree, null);
194182

195183
if (failed.get()) {
196-
return Truthiness.create(ImmutableSet.of(), ImmutableSet.of());
184+
return new Truthiness(ImmutableSet.of(), ImmutableSet.of());
197185
}
198186

199-
return Truthiness.create(requiredTrue.build(), requiredFalse.build());
187+
return new Truthiness(requiredTrue.build(), requiredFalse.build());
200188
}
201189

202190
/** Represents a constant expression. */
203-
@AutoOneOf(ConstantExpressionKind.class)
204-
public abstract static class ConstantExpression {
205-
/** The kind of a constant expression. */
206-
public enum ConstantExpressionKind {
207-
LITERAL,
208-
CONSTANT_EQUALS,
209-
PURE_METHOD,
210-
}
211-
212-
public abstract ConstantExpressionKind kind();
213-
214-
abstract Object literal();
191+
public sealed interface ConstantExpression {
192+
/** Represents a constant literal. */
193+
public static record Literal(Object object) implements ConstantExpression {
194+
@Override
195+
public void accept(ConstantExpressionVisitor visitor) {
196+
visitor.visitConstant(object());
197+
}
215198

216-
private static ConstantExpression literal(Object object) {
217-
return AutoOneOf_ConstantExpressions_ConstantExpression.literal(object);
199+
@Override
200+
public final String toString() {
201+
return object().toString();
202+
}
218203
}
219204

220-
abstract ConstantEquals constantEquals();
205+
/** Represents a binary equals call on two constant expressions. */
206+
public static record ConstantEquals(ConstantExpression lhs, ConstantExpression rhs)
207+
implements ConstantExpression {
208+
@Override
209+
public void accept(ConstantExpressionVisitor visitor) {
210+
lhs().accept(visitor);
211+
rhs().accept(visitor);
212+
}
221213

222-
private static ConstantExpression constantEquals(ConstantEquals constantEquals) {
223-
return AutoOneOf_ConstantExpressions_ConstantExpression.constantEquals(constantEquals);
224-
}
214+
@Override
215+
public final boolean equals(@Nullable Object other) {
216+
if (!(other instanceof ConstantEquals that)) {
217+
return false;
218+
}
219+
return (lhs().equals(that.lhs()) && rhs().equals(that.rhs()))
220+
|| (lhs().equals(that.rhs()) && rhs().equals(that.lhs()));
221+
}
225222

226-
public abstract PureMethodInvocation pureMethod();
223+
@Override
224+
public final String toString() {
225+
return format("%s equals %s", lhs(), rhs());
226+
}
227227

228-
private static ConstantExpression pureMethod(PureMethodInvocation pureMethodInvocation) {
229-
return AutoOneOf_ConstantExpressions_ConstantExpression.pureMethod(pureMethodInvocation);
228+
@Override
229+
public final int hashCode() {
230+
return lhs().hashCode() + rhs().hashCode();
231+
}
230232
}
231233

232-
@Override
233-
public final String toString() {
234-
return switch (kind()) {
235-
case LITERAL -> literal().toString();
236-
case CONSTANT_EQUALS -> constantEquals().toString();
237-
case PURE_METHOD -> pureMethod().toString();
238-
};
239-
}
234+
/**
235+
* Represents both a constant method call or a constant field/local access, depending on the
236+
* actual type of {@code symbol}.
237+
*/
238+
public static record PureMethodInvocation(
239+
Symbol symbol,
240+
ImmutableList<ConstantExpression> arguments,
241+
Optional<ConstantExpression> receiver)
242+
implements ConstantExpression {
243+
@Override
244+
public void accept(ConstantExpressionVisitor visitor) {
245+
visitor.visitIdentifier(symbol());
246+
arguments().forEach(a -> a.accept(visitor));
247+
receiver().ifPresent(r -> r.accept(visitor));
248+
}
240249

241-
public void accept(ConstantExpressionVisitor visitor) {
242-
switch (kind()) {
243-
case LITERAL -> visitor.visitConstant(literal());
244-
case CONSTANT_EQUALS -> {
245-
constantEquals().lhs().accept(visitor);
246-
constantEquals().rhs().accept(visitor);
250+
@Override
251+
public final String toString() {
252+
String receiver = receiver().map(r -> r + ".").orElse("");
253+
if (symbol() instanceof VarSymbol || symbol() instanceof ClassSymbol) {
254+
return receiver + symbol().getSimpleName();
247255
}
248-
case PURE_METHOD -> pureMethod().accept(visitor);
256+
return receiver
257+
+ (isStatic(symbol()) ? symbol().owner.getSimpleName() + "." : "")
258+
+ symbol().getSimpleName()
259+
+ arguments().stream().map(Object::toString).collect(joining(", ", "(", ")"));
249260
}
250261
}
262+
263+
void accept(ConstantExpressionVisitor visitor);
251264
}
252265

253266
public Optional<ConstantExpression> constantExpression(ExpressionTree tree, VisitorState state) {
@@ -257,15 +270,14 @@ public Optional<ConstantExpression> constantExpression(ExpressionTree tree, Visi
257270
Optional<ConstantExpression> lhs = constantExpression(binaryTree.getLeftOperand(), state);
258271
Optional<ConstantExpression> rhs = constantExpression(binaryTree.getRightOperand(), state);
259272
if (lhs.isPresent() && rhs.isPresent()) {
260-
return Optional.of(
261-
ConstantExpression.constantEquals(ConstantEquals.of(lhs.get(), rhs.get())));
273+
return Optional.of(new ConstantExpression.ConstantEquals(lhs.get(), rhs.get()));
262274
}
263275
}
264276
Object value = constValue(tree);
265277
if (value != null && tree instanceof LiteralTree) {
266-
return Optional.of(ConstantExpression.literal(value));
278+
return Optional.of(new ConstantExpression.Literal(value));
267279
}
268-
return symbolizeImmutableExpression(tree, state).map(ConstantExpression::pureMethod);
280+
return symbolizeImmutableExpression(tree, state).map(x -> x);
269281
}
270282

271283
/** Returns whether {@code aTree} and {@code bTree} seem to correspond to the same expression. */
@@ -278,81 +290,11 @@ public boolean isSame(ExpressionTree aTree, ExpressionTree bTree, VisitorState s
278290
return b.isPresent() && a.get().equals(b.get());
279291
}
280292

281-
/** Represents a binary equals call on two constant expressions. */
282-
@AutoValue
283-
public abstract static class ConstantEquals {
284-
abstract ConstantExpression lhs();
285-
286-
abstract ConstantExpression rhs();
287-
288-
@Override
289-
public final boolean equals(@Nullable Object other) {
290-
if (!(other instanceof ConstantEquals that)) {
291-
return false;
292-
}
293-
return (lhs().equals(that.lhs()) && rhs().equals(that.rhs()))
294-
|| (lhs().equals(that.rhs()) && rhs().equals(that.lhs()));
295-
}
296-
297-
@Override
298-
public final String toString() {
299-
return format("%s equals %s", lhs(), rhs());
300-
}
301-
302-
@Override
303-
public final int hashCode() {
304-
return lhs().hashCode() + rhs().hashCode();
305-
}
306-
307-
static ConstantEquals of(ConstantExpression lhs, ConstantExpression rhs) {
308-
return new AutoValue_ConstantExpressions_ConstantEquals(lhs, rhs);
309-
}
310-
}
311-
312-
/**
313-
* Represents both a constant method call or a constant field/local access, depending on the
314-
* actual type of {@code symbol}.
315-
*/
316-
@AutoValue
317-
public abstract static class PureMethodInvocation {
318-
public abstract Symbol symbol();
319-
320-
abstract ImmutableList<ConstantExpression> arguments();
321-
322-
public abstract Optional<ConstantExpression> receiver();
323-
324-
@Override
325-
public final String toString() {
326-
String receiver = receiver().map(r -> r + ".").orElse("");
327-
if (symbol() instanceof VarSymbol || symbol() instanceof ClassSymbol) {
328-
return receiver + symbol().getSimpleName();
329-
}
330-
return receiver
331-
+ (isStatic(symbol()) ? symbol().owner.getSimpleName() + "." : "")
332-
+ symbol().getSimpleName()
333-
+ arguments().stream().map(Object::toString).collect(joining(", ", "(", ")"));
334-
}
335-
336-
private static PureMethodInvocation of(
337-
Symbol symbol,
338-
Iterable<ConstantExpression> arguments,
339-
Optional<ConstantExpression> receiver) {
340-
return new AutoValue_ConstantExpressions_PureMethodInvocation(
341-
symbol, ImmutableList.copyOf(arguments), receiver);
342-
}
343-
344-
public void accept(ConstantExpressionVisitor visitor) {
345-
visitor.visitIdentifier(symbol());
346-
arguments().forEach(a -> a.accept(visitor));
347-
receiver().ifPresent(r -> r.accept(visitor));
348-
}
349-
}
350-
351293
/**
352294
* Returns a list of the methods called to get to this expression, as well as a terminating
353295
* variable if needed.
354296
*/
355-
public Optional<PureMethodInvocation> symbolizeImmutableExpression(
297+
public Optional<ConstantExpression.PureMethodInvocation> symbolizeImmutableExpression(
356298
ExpressionTree tree, VisitorState state) {
357299
var receiver =
358300
tree instanceof MethodInvocationTree || tree instanceof MemberSelectTree
@@ -372,7 +314,8 @@ public Optional<PureMethodInvocation> symbolizeImmutableExpression(
372314

373315
if (isPureIdentifier(tree)) {
374316
return Optional.of(
375-
PureMethodInvocation.of(getSymbol(tree), ImmutableList.of(), receiverConstant));
317+
new ConstantExpression.PureMethodInvocation(
318+
getSymbol(tree), ImmutableList.of(), receiverConstant));
376319
} else if (tree instanceof MethodInvocationTree methodInvocationTree
377320
&& pureMethods.matches(tree, state)) {
378321
ImmutableList.Builder<ConstantExpression> arguments = ImmutableList.builder();
@@ -384,7 +327,8 @@ public Optional<PureMethodInvocation> symbolizeImmutableExpression(
384327
arguments.add(argumentConstant.get());
385328
}
386329
return Optional.of(
387-
PureMethodInvocation.of(getSymbol(tree), arguments.build(), receiverConstant));
330+
new ConstantExpression.PureMethodInvocation(
331+
getSymbol(tree), arguments.build(), receiverConstant));
388332
} else {
389333
return Optional.empty();
390334
}

0 commit comments

Comments
 (0)