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: 517924852
  • Loading branch information
graememorgan authored and Error Prone Team committed Mar 20, 2023
1 parent 49fb044 commit e1db1b9
Show file tree
Hide file tree
Showing 21 changed files with 93 additions and 78 deletions.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 e1db1b9

Please sign in to comment.