diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java index df44f14e873..d147599d9cd 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java @@ -22,10 +22,8 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; import com.google.errorprone.annotations.CheckReturnValue; -import com.google.errorprone.annotations.Immutable; import com.google.errorprone.annotations.ImmutableTypeParameter; import com.google.errorprone.annotations.concurrent.LazyInit; -import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.threadsafety.ThreadSafety.Purpose; import com.google.errorprone.bugpatterns.threadsafety.ThreadSafety.Violation; import com.google.errorprone.fixes.SuggestedFix; @@ -45,6 +43,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.BiPredicate; import java.util.function.Predicate; import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; @@ -52,19 +51,19 @@ import org.checkerframework.checker.nullness.qual.Nullable; /** Analyzes types for deep immutability. */ -public class ImmutableAnalysis { +public final class ImmutableAnalysis { - private final BugChecker bugChecker; + private final BiPredicate suppressionChecker; private final VisitorState state; private final WellKnownMutability wellKnownMutability; private final ThreadSafety threadSafety; ImmutableAnalysis( - BugChecker bugChecker, + BiPredicate suppressionChecker, VisitorState state, WellKnownMutability wellKnownMutability, ImmutableSet immutableAnnotations) { - this.bugChecker = bugChecker; + this.suppressionChecker = suppressionChecker; this.state = state; this.wellKnownMutability = wellKnownMutability; this.threadSafety = @@ -76,11 +75,6 @@ public class ImmutableAnalysis { .build(state); } - public ImmutableAnalysis( - BugChecker bugChecker, VisitorState state, WellKnownMutability wellKnownMutability) { - this(bugChecker, state, wellKnownMutability, ImmutableSet.of(Immutable.class.getName())); - } - Violation isThreadSafeType( boolean allowContainerTypeParameters, Set containerTypeParameters, Type type) { return threadSafety.isThreadSafeType( @@ -140,11 +134,11 @@ public Violation checkForImmutability( if (interfaceAnnotation == null) { continue; } - info = + Violation superInfo = threadSafety.checkSuperInstantiation( immutableTyParams, interfaceAnnotation, interfaceType); - if (info.isPresent()) { - return info.plus( + if (superInfo.isPresent()) { + return superInfo.plus( String.format( "'%s' extends '%s'", threadSafety.getPrettyName(type.tsym), @@ -154,14 +148,14 @@ public Violation checkForImmutability( if (!type.asElement().isEnum()) { // don't check enum super types here to avoid double-reporting errors - info = checkSuper(immutableTyParams, type); - if (info.isPresent()) { - return info; + Violation superInfo = checkSuper(immutableTyParams, type, reporter); + if (superInfo.isPresent()) { + return superInfo; } } Type mutableEnclosing = threadSafety.mutableEnclosingInstance(tree, type); if (mutableEnclosing != null) { - return info.plus( + return Violation.of( String.format( "'%s' has mutable enclosing instance '%s'", threadSafety.getPrettyName(type.tsym), mutableEnclosing)); @@ -169,7 +163,8 @@ public Violation checkForImmutability( return Violation.absent(); } - private Violation checkSuper(ImmutableSet immutableTyParams, ClassType type) { + private Violation checkSuper( + ImmutableSet immutableTyParams, ClassType type, ViolationReporter reporter) { ClassType superType = (ClassType) state.getTypes().supertype(type); if (superType.getKind() == TypeKind.NONE || state.getTypes().isSameType(state.getSymtab().objectType, superType)) { @@ -198,18 +193,7 @@ private Violation checkSuper(ImmutableSet immutableTyParams, ClassType t // Recursive case: check if the supertype is 'effectively' immutable. Violation info = - checkForImmutability( - Optional.empty(), - immutableTyParams, - superType, - new ViolationReporter() { - @Override - public Description.Builder describe(Tree tree, Violation info) { - return bugChecker - .buildDescription(tree) - .setMessage(info.plus(info.message()).message()); - } - }); + checkForImmutability(Optional.empty(), immutableTyParams, superType, reporter); if (!info.isPresent()) { return Violation.absent(); } @@ -267,7 +251,7 @@ Violation isFieldImmutable( ClassType classType, VarSymbol var, ViolationReporter reporter) { - if (bugChecker.isSuppressed(var, state)) { + if (suppressionChecker.test(var, state)) { return Violation.absent(); } if (!var.getModifiers().contains(Modifier.FINAL) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java index 9941e24f1a3..8c6213dd5cd 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnnotationChecker.java @@ -90,7 +90,10 @@ public Description matchClass(ClassTree tree, VisitorState state) { Violation info = new ImmutableAnalysis( - this, state, wellKnownMutability, ImmutableSet.of(Immutable.class.getName())) + this::isSuppressed, + state, + wellKnownMutability, + ImmutableSet.of(Immutable.class.getName())) .checkForImmutability( Optional.of(tree), ImmutableSet.of(), getType(tree), this::describeClass); diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java index c3487d91847..b9c744e5d28 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java @@ -214,7 +214,8 @@ public Description matchMethod(MethodTree tree, VisitorState state) { } private ImmutableAnalysis createImmutableAnalysis(VisitorState state) { - return new ImmutableAnalysis(this, state, wellKnownMutability, immutableAnnotations); + return new ImmutableAnalysis( + this::isSuppressed, state, wellKnownMutability, immutableAnnotations); } private void checkInvocation( diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableEnumChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableEnumChecker.java index f559194cf36..12a5965c6eb 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableEnumChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableEnumChecker.java @@ -84,7 +84,10 @@ public Description matchClass(ClassTree tree, VisitorState state) { Violation info = new ImmutableAnalysis( - this, state, wellKnownMutability, ImmutableSet.of(Immutable.class.getName())) + this::isSuppressed, + state, + wellKnownMutability, + ImmutableSet.of(Immutable.class.getName())) .checkForImmutability( Optional.of(tree), ImmutableSet.of(), getType(tree), this::describe);