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.

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

Make ThreadSafeTypeParameter useful in the open-source version of ErrorProne. #4554

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@
* <li>it is annotated with {@link ThreadSafe}.
* </ul>
*
* <p>This first requirement means the type is at least inherently shallowly thread-safe.
* <p>This first requirement means the type is at least inherently shallowly thread-safe. For types
* with type parameters to be deemed deeply thread-safe, those of these types that denote
* containment must also be deemed thread-safe. A full explanation of this can be found in the
* {@link ThreadSafeTypeParameter} javadoc.
*
* <p>Fields annotated with {@code javax.annotation.concurrent.GuardedBy} are likely the meat of a
* mutable thread-safe class: these are things that need to be mutated, but should be done so in a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.annotations.ThreadSafe;
import com.google.errorprone.annotations.ThreadSafeTypeParameter;
import com.google.errorprone.bugpatterns.CanBeStaticAnalyzer;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.util.ASTHelpers;
Expand Down Expand Up @@ -95,7 +96,8 @@ public static ThreadSafety.Builder threadSafeBuilder(
.setPurpose(Purpose.FOR_THREAD_SAFE_CHECKER)
.knownTypes(wellKnownThreadSafety)
.markerAnnotations(ImmutableSet.of(ThreadSafe.class.getName()))
.acceptedAnnotations(ImmutableSet.of(Immutable.class.getName()));
.acceptedAnnotations(ImmutableSet.of(Immutable.class.getName()))
.typeParameterAnnotation(ImmutableSet.of(ThreadSafeTypeParameter.class.getName()));
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,29 @@ public void rawClass() {
.doTest();
}

@Ignore("b/26797524 - add tests for generic arguments")
@Test
public void threadSafeTypeParam() {
compilationHelper
.addSourceLines(
"X.java",
"import com.google.common.collect.ImmutableList;",
"import com.google.errorprone.annotations.ThreadSafe;",
"public class X {",
" final ImmutableList<@ThreadSafeTypeParameter ?> unknownSafeType;",
" X (ImmutableList<@ThreadSafeTypeParameter ?> unknownSafeType) {",
" this.unknownSafeType = unknownSafeType;",
" }",
"}")
.addSourceLines(
"Test.java",
"import com.google.common.collect.ImmutableList;",
"class Test {",
" final X badX = new X(ImmutableList.of(ImmutableList.<String>of()));",
"}")
.doTest();
}

@Ignore("b/26797524 - add tests for generic arguments")
@Test
public void mutableTypeParam() {
Expand Down Expand Up @@ -1001,6 +1024,78 @@ public void knownThreadSafeFlag() {
.doTest();
}

@Test
public void threadSafeTypeParameter() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafe;",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"@ThreadSafe class Test<@ThreadSafeTypeParameter T> {",
" final T t = null;",
"}")
.doTest();
}

@Test
public void threadSafeTypeParameterInstantiation() {
compilationHelper
.addSourceLines(
"A.java",
"import com.google.errorprone.annotations.ThreadSafe;",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"@ThreadSafe class A<@ThreadSafeTypeParameter T> {",
"}")
.addSourceLines(
"Test.java",
"class Test {",
" A<String> f() {",
" return new A<>();",
" }",
" A<Object> g() {",
" // BUG: Diagnostic contains: instantiation of 'T' is not "
+ "thread-safe, 'Object' is not thread-safe",
" return new A<>();",
" }",
"}")
.doTest();
}

@Test
public void threadSafeTypeParameterUsage() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"class Test {",
" static <@ThreadSafeTypeParameter T> void f() {}",
"}")
.doTest();
}

@Test
public void threadSafeTypeParameterUsage_interface() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafe;",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"@ThreadSafe interface Test<@ThreadSafeTypeParameter T> {",
"}")
.doTest();
}

@Test
public void threadSafeTypeParameterMutableClass() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"// BUG: Diagnostic contains: @ThreadSafeTypeParameter is only supported on",
"class A<@ThreadSafeTypeParameter T> {}")
.doTest();
}

@Test
public void annotatedClassType() {
compilationHelper
Expand All @@ -1015,10 +1110,65 @@ public void annotatedClassType() {
.doTest();
}

@Test
public void instantiationWithThreadSafeTypeParameter() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.common.collect.ImmutableList;",
"import com.google.errorprone.annotations.ThreadSafe;",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"@ThreadSafe public class Test<@ThreadSafeTypeParameter T> {",
" final ImmutableList<T> xs = ImmutableList.of();",
"}")
.doTest();
}

// Regression test for b/117937500
@Test
public void notAllTypeVarsInstantiated() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"import java.util.function.Function;",
"class Test {",
" public final <A> void f1(A transform) {}",
" public <B, @ThreadSafeTypeParameter C> C f2(Function<B, C> fn) {",
" return null;",
" }",
" public final <D, E> void f3(Function<D, E> fn) {",
" // BUG: Diagnostic contains: instantiation of 'C' is not thread-safe",
" // 'E' is a non-thread-safe type variable",
" f1(f2(fn));",
" }",
"}")
.doTest();
}

// javac does not instantiate type variables when they are not used for target typing, so we
// cannot check whether their instantiations are thread-safe.
@Ignore
@Test
public void notAllTypeVarsInstantiated_shouldFail() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.ThreadSafeTypeParameter;",
"import java.util.function.Function;",
"class Test {",
" public final <A> void f1(A transform) {}",
" public <@ThreadSafeTypeParameter B, C> C f2(Function<B, C> fn) {",
" return null;",
" }",
" public final <D, E> void f3(Function<D, E> fn) {",
" // BUG: Diagnostic contains: instantiation of 'B' is not thread-safe",
" // 'D' is a non-thread-safe type variable",
" f1(f2(fn));",
" }",
"}")
.doTest();
}

@Test
public void threadSafeUpperBound() {
Expand Down
Loading