Skip to content

Commit

Permalink
Lean into DI for obviously DI-able helper classes within EP.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 501673994
  • Loading branch information
graememorgan authored and Error Prone Team committed Mar 20, 2023
1 parent 49fb044 commit 0a21362
Show file tree
Hide file tree
Showing 21 changed files with 93 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,13 @@ public abstract class AbstractReturnValueIgnored extends BugChecker

private final ConstantExpressions constantExpressions;

// TODO(ghm): Remove once possible.
protected AbstractReturnValueIgnored() {
this(ErrorProneFlags.empty());
this(ConstantExpressions.fromFlags(ErrorProneFlags.empty()));
}

protected AbstractReturnValueIgnored(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
protected AbstractReturnValueIgnored(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.common.collect.HashMultiset;
import com.google.common.collect.Multiset;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
Expand Down Expand Up @@ -60,8 +59,8 @@ public final class AlreadyChecked extends BugChecker implements CompilationUnitT
private final ConstantExpressions constantExpressions;

@Inject
public AlreadyChecked(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public AlreadyChecked(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Multiset;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
Expand Down Expand Up @@ -138,8 +137,8 @@ void validate(MethodInvocationTree tree, String argument) {
private final ConstantExpressions constantExpressions;

@Inject
public AlwaysThrows(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public AlwaysThrows(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import com.google.errorprone.bugpatterns.checkreturnvalue.ResultUsePolicyEvaluator;
import com.google.errorprone.bugpatterns.checkreturnvalue.ResultUsePolicyEvaluator.MethodInfo;
import com.google.errorprone.bugpatterns.checkreturnvalue.ResultUseRule.RuleScope;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
Expand Down Expand Up @@ -142,8 +143,8 @@ public MethodKind getMethodKind(MethodSymbol method) {
private final ResultUsePolicyEvaluator<VisitorState, Symbol, MethodSymbol> evaluator;

@Inject
public CheckReturnValue(ErrorProneFlags flags) {
super(flags);
public CheckReturnValue(ErrorProneFlags flags, ConstantExpressions constantExpressions) {
super(constantExpressions);
this.messageTrailerStyle =
flags
.getEnum("CheckReturnValue:MessageTrailerStyle", MessageTrailerStyle.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
Expand Down Expand Up @@ -83,9 +82,10 @@ public final class FieldCanBeStatic extends BugChecker implements VariableTreeMa
private final ConstantExpressions constantExpressions;

@Inject
public FieldCanBeStatic(ErrorProneFlags flags) {
this.wellKnownMutability = WellKnownMutability.fromFlags(flags);
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public FieldCanBeStatic(
WellKnownMutability wellKnownMutability, ConstantExpressions constantExpressions) {
this.wellKnownMutability = wellKnownMutability;
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
Expand All @@ -37,6 +38,7 @@
import java.util.Optional;
import java.util.concurrent.CompletionService;
import java.util.concurrent.ForkJoinTask;
import javax.inject.Inject;

/** See BugPattern annotation. */
@BugPattern(
Expand Down Expand Up @@ -126,6 +128,11 @@ public boolean matches(ExpressionTree tree, VisitorState state) {
}
};

@Inject
FutureReturnValueIgnored(ConstantExpressions constantExpressions) {
super(constantExpressions);
}

@Override
public Matcher<ExpressionTree> specializedMatcher() {
return MATCHER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

import com.google.common.collect.ImmutableMap;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
Expand Down Expand Up @@ -58,13 +58,9 @@ public final class IgnoredPureGetter extends AbstractReturnValueIgnored {
VisitorState.memoize(
state -> state.getTypeFromString("com.google.protobuf.MutableMessageLite"));

public IgnoredPureGetter() {
this(ErrorProneFlags.empty());
}

@Inject
public IgnoredPureGetter(ErrorProneFlags flags) {
super(flags);
public IgnoredPureGetter(ConstantExpressions constantExpressions) {
super(constantExpressions);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.common.collect.HashMultiset;
import com.google.common.collect.Multiset;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
Expand Down Expand Up @@ -61,8 +60,8 @@ public final class OptionalNotPresent extends BugChecker implements CompilationU
private final ConstantExpressions constantExpressions;

@Inject
public OptionalNotPresent(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public OptionalNotPresent(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
Expand All @@ -37,6 +38,7 @@
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import javax.inject.Inject;

/** Highlights cases where a proto's build method has its return value ignored. */
@BugPattern(
Expand Down Expand Up @@ -70,6 +72,11 @@ public final class ProtoBuilderReturnValueIgnored extends AbstractReturnValueIgn
.onDescendantOf("com.google.protobuf.MessageLite")
.namedAnyOf("toBuilder"));

@Inject
ProtoBuilderReturnValueIgnored(ConstantExpressions constantExpressions) {
super(constantExpressions);
}

@Override
public Matcher<? super ExpressionTree> specializedMatcher() {
return MATCHER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree.Kind;
Expand Down Expand Up @@ -408,21 +409,14 @@ private static boolean functionalMethod(ExpressionTree tree, VisitorState state)
// keep-sorted end
);

private static final ImmutableSet<Matcher<? super ExpressionTree>> ALL_MATCHERS =
ImmutableSet.of(SPECIALIZED_MATCHER, CLASS_METHODS);

private static final ImmutableBiMap<String, Matcher<ExpressionTree>> FLAG_MATCHERS =
ImmutableBiMap.of("ReturnValueIgnored:ClassMethods", CLASS_METHODS);

private final Matcher<ExpressionTree> matcher;

public ReturnValueIgnored() {
this.matcher = anyOf(ALL_MATCHERS);
}

@Inject
public ReturnValueIgnored(ErrorProneFlags flags) {
super(flags);
public ReturnValueIgnored(ErrorProneFlags flags, ConstantExpressions constantExpressions) {
super(constantExpressions);
this.matcher = createMatcher(flags);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
Expand All @@ -39,6 +40,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import javax.inject.Inject;

/** A {@link BugChecker}; see the associated {@link BugPattern} for details. */
@BugPattern(
Expand Down Expand Up @@ -104,6 +106,11 @@ private static boolean isExemptedMethod(ExpressionTree tree, VisitorState state)
RxReturnValueIgnored::hasCirvAnnotation,
RxReturnValueIgnored::isExemptedMethod)));

@Inject
RxReturnValueIgnored(ConstantExpressions constantExpressions) {
super(constantExpressions);
}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Description description = super.matchMethodInvocation(tree, state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
Expand Down Expand Up @@ -54,16 +53,11 @@
severity = WARNING,
tags = StandardTags.FRAGILE_CODE)
public class UnsynchronizedOverridesSynchronized extends BugChecker implements MethodTreeMatcher {

private final ConstantExpressions constantExpressions;

public UnsynchronizedOverridesSynchronized() {
this(ErrorProneFlags.empty());
}

@Inject
public UnsynchronizedOverridesSynchronized(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public UnsynchronizedOverridesSynchronized(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.SwitchTreeMatcher;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
Expand Down Expand Up @@ -61,8 +60,8 @@ public final class WrongOneof extends BugChecker implements SwitchTreeMatcher {
private final ConstantExpressions constantExpressions;

@Inject
public WrongOneof(ErrorProneFlags flags) {
this.constantExpressions = ConstantExpressions.fromFlags(flags);
public WrongOneof(ConstantExpressions constantExpressions) {
this.constantExpressions = constantExpressions;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.AbstractReturnValueIgnored;
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import javax.inject.Inject;

/**
* @author avenet@google.com (Arnaud J. Venet)
Expand All @@ -34,6 +36,11 @@
summary = "Return value of android.graphics.Rect.intersect() must be checked",
severity = ERROR)
public final class RectIntersectReturnValueIgnored extends AbstractReturnValueIgnored {
@Inject
RectIntersectReturnValueIgnored(ConstantExpressions constantExpressions) {
super(constantExpressions);
}

@Override
public Matcher<? super ExpressionTree> specializedMatcher() {
return instanceMethod().onExactClass("android.graphics.Rect").named("intersect");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
import javax.inject.Inject;
import org.checkerframework.checker.nullness.qual.Nullable;

/** Helper for establishing whether expressions correspond to a constant expression. */
public final class ConstantExpressions {
private final Matcher<ExpressionTree> pureMethods;
private final Supplier<ThreadSafety> threadSafety;

public ConstantExpressions(WellKnownMutability wellKnownMutability) {
@Inject
ConstantExpressions(WellKnownMutability wellKnownMutability) {
this.pureMethods =
anyOf(
basePureMethods,
Expand All @@ -91,8 +93,7 @@ public ConstantExpressions(WellKnownMutability wellKnownMutability) {
}

public static ConstantExpressions fromFlags(ErrorProneFlags flags) {
WellKnownMutability wellKnownMutability = WellKnownMutability.fromFlags(flags);
return new ConstantExpressions(wellKnownMutability);
return new ConstantExpressions(WellKnownMutability.fromFlags(flags));
}

/** Represents sets of things known to be true and false if a boolean statement evaluated true. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.bugpatterns.BugChecker;
Expand Down Expand Up @@ -61,8 +60,8 @@ public class ImmutableAnnotationChecker extends BugChecker implements ClassTreeM
private final WellKnownMutability wellKnownMutability;

@Inject
public ImmutableAnnotationChecker(ErrorProneFlags flags) {
this.wellKnownMutability = WellKnownMutability.fromFlags(flags);
public ImmutableAnnotationChecker(WellKnownMutability wellKnownMutability) {
this.wellKnownMutability = wellKnownMutability;
}

@Override
Expand Down

0 comments on commit 0a21362

Please sign in to comment.