Consolidate handling of jar files in deps

PiperOrigin-RevId: 193533061

7 files changed, 79 insertions, 142 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index de8505553b..3ae6f66beb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -39,8 +39,8 @@ import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.rules.java.ClasspathConfiguredFragment;
 import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder;
 import com.google.devtools.build.lib.rules.java.JavaCommon;
-import com.google.devtools.build.lib.rules.java.JavaCompilationArgs;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
 import com.google.devtools.build.lib.rules.java.JavaCompilationHelper;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
@@ -511,12 +511,9 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
     // The dep may be a simple JAR and not a java rule, hence we can't simply do
     // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(),
     // so we reuse the logic within JavaCompilationArgs to handle both scenarios.
-    JavaCompilationArgs args =
-        JavaCompilationArgs.builder()
-            .addTransitiveTargets(
-                ImmutableList.of(deps), /*recursive=*/ true, ClasspathType.RUNTIME_ONLY)
-            .build();
-    return args.getRuntimeJars();
+    return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps))
+        .getRecursiveJavaCompilationArgs()
+        .getRuntimeJars();
   }
 
   private static String getAndCheckTestClass(
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 3933ad7e78..7de525e0c8 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
@@ -272,9 +272,16 @@ public class JavaCommon {
             .setIsNeverLink(isNeverLink)
             .setSrcLessDepsExport(srcLessDepsExport)
             .setCompilationArtifacts(getJavaCompilationArtifacts())
-            .setDeps(targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY))
-            .setRuntimeDeps(getRuntimeDeps(ruleContext))
-            .setExports(getExports(ruleContext))
+            .setDepsCompilationArgs(
+                ImmutableList.of(
+                    JavaCompilationArgsProvider.legacyFromTargets(
+                        targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY))))
+            .setRuntimeDepsCompilationArgs(
+                ImmutableList.of(
+                    JavaCompilationArgsProvider.legacyFromTargets(getRuntimeDeps(ruleContext))))
+            .setExportsCompilationArgs(
+                ImmutableList.of(
+                    JavaCompilationArgsProvider.legacyFromTargets(getExports(ruleContext))))
             .build());
   }
 
@@ -752,9 +759,9 @@ public class JavaCommon {
   private void processRuntimeDeps(JavaTargetAttributes.Builder attributes) {
     List<TransitiveInfoCollection> runtimeDepInfo = getRuntimeDeps(ruleContext);
     checkRuntimeDeps(ruleContext, runtimeDepInfo);
-    JavaCompilationArgs args = JavaCompilationArgs.builder()
-        .addTransitiveTargets(runtimeDepInfo, true, ClasspathType.RUNTIME_ONLY)
-        .build();
+    JavaCompilationArgs args =
+        JavaCompilationArgsProvider.legacyFromTargets(runtimeDepInfo)
+            .getRecursiveJavaCompilationArgs();
     attributes.addRuntimeClassPathEntries(args.getRuntimeJars());
     attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata());
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
index 74e2cac387..166d9edb2f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java
@@ -16,13 +16,10 @@ package com.google.devtools.build.lib.rules.java;
 
 import com.google.auto.value.AutoValue;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.FileProvider;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.util.FileType;
 
 /** A container of Java compilation artifacts. */
 @AutoValue
@@ -195,9 +192,8 @@ public abstract class JavaCompilationArgs {
 
     public Builder addTransitiveCompilationArgs(
         JavaCompilationArgsProvider dep, boolean recursive, ClasspathType type) {
-      JavaCompilationArgs args = recursive
-          ? dep.getRecursiveJavaCompilationArgs()
-          : dep.getJavaCompilationArgs();
+      JavaCompilationArgs args =
+          recursive ? dep.getRecursiveJavaCompilationArgs() : dep.getJavaCompilationArgs();
       addTransitiveArgs(args, type);
       return this;
     }
@@ -211,85 +207,6 @@ public abstract class JavaCompilationArgs {
     }
 
     /**
-     * Merges the artifacts of another target.
-     */
-    public Builder addTransitiveTarget(TransitiveInfoCollection dep, boolean recursive,
-        ClasspathType type) {
-      JavaCompilationArgsProvider provider =
-          JavaInfo.getProvider(JavaCompilationArgsProvider.class, dep);
-      if (provider != null) {
-        addTransitiveCompilationArgs(provider, recursive, type);
-        return this;
-      } else {
-        NestedSet<Artifact> filesToBuild =
-            dep.getProvider(FileProvider.class).getFilesToBuild();
-        for (Artifact jar : FileType.filter(filesToBuild, JavaSemantics.JAR)) {
-          addCompileTimeJarAsFullJar(jar);
-          addRuntimeJar(jar);
-        }
-      }
-      return this;
-    }
-
-    /**
-     * Merges the artifacts of a collection of targets.
-     */
-    public Builder addTransitiveTargets(Iterable<? extends TransitiveInfoCollection> deps,
-        boolean recursive, ClasspathType type) {
-      for (TransitiveInfoCollection dep : deps) {
-        addTransitiveTarget(dep, recursive, type);
-      }
-      return this;
-    }
-
-    /**
-     * Merges the artifacts of a collection of targets.
-     */
-    public Builder addTransitiveTargets(Iterable<? extends TransitiveInfoCollection> deps,
-        boolean recursive) {
-      return addTransitiveTargets(deps, recursive, ClasspathType.BOTH);
-    }
-
-    /**
-     * Merges the artifacts of a collection of targets.
-     */
-    public Builder addTransitiveTargets(Iterable<? extends TransitiveInfoCollection> deps) {
-      return addTransitiveTargets(deps, /*recursive=*/true, ClasspathType.BOTH);
-    }
-
-    /**
-     * Merges the artifacts of a collection of targets.
-     */
-    public Builder addTransitiveDependencies(Iterable<JavaCompilationArgsProvider> deps,
-        boolean recursive) {
-      for (JavaCompilationArgsProvider dep : deps) {
-        addTransitiveDependency(dep, recursive, ClasspathType.BOTH);
-      }
-      return this;
-    }
-
-    /** Merges the artifacts of a collection of targets. */
-    public Builder addTransitiveDependencies(
-        Iterable<JavaCompilationArgsProvider> deps, boolean recursive, ClasspathType type) {
-      for (JavaCompilationArgsProvider dep : deps) {
-        addTransitiveDependency(dep, recursive, type);
-      }
-      return this;
-    }
-
-    /**
-     * Merges the artifacts of another target.
-     */
-    private Builder addTransitiveDependency(JavaCompilationArgsProvider dep, boolean recursive,
-        ClasspathType type) {
-      JavaCompilationArgs args = recursive
-          ? dep.getRecursiveJavaCompilationArgs()
-          : dep.getJavaCompilationArgs();
-      addTransitiveArgs(args, type);
-      return this;
-    }
-
-    /**
      * Includes the contents of another instance of {@link JavaCompilationArgs}.
      *
      * @param args the {@link JavaCompilationArgs} instance
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java
index b25234d86b..23f423259b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java
@@ -16,9 +16,9 @@ package com.google.devtools.build.lib.rules.java;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import java.util.List;
 
+// TODO(cushon): delete this class, see discussion in cl/193146318
 @AutoValue
 abstract class JavaCompilationArgsHelper {
   abstract boolean recursive();
@@ -31,24 +31,15 @@ abstract class JavaCompilationArgsHelper {
 
   abstract List<JavaCompilationArgsProvider> depsCompilationArgs();
 
-  abstract Iterable<? extends TransitiveInfoCollection> deps();
-
   abstract List<JavaCompilationArgsProvider> runtimeDepsCompilationArgs();
 
-  abstract Iterable<? extends TransitiveInfoCollection> runtimeDeps();
-
   abstract List<JavaCompilationArgsProvider> exportsCompilationArgs();
 
-  abstract Iterable<? extends TransitiveInfoCollection> exports();
-
   static Builder builder() {
     return new AutoValue_JavaCompilationArgsHelper.Builder()
         .setDepsCompilationArgs(ImmutableList.of())
-        .setDeps(ImmutableList.of())
         .setRuntimeDepsCompilationArgs(ImmutableList.of())
-        .setRuntimeDeps(ImmutableList.of())
-        .setExportsCompilationArgs(ImmutableList.of())
-        .setExports(ImmutableList.of());
+        .setExportsCompilationArgs(ImmutableList.of());
   }
 
   public abstract Builder toBuilder();
@@ -65,16 +56,10 @@ abstract class JavaCompilationArgsHelper {
 
     abstract Builder setDepsCompilationArgs(List<JavaCompilationArgsProvider> value);
 
-    abstract Builder setDeps(Iterable<? extends TransitiveInfoCollection> value);
-
     abstract Builder setRuntimeDepsCompilationArgs(List<JavaCompilationArgsProvider> value);
 
-    abstract Builder setRuntimeDeps(Iterable<? extends TransitiveInfoCollection> value);
-
     abstract Builder setExportsCompilationArgs(List<JavaCompilationArgsProvider> value);
 
-    abstract Builder setExports(Iterable<? extends TransitiveInfoCollection> value);
-
     abstract JavaCompilationArgsHelper build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
index de476e970d..7ca70e2095 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java
@@ -17,12 +17,16 @@ package com.google.devtools.build.lib.rules.java;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 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.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.util.FileType;
 import java.util.Collection;
 
 /** An interface for objects that provide information on how to include them in Java builds. */
@@ -100,4 +104,41 @@ public abstract class JavaCompilationArgsProvider implements TransitiveInfoProvi
         recursiveJavaCompilationArgs.build(),
         compileTimeJavaDepArtifacts.build());
   }
+
+  /**
+   * Returns a {@link JavaCompilationArgsProvider} for the given {@link TransitiveInfoCollection}s.
+   *
+   * <p>If the given targets have a {@link JavaCompilationArgsProvider}, the information from that
+   * provider will be returned. Otherwise, any jar files provided by the targets will be wrapped in
+   * the returned provider.
+   *
+   * @deprecated The handling of raw jar files is present for legacy compatibility. All new
+   *     Java-based rules should require their dependencies to provide {@link
+   *     JavaCompilationArgsProvider}, and that precompiled jar files be wrapped in {@code
+   *     java_import}. New rules should not use this method, and existing rules should be cleaned up
+   *     to disallow jar files in their deps.
+   */
+  // TODO(b/11285003): disallow jar files in deps, require java_import instead
+  @Deprecated
+  public static JavaCompilationArgsProvider legacyFromTargets(
+      Iterable<? extends TransitiveInfoCollection> infos) {
+    JavaCompilationArgs.Builder argsBuilder = JavaCompilationArgs.builder();
+    JavaCompilationArgs.Builder recursiveArgsBuilder = JavaCompilationArgs.builder();
+    for (TransitiveInfoCollection info : infos) {
+      JavaCompilationArgsProvider provider =
+          JavaInfo.getProvider(JavaCompilationArgsProvider.class, info);
+      if (provider != null) {
+        argsBuilder.addTransitiveArgs(provider.getJavaCompilationArgs(), ClasspathType.BOTH);
+        recursiveArgsBuilder.addTransitiveArgs(
+            provider.getRecursiveJavaCompilationArgs(), ClasspathType.BOTH);
+      } else {
+        NestedSet<Artifact> filesToBuild = info.getProvider(FileProvider.class).getFilesToBuild();
+        for (Artifact jar : FileType.filter(filesToBuild, JavaSemantics.JAR)) {
+          argsBuilder.addRuntimeJar(jar).addCompileTimeJarAsFullJar(jar);
+          recursiveArgsBuilder.addRuntimeJar(jar).addCompileTimeJarAsFullJar(jar);
+        }
+      }
+    }
+    return create(argsBuilder.build(), recursiveArgsBuilder.build());
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
index fdd0c26082..e209ebc23d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -708,12 +709,11 @@ public final class JavaCompilationHelper {
   }
 
   private void addLibrariesToAttributesInternal(Iterable<? extends TransitiveInfoCollection> deps) {
-    JavaCompilationArgs args = JavaCompilationArgs.builder()
-        .addTransitiveTargets(deps).build();
+    JavaCompilationArgs args =
+        JavaCompilationArgsProvider.legacyFromTargets(deps).getRecursiveJavaCompilationArgs();
 
-    NestedSet<Artifact> directJars = isStrict()
-        ? getNonRecursiveCompileTimeJarsFromCollection(deps)
-        : null;
+    NestedSet<Artifact> directJars =
+        isStrict() ? getNonRecursiveCompileTimeJarsFromCollection(deps) : null;
     addArgsAndJarsToAttributes(args, directJars);
   }
 
@@ -724,7 +724,10 @@ public final class JavaCompilationHelper {
   private NestedSet<Artifact> getNonRecursiveCompileTimeJarsFromCollection(
       Iterable<? extends TransitiveInfoCollection> deps) {
     JavaCompilationArgs.Builder builder = JavaCompilationArgs.builder();
-    builder.addTransitiveTargets(deps, /*recursive=*/false);
+    builder.addTransitiveCompilationArgs(
+        JavaCompilationArgsProvider.legacyFromTargets(deps),
+        /*recursive=*/ false,
+        ClasspathType.BOTH);
     return builder.build().getCompileTimeJars();
   }
 
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 0a153af354..fb6668040b 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
@@ -332,28 +332,19 @@ public final class JavaLibraryHelper {
   }
 
   private void addDepsToAttributes(JavaTargetAttributes.Builder attributes) {
-    NestedSet<Artifact> directJars;
+    JavaCompilationArgsProvider argsProvider = JavaCompilationArgsProvider.merge(deps);
+
     if (isStrict()) {
-      directJars = getNonRecursiveCompileTimeJarsFromDeps();
+      NestedSet<Artifact> directJars = argsProvider.getJavaCompilationArgs().getCompileTimeJars();
       if (directJars != null) {
         attributes.addDirectJars(directJars);
       }
     }
 
-    JavaCompilationArgs args =
-        JavaCompilationArgs.builder()
-            .addTransitiveDependencies(deps, true)
-            .build();
-    attributes.addCompileTimeClassPathEntries(args.getCompileTimeJars());
-    attributes.addRuntimeClassPathEntries(args.getRuntimeJars());
-    attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata());
-  }
-
-  private NestedSet<Artifact> getNonRecursiveCompileTimeJarsFromDeps() {
-    return JavaCompilationArgs.builder()
-        .addTransitiveDependencies(deps, false)
-        .build()
-        .getCompileTimeJars();
+    JavaCompilationArgs recursiveArgs = argsProvider.getRecursiveJavaCompilationArgs();
+    attributes.addCompileTimeClassPathEntries(recursiveArgs.getCompileTimeJars());
+    attributes.addRuntimeClassPathEntries(recursiveArgs.getRuntimeJars());
+    attributes.addInstrumentationMetadataEntries(recursiveArgs.getInstrumentationMetadata());
   }
 
   static JavaCompilationArgs getJavaCompilationArgs(JavaCompilationArgsHelper helper) {
@@ -361,18 +352,14 @@ public final class JavaLibraryHelper {
     JavaCompilationArgs.Builder builder =
         JavaCompilationArgs.builder()
             .merge(helper.compilationArtifacts(), helper.isNeverLink())
-            .addTransitiveTargets(helper.exports(), helper.recursive(), type)
             .addTransitiveCompilationArgs(
                 helper.exportsCompilationArgs(), helper.recursive(), type);
     // TODO(bazel-team): remove srcs-less behaviour after android_library users are refactored
     if (helper.recursive() || helper.srcLessDepsExport()) {
       builder
           .addTransitiveCompilationArgs(helper.depsCompilationArgs(), helper.recursive(), type)
-          .addTransitiveTargets(helper.deps(), helper.recursive(), type)
           .addTransitiveCompilationArgs(
-              helper.runtimeDepsCompilationArgs(), helper.recursive(), ClasspathType.RUNTIME_ONLY)
-          .addTransitiveTargets(
-              helper.runtimeDeps(), helper.recursive(), ClasspathType.RUNTIME_ONLY);
+              helper.runtimeDepsCompilationArgs(), helper.recursive(), ClasspathType.RUNTIME_ONLY);
     }
     return builder.build();
   }