Change the return type of compilation_info.javac_options to depset

RELNOTES: JavaInfo.compilation_info.javac_options now returns a depset. Use tokenize_javacopts from @rules_java to get the options as a correctly ordered list.
PiperOrigin-RevId: 586606843
Change-Id: If0c7cea372e5811a170dace1018375ec3fe8ab37

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

@@ -830,7 +830,7 @@ public RuleContext getRuleContext() {
 
   private JavaCompilationInfoProvider createCompilationInfoProvider() throws RuleErrorException {
     return new JavaCompilationInfoProvider.Builder()
-        .setJavacOpts(javacOpts)
+        .setJavacOpts(JavaHelper.detokenizeJavaOptions(javacOpts))
         .setBootClasspath(getBootClasspath())
         .setCompilationClasspath(getCompileTimeClasspath())
         .setRuntimeClasspath(getRuntimeClasspath())

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

@@ -83,8 +83,7 @@ public static JavaCompilationInfoProvider fromStarlarkCompilationInfo(Object val
       Builder builder =
           new Builder()
               .setJavacOpts(
-                  Sequence.cast(info.getValue("javac_options"), String.class, "javac_options")
-                      .getImmutableList())
+                  Depset.cast(info.getValue("javac_options"), String.class, "javac_options"))
               .setBootClasspath(
                   NestedSetBuilder.wrap(
                       Order.NAIVE_LINK_ORDER,
@@ -112,13 +111,13 @@ public boolean isImmutable() {
 
   /** Builder for {@link JavaCompilationInfoProvider}. */
   public static class Builder {
-    private ImmutableList<String> javacOpts = ImmutableList.of();
+    private NestedSet<String> javacOpts = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
     private NestedSet<Artifact> runtimeClasspath;
     private NestedSet<Artifact> compilationClasspath;
     private NestedSet<Artifact> bootClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
 
     @CanIgnoreReturnValue
-    public Builder setJavacOpts(@Nonnull ImmutableList<String> javacOpts) {
+    public Builder setJavacOpts(@Nonnull NestedSet<String> javacOpts) {
       this.javacOpts = javacOpts;
       return this;
     }
@@ -143,15 +142,21 @@ public Builder setBootClasspath(NestedSet<Artifact> bootClasspath) {
 
     public JavaCompilationInfoProvider build() throws RuleErrorException {
       return new AutoValue_JavaCompilationInfoProvider(
-          JavaCompilationHelper.internJavacOpts(javacOpts),
-          runtimeClasspath,
-          compilationClasspath,
-          bootClasspath);
+          javacOpts, runtimeClasspath, compilationClasspath, bootClasspath);
     }
   }
 
+  public abstract NestedSet<String> getJavacOpts();
+
   @Override
-  public abstract ImmutableList<String> getJavacOpts();
+  public Depset getJavacOptsStarlark() {
+    return Depset.of(String.class, getJavacOpts());
+  }
+
+  @Override
+  public ImmutableList<String> getJavacOptsList() {
+    return JavaCompilationHelper.internJavacOpts(JavaHelper.tokenizeJavaOptions(getJavacOpts()));
+  }
 
   @Nullable
   public abstract NestedSet<Artifact> runtimeClasspath();
@@ -203,7 +208,7 @@ public final boolean equals(Object obj) {
       return false;
     }
     JavaCompilationInfoProvider other = (JavaCompilationInfoProvider) obj;
-    return Objects.equals(getJavacOpts(), other.getJavacOpts())
+    return getJavacOpts().shallowEquals(other.getJavacOpts())
         && Objects.equals(getRuntimeClasspath(), other.getRuntimeClasspath())
         && Objects.equals(getCompilationClasspath(), other.getCompilationClasspath())
         && bootClasspath().shallowEquals(other.bootClasspath());

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCompilationInfoProviderApi.java

@@ -29,8 +29,13 @@
     doc = "Provides access to compilation information for Java rules.")
 public interface JavaCompilationInfoProviderApi<FileT extends FileApi> extends StarlarkValue {
 
-  @StarlarkMethod(name = "javac_options", structField = true, doc = "Options to java compiler.")
-  ImmutableList<String> getJavacOpts();
+  @StarlarkMethod(
+      name = "javac_options",
+      structField = true,
+      doc =
+          "A depset of options to java compiler. To get the exact list of options passed to javac"
+              + " in the correct order, use the tokenize_javacopts utility in rules_java")
+  Depset getJavacOptsStarlark();
 
   @StarlarkMethod(
       name = "javac_options_list",
@@ -39,9 +44,7 @@ public interface JavaCompilationInfoProviderApi<FileT extends FileApi> extends S
           "A list of options to java compiler. This exists temporarily for migration purposes. "
               + "javac_options will return a depset in the future, and this method will be dropped "
               + "once all usages have been updated to handle depsets.")
-  default ImmutableList<String> getJavacOptsList() {
-    return getJavacOpts();
-  }
+  ImmutableList<String> getJavacOptsList();
 
   @StarlarkMethod(
       name = "runtime_classpath",

src/main/starlark/builtins_bzl/common/java/android_lint.bzl

@@ -14,8 +14,12 @@
 
 """Creates the android lint action for java rules"""
 
+load(":common/java/java_helper.bzl", "helper")
 load(":common/java/java_semantics.bzl", "semantics")
 
+def _tokenize_opts(opts_depset):
+    return helper.tokenize_javacopts(ctx = None, opts = opts_depset)
+
 def _android_lint_action(ctx, source_files, source_jars, compilation_info):
     """
     Creates an action that runs Android lint against Java source files.
@@ -97,8 +101,9 @@ def _android_lint_action(ctx, source_files, source_jars, compilation_info):
     args.add("--target_label", ctx.label)
 
     javac_opts = compilation_info.javac_options
-    if (javac_opts):
-        args.add_all("--javacopts", javac_opts)
+    if javac_opts:
+        # wrap in a list so that map_each passes the depset to _tokenize_opts
+        args.add_all("--javacopts", [javac_opts], map_each = _tokenize_opts)
         args.add("--")
 
     args.add("--lintopts")

src/main/starlark/builtins_bzl/common/java/compile_action.bzl

@@ -162,7 +162,7 @@ def compile_action(
         runfiles = [output_class_jar] if source_files or source_jars or resources else [],
         # TODO(ilist): collect compile_jars from JavaInfo in deps & exports
         compilation_classpath = java_info.compilation_info.compilation_classpath,
-        javac_options = java_info.compilation_info.javac_options_list,
+        javac_options = java_info.compilation_info.javac_options,
         plugins = _collect_plugins(deps, plugins),
     )
 

src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl

@@ -282,7 +282,7 @@ def compile(
         direct_runtime_jars = []
 
     compilation_info = struct(
-        javac_options = all_javac_opts_list,
+        javac_options = all_javac_opts,
         javac_options_list = all_javac_opts_list,
         # needs to be flattened because the public API is a list
         boot_classpath = (bootclasspath.bootclasspath if bootclasspath else java_toolchain.bootclasspath).to_list(),

src/main/starlark/builtins_bzl/common/java/java_info.bzl

@@ -82,7 +82,9 @@ JavaCompilationInfo = provider(
     doc = "Compilation information in Java rules, for perusal of aspects and tools.",
     fields = {
         "boot_classpath": "Boot classpath for this Java target.",
-        "javac_options": "Options to the java compiler.",
+        "javac_options": """Depset of options to the java compiler. To get the
+            exact list of options passed to javac in the correct order, use the
+            tokenize_javacopts utility in rules_java""",
         "javac_options_list": """A list of options to java compiler. This exists
             temporarily for migration purposes. javac_options will return a depset
             in the future, and this method will be dropped once all usages have
@@ -475,7 +477,7 @@ def java_info_for_compilation(
     if compilation_info:
         result.update(
             compilation_info = JavaCompilationInfo(
-                javac_options = _java_common_internal.intern_javac_opts(compilation_info.javac_options),
+                javac_options = compilation_info.javac_options,
                 javac_options_list = _java_common_internal.intern_javac_opts(compilation_info.javac_options_list),
                 boot_classpath = compilation_info.boot_classpath,
                 compilation_classpath = compilation_info.compilation_classpath,

src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java

@@ -1079,7 +1079,10 @@ public void translateStarlarkJavaInfo_compilationInfo() throws Exception {
                 "compilation_info",
                 makeStruct(
                     ImmutableMap.of(
-                        "javac_options", StarlarkList.immutableOf("opt1", "opt2"),
+                        "javac_options",
+                            Depset.of(
+                                String.class,
+                                NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2")),
                         "boot_classpath", StarlarkList.immutableOf(createArtifact("cp.jar")))))
             .buildOrThrow();
     StarlarkInfo starlarkInfo = makeStruct(fields);
@@ -1088,7 +1091,7 @@ public void translateStarlarkJavaInfo_compilationInfo() throws Exception {
 
     assertThat(javaInfo).isNotNull();
     assertThat(javaInfo.getCompilationInfoProvider()).isNotNull();
-    assertThat(javaInfo.getCompilationInfoProvider().getJavacOpts())
+    assertThat(javaInfo.getCompilationInfoProvider().getJavacOptsList())
         .containsExactly("opt1", "opt2");
     assertThat(javaInfo.getCompilationInfoProvider().getBootClasspathList()).hasSize(1);
     assertThat(prettyArtifactNames(javaInfo.getCompilationInfoProvider().getBootClasspathList()))
@@ -1107,13 +1110,16 @@ public void translatedStarlarkCompilationInfoEqualsNativeInstance() throws Excep
             ImmutableMap.of(
                 "compilation_classpath", Depset.of(Artifact.class, compilationClasspath),
                 "runtime_classpath", Depset.of(Artifact.class, runtimeClasspath),
-                "javac_options", StarlarkList.immutableOf("opt1", "opt2"),
+                "javac_options",
+                    Depset.of(
+                        String.class,
+                        NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2")),
                 "boot_classpath", StarlarkList.immutableOf(bootClasspathArtifact)));
     JavaCompilationInfoProvider nativeCompilationInfo =
         new JavaCompilationInfoProvider.Builder()
             .setCompilationClasspath(compilationClasspath)
             .setRuntimeClasspath(runtimeClasspath)
-            .setJavacOpts(ImmutableList.of("opt1", "opt2"))
+            .setJavacOpts(NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2"))
             .setBootClasspath(
                 NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, bootClasspathArtifact))
             .build();

src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java

@@ -681,7 +681,7 @@ public void testJavaCommonCompileCompilationInfo() throws Exception {
     assertThat(prettyArtifactNames(compilationInfo.getRuntimeClasspath().toList(Artifact.class)))
         .containsExactly("java/test/libdep.jar", "java/test/libcustom.jar");
 
-    assertThat(compilationInfo.getJavacOpts()).contains("-XDone");
+    assertThat(compilationInfo.getJavacOptsList()).contains("-XDone");
   }
 
   @Test
@@ -2188,7 +2188,7 @@ public void javaInfoGetCompilationInfoProvider() throws Exception {
             prettyArtifactNames(
                 javaCompilationInfoProvider.getRuntimeClasspath().getSet(Artifact.class)))
         .containsExactly("foo/libmy_java_lib_a.jar");
-    assertThat(javaCompilationInfoProvider.getJavacOpts()).contains("opt1");
+    assertThat(javaCompilationInfoProvider.getJavacOpts().toList()).contains("opt1");
     assertThat(javaCompilationInfoProvider.getJavacOptsList()).contains("opt1");
   }