add plus one deps transitive mode for compilation

.travis.yml

@@ -19,10 +19,10 @@ env:
   # See https://github.com/bazelbuild/rules_scala/pull/622
   # we want to test the last release
   #- V=0.16.1 TEST_SCRIPT=test_lint.sh
-  - V=0.17.1 TEST_SCRIPT=test_rules_scala.sh
+  #- V=0.17.1 TEST_SCRIPT=test_rules_scala.sh
   - V=0.22.0 TEST_SCRIPT=test_rules_scala.sh
     #- V=0.14.1 TEST_SCRIPT=test_intellij_aspect.sh
-  - V=0.17.1 TEST_SCRIPT=test_reproducibility.sh
+  #- V=0.17.1 TEST_SCRIPT=test_reproducibility.sh
   - V=0.22.0 TEST_SCRIPT=test_reproducibility.sh
 
 before_install:

WORKSPACE

@@ -171,4 +171,30 @@ git_repository(
     name = "bazel_skylib",
     remote = "https://github.com/bazelbuild/bazel-skylib.git",
     tag = "0.6.0",
-)
+)
+
+## deps for tests of limited deps support
+scala_maven_import_external(
+    name = "org_springframework_spring_core",
+    artifact = "org.springframework:spring-core:5.1.5.RELEASE",
+    jar_sha256 = "f771b605019eb9d2cf8f60c25c050233e39487ff54d74c93d687ea8de8b7285a",
+    licenses = ["notice"],  # Apache 2.0
+    server_urls = [
+        "https://repo1.maven.org/maven2/",
+        "https://mirror.bazel.build/repo1.maven.org/maven2",
+        ],
+)
+
+scala_maven_import_external(
+    name = "org_springframework_spring_tx",
+    artifact = "org.springframework:spring-tx:5.1.5.RELEASE",
+    jar_sha256 = "666f72b73c7e6b34e5bb92a0d77a14cdeef491c00fcb07a1e89eb62b08500135",
+    licenses = ["notice"],  # Apache 2.0
+    server_urls = [
+        "https://repo1.maven.org/maven2/",
+        "https://mirror.bazel.build/repo1.maven.org/maven2",
+        ],
+    deps = [
+        "@org_springframework_spring_core"
+    ]
+)

scala/private/common.bzl

@@ -22,21 +22,25 @@ def collect_srcjars(targets):
 def collect_jars(
         dep_targets,
         dependency_analyzer_is_off = True,
-        unused_dependency_checker_is_off = True):
+        unused_dependency_checker_is_off = True,
+        plus_one_deps_is_off = True):
     """Compute the runtime and compile-time dependencies from the given targets"""  # noqa
 
     if dependency_analyzer_is_off:
         return _collect_jars_when_dependency_analyzer_is_off(
             dep_targets,
             unused_dependency_checker_is_off,
+            plus_one_deps_is_off,
         )
     else:
         return _collect_jars_when_dependency_analyzer_is_on(dep_targets)
 
 def _collect_jars_when_dependency_analyzer_is_off(
         dep_targets,
-        unused_dependency_checker_is_off):
+        unused_dependency_checker_is_off,
+        plus_one_deps_is_off):
     compile_jars = []
+    plus_one_deps_compile_jars = []
     runtime_jars = []
     jars2labels = {}
 
@@ -58,11 +62,16 @@ def _collect_jars_when_dependency_analyzer_is_off(
         else:
             print("ignored dependency, has no JavaInfo: " + str(dep_target))
 
+        if (not plus_one_deps_is_off) and (PlusOneDeps in dep_target):
+            plus_one_deps_compile_jars.append(
+                depset(transitive = [dep[JavaInfo].compile_jars for dep in dep_target[PlusOneDeps].direct_deps if JavaInfo in dep ])
+            )
+
     return struct(
         compile_jars = depset(transitive = compile_jars),
         transitive_runtime_jars = depset(transitive = runtime_jars),
         jars2labels = JarsToLabelsInfo(jars_to_labels = jars2labels),
-        transitive_compile_jars = depset(),
+        transitive_compile_jars = depset(transitive = compile_jars + plus_one_deps_compile_jars),
     )
 
 def _collect_jars_when_dependency_analyzer_is_on(dep_targets):
@@ -148,3 +157,16 @@ def create_java_provider(scalaattr, transitive_compile_time_jars):
         ),
         transitive_runtime_jars = scalaattr.transitive_runtime_jars,
     )
+
+PlusOneDeps = provider(
+    fields = {
+        'direct_deps' : 'list of direct compile dependencies of a target',
+    }
+)
+
+def _collect_plus_one_deps_aspect_impl(target, ctx):
+    return [PlusOneDeps(direct_deps = getattr(ctx.rule.attr,'deps',[]))]
+
+collect_plus_one_deps_aspect = aspect(implementation = _collect_plus_one_deps_aspect_impl,
+    attr_aspects = ['deps'],
+)

scala/private/rule_impls.bzl

@@ -220,6 +220,8 @@ CurrentTarget: {current_target}
             ignored_targets = ignored_targets,
             current_target = current_target,
         )
+    if is_dependency_analyzer_off(ctx) and not _is_plus_one_deps_off(ctx):
+       compiler_classpath_jars = transitive_compile_jars
 
     plugins_list = plugins.to_list()
     plugin_arg = _join_path(plugins_list)
@@ -616,6 +618,9 @@ def is_dependency_analyzer_on(ctx):
 def is_dependency_analyzer_off(ctx):
     return not is_dependency_analyzer_on(ctx)
 
+def _is_plus_one_deps_off(ctx):
+    return not ("plus_one_deps" in ctx.var)
+
 # Extract very common code out from dependency analysis into single place
 # automatically adds dependency on scala-library and scala-reflect
 # collects jars from deps, runtime jars from runtime_deps, and
@@ -631,6 +636,7 @@ def _collect_jars_from_common_ctx(
         ctx.attr.deps + extra_deps + base_classpath,
         dependency_analyzer_is_off,
         unused_dependency_checker_is_off,
+        _is_plus_one_deps_off(ctx),
     )
 
     (
@@ -985,9 +991,10 @@ def scala_test_impl(ctx):
     ]
     unused_dependency_checker_is_off = unused_dependency_checker_mode == "off"
 
+    scalatest_base_classpath = scalac_provider.default_classpath + [ctx.attr._scalatest]
     jars = _collect_jars_from_common_ctx(
         ctx,
-        scalac_provider.default_classpath,
+        scalatest_base_classpath,
         extra_runtime_deps = [
             ctx.attr._scalatest_reporter,
             ctx.attr._scalatest_runner,
@@ -1006,36 +1013,6 @@ def scala_test_impl(ctx):
         jars.jars2labels,
     )
 
-    # _scalatest is an http_jar, so its compile jar is run through ijar
-    # however, contains macros, so need to handle separately
-    scalatest_jars = collect_jars([ctx.attr._scalatest]).transitive_runtime_jars
-    cjars = depset(transitive = [cjars, scalatest_jars])
-    transitive_rjars = depset(transitive = [transitive_rjars, scalatest_jars])
-
-    if is_dependency_analyzer_on(ctx):
-        transitive_compile_jars = depset(
-            transitive = [scalatest_jars, transitive_compile_jars],
-        )
-        scalatest_jars_list = scalatest_jars.to_list()
-        j2l = jars_to_labels.jars_to_labels
-        add_labels_of_jars_to(
-            j2l,
-            ctx.attr._scalatest,
-            scalatest_jars_list,
-            scalatest_jars_list,
-        )
-        jars_to_labels = JarsToLabelsInfo(jars_to_labels = j2l)
-
-    elif not unused_dependency_checker_is_off:
-        j2l = jars_to_labels.jars_to_labels
-        add_labels_of_jars_to(
-            j2l,
-            ctx.attr._scalatest,
-            [],
-            scalatest_jars.to_list(),
-        )
-        jars_to_labels = JarsToLabelsInfo(jars_to_labels = j2l)
-
     args = " ".join([
         "-R \"{path}\"".format(path = ctx.outputs.jar.short_path),
         _scala_test_flags(ctx),

scala/scala.bzl

@@ -24,6 +24,10 @@ load(
     "@io_bazel_rules_scala//specs2:specs2_junit.bzl",
     _specs2_junit_dependencies = "specs2_junit_dependencies",
 )
+load(
+    "@io_bazel_rules_scala//scala/private:common.bzl",
+    _collect_plus_one_deps_aspect = "collect_plus_one_deps_aspect",
+)
 
 _launcher_template = {
     "_java_stub_template": attr.label(
@@ -105,7 +109,7 @@ _junit_resolve_deps = {
 # Common attributes reused across multiple rules.
 _common_attrs_for_plugin_bootstrapping = {
     "srcs": attr.label_list(allow_files = [".scala", ".srcjar", ".java"]),
-    "deps": attr.label_list(),
+    "deps": attr.label_list(aspects = [_collect_plus_one_deps_aspect]),
     "plugins": attr.label_list(allow_files = [".jar"]),
     "runtime_deps": attr.label_list(providers = [[JavaInfo]]),
     "data": attr.label_list(allow_files = True),

test_expect_failure/plus_one_deps/exports_deps/A.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class A {
+  println(new B().hi)
+}

test_expect_failure/plus_one_deps/exports_deps/B.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class B extends C {
+  def hi: String = "hi"
+}

test_expect_failure/plus_one_deps/exports_deps/BUILD.bazel

@@ -0,0 +1,21 @@
+load("//scala:scala.bzl", "scala_library")
+scala_library(
+    name = "a",
+    srcs = ["A.scala"],
+    deps = [":b"],
+)
+scala_library(
+    name = "b",
+    srcs = ["B.scala"],
+    deps = [":c"],
+)
+scala_library(
+    name = "c",
+    srcs = ["C.scala"],
+    deps = [":d"],
+    exports = ["d"],
+)
+scala_library(
+    name = "d",
+    srcs = ["D.scala"],
+)

test_expect_failure/plus_one_deps/exports_deps/C.scala

@@ -0,0 +1,3 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class C extends D

test_expect_failure/plus_one_deps/exports_deps/D.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class D {
+
+}

test_expect_failure/plus_one_deps/external_deps/A.scala

@@ -0,0 +1,6 @@
+package scalarules.test_expect_failure.plus_one_deps.external_deps
+import org.springframework.dao.DataAccessException
+
+class A {
+  println(classOf[DataAccessException])
+}

test_expect_failure/plus_one_deps/external_deps/BUILD.bazel

@@ -0,0 +1,7 @@
+load("//scala:scala.bzl", "scala_library")
+scala_library(
+    name = "a",
+    srcs = ["A.scala"],
+    deps = ["@org_springframework_spring_tx"]
+
+)

test_expect_failure/plus_one_deps/internal_deps/A.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class A {
+  println(new B().hi)
+}

test_expect_failure/plus_one_deps/internal_deps/B.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class B extends C {
+  def hi: String = "hi"
+}

test_expect_failure/plus_one_deps/internal_deps/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//scala:scala.bzl", "scala_library")
+scala_library(
+    name = "a",
+    srcs = ["A.scala"],
+    deps = [":b"],
+)
+scala_library(
+    name = "b",
+    srcs = ["B.scala"],
+    deps = [":c"],
+)
+scala_library(
+    name = "c",
+    srcs = ["C.scala"],
+)

test_expect_failure/plus_one_deps/internal_deps/C.scala

@@ -0,0 +1,3 @@
+package scalarules.test_expect_failure.plus_one_deps.internal_deps
+
+class C

test_expect_failure/plus_one_deps/with_unused_deps/A.scala

@@ -0,0 +1,5 @@
+package scalarules.test_expect_failure.plus_one_deps.with_unused_deps
+
+class A {
+  println(new B().hi)
+}

test_expect_failure/plus_one_deps/with_unused_deps/B.scala

@@ -0,0 +1,8 @@
+package scalarules.test_expect_failure.plus_one_deps.with_unused_deps
+
+class B {
+  def hi: String = {
+    println(classOf[C])
+    "hi"
+  }
+}

test_expect_failure/plus_one_deps/with_unused_deps/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//scala:scala.bzl", "scala_library")
+scala_library(
+    name = "a",
+    srcs = ["A.scala"],
+    deps = [":b",":c"],
+)
+scala_library(
+    name = "b",
+    srcs = ["B.scala"],
+    deps = [":c"],
+)
+scala_library(
+    name = "c",
+    srcs = ["C.scala"],
+)

test_expect_failure/plus_one_deps/with_unused_deps/C.scala

@@ -0,0 +1,3 @@
+package scalarules.test_expect_failure.plus_one_deps.with_unused_deps
+
+class C

test_expect_failure/scala_import/ScalaImportPropagatesCompileDepsTest.scala

@@ -7,7 +7,7 @@ import org.specs2.mutable.SpecificationWithJUnit
 class ScalaImportPropagatesCompileDepsTest extends SpecificationWithJUnit {
 
   "scala_import" should {
-    "propagate runtime deps" in {
+    "propagate compile time deps" in {
       println(classOf[Cache[String, String]])
       println(classOf[ArrayUtils])
       println(classOf[cats.Applicative[Any]])

test_rules_scala.sh

@@ -904,6 +904,26 @@ test_scala_import_fetch_sources() {
   assert_file_exists $bazel_out_external_guava_21/$srcjar_name
 }
 
+test_compilation_succeeds_with_plus_one_deps_on() {
+  bazel build --define plus_one_deps=true //test_expect_failure/plus_one_deps/internal_deps:a
+}
+test_compilation_fails_with_plus_one_deps_undefined() {
+  action_should_fail build //test_expect_failure/plus_one_deps/internal_deps:a
+}
+test_compilation_succeeds_with_plus_one_deps_on_for_external_deps() {
+  bazel build --define plus_one_deps=true //test_expect_failure/plus_one_deps/external_deps:a
+}
+test_compilation_succeeds_with_plus_one_deps_on_also_for_exports() {
+  bazel build --define plus_one_deps=true //test_expect_failure/plus_one_deps/exports_deps:a
+}
+test_plus_one_deps_only_works_for_java_info_targets() {
+  #for example doesn't break scala proto which depends on proto_library
+  bazel build --define plus_one_deps=true //test/proto:test_proto
+}
+test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps() {
+  action_should_fail build //test_expect_failure/plus_one_deps/with_unused_deps:a
+}
+
 assert_file_exists() {
   if [[ -f $1 ]]; then
     echo "File $1 exists."
@@ -997,3 +1017,11 @@ $runner test_scala_import_source_jar_should_be_fetched_when_env_bazel_jvm_fetch_
 $runner test_scala_import_source_jar_should_not_be_fetched_when_env_bazel_jvm_fetch_sources_is_set_to_non_true
 $runner test_unused_dependency_checker_mode_warn
 $runner test_override_javabin
+$runner test_compilation_succeeds_with_plus_one_deps_on
+$runner test_compilation_fails_with_plus_one_deps_undefined
+$runner test_compilation_succeeds_with_plus_one_deps_on_for_external_deps
+$runner test_compilation_succeeds_with_plus_one_deps_on_also_for_exports
+$runner test_plus_one_deps_only_works_for_java_info_targets
+$runner bazel test //test/... --define plus_one_deps=true
+$runner test_unused_dependency_fails_even_if_also_exists_in_plus_one_deps
+