Use a public java_compile_toolchain attr instead of _java_toolchain (#1389)

* Use a public java_compile_toolchain common attr instead of _java_toolchain

Co-authored-by: Shane Delmore <[email protected]>

* Add _java_toolchain default for aspects

* Remove missing test

* use specified_java_compile_toolchain in scala_import.bzl; lint

* assertTrue -> assertEquals

Co-authored-by: Shane Delmore <[email protected]>

Author: wiwa

scala/private/common_attributes.bzl

@@ -28,6 +28,10 @@ common_attrs_for_plugin_bootstrapping = {
     "resources": attr.label_list(allow_files = True),
     "resource_strip_prefix": attr.string(),
     "resource_jars": attr.label_list(allow_files = True),
+    "java_compile_toolchain": attr.label(
+        default = Label("@bazel_tools//tools/jdk:current_java_toolchain"),
+        providers = [java_common.JavaToolchainInfo],
+    ),
     "scalacopts": attr.string_list(),
     "javacopts": attr.string_list(),
     "scalac_jvm_flags": attr.string_list(),
@@ -89,9 +93,6 @@ implicit_deps = {
         default = Label("@bazel_tools//tools/zip:zipper"),
         allow_files = True,
     ),
-    "_java_toolchain": attr.label(
-        default = Label("@bazel_tools//tools/jdk:current_java_toolchain"),
-    ),
     "_java_runtime": attr.label(
         default = Label("@bazel_tools//tools/jdk:current_java_runtime"),
     ),

scala/private/phases/phase_compile.bzl

@@ -3,7 +3,6 @@
 #
 # DOCUMENT THIS
 #
-load("@bazel_tools//tools/jdk:toolchain_utils.bzl", "find_java_toolchain")
 load(
     "@io_bazel_rules_scala//scala/private:paths.bzl",
     _get_files_with_extension = "get_files_with_extension",
@@ -13,6 +12,7 @@ load(
 )
 load(
     "@io_bazel_rules_scala//scala/private:rule_impls.bzl",
+    "specified_java_compile_toolchain",
     _compile_java = "compile_java",
     _compile_scala = "compile_scala",
 )
@@ -217,7 +217,7 @@ def _compile_or_empty(
                 ctx.actions,
                 jar = ctx.outputs.jar,
                 target_label = ctx.label,
-                java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+                java_toolchain = specified_java_compile_toolchain(ctx),
             )
         else:
             #  macro code needs to be available at compile-time,
@@ -333,7 +333,7 @@ def _pack_source_jar(ctx, scala_srcs, input_srcjars):
             output_source_jar = output_source_jar,
             sources = scala_srcs,
             source_jars = input_srcjars,
-            java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+            java_toolchain = specified_java_compile_toolchain(ctx),
         )
 
 def _try_to_compile_java_jar(

scala/private/rule_impls.bzl

@@ -141,6 +141,8 @@ def compile_scala(
     )
 
 def compile_java(ctx, source_jars, source_files, output, extra_javac_opts, providers_of_dependencies):
+    java_toolchain = specified_java_compile_toolchain(ctx)
+
     return java_common.compile(
         ctx,
         source_jars = source_jars,
@@ -150,7 +152,7 @@ def compile_java(ctx, source_jars, source_files, output, extra_javac_opts, provi
             ctx,
             extra_javac_opts +
             java_common.default_javac_opts(
-                java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
+                java_toolchain = java_toolchain,
             ),
         ),
         deps = providers_of_dependencies,
@@ -159,13 +161,24 @@ def compile_java(ctx, source_jars, source_files, output, extra_javac_opts, provi
         #workaround until https://github.com/bazelbuild/bazel/issues/3528 is resolved
         exports = [],
         neverlink = getattr(ctx.attr, "neverlink", False),
-        java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+        java_toolchain = java_toolchain,
         strict_deps = ctx.fragments.java.strict_java_deps,
     )
 
 def runfiles_root(ctx):
     return "${TEST_SRCDIR}/%s" % ctx.workspace_name
 
+def specified_java_compile_toolchain(ctx):
+    # Aspects such as scrooge_java_aspect are not allowed public label attrs
+    # And so will still use an implicit _java_toolchain
+    java_compile_toolchain = getattr(
+        ctx.attr,
+        "java_compile_toolchain",
+        getattr(ctx.attr, "_java_toolchain", None),
+    )
+
+    return find_java_toolchain(ctx, java_compile_toolchain)
+
 def specified_java_runtime(ctx, default_runtime = None):
     use_specified_java = "runtime_jdk" in dir(ctx.attr)
     if use_specified_java:

scala/scala_import.bzl

@@ -1,5 +1,9 @@
 load("@io_bazel_rules_scala//scala:jars_to_labels.bzl", "JarsToLabelsInfo")
 load("//scala/settings:stamp_settings.bzl", "StampScalaImport")
+load(
+    "@io_bazel_rules_scala//scala/private:rule_impls.bzl",
+    "specified_java_compile_toolchain",
+)
 
 def _stamp_jar(ctx, jar):
     stamped_jar_filename = "%s.stamp/%s" % (ctx.label.name, jar.basename)
@@ -9,7 +13,7 @@ def _stamp_jar(ctx, jar):
         actions = ctx.actions,
         jar = symlink_file,
         target_label = ctx.label,
-        java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
+        java_toolchain = specified_java_compile_toolchain(ctx),
     )
 
 # intellij part is tested manually, tread lightly when changing there
@@ -139,7 +143,7 @@ scala_import = rule(
             doc = "Adds Target-Label attribute to MANIFEST.MF for dep tracking",
             default = Label("@io_bazel_rules_scala//scala/settings:stamp_scala_import"),
         ),
-        "_java_toolchain": attr.label(
+        "java_compile_toolchain": attr.label(
             default = Label("@bazel_tools//tools/jdk:current_java_toolchain"),
         ),
     },

scala_proto/private/scala_proto_aspect.bzl

@@ -1,12 +1,12 @@
 load("@rules_proto//proto:defs.bzl", "ProtoInfo")
 load("//scala/private:common.bzl", "write_manifest_file")
 load("//scala/private:dependency.bzl", "legacy_unclear_dependency_info_for_protobuf_scrooge")
-load("//scala/private:rule_impls.bzl", "compile_scala")
-load("//scala/private/toolchain_deps:toolchain_deps.bzl", "find_deps_info_on")
 load(
-    "@bazel_tools//tools/jdk:toolchain_utils.bzl",
-    "find_java_toolchain",
+    "//scala/private:rule_impls.bzl",
+    "compile_scala",
+    "specified_java_compile_toolchain",
 )
+load("//scala/private/toolchain_deps:toolchain_deps.bzl", "find_deps_info_on")
 load(
     "@io_bazel_rules_scala//scala_proto/private:scala_proto_aspect_provider.bzl",
     "ScalaProtoAspectInfo",
@@ -51,7 +51,7 @@ def _pack_sources(ctx, src_jars):
         ctx.actions,
         source_jars = src_jars,
         output_source_jar = ctx.actions.declare_file(ctx.label.name + "_scalapb-src.jar"),
-        java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+        java_toolchain = specified_java_compile_toolchain(ctx),
     )
 
 def _generate_sources(ctx, toolchain, proto):

test/BUILD

@@ -482,6 +482,18 @@ scala_binary(
     runtime_jdk = "@bazel_tools//tools/jdk:remote_jdk11",
 )
 
+# Make sure scala_library respects java_compile_toolchain during builds
+scala_junit_test(
+    name = "CheckBytecodeMajorVersion",
+    srcs = ["CheckBytecodeMajorVersionTest.scala"],
+    suffixes = ["Test"],
+    runtime_deps = [
+        "//test/src/main/resources/java_sources:CompiledWithJava11",
+        "//test/src/main/resources/java_sources:CompiledWithJava8",
+    ],
+    deps = ["@io_bazel_rules_scala_junit_junit"],
+)
+
 # Generate a file containing the rootpaths of a Scala binary.
 genrule(
     name = "rootpath-script",

test/CheckBytecodeMajorVersionTest.scala

@@ -0,0 +1,50 @@
+package scalarules.test
+
+import java.nio.file.Files
+import java.io.InputStream
+
+import org.junit.Assert;
+import org.junit.Test;
+
+class CheckBytecodeMajorVersionTest {
+
+  // https://stackoverflow.com/questions/27065/tool-to-read-and-display-java-class-versions
+  def getBytecodeMajorVersion(classFileInput: InputStream): Int = {
+    val eighthByte = classFileInput.readNBytes(8)(7)
+    eighthByte.toInt
+  }
+
+  // https://stackoverflow.com/questions/9170832/list-of-java-class-file-format-major-version-numbers
+  val majorVersionToJdkVersion = Map(
+    52 -> 8,
+    55 -> 11
+  )
+
+  @Test
+  def someTest(): Unit = {
+
+    val expectJava8 = getBytecodeMajorVersion(
+      getClass
+        .getClassLoader
+        .getResourceAsStream("java_sources/SimpleJavaSourceFileA.class")
+    )
+
+    val expectJava11 =  getBytecodeMajorVersion(
+      getClass
+        .getClassLoader
+        .getResourceAsStream("java_sources/SimpleJavaSourceFileB.class")
+    )
+      
+    Assert.assertEquals(
+      s"Expected Java 8 bytecode (major version 52) but got major version $expectJava8",
+      8,
+      majorVersionToJdkVersion(expectJava8)
+    )
+      
+    Assert.assertEquals(
+      s"Expected Java 11 bytecode (major version 55) but got major version $expectJava11",
+      11,
+      majorVersionToJdkVersion(expectJava11)
+    )
+  }
+}

test/src/main/resources/java_sources/BUILD

@@ -0,0 +1,15 @@
+load("//scala:scala.bzl", "scala_library")
+
+package(default_visibility = ["//visibility:public"])
+
+scala_library(
+    name = "CompiledWithJava8",
+    srcs = ["SimpleJavaSourceFileA.java"],
+    java_compile_toolchain = "@bazel_tools//tools/jdk:toolchain_java8",
+)
+
+scala_library(
+    name = "CompiledWithJava11",
+    srcs = ["SimpleJavaSourceFileB.java"],
+    java_compile_toolchain = "@bazel_tools//tools/jdk:toolchain_java11",
+)

test/src/main/resources/java_sources/SimpleJavaSourceFileA.java

@@ -0,0 +1,11 @@
+package java_sources;
+
+public class SimpleJavaSourceFileA {
+
+  public int someIntField;
+
+  public SimpleJavaSourceFileA() {
+    this.someIntField = 0;
+  }
+  
+}

test/src/main/resources/java_sources/SimpleJavaSourceFileB.java

@@ -0,0 +1,11 @@
+package java_sources;
+
+public class SimpleJavaSourceFileB {
+
+  public int someIntField;
+
+  public SimpleJavaSourceFileB() {
+    this.someIntField = 0;
+  }
+  
+}