Skip to content

Commit

Permalink
Add support in ErrorProne for a new ThreadSafeTypeParamerter annota…
Browse files Browse the repository at this point in the history
…tion that is replacing `ThreadSafe.TypeParameter`.

A follow-up CL will actually create the annotation once ErrorProne has this CL released to avoid premature dependencies on the new annotation.

This is the first step in open sourcing the threadsafe equivalent of `ImmutableTypeParameter`. After this an LSC will be performed to replace all usages of `ThreadSafe.TypeParameter` with `ThreadSafeTypeParameter` after which and remaining references to the former (e.g. in messages) will be removed.

PiperOrigin-RevId: 651816128
  • Loading branch information
java-team-github-bot authored and Error Prone Team committed Jul 12, 2024
1 parent 77e4f72 commit 3b9ffc2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState

@Override
public Description matchNewClass(NewClassTree tree, VisitorState state) {
// check instantiations of `@ThreadSafe.TypeParameter`s in generic constructor invocations
// check instantiations of `@ThreadSafeTypeParameter`s in generic constructor invocations
checkInvocation(
tree, ((JCNewClass) tree).constructorType, state, ((JCNewClass) tree).constructor);
// check instantiations of `@ThreadSafe.TypeParameter`s in class constructor invocations
// check instantiations of `@ThreadSafeTypeParameter`s in class constructor invocations
ThreadSafeAnalysis analysis = new ThreadSafeAnalysis(this, state, wellKnownThreadSafety);
Violation info =
analysis.checkInstantiation(
Expand Down Expand Up @@ -137,6 +137,7 @@ public Description matchTypeParameter(TypeParameterTree tree, VisitorState state
ThreadSafeAnalysis analysis = new ThreadSafeAnalysis(this, state, wellKnownThreadSafety);
if (analysis.hasThreadSafeTypeParameterAnnotation((TypeVariableSymbol) sym)) {
if (analysis.getThreadSafeAnnotation(sym.owner, state) == null) {
// TODO: b/324092874 -- Update this message to use the new annotation name.
return buildDescription(tree)
.setMessage("@ThreadSafe.TypeParameter is only supported on threadsafe classes")
.build();
Expand Down Expand Up @@ -198,6 +199,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
.map(Entry::getKey)
.collect(toImmutableSet());
if (!threadSafeAndContainer.isEmpty()) {
// TODO: b/324092874 -- Update this message to use the new annotation name.
return buildDescription(tree)
.setMessage(
String.format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,14 @@
public class ThreadSafeCheckerTest {

private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(ThreadSafeChecker.class, getClass());
CompilationTestHelper.newInstance(ThreadSafeChecker.class, getClass())
// TODO: b/339025111 - Remove this once ThreadSafeTypeParameter actually exists.
.addSourceLines(
"ThreadSafeTypeParameter.java",
"package com.google.errorprone.annotations;",
"@java.lang.annotation.Target(java.lang.annotation.ElementType.TYPE_PARAMETER)",
"@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)",
"public @interface ThreadSafeTypeParameter {}");

private final BugCheckerRefactoringTestHelper refactoringHelper =
BugCheckerRefactoringTestHelper.newInstance(ThreadSafeChecker.class, getClass());
Expand Down Expand Up @@ -1001,6 +1008,16 @@ public void knownThreadSafeFlag() {
.doTest();
}

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

@Test
public void annotatedClassType() {
compilationHelper
Expand All @@ -1015,7 +1032,16 @@ public void annotatedClassType() {
.doTest();
}

// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// Regression test for b/117937500
// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// Regression test for b/117937500

// javac does not instantiate type variables when they are not used for target typing, so we
// cannot check whether their instantiations are thread-safe.
// TODO: b/324092874 - Remove this test once ThreadSafe.TypeParameter is removed.

// javac does not instantiate type variables when they are not used for target typing, so we
// cannot check whether their instantiations are thread-safe.
Expand Down

0 comments on commit 3b9ffc2

Please sign in to comment.