java_proto_library strict deps: add attributes for gradual migration

Control strict-deps through a rule-level and a package-level attribute, allowing finer-grained migration in conjunction with a global flag.

RELNOTES: java_proto_library: control strict-deps through a rule-level and a package-level attribute.

--
MOS_MIGRATED_REVID=128542363

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

@@ -17,6 +17,7 @@
 import static com.google.devtools.build.lib.packages.Aspect.INJECTING_RULE_KIND_PARAMETER_KEY;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
+import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 
 import com.google.common.base.Function;
 import com.google.devtools.build.lib.analysis.BaseRuleClasses;
@@ -25,7 +26,7 @@
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
-
+import com.google.devtools.build.lib.rules.java.JavaConfiguration;
 import javax.annotation.Nullable;
 
 /** Declaration of the {@code java_proto_library} rule. */
@@ -53,6 +54,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
     return builder
         // This rule isn't ready for use yet.
         .setUndocumented()
+        .requiresConfigurationFragments(JavaConfiguration.class)
         /* <!-- #BLAZE_RULE(java_proto_library).ATTRIBUTE(deps) -->
         The list of <a href="protocol-buffer.html#proto_library"><code>proto_library</code></a>
         rules to generate Java code for.
@@ -62,6 +64,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
                 .aspect(javaProtoAspect, ASPECT_PARAMETERS))
+        .add(attr("strict_deps", BOOLEAN).undocumented("for migration"))
         .build();
   }
 

src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -38,7 +38,6 @@
 import com.google.devtools.build.lib.vfs.Canonicalizer;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -47,7 +46,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
 import javax.annotation.Nullable;
 
 /**
@@ -141,10 +139,10 @@ protected NameConflictException(String message) {
    */
   private String defaultHdrsCheck;
 
-  /**
-   * Default copts for cc_* rules.  The rules' individual copts will append to
-   * this value.
-   */
+  /** See getter. */
+  private TriState defaultStrictDepsJavaProtos = TriState.AUTO;
+
+  /** Default copts for cc_* rules. The rules' individual copts will append to this value. */
   private ImmutableList<String> defaultCopts;
 
   /**
@@ -590,8 +588,15 @@ public String getDefaultDeprecation() {
   }
 
   /**
-   * Gets the default header checking mode.
+   * Default for 'strict_deps' of Java proto rules, when they aren't explicitly specified.
+   *
+   * <p>A value of AUTO is returned when the package didn't itself explicitly specify this value.
    */
+  public TriState getDefaultStrictDepsJavaProtos() {
+    return defaultStrictDepsJavaProtos;
+  }
+
+  /** Gets the default header checking mode. */
   public String getDefaultHdrsCheck() {
     return defaultHdrsCheck != null ? defaultHdrsCheck : "strict";
   }
@@ -907,9 +912,12 @@ public Builder setDefaultHdrsCheck(String hdrsCheck) {
       return this;
     }
 
-    /**
-     * Sets the default value of copts. Rule-level copts will append to this.
-     */
+    public Builder setDefaultStrictDepsJavaProtos(TriState value) {
+      pkg.defaultStrictDepsJavaProtos = value;
+      return this;
+    }
+
+    /** Sets the default value of copts. Rule-level copts will append to this. */
     public Builder setDefaultCopts(List<String> defaultCopts) {
       this.defaultCopts = defaultCopts;
       return this;

src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -68,7 +68,6 @@
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.UnixGlob;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -81,7 +80,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Logger;
-
 import javax.annotation.Nullable;
 
 /**
@@ -264,6 +262,21 @@ protected void process(Package.Builder pkgBuilder, Location location,
     }
   }
 
+  /**
+   * Declares the package() attribute specifying the default value for
+   * java_proto_library.strict_deps.
+   */
+  private static class DefaultStrictDepsJavaProtos extends PackageArgument<Boolean> {
+    private DefaultStrictDepsJavaProtos() {
+      super("default_strict_deps_java_protos", Type.BOOLEAN);
+    }
+
+    @Override
+    protected void process(Package.Builder pkgBuilder, Location location, Boolean value) {
+      pkgBuilder.setDefaultStrictDepsJavaProtos(value ? TriState.YES : TriState.NO);
+    }
+  }
+
   public static final String PKG_CONTEXT = "$pkg_context";
 
   // Used outside of Bazel!
@@ -457,14 +470,15 @@ public ImmutableList<EnvironmentExtension> getEnvironmentExtensions() {
   private ImmutableMap<String, PackageArgument<?>> createPackageArguments() {
     ImmutableList.Builder<PackageArgument<?>> arguments =
         ImmutableList.<PackageArgument<?>>builder()
-           .add(new DefaultDeprecation())
-           .add(new DefaultDistribs())
-           .add(new DefaultLicenses())
-           .add(new DefaultTestOnly())
-           .add(new DefaultVisibility())
-           .add(new Features())
-           .add(new DefaultCompatibleWith())
-           .add(new DefaultRestrictedTo());
+            .add(new DefaultDeprecation())
+            .add(new DefaultStrictDepsJavaProtos())
+            .add(new DefaultDistribs())
+            .add(new DefaultLicenses())
+            .add(new DefaultTestOnly())
+            .add(new DefaultVisibility())
+            .add(new Features())
+            .add(new DefaultCompatibleWith())
+            .add(new DefaultRestrictedTo());
 
     for (EnvironmentExtension extension : environmentExtensions) {
       arguments.addAll(extension.getPackageArguments());

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

@@ -30,9 +30,7 @@
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.common.options.TriState;
-
 import java.util.List;
-
 import javax.annotation.Nullable;
 
 /** A java compiler configuration containing the flags required for compilation. */
@@ -131,7 +129,7 @@ public boolean alwaysGenerateOutputMapping() {
   private final boolean useHeaderCompilation;
   private final boolean optimizeHeaderCompilationAnnotationProcessing;
   private final boolean generateJavaDeps;
-  private final boolean javaProtoLibraryDepsAreStrict;
+  private final boolean strictDepsJavaProtos;
   private final JavaClasspathMode javaClasspath;
   private final ImmutableList<String> javaWarns;
   private final ImmutableList<String> defaultJvmFlags;
@@ -173,7 +171,7 @@ public boolean alwaysGenerateOutputMapping() {
     this.javaToolchain = javaToolchain;
     this.javaOptimizationMode = javaOptions.javaOptimizationMode;
     this.legacyBazelJavaTest = javaOptions.legacyBazelJavaTest;
-    this.javaProtoLibraryDepsAreStrict = javaOptions.javaProtoLibraryDepsAreStrict;
+    this.strictDepsJavaProtos = javaOptions.strictDepsJavaProtos;
 
     ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
     for (String s : javaOptions.translationTargets) {
@@ -339,8 +337,7 @@ public boolean useLegacyBazelJavaTest() {
     return legacyBazelJavaTest;
   }
 
-  // TODO(b/29867858): Replace with a BUILD-level attribute.
-  public boolean javaProtoLibraryDepsAreStrict() {
-    return javaProtoLibraryDepsAreStrict;
+  public boolean strictDepsJavaProtos() {
+    return strictDepsJavaProtos;
   }
 }

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

@@ -25,7 +25,6 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
 import com.google.devtools.build.lib.util.Preconditions;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -112,9 +111,17 @@ public JavaLibraryHelper setJavacOpts(Iterable<String> javacOpts) {
   }
 
   /**
-   * Sets the mode that determines how strictly dependencies are checked.
+   * When in strict mode, compiling the source-jars passed to this JavaLibraryHelper will break if
+   * they depend on classes not in any of the {@link
+   * JavaCompilationArgsProvider#javaCompilationArgs} passed in {@link #addDep}, even if they do
+   * appear in {@link JavaCompilationArgsProvider#recursiveJavaCompilationArgs}. That is, depending
+   * on a class requires a direct dependency on it.
+   *
+   * <p>Contrast this with the strictness-parameter to {@link #buildCompilationArgsProvider}, which
+   * controls whether others depending on the result of this compilation, can perform strict-deps
+   * checks at all.
    */
-  public JavaLibraryHelper setStrictDepsMode(StrictDepsMode strictDepsMode) {
+  public JavaLibraryHelper setCompilationStrictDepsMode(StrictDepsMode strictDepsMode) {
     this.strictDepsMode = strictDepsMode;
     return this;
   }
@@ -155,18 +162,25 @@ public JavaCompilationArgs build(JavaSemantics semantics) {
 
   /**
    * 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 by direct vs transitive
-   * jars).
+   * result of a call to build(). (that is, it contains the compile-time and runtime jars, separated
+   * by direct vs transitive jars).
+   *
+   * @param isReportedAsStrict if true, the result's direct JavaCompilationArgs only contain classes
+   *     resulting from compiling the source-jars. If false, the direct JavaCompilationArgs contain
+   *     both these classes, as well as any classes from transitive dependencies. A value of 'false'
+   *     means this compilation cannot be checked for strict-deps, by any consumer (depending)
+   *     compilation. Contrast this with {@link #setCompilationStrictDepsMode}.
    */
-  public JavaCompilationArgsProvider buildCompilationArgsProvider(JavaCompilationArgs directArgs) {
-    JavaCompilationArgs transitiveArgs = JavaCompilationArgs.builder()
-        .addTransitiveArgs(directArgs, BOTH)
-        .addTransitiveDependencies(deps, true /* recursive */)
+  public JavaCompilationArgsProvider buildCompilationArgsProvider(
+      JavaCompilationArgs directArgs, boolean isReportedAsStrict) {
+    JavaCompilationArgs transitiveArgs =
+        JavaCompilationArgs.builder()
+            .addTransitiveArgs(directArgs, BOTH)
+            .addTransitiveDependencies(deps, true /* recursive */)
             .build();
 
     return new JavaCompilationArgsProvider(
-        isStrict() ? directArgs : transitiveArgs, transitiveArgs);
+        isReportedAsStrict ? directArgs : transitiveArgs, transitiveArgs);
   }
 
   private void addDepsToAttributes(JavaTargetAttributes.Builder attributes) {

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

@@ -351,17 +351,17 @@ public JavaOptimizationModeConverter() {
   public boolean legacyBazelJavaTest;
 
   @Option(
-    name = "java_proto_library_deps_are_strict",
-    defaultValue = "false",
+    name = "strict_deps_java_protos",
+    defaultValue = "true",
     category = "undocumented",
     help =
-        "This only applies to java_proto_library. "
-            + "If true: (1) if a Java file uses proto Foo, it must depend on a  "
-            + "java_{lite,...}_proto_library that directly depends on a proto_library that has Foo "
-            + "in its srcs. (2) strict-deps violations are reported for the proto_library rules "
-            + "themselves."
+        "When 'strict-deps' is on, .java files that depend on classes not declared in their rule's "
+            + "'deps' fail to build. In other words, it's forbidden to depend on classes obtained "
+            + "transitively. This flag controls the behavior of Java proto rules when their "
+            + "'strict_deps' attribute is unspecified, and its containing package doesn't specify "
+            + "'default_strict_deps_java_protos'."
   )
-  public boolean javaProtoLibraryDepsAreStrict;
+  public boolean strictDepsJavaProtos;
 
   @Override
   public FragmentOptions getHost(boolean fallback) {

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

@@ -174,7 +174,6 @@ private static class Impl {
     private final RuleContext ruleContext;
     private final SupportData supportData;
 
-    private final boolean isStrictDeps;
     private final String protoRuntimeAttr;
     private final JavaSemantics javaSemantics;
 
@@ -197,9 +196,6 @@ private static class Impl {
       this.protoCompilerPluginOptions = protoCompilerPluginOptions;
       this.javaSemantics = javaSemantics;
 
-      isStrictDeps =
-          ruleContext.getFragment(JavaConfiguration.class).javaProtoLibraryDepsAreStrict();
-
       dependencyCompilationArgs =
           JavaCompilationArgsProvider.merge(
               Iterables.<JavaCompilationArgsAspectProvider, JavaCompilationArgsProvider>transform(
@@ -312,13 +308,14 @@ private JavaCompilationArgsProvider createJavaCompileAction(
               .setOutput(outputJar)
               .addSourceJars(sourceJar)
               .setJavacOpts(constructJavacOpts());
-      helper.addDep(dependencyCompilationArgs);
       helper
+          .addDep(dependencyCompilationArgs)
           .addDep(
               ruleContext.getPrerequisite(
                   protoRuntimeAttr, Mode.TARGET, JavaCompilationArgsProvider.class))
-          .setStrictDepsMode(isStrictDeps ? StrictDepsMode.WARN : StrictDepsMode.OFF);
-      return helper.buildCompilationArgsProvider(helper.build(javaSemantics));
+          .setCompilationStrictDepsMode(StrictDepsMode.OFF);
+      return helper.buildCompilationArgsProvider(
+          helper.build(javaSemantics), true /* isReportedAsStrict */);
     }
 
     private Artifact getSourceJarArtifact() {

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

@@ -50,6 +50,10 @@ public ConfiguredTarget create(final RuleContext ruleContext)
                     ruleContext, JavaCompilationArgsAspectProvider.class),
                 JavaCompilationArgsAspectProvider.GET_PROVIDER));
 
+    if (!StrictDepsUtils.isStrictDepsJavaProtoLibrary(ruleContext)) {
+      dependencyArgsProviders = StrictDepsUtils.makeNonStrict(dependencyArgsProviders);
+    }
+
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
             .addArtifacts(