From 1d040949c37c1d5b5105f6b8fe2540101897b43c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 17 Oct 2024 08:02:55 -0700 Subject: [PATCH] A couple of fixes in MoreAnnotations I discovered these issues while using this code in NullAway; see https://github.com/uber/NullAway/pull/1055. (We support older Error Prone versions so can't directly call this API and had to adapt the code instead.) The previous code didn't handle explicit annotations on lambda parameters (e.g., `(@Nullable Object x) -> { ... }`) and annotations on enum constants. Not sure the best way to add tests for this but happy to add them if given a suggestion. Actually the enum constant case was weird; I was unable to repro with bytecodes output by `javac` and I only observed the case with an `ijar` from Bazel. FYI @cpovirk @cushon Fixes #4620 COPYBARA_INTEGRATE_REVIEW=https://github.com/google/error-prone/pull/4620 from msridhar:more-annotations-tweaks fb3690ba6efaae689fe08f8c064770925d6c34e0 PiperOrigin-RevId: 686916301 --- .../errorprone/util/MoreAnnotations.java | 15 ++++++++-- .../errorprone/util/MoreAnnotationsTest.java | 29 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java b/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java index 5ea7a61acd0..6e3d2695ee5 100644 --- a/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java +++ b/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java @@ -30,6 +30,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeAnnotationPosition; import com.sun.tools.javac.code.TypeTag; +import com.sun.tools.javac.tree.JCTree; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -130,6 +131,8 @@ private static boolean targetTypeMatches(Symbol sym, TypeAnnotationPosition posi case LOCAL_VARIABLE: return position.type == TargetType.LOCAL_VARIABLE; case FIELD: + // treated like a field + case ENUM_CONSTANT: return position.type == TargetType.FIELD; case CONSTRUCTOR: case METHOD: @@ -137,8 +140,16 @@ private static boolean targetTypeMatches(Symbol sym, TypeAnnotationPosition posi case PARAMETER: switch (position.type) { case METHOD_FORMAL_PARAMETER: - return ((MethodSymbol) sym.owner).getParameters().indexOf(sym) - == position.parameter_index; + int parameterIndex = position.parameter_index; + if (position.onLambda != null) { + com.sun.tools.javac.util.List lambdaParams = + position.onLambda.params; + return parameterIndex < lambdaParams.size() + && lambdaParams.get(parameterIndex).sym.equals(sym); + } else { + return ((Symbol.MethodSymbol) sym.owner).getParameters().indexOf(sym) + == parameterIndex; + } default: return false; } diff --git a/check_api/src/test/java/com/google/errorprone/util/MoreAnnotationsTest.java b/check_api/src/test/java/com/google/errorprone/util/MoreAnnotationsTest.java index 3c8bdeef03a..0ab74fe4610 100644 --- a/check_api/src/test/java/com/google/errorprone/util/MoreAnnotationsTest.java +++ b/check_api/src/test/java/com/google/errorprone/util/MoreAnnotationsTest.java @@ -223,4 +223,33 @@ class InnerStatic {} """) .doTest(); } + + @Test + public void explicitlyAnnotatedLambdaTest() { + CompilationTestHelper.newInstance(GetTopLevelTypeAttributesTester.class, getClass()) + .addSourceLines( + "Annos.java", + """ + import static java.lang.annotation.ElementType.TYPE_USE; + import java.lang.annotation.Target; + + @Target(TYPE_USE) + @interface A {} + """) + .addSourceLines( + "Test.java", + """ + import java.util.function.Consumer; + + class Test { + Consumer<@A String> c; + + void test() { + // BUG: Diagnostic contains: A + c = (@A String s) -> {}; + } + } + """) + .doTest(); + } }