Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Automatic code cleanup. #4348

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Automatic code cleanup. #4348

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 47 additions & 9 deletions core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafety.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ public final class ThreadSafety {
private final ImmutableSet<String> suppressAnnotation;
private final ImmutableSet<String> typeParameterAnnotation;

/** Stores recursive invocations of {@link #isTypeParameterThreadSafe} */
private final Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter = new HashSet<>();

public static Builder builder() {
return new Builder();
}
Expand Down Expand Up @@ -418,6 +415,14 @@ public static Violation absent() {
*/
public Violation threadSafeInstantiation(
Set<String> containerTypeParameters, AnnotationInfo annotation, Type type) {
return threadSafeInstantiation(containerTypeParameters, annotation, type, new HashSet<>());
}

public Violation threadSafeInstantiation(
Set<String> containerTypeParameters,
AnnotationInfo annotation,
Type type,
Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter) {
for (int i = 0; i < type.tsym.getTypeParameters().size(); i++) {
TypeVariableSymbol typaram = type.tsym.getTypeParameters().get(i);
boolean immutableTypeParameter = hasThreadSafeTypeParameterAnnotation(typaram);
Expand Down Expand Up @@ -445,7 +450,12 @@ public Violation threadSafeInstantiation(
.toString()))) {
continue;
}
Violation info = isThreadSafeType(!immutableTypeParameter, containerTypeParameters, tyarg);
Violation info =
isThreadSafeTypeInternal(
!immutableTypeParameter,
containerTypeParameters,
tyarg,
recursiveThreadSafeTypeParameter);
if (info.isPresent()) {
return info.plus(
String.format(
Expand Down Expand Up @@ -532,18 +542,35 @@ private boolean containerOfSubtyping(
*/
public Violation isThreadSafeType(
boolean allowContainerTypeParameters, Set<String> containerTypeParameters, Type type) {
return isThreadSafeTypeInternal(
allowContainerTypeParameters, containerTypeParameters, type, new HashSet<>());
}

private Violation isThreadSafeTypeInternal(
boolean allowContainerTypeParameters,
Set<String> containerTypeParameters,
Type type,
Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter) {
return type.accept(
new ThreadSafeTypeVisitor(allowContainerTypeParameters, containerTypeParameters), null);
new ThreadSafeTypeVisitor(
allowContainerTypeParameters,
containerTypeParameters,
recursiveThreadSafeTypeParameter),
null);
}

private class ThreadSafeTypeVisitor extends Types.SimpleVisitor<Violation, Void> {

private final boolean allowContainerTypeParameters;
private final Set<String> containerTypeParameters;
private final Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter;

private ThreadSafeTypeVisitor(
boolean allowContainerTypeParameters, Set<String> containerTypeParameters) {
boolean allowContainerTypeParameters,
Set<String> containerTypeParameters,
Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter) {
this.allowContainerTypeParameters = allowContainerTypeParameters;
this.recursiveThreadSafeTypeParameter = recursiveThreadSafeTypeParameter;
this.containerTypeParameters =
!allowContainerTypeParameters ? ImmutableSet.of() : containerTypeParameters;
}
Expand All @@ -564,7 +591,8 @@ public Violation visitTypeVar(TypeVar type, Void s) {
if (containerTypeParameters.contains(tyvar.getSimpleName().toString())) {
return Violation.absent();
}
if (isTypeParameterThreadSafe(tyvar, containerTypeParameters)) {
if (isTypeParameterThreadSafe(
tyvar, containerTypeParameters, recursiveThreadSafeTypeParameter)) {
return Violation.absent();
}
String message;
Expand Down Expand Up @@ -614,7 +642,8 @@ public Violation visitType(Type type, Void s) {
}
AnnotationInfo annotation = getMarkerOrAcceptedAnnotation(type.tsym, state);
if (annotation != null) {
return threadSafeInstantiation(containerTypeParameters, annotation, type);
return threadSafeInstantiation(
containerTypeParameters, annotation, type, recursiveThreadSafeTypeParameter);
}
String nameStr = type.tsym.flatName().toString();
if (knownTypes.getKnownUnsafeClasses().contains(nameStr)) {
Expand Down Expand Up @@ -677,14 +706,23 @@ public boolean hasThreadSafeElementAnnotation(TypeVariableSymbol symbol) {
*/
private boolean isTypeParameterThreadSafe(
TypeVariableSymbol symbol, Set<String> containerTypeParameters) {
return isTypeParameterThreadSafe(symbol, containerTypeParameters, new HashSet<>());
}

private boolean isTypeParameterThreadSafe(
TypeVariableSymbol symbol,
Set<String> containerTypeParameters,
Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter) {
if (!recursiveThreadSafeTypeParameter.add(symbol)) {
return true;
}
// TODO(b/77695285): Prevent type variables that are immutable because of an immutable upper
// bound to be marked thread-safe via containerOf or typeParameterAnnotation.
try {
for (Type bound : symbol.getBounds()) {
if (!isThreadSafeType(true, containerTypeParameters, bound).isPresent()) {
if (!isThreadSafeTypeInternal(
true, containerTypeParameters, bound, recursiveThreadSafeTypeParameter)
.isPresent()) {
// A type variable is thread-safe if any upper bound is thread-safe.
return true;
}
Expand Down