From 3b9ffc2543080ad9150d41197aa9a3942e150849 Mon Sep 17 00:00:00 2001 From: Error Prone Team Date: Fri, 12 Jul 2024 10:29:12 -0700 Subject: [PATCH] Add support in ErrorProne for a new `ThreadSafeTypeParamerter` annotation 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 --- .../threadsafety/ThreadSafeChecker.java | 6 ++-- .../threadsafety/ThreadSafeCheckerTest.java | 28 ++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeChecker.java index 29c3957eee2..b2bd5a0314b 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeChecker.java @@ -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( @@ -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(); @@ -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( diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java index d4a5d2316ce..0c04d598bc2 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ThreadSafeCheckerTest.java @@ -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()); @@ -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 @@ -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.