Populate JavaInfo's annotation_processing field in java_common.compile

RELNOTES: None.

PiperOrigin-RevId: 200471197

5 files changed, 195 insertions, 82 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index cb4b60726f..9557005d4b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -59,7 +59,6 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -213,22 +212,6 @@ public class JavaCommon {
     return javaArtifacts;
   }
 
-  public NestedSet<Artifact> getProcessorClasspathJars() {
-    NestedSetBuilder<Artifact> builder = NestedSetBuilder.naiveLinkOrder();
-    for (JavaPluginInfoProvider plugin : activePlugins) {
-      builder.addTransitive(plugin.getProcessorClasspath());
-    }
-    return builder.build();
-  }
-
-  public ImmutableList<String> getProcessorClassNames() {
-    Set<String> processorNames = new LinkedHashSet<>();
-    for (JavaPluginInfoProvider plugin : activePlugins) {
-      processorNames.addAll(plugin.getProcessorClasses());
-    }
-    return ImmutableList.copyOf(processorNames);
-  }
-
   /**
    * Creates the java.library.path from a list of the native libraries.
    * Concatenates the parent directories of the shared libraries into a Java
@@ -411,37 +394,6 @@ public class JavaCommon {
   }
 
   /**
-   * Collects transitive gen jars for the current rule.
-   */
-  private JavaGenJarsProvider collectTransitiveGenJars(
-          boolean usesAnnotationProcessing,
-          @Nullable Artifact genClassJar,
-          @Nullable Artifact genSourceJar) {
-    NestedSetBuilder<Artifact> classJarsBuilder = NestedSetBuilder.stableOrder();
-    NestedSetBuilder<Artifact> sourceJarsBuilder = NestedSetBuilder.stableOrder();
-
-    if (genClassJar != null) {
-      classJarsBuilder.add(genClassJar);
-    }
-    if (genSourceJar != null) {
-      sourceJarsBuilder.add(genSourceJar);
-    }
-    for (JavaGenJarsProvider dep : getDependencies(JavaGenJarsProvider.class)) {
-      classJarsBuilder.addTransitive(dep.getTransitiveGenClassJars());
-      sourceJarsBuilder.addTransitive(dep.getTransitiveGenSourceJars());
-    }
-    return new JavaGenJarsProvider(
-        usesAnnotationProcessing,
-        genClassJar,
-        genSourceJar,
-        getProcessorClasspathJars(),
-        getProcessorClassNames(),
-        classJarsBuilder.build(),
-        sourceJarsBuilder.build()
-    );
-  }
-
-  /**
    * Collects labels of targets and artifacts reached transitively via the "exports" attribute.
    */
   protected JavaExportsProvider collectTransitiveExports() {
@@ -749,16 +701,20 @@ public class JavaCommon {
       JavaInfo.Builder javaInfoBuilder,
       @Nullable Artifact genClassJar,
       @Nullable Artifact genSourceJar) {
-    JavaGenJarsProvider genJarsProvider = collectTransitiveGenJars(
-        javaCompilationHelper.usesAnnotationProcessing(),
-        genClassJar, genSourceJar);
+    JavaGenJarsProvider genJarsProvider =
+        JavaGenJarsProvider.create(
+            javaCompilationHelper.usesAnnotationProcessing(),
+            genClassJar,
+            genSourceJar,
+            activePlugins,
+            getDependencies(JavaGenJarsProvider.class));
 
     NestedSetBuilder<Artifact> genJarsBuilder = NestedSetBuilder.stableOrder();
     genJarsBuilder.addTransitive(genJarsProvider.getTransitiveGenClassJars());
     genJarsBuilder.addTransitive(genJarsProvider.getTransitiveGenSourceJars());
 
     builder
-        .add(JavaGenJarsProvider.class, genJarsProvider)
+        .addProvider(JavaGenJarsProvider.class, genJarsProvider)
         .addOutputGroup(JavaSemantics.GENERATED_JARS_OUTPUT_GROUP, genJarsBuilder.build());
 
     javaInfoBuilder.addProvider(JavaGenJarsProvider.class, genJarsProvider);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
index d58281c5d5..c5923776c5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
@@ -18,9 +18,13 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkbuildapi.java.JavaAnnotationProcessingApi;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
 import javax.annotation.Nullable;
 
 /** The collection of gen jars from the transitive closure. */
@@ -41,6 +45,44 @@ public final class JavaGenJarsProvider
   private final NestedSet<Artifact> transitiveGenClassJars;
   private final NestedSet<Artifact> transitiveGenSourceJars;
 
+  static JavaGenJarsProvider create(
+      boolean usesAnnotationProcessing,
+      @Nullable Artifact genClassJar,
+      @Nullable Artifact genSourceJar,
+      List<JavaPluginInfoProvider> plugins,
+      Iterable<JavaGenJarsProvider> transitiveJavaGenJars) {
+    NestedSetBuilder<Artifact> classJarsBuilder = NestedSetBuilder.stableOrder();
+    NestedSetBuilder<Artifact> sourceJarsBuilder = NestedSetBuilder.stableOrder();
+
+    if (genClassJar != null) {
+      classJarsBuilder.add(genClassJar);
+    }
+    if (genSourceJar != null) {
+      sourceJarsBuilder.add(genSourceJar);
+    }
+    for (JavaGenJarsProvider dep : transitiveJavaGenJars) {
+      classJarsBuilder.addTransitive(dep.getTransitiveGenClassJars());
+      sourceJarsBuilder.addTransitive(dep.getTransitiveGenSourceJars());
+    }
+
+    NestedSetBuilder<Artifact> processorClasspathsBuilder = NestedSetBuilder.naiveLinkOrder();
+    Set<String> processorNames = new LinkedHashSet<>();
+    for (JavaPluginInfoProvider plugin : plugins) {
+      processorClasspathsBuilder.addTransitive(plugin.getProcessorClasspath());
+      processorNames.addAll(plugin.getProcessorClasses());
+    }
+
+    return new JavaGenJarsProvider(
+        usesAnnotationProcessing,
+        genClassJar,
+        genSourceJar,
+        processorClasspathsBuilder.build(),
+        ImmutableList.copyOf(processorNames),
+        classJarsBuilder.build(),
+        sourceJarsBuilder.build());
+  }
+
+  // Package-private for @AutoCodec
   JavaGenJarsProvider(
       boolean usesAnnotationProcessing,
       @Nullable Artifact genClassJar,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java
index e5c2cde4a3..369e198287 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java
@@ -437,7 +437,7 @@ final class JavaInfoBuildHelper {
       SkylarkList<Artifact> resources,
       Boolean neverlink,
       JavaSemantics javaSemantics)
-      throws EvalException, InterruptedException {
+      throws EvalException {
     if (sourceJars.isEmpty() && sourceFiles.isEmpty() && exports.isEmpty()) {
       throw new EvalException(
           null, "source_jars, sources and exports cannot be simultaneous empty");
@@ -499,9 +499,15 @@ final class JavaInfoBuildHelper {
             javaRuntimeInfo,
             SkylarkList.createImmutable(ImmutableList.of()),
             outputJarsBuilder,
-            /*createOutputSourceJar*/ generateMergedSourceJar,
+            /*createOutputSourceJar=*/ generateMergedSourceJar,
             outputSourceJar,
-            javaInfoBuilder);
+            javaInfoBuilder,
+            // Include JavaGenJarsProviders from both deps and exports in the JavaGenJarsProvider
+            // added to javaInfoBuilder for this target.
+            NestedSetBuilder.<JavaGenJarsProvider>stableOrder()
+                .addAll(JavaInfo.fetchProvidersFromList(deps, JavaGenJarsProvider.class))
+                .addAll(JavaInfo.fetchProvidersFromList(exports, JavaGenJarsProvider.class))
+                .build());
 
     JavaCompilationArgsProvider javaCompilationArgsProvider =
         helper.buildCompilationArgsProvider(artifacts, true, neverlink);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java
index f39e5b1029..81de790938 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java
@@ -195,7 +195,8 @@ public final class JavaLibraryHelper {
         outputJarsBuilder,
         createOutputSourceJar,
         outputSourceJar,
-        /* javaInfoBuilder= */ null);
+        /* javaInfoBuilder= */ null,
+        ImmutableList.of()); // ignored when javaInfoBuilder is null
   }
 
   public JavaCompilationArtifacts build(
@@ -206,7 +207,8 @@ public final class JavaLibraryHelper {
       JavaRuleOutputJarsProvider.Builder outputJarsBuilder,
       boolean createOutputSourceJar,
       @Nullable Artifact outputSourceJar,
-      @Nullable JavaInfo.Builder javaInfoBuilder) {
+      @Nullable JavaInfo.Builder javaInfoBuilder,
+      Iterable<JavaGenJarsProvider> transitiveJavaGenJars) {
     Preconditions.checkState(output != null, "must have an output file; use setOutput()");
     Preconditions.checkState(
         !createOutputSourceJar || outputSourceJar != null,
@@ -272,12 +274,12 @@ public final class JavaLibraryHelper {
 
     JavaCompilationArtifacts javaArtifacts = artifactsBuilder.build();
     if (javaInfoBuilder != null) {
-      ClasspathConfiguredFragment classpathFragment = new ClasspathConfiguredFragment(
-        javaArtifacts,
-        attributes.build(),
-        neverlink,
-        JavaCompilationHelper.getBootClasspath(javaToolchainProvider)
-      );
+      ClasspathConfiguredFragment classpathFragment =
+          new ClasspathConfiguredFragment(
+              javaArtifacts,
+              attributes.build(),
+              neverlink,
+              JavaCompilationHelper.getBootClasspath(javaToolchainProvider));
 
       javaInfoBuilder.addProvider(
           JavaCompilationInfoProvider.class,
@@ -287,11 +289,28 @@ public final class JavaLibraryHelper {
               .setCompilationClasspath(classpathFragment.getCompileTimeClasspath())
               .setRuntimeClasspath(classpathFragment.getRuntimeClasspath())
               .build());
+
+      javaInfoBuilder.addProvider(
+          JavaGenJarsProvider.class,
+          createJavaGenJarsProvider(helper, genClassJar, genSourceJar, transitiveJavaGenJars));
     }
 
     return javaArtifacts;
   }
 
+  private JavaGenJarsProvider createJavaGenJarsProvider(
+      JavaCompilationHelper helper,
+      @Nullable Artifact genClassJar,
+      @Nullable Artifact genSourceJar,
+      Iterable<JavaGenJarsProvider> transitiveJavaGenJars) {
+    return JavaGenJarsProvider.create(
+        helper.usesAnnotationProcessing(),
+        genClassJar,
+        genSourceJar,
+        plugins,
+        transitiveJavaGenJars);
+  }
+
   /**
    * Returns a JavaCompilationArgsProvider that fully encapsulates this compilation, based on the
    * result of a call to build(). (that is, it contains the compile-time and runtime jars, separated
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
index 6a28c8c991..47036d774c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.java;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames;
 
-import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -309,6 +308,52 @@ public class JavaSkylarkApiTest extends BuildViewTestCase {
     assertThat(outputs.getJdeps().getFilename()).isEqualTo("libdep.jdeps");
   }
 
+  /**
+   * Tests that JavaInfo.java_annotation_processing returned from java_common.compile looks as
+   * expected, and specifically, looks as if java_library was used instead.
+   */
+  @Test
+  public void testJavaCommonCompileExposesAnnotationProcessingInfo() throws Exception {
+    // Set up a Skylark rule that uses java_common.compile and supports annotation processing in
+    // the same way as java_library, then use a helper method to test that the custom rule produces
+    // the same annotation processing information as java_library would.
+    writeBuildFileForJavaToolchain();
+    scratch.file(
+        "java/test/custom_rule.bzl",
+        "def _impl(ctx):",
+        "  output_jar = ctx.actions.declare_file('lib' + ctx.label.name + '.jar')",
+        "  return java_common.compile(",
+        "    ctx,",
+        "    source_files = ctx.files.srcs,",
+        "    deps = [d[JavaInfo] for d in ctx.attr.deps],",
+        "    exports = [e[JavaInfo] for e in ctx.attr.exports],",
+        "    plugins = [p[JavaInfo] for p in ctx.attr.plugins],",
+        "    output = output_jar,",
+        "    java_toolchain = ctx.attr._java_toolchain,",
+        "    host_javabase = ctx.attr._host_javabase",
+        "  )",
+        "java_custom_library = rule(",
+        "  implementation = _impl,",
+        "  outputs = {",
+        "    'my_output': 'lib%{name}.jar'",
+        "  },",
+        "  attrs = {",
+        "    'srcs': attr.label_list(allow_files=['.java']),",
+        "    'deps': attr.label_list(),",
+        "    'exports': attr.label_list(),",
+        "    'plugins': attr.label_list(),",
+        "    '_java_toolchain': attr.label(default = Label('//java/com/google/test:toolchain')),",
+        "    '_host_javabase': attr.label(",
+        "        default = Label('" + HOST_JAVA_RUNTIME_LABEL + "'))",
+        "  },",
+        "  fragments = ['java']",
+        ")");
+
+    testAnnotationProcessingInfoIsSkylarkAccessible(
+        /*toBeProcessedRuleName=*/ "java_custom_library",
+        /*extraLoad=*/ "load(':custom_rule.bzl', 'java_custom_library')");
+  }
+
   @Test
   public void testJavaCommonCompileCompilationInfo() throws Exception {
     writeBuildFileForJavaToolchain();
@@ -702,19 +747,38 @@ public class JavaSkylarkApiTest extends BuildViewTestCase {
     return result;
   }
 
+  /**
+   * Tests that a java_library exposes java_processing_info as expected when annotation processing
+   * is used.
+   */
   @Test
   public void testJavaPlugin() throws Exception {
+    testAnnotationProcessingInfoIsSkylarkAccessible(
+        /*toBeProcessedRuleName=*/ "java_library", /*extraLoad=*/ "");
+  }
+
+  /**
+   * Tests that JavaInfo's java_annotation_processing looks as expected with a target that's
+   * assumed to use annotation processing itself and has a dep and an export that likewise use
+   * annotation processing.
+   */
+  private void testAnnotationProcessingInfoIsSkylarkAccessible(
+      String toBeProcessedRuleName, String extraLoad) throws Exception {
     scratch.file(
-      "java/test/extension.bzl",
-      "result = provider()",
-      "def impl(ctx):",
-      "   depj = ctx.attr.dep.java",
-      "   return [result(",
-      "             processor_classpath = depj.annotation_processing.processor_classpath,",
-      "             processor_classnames = depj.annotation_processing.processor_classnames,",
-      "          )]",
-      "my_rule = rule(impl, attrs = { 'dep' : attr.label() })"
-    );
+        "java/test/extension.bzl",
+        "result = provider()",
+        "def impl(ctx):",
+        "   depj = ctx.attr.dep[JavaInfo]",
+        "   return [result(",
+        "             enabled = depj.annotation_processing.enabled,",
+        "             class_jar = depj.annotation_processing.class_jar,",
+        "             source_jar = depj.annotation_processing.source_jar,",
+        "             processor_classpath = depj.annotation_processing.processor_classpath,",
+        "             processor_classnames = depj.annotation_processing.processor_classnames,",
+        "             transitive_class_jars = depj.annotation_processing.transitive_class_jars,",
+        "             transitive_source_jars = depj.annotation_processing.transitive_source_jars,",
+        "          )]",
+        "my_rule = rule(impl, attrs = { 'dep' : attr.label() })");
     scratch.file(
       "java/test/BUILD",
       "load(':extension.bzl', 'my_rule')",
@@ -724,27 +788,53 @@ public class JavaSkylarkApiTest extends BuildViewTestCase {
       "    srcs = ['AnnotationProcessor.java'],",
       "    processor_class = 'com.google.process.stuff',",
       "    deps = [ ':plugin_dep' ])",
-      "java_library(name = 'to_be_processed',",
+      extraLoad,
+      toBeProcessedRuleName + "(",
+      "    name = 'to_be_processed',",
       "    plugins = [':plugin'],",
-      "    srcs = ['ToBeProcessed.java'])",
+      "    srcs = ['ToBeProcessed.java'],",
+      "    deps = [':dep'],",
+      "    exports = [':export'],",
+      ")",
+      "java_library(",
+      "  name = 'dep',",
+      "  srcs = ['Dep.java'],",
+      "  plugins = [':plugin']",
+      ")",
+      "java_library(",
+      "  name = 'export',",
+      "  srcs = ['Export.java'],",
+      "  plugins = [':plugin']",
+      ")",
       "my_rule(name = 'my', dep = ':to_be_processed')");
+
+    // Assert that java_annotation_processing for :to_be_processed looks as expected:
+    // the target itself uses :plugin as the processor, and transitive information includes
+    // the target's own as well as :dep's and :export's annotation processing outputs, since those
+    // two targets also use annotation processing.
     ConfiguredTarget configuredTarget = getConfiguredTarget("//java/test:my");
     Info info =
         configuredTarget.get(
             new SkylarkKey(Label.parseAbsolute("//java/test:extension.bzl"), "result"));
 
+    assertThat(info.getValue("enabled")).isEqualTo(Boolean.TRUE);
+    assertThat(info.getValue("class_jar")).isNotNull();
+    assertThat(info.getValue("source_jar")).isNotNull();
     assertThat((List<?>) info.getValue("processor_classnames"))
         .containsExactly("com.google.process.stuff");
     assertThat(
             Iterables.transform(
                 ((SkylarkNestedSet) info.getValue("processor_classpath")).toCollection(),
-                new Function<Object, String>() {
-                  @Override
-                  public String apply(Object o) {
-                    return ((Artifact) o).getFilename();
-                  }
-                }))
+                o -> ((Artifact) o).getFilename()))
         .containsExactly("libplugin.jar", "libplugin_dep.jar");
+    assertThat(((SkylarkNestedSet) info.getValue("transitive_class_jars")).toCollection())
+        .hasSize(3); // from to_be_processed, dep, and export
+    assertThat(((SkylarkNestedSet) info.getValue("transitive_class_jars")).toCollection())
+        .contains(info.getValue("class_jar"));
+    assertThat(((SkylarkNestedSet) info.getValue("transitive_source_jars")).toCollection())
+        .hasSize(3); // from to_be_processed, dep, and export
+    assertThat(((SkylarkNestedSet) info.getValue("transitive_source_jars")).toCollection())
+        .contains(info.getValue("source_jar"));
   }
 
   @Test