Make java_common.compile's javacopt handling consistent with native J…

…ava rules

Compilations performed by java_common.compile now use the javacopts in the
java_toolchain by default, instead of requiring them to be explicitly retrived
using java_common.default_javac_opts, for consistency with the native rules.

java_common.compile(javac_opts=...) can still be used to pass additional javacopts.

RELNOTES: Make java_common.compile now uses java_toolchain javacopts by default; explicitly retrieving them using java_common.default_javac_opts is unnecessary.
PiperOrigin-RevId: 194254098

src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidSemantics.java

@@ -43,13 +43,11 @@ public String getNativeDepsFileName() {
   }
 
   @Override
-  public ImmutableList<String> getJavacArguments(RuleContext ruleContext) {
+  public ImmutableList<String> getCompatibleJavacOptions(RuleContext ruleContext) {
     ImmutableList.Builder<String> javacArgs = new ImmutableList.Builder<>();
-
     if (!ruleContext.getFragment(AndroidConfiguration.class).desugarJava8()) {
       javacArgs.add("-source", "7", "-target", "7");
     }
-
     return javacArgs.build();
   }
 

src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java

@@ -62,6 +62,7 @@
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
+import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
 import com.google.devtools.build.lib.rules.java.JavaUtil;
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -553,8 +554,9 @@ public void collectTargetsTreatedAsDeps(
   }
 
   @Override
-  public Iterable<String> getExtraJavacOpts(RuleContext ruleContext) {
-    return ImmutableList.<String>of();
+  public ImmutableList<String> getCompatibleJavacOptions(
+      RuleContext ruleContext, JavaToolchainProvider toolchain) {
+    return ImmutableList.of();
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java

@@ -16,7 +16,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
@@ -466,13 +465,14 @@ public JavaTargetAttributes init(
       bootclasspath =
           ImmutableList.of(AndroidSdkProvider.fromRuleContext(ruleContext).getAndroidJar());
     }
-    Iterable<String> javacopts = androidSemantics.getJavacArguments(ruleContext);
+    ImmutableList.Builder<String> javacopts = ImmutableList.builder();
+    javacopts.addAll(androidSemantics.getCompatibleJavacOptions(ruleContext));
     if (DataBinding.isEnabled(ruleContext)) {
-      javacopts = Iterables.concat(javacopts, DataBinding.getJavacopts(ruleContext, isBinary));
+      javacopts.addAll(DataBinding.getJavacOpts(ruleContext, isBinary));
     }
     JavaTargetAttributes.Builder attributes =
         javaCommon
-            .initCommon(idlHelper.getIdlGeneratedJavaSources(), javacopts)
+            .initCommon(idlHelper.getIdlGeneratedJavaSources(), javacopts.build())
             .setBootClassPath(bootclasspath);
     if (DataBinding.isEnabled(ruleContext)) {
       DataBinding.addAnnotationProcessor(ruleContext, attributes);

src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java

@@ -69,10 +69,10 @@ default ApplicationManifest getManifestForRule(RuleContext ruleContext)
    * Returns the command line options to be used when compiling Java code for {@code android_*}
    * rules.
    *
-   * <p>These will come after the default options specified by the toolchain and the ones in the
-   * {@code javacopts} attribute.
+   * <p>These will come after the default options specified by the toolchain, and before the ones in
+   * the {@code javacopts} attribute.
    */
-  ImmutableList<String> getJavacArguments(RuleContext ruleContext);
+  ImmutableList<String> getCompatibleJavacOptions(RuleContext ruleContext);
 
   /**
    * Configures the builder for generating the output jar used to configure the main dex file.

src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java

@@ -169,7 +169,7 @@ static void addAnnotationProcessor(
   }
 
   /** The javac flags that are needed to configure data binding's annotation processor. */
-  static ImmutableList<String> getJavacopts(RuleContext ruleContext, boolean isBinary) {
+  static ImmutableList<String> getJavacOpts(RuleContext ruleContext, boolean isBinary) {
     ImmutableList.Builder<String> flags = ImmutableList.builder();
     String metadataOutputDir = getDataBindingExecPath(ruleContext).getPathString();
 

src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java

@@ -61,7 +61,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 /**
@@ -108,6 +107,7 @@ public void collectMetadataArtifacts(
 
   private final RuleContext ruleContext;
   private final JavaSemantics semantics;
+  private final JavaToolchainProvider javaToolchain;
   private JavaCompilationHelper javaCompilationHelper;
 
   public JavaCommon(RuleContext ruleContext, JavaSemantics semantics) {
@@ -144,6 +144,7 @@ public JavaCommon(RuleContext ruleContext,
       ImmutableList<TransitiveInfoCollection> runtimeDeps,
       ImmutableList<TransitiveInfoCollection> bothDeps) {
     this.ruleContext = ruleContext;
+    this.javaToolchain = JavaToolchainProvider.from(ruleContext);
     this.semantics = semantics;
     this.sources = sources;
     this.targetsTreatedAsDeps = ImmutableMap.of(
@@ -451,39 +452,34 @@ protected JavaExportsProvider collectTransitiveExports() {
 
   public final void initializeJavacOpts() {
     Preconditions.checkState(javacOpts == null);
-    javacOpts = computeJavacOpts(semantics.getExtraJavacOpts(ruleContext));
+    javacOpts = computeJavacOpts(getCompatibleJavacOptions());
   }
 
-  /**
-   * For backwards compatibility, this method allows multiple calls to set the Javac opts. Do not
-   * use this.
-   */
-  @Deprecated
-  public final void initializeJavacOpts(Iterable<String> extraJavacOpts) {
-    javacOpts = computeJavacOpts(extraJavacOpts);
+  /** Computes javacopts for the current rule. */
+  private ImmutableList<String> computeJavacOpts(Collection<String> extraRuleJavacOpts) {
+    return ImmutableList.<String>builder()
+        .addAll(computeToolchainJavacOpts(ruleContext, javaToolchain))
+        .addAll(extraRuleJavacOpts)
+        .addAll(ruleContext.getExpander().withDataLocations().tokenized("javacopts"))
+        .build();
   }
 
-  private ImmutableList<String> computeJavacOpts(Iterable<String> extraJavacOpts) {
+  /**
+   * Returns the toolchain javacopts for the current rule, including global defaults as well as any
+   * per-package configuration.
+   */
+  public static ImmutableList<String> computeToolchainJavacOpts(
+      RuleContext ruleContext, JavaToolchainProvider toolchain) {
     return Streams.concat(
-            toolchainJavacOpts(ruleContext),
-            Streams.stream(extraJavacOpts),
-            ruleContext.getExpander().withDataLocations().tokenized("javacopts").stream())
+            toolchain.getJavacOptions().stream(),
+            toolchain
+                .packageConfiguration()
+                .stream()
+                .filter(p -> p.matches(ruleContext.getLabel()))
+                .flatMap(p -> p.javacopts().stream()))
         .collect(toImmutableList());
   }
 
-  private Stream<String> toolchainJavacOpts(RuleContext ruleContext) {
-    JavaToolchainProvider toolchain = JavaToolchainProvider.from(ruleContext);
-    return Stream.concat(
-        toolchain.getJavacOptions().stream(),
-        // Enable any javacopts from java_toolchain.packages that are configured for the current
-        // package.
-        toolchain
-            .packageConfiguration()
-            .stream()
-            .filter(p -> p.matches(ruleContext.getLabel()))
-            .flatMap(p -> p.javacopts().stream()));
-  }
-
   public static PathFragment getHostJavaExecutable(RuleContext ruleContext) {
     return JavaRuntimeInfo.forHost(ruleContext).javaBinaryExecPath();
   }
@@ -600,7 +596,11 @@ private static List<TransitiveInfoCollection> getRuntimeDeps(RuleContext ruleCon
   }
 
   public JavaTargetAttributes.Builder initCommon() {
-    return initCommon(ImmutableList.<Artifact>of(), semantics.getExtraJavacOpts(ruleContext));
+    return initCommon(ImmutableList.<Artifact>of(), getCompatibleJavacOptions());
+  }
+
+  private ImmutableList<String> getCompatibleJavacOptions() {
+    return semantics.getCompatibleJavacOptions(ruleContext, javaToolchain);
   }
 
   /**
@@ -614,7 +614,7 @@ public JavaTargetAttributes.Builder initCommon() {
   public JavaTargetAttributes.Builder initCommon(
       Collection<Artifact> extraSrcs, Iterable<String> extraJavacOpts) {
     Preconditions.checkState(javacOpts == null);
-    javacOpts = computeJavacOpts(extraJavacOpts);
+    javacOpts = computeJavacOpts(ImmutableList.copyOf(extraJavacOpts));
     activePlugins = collectPlugins();
 
     JavaTargetAttributes.Builder javaTargetAttributes = new JavaTargetAttributes.Builder(semantics);

src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java

@@ -451,14 +451,25 @@ public JavaInfo createJavaCompileAction(
       throw new EvalException(null, "'host_javabase' must point to a Java runtime");
     }
 
+    JavaToolchainProvider toolchainProvider = getJavaToolchainProvider(javaToolchain);
+
     JavaLibraryHelper helper =
         new JavaLibraryHelper(skylarkRuleContext.getRuleContext())
             .setOutput(outputJar)
             .addSourceJars(sourceJars)
             .addSourceFiles(sourceFiles)
             .addResources(resources)
             .setSourcePathEntries(sourcepathEntries)
-            .setJavacOpts(javacOpts);
+            .setJavacOpts(
+                ImmutableList.<String>builder()
+                    .addAll(
+                        JavaCommon.computeToolchainJavacOpts(
+                            skylarkRuleContext.getRuleContext(), toolchainProvider))
+                    .addAll(
+                        javaSemantics.getCompatibleJavacOptions(
+                            skylarkRuleContext.getRuleContext(), toolchainProvider))
+                    .addAll(javacOpts)
+                    .build());
 
     List<JavaCompilationArgsProvider> depsCompilationArgsProviders =
         JavaInfo.fetchProvidersFromList(deps, JavaCompilationArgsProvider.class);
@@ -486,7 +497,7 @@ public JavaInfo createJavaCompileAction(
     JavaCompilationArtifacts artifacts =
         helper.build(
             javaSemantics,
-            getJavaToolchainProvider(javaToolchain),
+            toolchainProvider,
             javaRuntimeInfo,
             SkylarkList.createImmutable(ImmutableList.of()),
             outputJarsBuilder,

src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java

@@ -364,9 +364,13 @@ void addRunfilesForBinary(RuleContext ruleContext, Artifact launcher,
   void addRunfilesForLibrary(RuleContext ruleContext, Runfiles.Builder runfilesBuilder);
 
   /**
-   * Returns the additional options to be passed to javac.
+   * Returns the command line options to be used when compiling Java code for {@code java_*} rules.
+   *
+   * <p>These will come after the default options specified by the toolchain, and before the ones in
+   * the {@code javacopts} attribute.
    */
-  Iterable<String> getExtraJavacOpts(RuleContext ruleContext);
+  ImmutableList<String> getCompatibleJavacOptions(
+      RuleContext ruleContext, JavaToolchainProvider toolchain);
 
   /**
    * Add additional targets to be treated as direct dependencies.

src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java

@@ -524,8 +524,9 @@ public Artifact packSources(
       mandatoryPositionals = 1,
       parameters = {
         @Param(name = "java_toolchain_attr", positional = false, named = true, type = String.class)
-      }
-  )
+      })
+  // TODO(b/78512644): migrate callers to passing explicit javacopts or using custom toolchains, and
+  // delete
   public static ImmutableList<String> getDefaultJavacOpts(
       SkylarkRuleContext skylarkRuleContext, String javaToolchainAttr) throws EvalException {
     RuleContext ruleContext = skylarkRuleContext.getRuleContext();

src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java

@@ -23,7 +23,6 @@
 import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
-import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
 import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder;
 import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainProvider;
@@ -136,18 +135,16 @@ public static JavaInfo getRuntimeToolchainProvider(
       @Param(name = "java_toolchain_attr", positional = false, named = true, type = String.class)
     }
   )
-  // TODO(elenairina): Consider a nicer way of returning this, taking in a JavaToolchainProvider.
+  // TODO(b/78512644): migrate callers to passing explicit proto javacopts or using custom
+  // toolchains, and delete
   public static ImmutableList<String> getJavacOpts(
       SkylarkRuleContext skylarkRuleContext, String javaToolchainAttr) throws EvalException {
     ConfiguredTarget javaToolchainConfigTarget =
         (ConfiguredTarget) checkNotNull(skylarkRuleContext.getAttr().getValue(javaToolchainAttr));
     JavaToolchainProvider toolchain =
         checkNotNull(JavaToolchainProvider.from(javaToolchainConfigTarget));
 
-    return ImmutableList.<String>builder()
-        .addAll(toolchain.getJavacOptions())
-        .addAll(toolchain.getCompatibleJavacOptions(JavaSemantics.PROTO_JAVACOPTS_KEY))
-        .build();
+    return ProtoJavacOpts.constructJavacOpts(skylarkRuleContext.getRuleContext(), toolchain);
   }
 
   private static ProtoLangToolchainProvider getProtoToolchainProvider(

src/main/java/com/google/devtools/build/lib/rules/java/proto/ProtoJavacOpts.java

@@ -30,7 +30,16 @@ public class ProtoJavacOpts {
    * <p>See java_toolchain.compatible_javacopts for the javacopts required for protos.
    */
   public static ImmutableList<String> constructJavacOpts(RuleContext ruleContext) {
-    JavaToolchainProvider toolchain = JavaToolchainProvider.from(ruleContext);
+    return constructJavacOpts(ruleContext, JavaToolchainProvider.from(ruleContext));
+  }
+
+  /**
+   * Returns javacopts for compiling the Java source files generated by the proto compiler.
+   *
+   * <p>See java_toolchain.compatible_javacopts for the javacopts required for protos.
+   */
+  public static ImmutableList<String> constructJavacOpts(
+      RuleContext ruleContext, JavaToolchainProvider toolchain) {
     return ImmutableList.<String>builder()
         .addAll(toolchain.getJavacOptions())
         .addAll(toolchain.getCompatibleJavacOptions(JavaSemantics.PROTO_JAVACOPTS_KEY))