Replace java_plugin_configuration with java_package_configuration

java_toolchain.plugin_configuration creates cycles in the unconfigured
target graph. Instead, use the same per-package configuration approach
to allow enabling javacopts, which can be used to enable built-in
checks on a per-package basis.

PiperOrigin-RevId: 179598500

9 files changed, 72 insertions, 71 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index d16c7a4175..22147ec52c 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -712,9 +712,9 @@ java_library(
         "rules/java/JavaImportBaseRule.java",
         "rules/java/JavaInfo.java",
         "rules/java/JavaLibrary.java",
+        "rules/java/JavaPackageConfiguration.java",
+        "rules/java/JavaPackageConfigurationRule.java",
         "rules/java/JavaPlugin.java",
-        "rules/java/JavaPluginConfiguration.java",
-        "rules/java/JavaPluginConfigurationRule.java",
         "rules/java/JavaPrimaryClassProvider.java",
         "rules/java/JavaRuntime.java",
         "rules/java/JavaRuntimeAlias.java",
@@ -813,7 +813,7 @@ java_library(
         "rules/java/JavaLibraryHelper.java",
         "rules/java/JavaNativeLibraryProvider.java",
         "rules/java/JavaOptions.java",
-        "rules/java/JavaPluginConfigurationProvider.java",
+        "rules/java/JavaPackageConfigurationProvider.java",
         "rules/java/JavaPluginInfoProvider.java",
         "rules/java/JavaRuleClasses.java",
         "rules/java/JavaRuleOutputJarsProvider.java",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 0cad20cd0f..f503af88ee 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -118,7 +118,7 @@ import com.google.devtools.build.lib.rules.java.JavaConfigurationLoader;
 import com.google.devtools.build.lib.rules.java.JavaImportBaseRule;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.rules.java.JavaOptions;
-import com.google.devtools.build.lib.rules.java.JavaPluginConfigurationRule;
+import com.google.devtools.build.lib.rules.java.JavaPackageConfigurationRule;
 import com.google.devtools.build.lib.rules.java.JavaRuleClasses.IjarBaseRule;
 import com.google.devtools.build.lib.rules.java.JavaRuntimeAlias;
 import com.google.devtools.build.lib.rules.java.JavaRuntimeRule;
@@ -397,7 +397,7 @@ public class BazelRuleClassProvider {
           builder.addRuleDefinition(new BazelJavaTestRule());
           builder.addRuleDefinition(new BazelJavaPluginRule());
           builder.addRuleDefinition(new JavaToolchainRule());
-          builder.addRuleDefinition(new JavaPluginConfigurationRule());
+          builder.addRuleDefinition(new JavaPackageConfigurationRule());
           builder.addRuleDefinition(new JavaRuntimeRule());
           builder.addRuleDefinition(new JavaRuntimeSuiteRule());
           builder.addRuleDefinition(new JavaRuntimeAlias.JavaRuntimeAliasRule());
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 4b246657c3..5b018b2fb6 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
@@ -63,6 +63,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 /**
@@ -459,12 +460,25 @@ public class JavaCommon {
 
   private ImmutableList<String> computeJavacOpts(Iterable<String> extraJavacOpts) {
     return Streams.concat(
-            JavaToolchainProvider.from(ruleContext).getJavacOptions().stream(),
+            toolchainJavacOpts(ruleContext),
             Streams.stream(extraJavacOpts),
             ruleContext.getExpander().withDataLocations().tokenized("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) {
     JavaRuntimeInfo javaRuntime = JavaHelper.getHostJavaRuntime(ruleContext);
     return javaRuntime != null
@@ -824,13 +838,6 @@ public class JavaCommon {
         getPluginInfoProvidersForAttribute(ruleContext, ":java_plugins", Mode.HOST));
     Iterables.addAll(result, getPluginInfoProvidersForAttribute(ruleContext, "plugins", Mode.HOST));
     Iterables.addAll(result, getPluginInfoProvidersForAttribute(ruleContext, "deps", Mode.TARGET));
-    // Enable any plugins from java_toolchain.plugins that are configured for the current package.
-    JavaToolchainProvider.from(ruleContext)
-        .pluginConfiguration()
-        .stream()
-        .filter(p -> p.matches(ruleContext.getLabel()))
-        .map(JavaPluginConfigurationProvider::plugin)
-        .forEachOrdered(result::add);
     return ImmutableList.copyOf(result);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfiguration.java
index ac9ac3017c..649957c3d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfiguration.java
@@ -25,10 +25,11 @@ import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.syntax.Type;
 import java.util.List;
 
-/** Implementation for the java_plugin_configuration rule. */
-public class JavaPluginConfiguration implements RuleConfiguredTargetFactory {
+/** Implementation for the java_package_configuration rule. */
+public class JavaPackageConfiguration implements RuleConfiguredTargetFactory {
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext) {
@@ -36,14 +37,11 @@ public class JavaPluginConfiguration implements RuleConfiguredTargetFactory {
         ImmutableList.copyOf(
             ruleContext.getPrerequisites(
                 "packages", Mode.HOST, PackageSpecificationProvider.class));
-    JavaPluginInfoProvider plugins =
-        JavaPluginInfoProvider.merge(
-            JavaInfo.getProvidersFromListOfTargets(
-                JavaPluginInfoProvider.class, ruleContext.getPrerequisites("plugins", Mode.HOST)));
+    List<String> javacopts = ruleContext.attributes().get("javacopts", Type.STRING_LIST);
     return new RuleConfiguredTargetBuilder(ruleContext)
         .addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))
         .setFilesToBuild(NestedSetBuilder.emptySet(Order.STABLE_ORDER))
-        .addProvider(JavaPluginConfigurationProvider.create(packages, plugins))
+        .addProvider(JavaPackageConfigurationProvider.create(packages, javacopts))
         .build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfigurationProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfigurationProvider.java
index e1ca135759..3915e4c13d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfigurationProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfigurationProvider.java
@@ -22,26 +22,26 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import java.util.List;
 
-/** A provider for Java plugin configuration. */
+/** A provider for Java per-package configuration. */
 @AutoValue
 @Immutable
-public abstract class JavaPluginConfigurationProvider implements TransitiveInfoProvider {
+public abstract class JavaPackageConfigurationProvider implements TransitiveInfoProvider {
 
-  /** Creates a {@link JavaPluginConfigurationProvider}. */
-  public static JavaPluginConfigurationProvider create(
-      List<PackageSpecificationProvider> packageSpecifications, JavaPluginInfoProvider plugin) {
-    return new AutoValue_JavaPluginConfigurationProvider(packageSpecifications, plugin);
+  /** Creates a {@link JavaPackageConfigurationProvider}. */
+  public static JavaPackageConfigurationProvider create(
+      List<PackageSpecificationProvider> packageSpecifications, List<String> javacopts) {
+    return new AutoValue_JavaPackageConfigurationProvider(packageSpecifications, javacopts);
   }
 
-  /** Package specifications for which the plugins should be enabled. */
+  /** Package specifications for which the configuration should be applied. */
   abstract List<PackageSpecificationProvider> packageSpecifications();
 
-  /** The Java plugins described by this configuration. */
-  abstract JavaPluginInfoProvider plugin();
+  /** The javacopts for this configuration. */
+  abstract List<String> javacopts();
 
   /**
-   * Returns true if this plugin configuration matches the current label: that is, if the label's
-   * package is contained by any of the {@link #packageSpecifications}.
+   * Returns true if this configuration matches the current label: that is, if the label's package
+   * is contained by any of the {@link #packageSpecifications}.
    */
   public boolean matches(Label label) {
     return packageSpecifications()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfigurationRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfigurationRule.java
index 5e4522201d..2955f955c2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginConfigurationRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPackageConfigurationRule.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.rules.java;
 import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LICENSE;
-import static com.google.devtools.build.lib.rules.java.JavaRuleClasses.CONTAINS_JAVA_PROVIDER;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
@@ -27,31 +26,27 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
+import com.google.devtools.build.lib.syntax.Type;
 
-/** Rule definition for {@code java_plugin_configuration} */
-public class JavaPluginConfigurationRule implements RuleDefinition {
+/** Rule definition for {@code java_package_configuration} */
+public class JavaPackageConfigurationRule implements RuleDefinition {
 
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
     return builder
-        /* <!-- #BLAZE_RULE(java_plugin_configuration).ATTRIBUTE(packages) -->
+        /* <!-- #BLAZE_RULE(java_package_configuration).ATTRIBUTE(packages) -->
         The set of <code><a href="${link package_group}">package_group</a></code>s
-        the plugins in this configuration should be enabled for.
+        the configuration should be applied to.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(
             attr("packages", BuildType.LABEL_LIST)
                 .cfg(HOST)
                 .allowedFileTypes()
                 .mandatoryNativeProviders(ImmutableList.of(PackageSpecificationProvider.class)))
-        /* <!-- #BLAZE_RULE(java_plugin_configuration).ATTRIBUTE(packages) -->
-        The list of <code><a href="${link java_plugin}">java_plugin</a></code>s included in this
-        configuration.
+        /* <!-- #BLAZE_RULE(java_package_configuration).ATTRIBUTE(javacopts) -->
+        Java compiler flags.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(
-            attr("plugins", BuildType.LABEL_LIST)
-                .cfg(HOST)
-                .allowedFileTypes()
-                .mandatoryProvidersList(ImmutableList.of(CONTAINS_JAVA_PROVIDER)))
+        .add(attr("javacopts", Type.STRING_LIST))
         .add(attr("output_licenses", LICENSE))
         .build();
   }
@@ -59,30 +54,31 @@ public class JavaPluginConfigurationRule implements RuleDefinition {
   @Override
   public Metadata getMetadata() {
     return RuleDefinition.Metadata.builder()
-        .name("java_plugin_configuration")
+        .name("java_package_configuration")
         .ancestors(BaseRuleClasses.BaseRule.class)
-        .factoryClass(JavaPluginConfiguration.class)
+        .factoryClass(JavaPackageConfiguration.class)
         .build();
   }
 }
-/*<!-- #BLAZE_RULE (NAME = java_plugin_configuration, TYPE = OTHER, FAMILY = Java) -->
+/*<!-- #BLAZE_RULE (NAME = java_package_configuration, TYPE = OTHER, FAMILY = Java) -->
 
 <p>
-Configures a set of <code><a href="${link java_plugin}">java_plugin</a></code>s to run on a set of
-packages. Plugin configurations can be added to
-<code><a href="${link java_toolchain.plugins}">java_toolchain.plugins</a></code>s.
+Configuration to apply to a set of packages.
+Configurations can be added to
+<code><a href="${link java_toolchain.javacopts}">java_toolchain.javacopts</a></code>s.
 </p>
 
-<h4 id="java_plugin_configuration">Example:</h4>
+<h4 id="java_package_configuration">Example:</h4>
 
 <pre class="code">
-java_plugin_configuration(
-    plugins = [":my_java_plugin"],
-    packages = [":plugin_packages"],
+java_package_configuration(
+    name = "my_configuration",
+    packages = [":my_packages"],
+    javacopts = ["-Werror"],
 )
 
 package_group(
-    name = "plugin_packages",
+    name = "my_packages",
     packages = [
         "//com/my/project/...",
         "-//com/my/project/testing/...",
@@ -91,8 +87,8 @@ package_group(
 
 java_toolchain(
     ...,
-    plugin_configuration = [
-        ":my_plugin",
+    package_configuration = [
+        ":my_configuration",
     ]
 )
 </pre>
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java
index 69bcc7374d..edb67987ff 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchain.java
@@ -82,10 +82,10 @@ public final class JavaToolchain implements RuleConfiguredTargetFactory {
             ImmutableMap.<Label, ImmutableCollection<Artifact>>of(
                 AliasProvider.getDependencyLabel(javacDep), ImmutableList.of(javac)));
 
-    ImmutableList<JavaPluginConfigurationProvider> pluginConfiguration =
+    ImmutableList<JavaPackageConfigurationProvider> packageConfiguration =
         ImmutableList.copyOf(
             ruleContext.getPrerequisites(
-                "plugin_configuration", Mode.HOST, JavaPluginConfigurationProvider.class));
+                "package_configuration", Mode.HOST, JavaPackageConfigurationProvider.class));
 
     JavaToolchainData toolchainData =
         new JavaToolchainData(
@@ -115,7 +115,7 @@ public final class JavaToolchain implements RuleConfiguredTargetFactory {
             timezoneData,
             ijar,
             compatibleJavacOptions,
-            pluginConfiguration);
+            packageConfiguration);
     RuleConfiguredTargetBuilder builder =
         new RuleConfiguredTargetBuilder(ruleContext)
             .addSkylarkTransitiveInfo(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java
index 2306e24b5b..725a5f74d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java
@@ -81,7 +81,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
       @Nullable Artifact timezoneData,
       FilesToRunProvider ijar,
       ImmutableListMultimap<String, String> compatibleJavacOptions,
-      ImmutableList<JavaPluginConfigurationProvider> pluginConfiguration) {
+      ImmutableList<JavaPackageConfigurationProvider> packageConfiguration) {
     return new JavaToolchainProvider(
         label,
         bootclasspath,
@@ -107,7 +107,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
             .build(),
         data.getJvmOptions(),
         data.getJavacSupportsWorkers(),
-        pluginConfiguration);
+        packageConfiguration);
   }
 
   private final Label label;
@@ -129,7 +129,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
   private final ImmutableList<String> javacOptions;
   private final ImmutableList<String> jvmOptions;
   private final boolean javacSupportsWorkers;
-  private final ImmutableList<JavaPluginConfigurationProvider> pluginConfiguration;
+  private final ImmutableList<JavaPackageConfigurationProvider> packageConfiguration;
 
   private JavaToolchainProvider(
       Label label,
@@ -151,7 +151,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
       ImmutableList<String> javacOptions,
       ImmutableList<String> jvmOptions,
       boolean javacSupportsWorkers,
-      ImmutableList<JavaPluginConfigurationProvider> pluginConfiguration) {
+      ImmutableList<JavaPackageConfigurationProvider> packageConfiguration) {
     super(ImmutableMap.of(), Location.BUILTIN);
 
     this.label = label;
@@ -173,7 +173,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
     this.javacOptions = javacOptions;
     this.jvmOptions = jvmOptions;
     this.javacSupportsWorkers = javacSupportsWorkers;
-    this.pluginConfiguration = pluginConfiguration;
+    this.packageConfiguration = packageConfiguration;
   }
 
   /** Returns the label for this {@code java_toolchain}. */
@@ -291,7 +291,7 @@ public class JavaToolchainProvider extends ToolchainInfo {
   }
 
   /** Returns the global {@code java_plugin_configuration} data. */
-  public ImmutableList<JavaPluginConfigurationProvider> pluginConfiguration() {
-    return pluginConfiguration;
+  public ImmutableList<JavaPackageConfigurationProvider> packageConfiguration() {
+    return packageConfiguration;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java
index f3cdf60d5d..2ec3fd3dd4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java
@@ -205,14 +205,14 @@ public final class JavaToolchainRule implements RuleDefinition {
             attr("compatible_javacopts", STRING_LIST_DICT)
                 .undocumented("internal")
                 .value(ImmutableMap.<String, List<String>>of()))
-        /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(plugin_configuration) -->
-        Java plugins that should be enabled for the specified package groups.
+        /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(package_configuration) -->
+        Configuration that should be applied to the specified package groups.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(
-            attr("plugin_configuration", LABEL_LIST)
+            attr("package_configuration", LABEL_LIST)
                 .cfg(HOST)
                 .allowedFileTypes()
-                .mandatoryNativeProviders(ImmutableList.of(JavaPluginConfigurationProvider.class)))
+                .mandatoryNativeProviders(ImmutableList.of(JavaPackageConfigurationProvider.class)))
         .build();
   }