Skip to content

Commit 7b5a8ad

Browse files
graememorganError Prone Team
authored andcommitted
Rollforward of e1b16e2: @ImmutableTypeParameter is really a subset of @ThreadSafeTypeParameter, much as @Immutable is a subset of @ThreadSafe.
Fixed to distinguish between "this is the canonical type parameter" and "this is an acceptable one". Ugh. I'll flume as well to pick up any outrageous issues. PiperOrigin-RevId: 808631828
1 parent 6a6afc9 commit 7b5a8ad

File tree

2 files changed

+67
-6
lines changed

2 files changed

+67
-6
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafety.java

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.errorprone.VisitorState;
3333
import com.google.errorprone.annotations.CanIgnoreReturnValue;
3434
import com.google.errorprone.annotations.Immutable;
35+
import com.google.errorprone.annotations.ImmutableTypeParameter;
3536
import com.google.errorprone.annotations.ThreadSafe;
3637
import com.google.errorprone.annotations.ThreadSafeTypeParameter;
3738
import com.google.errorprone.bugpatterns.CanBeStaticAnalyzer;
@@ -81,6 +82,7 @@ public final class ThreadSafety {
8182
private final ImmutableSet<String> containerOfAnnotation;
8283
private final ImmutableSet<String> suppressAnnotation;
8384
private final ImmutableSet<String> typeParameterAnnotation;
85+
private final ImmutableSet<String> acceptedTypeParameterAnnotation;
8486

8587
public static Builder builder() {
8688
return new Builder();
@@ -94,7 +96,9 @@ public static ThreadSafety.Builder threadSafeBuilder(
9496
.knownTypes(wellKnownThreadSafety)
9597
.markerAnnotations(ImmutableSet.of(ThreadSafe.class.getName()))
9698
.acceptedAnnotations(ImmutableSet.of(Immutable.class.getName()))
97-
.typeParameterAnnotation(ImmutableSet.of(ThreadSafeTypeParameter.class.getName()));
99+
.typeParameterAnnotation(ImmutableSet.of(ThreadSafeTypeParameter.class.getName()))
100+
.acceptedTypeParameterAnnotation(
101+
ImmutableSet.of(ImmutableTypeParameter.class.getName()));
98102
return builder;
99103
}
100104

@@ -156,6 +160,7 @@ private Builder() {}
156160
private ImmutableSet<String> containerOfAnnotation = ImmutableSet.of();
157161
private ImmutableSet<String> suppressAnnotation = ImmutableSet.of();
158162
private ImmutableSet<String> typeParameterAnnotation = ImmutableSet.of();
163+
private ImmutableSet<String> acceptedTypeParameterAnnotation = ImmutableSet.of();
159164

160165
/** See {@link Purpose}. */
161166
@CanIgnoreReturnValue
@@ -249,11 +254,21 @@ public Builder typeParameterAnnotation(Class<? extends Annotation> typeParameter
249254
*/
250255
@CanIgnoreReturnValue
251256
public Builder typeParameterAnnotation(Iterable<String> typeParameterAnnotation) {
252-
checkNotNull(typeParameterAnnotation);
253257
this.typeParameterAnnotation = ImmutableSet.copyOf(typeParameterAnnotation);
254258
return this;
255259
}
256260

261+
/**
262+
* An annotation which, when found on a type parameter, indicates that the type parameter may
263+
* only be instantiated with thread-safe types.
264+
*/
265+
@CanIgnoreReturnValue
266+
public Builder acceptedTypeParameterAnnotation(
267+
Iterable<String> acceptedTypeParameterAnnotation) {
268+
this.acceptedTypeParameterAnnotation = ImmutableSet.copyOf(acceptedTypeParameterAnnotation);
269+
return this;
270+
}
271+
257272
public ThreadSafety build(VisitorState state) {
258273
checkNotNull(knownTypes);
259274
checkNotNull(markerAnnotations);
@@ -265,7 +280,8 @@ public ThreadSafety build(VisitorState state) {
265280
acceptedAnnotations,
266281
containerOfAnnotation,
267282
suppressAnnotation,
268-
typeParameterAnnotation);
283+
typeParameterAnnotation,
284+
acceptedTypeParameterAnnotation);
269285
}
270286
}
271287

@@ -277,7 +293,8 @@ private ThreadSafety(
277293
Set<String> acceptedAnnotations,
278294
Set<String> containerOfAnnotation,
279295
Set<String> suppressAnnotation,
280-
Set<String> typeParameterAnnotation) {
296+
Set<String> typeParameterAnnotation,
297+
Set<String> acceptedTypeParameterAnnotation) {
281298
this.state = checkNotNull(state);
282299
this.purpose = purpose;
283300
this.knownTypes = checkNotNull(knownTypes);
@@ -286,6 +303,8 @@ private ThreadSafety(
286303
this.containerOfAnnotation = ImmutableSet.copyOf(checkNotNull(containerOfAnnotation));
287304
this.suppressAnnotation = ImmutableSet.copyOf(checkNotNull(suppressAnnotation));
288305
this.typeParameterAnnotation = ImmutableSet.copyOf(checkNotNull(typeParameterAnnotation));
306+
this.acceptedTypeParameterAnnotation =
307+
ImmutableSet.copyOf(checkNotNull(acceptedTypeParameterAnnotation));
289308
}
290309

291310
/**
@@ -355,7 +374,7 @@ public Violation threadSafeInstantiation(
355374
Set<TypeVariableSymbol> recursiveThreadSafeTypeParameter) {
356375
for (int i = 0; i < type.tsym.getTypeParameters().size(); i++) {
357376
TypeVariableSymbol typaram = type.tsym.getTypeParameters().get(i);
358-
boolean immutableTypeParameter = hasThreadSafeTypeParameterAnnotation(typaram);
377+
boolean immutableTypeParameter = hasAcceptedThreadSafeTypeParameterAnnotation(typaram);
359378
if (annotation.containerOf().contains(typaram.getSimpleName().toString())
360379
|| immutableTypeParameter) {
361380
if (type.getTypeArguments().isEmpty()) {
@@ -608,6 +627,16 @@ public boolean hasThreadSafeTypeParameterAnnotation(TypeVariableSymbol symbol) {
608627
.anyMatch(t -> typeParameterAnnotation.contains(t.type.tsym.flatName().toString()));
609628
}
610629

630+
/**
631+
* Returns whether the given type parameter's declaration is annotated with {@link
632+
* #typeParameterAnnotation} or another acceptable annotation.
633+
*/
634+
public boolean hasAcceptedThreadSafeTypeParameterAnnotation(TypeVariableSymbol symbol) {
635+
Set<String> annotations = Sets.union(typeParameterAnnotation, acceptedTypeParameterAnnotation);
636+
return symbol.getAnnotationMirrors().stream()
637+
.anyMatch(t -> annotations.contains(t.type.tsym.flatName().toString()));
638+
}
639+
611640
/**
612641
* Returns whether the given type parameter's declaration is annotated with {@link
613642
* #containerOfAnnotation} indicating its type-safety determines the type-safety of the outer
@@ -658,7 +687,7 @@ private boolean isTypeParameterThreadSafe(
658687
return true;
659688
}
660689
}
661-
return hasThreadSafeTypeParameterAnnotation(symbol);
690+
return hasAcceptedThreadSafeTypeParameterAnnotation(symbol);
662691
} finally {
663692
recursiveThreadSafeTypeParameter.remove(symbol);
664693
}

core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,38 @@ class Test<@ThreadSafeTypeParameter T> {
13531353
.doTest();
13541354
}
13551355

1356+
@Test
1357+
public void immutableTypeParam_alsoThreadSafe() {
1358+
compilationHelper
1359+
.addSourceLines(
1360+
"Test.java",
1361+
"""
1362+
import com.google.errorprone.annotations.ThreadSafe;
1363+
import com.google.errorprone.annotations.ImmutableTypeParameter;
1364+
1365+
@ThreadSafe
1366+
class Test<@ImmutableTypeParameter T> {
1367+
final T t = null;
1368+
}
1369+
""")
1370+
.doTest();
1371+
}
1372+
1373+
@Test
1374+
public void immutableTypeParam_notInThreadSafeClass_ok() {
1375+
compilationHelper
1376+
.addSourceLines(
1377+
"Test.java",
1378+
"""
1379+
import com.google.errorprone.annotations.ImmutableTypeParameter;
1380+
1381+
class Test<@ImmutableTypeParameter T> {
1382+
final T t = null;
1383+
}
1384+
""")
1385+
.doTest();
1386+
}
1387+
13561388
@Test
13571389
public void threadSafeTypeParameterInstantiation() {
13581390
compilationHelper

0 commit comments

Comments
 (0)