A couple of fixes in MoreAnnotations

I discovered these issues while using this code in NullAway; see uber/NullAway#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=#4620 from msridhar:more-annotations-tweaks fb3690b PiperOrigin-RevId: 686916301
google · Oct 17, 2024 · 1d04094 · 1d04094
1 parent 6203a0e
commit 1d04094
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 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
@@ -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,15 +131,25 @@ 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:
         return position.type == TargetType.METHOD_RETURN;
       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<JCTree.JCVariableDecl> 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
@@ -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();
+  }
 }