Make java_proto_library's strict_deps default to true.

Remove package-level attribute to set the default of strict_deps.
Change the semantics to --strict_deps_java_protos to mean force strict deps of all Java protos to be true regardless of their strict_deps attribute.

--
MOS_MIGRATED_REVID=133789725

7 files changed, 12 insertions, 128 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
index f04d0e0d4a..ed3356469b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java
@@ -69,11 +69,11 @@ public class BazelJavaLiteProtoLibraryRule implements RuleDefinition {
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
                 .aspect(javaProtoAspect, ASPECT_PARAMETERS))
-        /* <!-- #BLAZE_RULE(java_lite_proto_library).ATTRIBUTE(deps) -->
+        /* <!-- #BLAZE_RULE(java_lite_proto_library).ATTRIBUTE(strict_deps) -->
         When True, this rule only exposes the protos that it wraps directly. Depending on indirect
         protos will break the build and print an 'add_dep' command to correct the build.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("strict_deps", BOOLEAN))
+        .add(attr("strict_deps", BOOLEAN).value(true))
         .add(
             attr(LITE_PROTO_RUNTIME_ATTR, LABEL)
                 .legacyAllowAnyFileType()
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
index db98d3554c..0d2c5991a6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoLibraryRule.java
@@ -65,7 +65,7 @@ public class BazelJavaProtoLibraryRule implements RuleDefinition {
                 .allowedRuleClasses("proto_library")
                 .allowedFileTypes()
                 .aspect(javaProtoAspect, ASPECT_PARAMETERS))
-        .add(attr("strict_deps", BOOLEAN).undocumented("for migration"))
+        .add(attr("strict_deps", BOOLEAN).value(true).undocumented("for migration"))
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index f1d9b51878..cbd672e728 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -139,9 +139,6 @@ public class Package {
    */
   private String defaultHdrsCheck;
 
-  /** 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;
 
@@ -587,15 +584,6 @@ public class Package {
     return defaultDeprecation;
   }
 
-  /**
-   * 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";
@@ -912,11 +900,6 @@ public class Package {
       return 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;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 4b8db490f3..86406f447a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -262,24 +262,6 @@ public final class PackageFactory {
     }
   }
 
-  /**
-   * Declares the package() attribute specifying the default value for
-   * java_proto_library.strict_deps.
-   *
-   * <p>This attribute should be considered as undocumented/experimental, and is subject to change
-   * at any time without prior notice.
-   */
-  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!
@@ -481,7 +463,6 @@ public final class PackageFactory {
     ImmutableList.Builder<PackageArgument<?>> arguments =
         ImmutableList.<PackageArgument<?>>builder()
             .add(new DefaultDeprecation())
-            .add(new DefaultStrictDepsJavaProtos())
             .add(new DefaultDistribs())
             .add(new DefaultLicenses())
             .add(new DefaultTestOnly())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index 65049fef5f..8a0135ae9c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -352,14 +352,13 @@ public class JavaOptions extends FragmentOptions {
 
   @Option(
     name = "strict_deps_java_protos",
-    defaultValue = "true",
+    defaultValue = "false",
     category = "undocumented",
     help =
         "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'."
+            + "transitively. When true, Java protos are strict regardless of their 'strict_deps' "
+            + "attribute."
   )
   public boolean strictDepsJavaProtos;
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtils.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtils.java
index 889c71c9e6..8b9d5c2de3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtils.java
@@ -17,8 +17,6 @@ package com.google.devtools.build.lib.rules.java.proto;
 import static com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType.BOTH;
 
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.packages.AttributeContainer;
-import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgs;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
@@ -28,26 +26,13 @@ public class StrictDepsUtils {
   /**
    * Returns true iff 'ruleContext' should enforce strict-deps.
    *
-   * <ol>
-   * <li>If the rule explicitly specifies the 'strict_deps' attribute, returns its value.
-   * <li>Otherwise, if the package explicitly specifies 'default_strict_deps_java_proto_library',
-   *     returns that value.
-   * <li>Otherwise, returns the value of the --strict_deps_java_proto_library flag.
-   * </ol>
-   *
-   * Using this method requires requesting the JavaConfiguration fragment.
+   * <p>Using this method requires requesting the JavaConfiguration fragment.
    */
   public static boolean isStrictDepsJavaProtoLibrary(RuleContext ruleContext) {
-    AttributeContainer attributeContainer = ruleContext.getRule().getAttributeContainer();
-    if (attributeContainer.isAttributeValueExplicitlySpecified("strict_deps")) {
-      return (boolean) attributeContainer.getAttr("strict_deps");
-    }
-    TriState defaultJavaProtoLibraryStrictDeps =
-        ruleContext.getRule().getPackage().getDefaultStrictDepsJavaProtos();
-    if (defaultJavaProtoLibraryStrictDeps == TriState.AUTO) {
-      return ruleContext.getFragment(JavaConfiguration.class).strictDepsJavaProtos();
+    if (ruleContext.getFragment(JavaConfiguration.class).strictDepsJavaProtos()) {
+      return true;
     }
-    return defaultJavaProtoLibraryStrictDeps == TriState.YES;
+    return (boolean) ruleContext.getRule().getAttributeContainer().getAttr("strict_deps");
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtilsTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtilsTest.java
index 7a6c3b8989..df6c1d9f36 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/proto/StrictDepsUtilsTest.java
@@ -35,44 +35,12 @@ public class StrictDepsUtilsTest extends BuildViewTestCase {
         "java_proto_library(name = 'b', strict_deps = 0)",
         "java_proto_library(name = 'c', strict_deps = 1)");
 
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:a"))).isFalse();
+    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:a"))).isTrue();
     assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:b"))).isFalse();
     assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:c"))).isTrue();
   }
 
   @Test
-  public void isStrictDepsJavaProtoLibrary_flagIsFalse_packageLevelIs0() throws Exception {
-    useConfiguration("--strict_deps_java_protos=false");
-
-    scratch.file(
-        "y/BUILD",
-        "package(default_strict_deps_java_protos = 0)",
-        "java_proto_library(name = 'a')",
-        "java_proto_library(name = 'b', strict_deps = 0)",
-        "java_proto_library(name = 'c', strict_deps = 1)");
-
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:a"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:b"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:c"))).isTrue();
-  }
-
-  @Test
-  public void isStrictDepsJavaProtoLibrary_flagIsFalse_packageLevelIs1() throws Exception {
-    useConfiguration("--strict_deps_java_protos=false");
-
-    scratch.file(
-        "z/BUILD",
-        "package(default_strict_deps_java_protos = 1)",
-        "java_proto_library(name = 'a')",
-        "java_proto_library(name = 'b', strict_deps = 0)",
-        "java_proto_library(name = 'c', strict_deps = 1)");
-
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:a"))).isTrue();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:b"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:c"))).isTrue();
-  }
-
-  @Test
   public void isStrictDepsJavaProtoLibrary_flagIsTrue_noPackageLevelAttribute() throws Exception {
     useConfiguration("--strict_deps_java_protos=true");
 
@@ -83,42 +51,10 @@ public class StrictDepsUtilsTest extends BuildViewTestCase {
         "java_proto_library(name = 'c', strict_deps = 1)");
 
     assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:a"))).isTrue();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:b"))).isFalse();
+    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:b"))).isTrue();
     assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//x:c"))).isTrue();
   }
 
-  @Test
-  public void isStrictDepsJavaProtoLibrary_flagIsTrue_packageLevelIs0() throws Exception {
-    useConfiguration("--strict_deps_java_protos=true");
-
-    scratch.file(
-        "y/BUILD",
-        "package(default_strict_deps_java_protos = 0)",
-        "java_proto_library(name = 'a')",
-        "java_proto_library(name = 'b', strict_deps = 0)",
-        "java_proto_library(name = 'c', strict_deps = 1)");
-
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:a"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:b"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//y:c"))).isTrue();
-  }
-
-  @Test
-  public void isStrictDepsJavaProtoLibrary_flagIsTrue_packageLevelIs1() throws Exception {
-    useConfiguration("--strict_deps_java_protos=true");
-
-    scratch.file(
-        "z/BUILD",
-        "package(default_strict_deps_java_protos = 1)",
-        "java_proto_library(name = 'a')",
-        "java_proto_library(name = 'b', strict_deps = 0)",
-        "java_proto_library(name = 'c', strict_deps = 1)");
-
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:a"))).isTrue();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:b"))).isFalse();
-    assertThat(StrictDepsUtils.isStrictDepsJavaProtoLibrary(getRuleContext("//z:c"))).isTrue();
-  }
-
   private RuleContext getRuleContext(String label) throws Exception {
     return getRuleContext(getConfiguredTarget(label));
   }