Skip to content

Commit

Permalink
Inline DelayedThreadSafeAnalysis into the checker.
Browse files Browse the repository at this point in the history
It's only used here. I kept staring at it trying to dedupe with ThreadSafeAnalysis, but it's actually different.

PiperOrigin-RevId: 523952698
  • Loading branch information
graememorgan authored and Error Prone Team committed Apr 14, 2023
1 parent d348ff7 commit f8f7756
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 35 deletions.
Expand Up @@ -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;
Expand All @@ -45,26 +43,27 @@
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;
import javax.lang.model.type.TypeKind;
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<Symbol, VisitorState> suppressionChecker;
private final VisitorState state;
private final WellKnownMutability wellKnownMutability;
private final ThreadSafety threadSafety;

ImmutableAnalysis(
BugChecker bugChecker,
BiPredicate<Symbol, VisitorState> suppressionChecker,
VisitorState state,
WellKnownMutability wellKnownMutability,
ImmutableSet<String> immutableAnnotations) {
this.bugChecker = bugChecker;
this.suppressionChecker = suppressionChecker;
this.state = state;
this.wellKnownMutability = wellKnownMutability;
this.threadSafety =
Expand All @@ -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<String> containerTypeParameters, Type type) {
return threadSafety.isThreadSafeType(
Expand Down Expand Up @@ -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),
Expand All @@ -154,22 +148,23 @@ 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));
}
return Violation.absent();
}

private Violation checkSuper(ImmutableSet<String> immutableTyParams, ClassType type) {
private Violation checkSuper(
ImmutableSet<String> immutableTyParams, ClassType type, ViolationReporter reporter) {
ClassType superType = (ClassType) state.getTypes().supertype(type);
if (superType.getKind() == TypeKind.NONE
|| state.getTypes().isSameType(state.getSymtab().objectType, superType)) {
Expand Down Expand Up @@ -198,18 +193,7 @@ private Violation checkSuper(ImmutableSet<String> immutableTyParams, ClassType t

// Recursive case: check if the supertype is 'effectively' immutable.
Violation info =
checkForImmutability(
Optional.<ClassTree>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.<ClassTree>empty(), immutableTyParams, superType, reporter);
if (!info.isPresent()) {
return Violation.absent();
}
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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);

Expand Down
Expand Up @@ -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(
Expand Down
Expand Up @@ -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);

Expand Down

0 comments on commit f8f7756

Please sign in to comment.