Make java_toolchain.{javac,javabuilder,singlejar,genclass,ijar} mandatory

--
MOS_MIGRATED_REVID=120241405

4 files changed, 63 insertions, 56 deletions

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 651ba7ec6a..51c5ebae40 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
@@ -53,14 +53,12 @@ public final class JavaToolchain implements RuleConfiguredTargetFactory {
     Artifact singleJar = getArtifact("singlejar", ruleContext);
     Artifact genClass = getArtifact("genclass", ruleContext);
     FilesToRunProvider ijar = ruleContext.getExecutablePrerequisite("ijar", Mode.HOST);
-    // TODO(cushon): clean up nulls once migration from --javac_bootclasspath and --javac_extdir
-    // is complete, and java_toolchain.{bootclasspath,extclasspath} are mandatory
     final JavaToolchainData toolchainData =
         new JavaToolchainData(
             source,
             target,
-            execPathsOrNull(bootclasspath),
-            execPathsOrNull(extclasspath),
+            Artifact.toExecPaths(bootclasspath),
+            Artifact.toExecPaths(extclasspath),
             encoding,
             xlint,
             misc,
@@ -86,11 +84,6 @@ public final class JavaToolchain implements RuleConfiguredTargetFactory {
     return builder.build();
   }
 
-  /** Returns the exec paths of the given artifacts, or {@code null}. */
-  private Iterable<String> execPathsOrNull(NestedSet<Artifact> artifacts) {
-    return artifacts != null ? Artifact.toExecPaths(artifacts) : null;
-  }
-
   private Artifact getArtifact(String attributeName, RuleContext ruleContext) {
     TransitiveInfoCollection prerequisite = ruleContext.getPrerequisite(attributeName, Mode.HOST);
     if (prerequisite == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainData.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainData.java
index f4df3d0dbb..49a0663a18 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainData.java
@@ -22,8 +22,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 
 import java.util.List;
 
-import javax.annotation.Nullable;
-
 /**
  * Information about the JDK used by the <code>java_*</code> rules.
  *
@@ -35,10 +33,8 @@ public class JavaToolchainData {
 
   private final String sourceVersion;
   private final String targetVersion;
-  // TODO(cushon): remove @Nullable once migration from --javac_bootclasspath and --javac_extdir
-  // is complete, and java_toolchain.{bootclasspath,extclasspath} are mandatory
-  @Nullable private final Iterable<String> bootclasspath;
-  @Nullable private final Iterable<String> extclasspath;
+  private final Iterable<String> bootclasspath;
+  private final Iterable<String> extclasspath;
   private final String encoding;
   private final ImmutableList<String> options;
   private final ImmutableList<String> jvmOpts;
@@ -46,16 +42,16 @@ public class JavaToolchainData {
   public JavaToolchainData(
       String sourceVersion,
       String targetVersion,
-      @Nullable Iterable<String> bootclasspath,
-      @Nullable Iterable<String> extclasspath,
+      Iterable<String> bootclasspath,
+      Iterable<String> extclasspath,
       String encoding,
       List<String> xlint,
       List<String> misc,
       List<String> jvmOpts) {
     this.sourceVersion = checkNotNull(sourceVersion, "sourceVersion must not be null");
     this.targetVersion = checkNotNull(targetVersion, "targetVersion must not be null");
-    this.bootclasspath = bootclasspath;
-    this.extclasspath = extclasspath;
+    this.bootclasspath = checkNotNull(bootclasspath, "bootclasspath must not be null");
+    this.extclasspath = checkNotNull(extclasspath, "extclasspath must not be null");
     this.encoding = checkNotNull(encoding, "encoding must not be null");
 
     this.jvmOpts = ImmutableList.copyOf(jvmOpts);
@@ -97,12 +93,10 @@ public class JavaToolchainData {
     return targetVersion;
   }
 
-  @Nullable
   public Iterable<String> getBootclasspath() {
     return bootclasspath;
   }
 
-  @Nullable
   public Iterable<String> getExtclasspath() {
     return extclasspath;
   }
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 f07f320132..cd509076e4 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
@@ -41,40 +41,40 @@ public final class JavaToolchainProvider implements TransitiveInfoProvider {
 
   private final String sourceVersion;
   private final String targetVersion;
-  @Nullable private final NestedSet<Artifact> bootclasspath;
-  @Nullable private final NestedSet<Artifact> extclasspath;
+  private final NestedSet<Artifact> bootclasspath;
+  private final NestedSet<Artifact> extclasspath;
   private final String encoding;
   private final ImmutableList<String> javacOptions;
   private final ImmutableList<String> javacJvmOptions;
-  @Nullable private final Artifact javac;
-  @Nullable private final Artifact javaBuilder;
-  @Nullable private final Artifact headerCompiler;
+  private final Artifact javac;
+  private final Artifact javaBuilder;
+  private final Artifact headerCompiler;
   @Nullable private final Artifact singleJar;
-  @Nullable private final Artifact genClass;
-  @Nullable private final FilesToRunProvider ijar;
+  private final Artifact genClass;
+  private final FilesToRunProvider ijar;
 
   public JavaToolchainProvider(
       JavaToolchainData data,
-      @Nullable NestedSet<Artifact> bootclasspath,
-      @Nullable NestedSet<Artifact> extclasspath,
+      NestedSet<Artifact> bootclasspath,
+      NestedSet<Artifact> extclasspath,
       List<String> defaultJavacFlags,
-      @Nullable Artifact javac,
-      @Nullable Artifact javaBuilder,
+      Artifact javac,
+      Artifact javaBuilder,
       @Nullable Artifact headerCompiler,
-      @Nullable Artifact singleJar,
-      @Nullable Artifact genClass,
-      @Nullable FilesToRunProvider ijar) {
+      Artifact singleJar,
+      Artifact genClass,
+      FilesToRunProvider ijar) {
     this.sourceVersion = checkNotNull(data.getSourceVersion(), "sourceVersion must not be null");
     this.targetVersion = checkNotNull(data.getTargetVersion(), "targetVersion must not be null");
-    this.bootclasspath = bootclasspath;
-    this.extclasspath = extclasspath;
+    this.bootclasspath = checkNotNull(bootclasspath, "bootclasspath must not be null");
+    this.extclasspath = checkNotNull(extclasspath, "extclasspath must not be null");
     this.encoding = checkNotNull(data.getEncoding(), "encoding must not be null");
-    this.javac = javac;
-    this.javaBuilder = javaBuilder;
+    this.javac = checkNotNull(javac, "javac must not be null");
+    this.javaBuilder = checkNotNull(javaBuilder, "javaBuilder must not be null");
     this.headerCompiler = headerCompiler;
-    this.singleJar = singleJar;
-    this.genClass = genClass;
-    this.ijar = ijar;
+    this.singleJar = checkNotNull(singleJar, "singleJar must not be null");
+    this.genClass = checkNotNull(genClass, "genClass must not be null");
+    this.ijar = checkNotNull(ijar, "ijar must not be null");
 
     // merges the defaultJavacFlags from
     // {@link JavaConfiguration} with the flags from the {@code java_toolchain} rule.
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 cd8f824ef0..ff8f2d437a 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
@@ -48,13 +48,19 @@ public final class JavaToolchainRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(bootclasspath) -->
         The Java target exdir entries. Corresponds to javac's -bootclasspath flag.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --javac_bootclasspath is complete
-        .add(attr("bootclasspath", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE))
+        .add(
+            attr("bootclasspath", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE))
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(extclasspath) -->
         The Java target exdir entries. Corresponds to javac's -extdir flag.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --javac_extdir is complete
-        .add(attr("extclasspath", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE))
+        .add(
+            attr("extclasspath", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE))
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(encoding) -->
         The encoding of the java files (e.g., 'UTF-8').
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
@@ -77,29 +83,43 @@ public final class JavaToolchainRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(javac) -->
         Label of the javac jar.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --java_langtools is complete
-        .add(attr("javac", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE))
+        .add(attr("javac", LABEL_LIST).mandatory().cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE))
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(javabuilder) -->
         Label of the JavaBuilder deploy jar.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --singlejar_top is complete
         .add(
-            attr("javabuilder", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE).exec())
+            attr("javabuilder", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE)
+                .exec())
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(singlejar) -->
         Label of the SingleJar deploy jar.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --genclass_top is complete
-        .add(attr("singlejar", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE).exec())
+        .add(
+            attr("singlejar", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE)
+                .exec())
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(genclass) -->
         Label of the GenClass deploy jar.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --javabuilder_top is complete
-        .add(attr("genclass", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE).exec())
+        .add(
+            attr("genclass", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE)
+                .exec())
         /* <!-- #BLAZE_RULE(java_toolchain).ATTRIBUTE(ijar) -->
         Label of the ijar executable.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        // TODO(cushon): make mandatory once migration from --ijar_top is complete
-        .add(attr("ijar", LABEL_LIST).cfg(HOST).allowedFileTypes(FileTypeSet.ANY_FILE).exec())
+        .add(
+            attr("ijar", LABEL_LIST)
+                .mandatory()
+                .cfg(HOST)
+                .allowedFileTypes(FileTypeSet.ANY_FILE)
+                .exec())
         .add(
             attr("header_compiler", LABEL_LIST)
                 .cfg(HOST)