Mark unused deps as ignored for mixed source targets (#1499)

* Mark unused deps as ignored for mixed source targets

When Scala target include Java sources, unused deps are reported from Scala sources, which still may be required by Java code. We mark unused deps as ignored so that sdeps consumers could act accordingly.

Also fixed some doc strings.

Author: liucijus

scala/scala_toolchain.bzl

@@ -136,11 +136,11 @@ scala_toolchain = rule(
             values = ["ast-plus", "ast", "high-level", "default"],
         ),
         "dependency_tracking_strict_deps_patterns": attr.string_list(
-            doc = "List of target prefixes included for strict deps analysis. Exclude patetrns with '-'",
+            doc = "List of target prefixes included for strict deps analysis. Exclude patterns with '-'",
             default = [""],
         ),
         "dependency_tracking_unused_deps_patterns": attr.string_list(
-            doc = "List of target prefixes included for unused deps analysis. Exclude patetrns with '-'",
+            doc = "List of target prefixes included for unused deps analysis. Exclude patterns with '-'",
             default = [""],
         ),
         "scalac_jvm_flags": attr.string_list(),

src/java/io/bazel/rulesscala/scalac/reporter/DepsTrackingReporter.java

@@ -141,7 +141,7 @@ public void prepareReport() throws IOException {
               ops.directJars[i],
               directTarget,
               Kind.UNUSED,
-              ignoredTargets.contains(directTarget)
+              ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
           )
       );
     }

src/java/io/bazel/rulesscala/scalac/reporter/scala_deps.proto

@@ -24,7 +24,7 @@ message Dependency {
   // Target label for this dependency
   string label = 3;
 
-  // Path to the full
+  // Path to the full jar
   string path = 4;
 
   // Ignored in dep tracking

test/shell/test_compiler_dependency_tracking.sh

@@ -24,6 +24,11 @@ test_fails_for_strict_dep() {
     build --extra_toolchains="//test_expect_failure/compiler_dependency_tracker:ast_plus_error" //test_expect_failure/compiler_dependency_tracker:missing_source_dep
 }
 
+test_sdeps() {
+  bazel test --extra_toolchains=//test_expect_failure/compiler_dependency_tracker:ast_plus_warn //test_expect_failure/compiler_dependency_tracker/sdeps/...
+}
+
 $runner test_fails_for_unused_dep
 $runner test_fails_for_missing_compile_dep
 $runner test_fails_for_strict_dep
+$runner test_sdeps

test_expect_failure/compiler_dependency_tracker/BUILD

@@ -18,6 +18,23 @@ toolchain(
     visibility = ["//visibility:public"],
 )
 
+scala_toolchain(
+    name = "ast_plus_warn_impl",
+    compiler_deps_mode = "warn",
+    dependency_mode = "plus-one",
+    dependency_tracking_method = "ast-plus",
+    strict_deps_mode = "warn",
+    unused_dependency_checker_mode = "warn",
+    visibility = ["//visibility:public"],
+)
+
+toolchain(
+    name = "ast_plus_warn",
+    toolchain = ":ast_plus_warn_impl",
+    toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
+    visibility = ["//visibility:public"],
+)
+
 scala_library(
     name = "unused_dep",
     srcs = ["A.scala"],

test_expect_failure/compiler_dependency_tracker/sdeps/AnotherScalaDep.scala

@@ -0,0 +1,3 @@
+package sdeps
+
+class AnotherScalaDep

test_expect_failure/compiler_dependency_tracker/sdeps/BUILD

@@ -0,0 +1,61 @@
+load("//scala:scala.bzl", "scala_library", "scala_specs2_junit_test")
+
+scala_library(
+    name = "only_scala_sources",
+    srcs = [
+        "SomeScalaWithDeps.scala",
+    ],
+    unused_dependency_checker_ignored_targets = [
+        ":scala_ignored_unused_dep",
+    ],
+    deps = [
+        ":scala_dep",
+        ":scala_ignored_unused_dep",
+        ":scala_unused_dep",
+    ],
+)
+
+scala_library(
+    name = "mixed_sources",
+    srcs = [
+        "SomeJava.java",
+        "SomeScalaWithDeps.scala",
+    ],
+    unused_dependency_checker_ignored_targets = [
+        ":scala_ignored_unused_dep",
+    ],
+    deps = [
+        ":scala_dep",
+        ":scala_ignored_unused_dep",
+        ":scala_unused_dep",
+    ],
+)
+
+scala_library(
+    name = "scala_dep",
+    srcs = ["ScalaDep.scala"],
+)
+
+scala_library(
+    name = "scala_unused_dep",
+    srcs = ["AnotherScalaDep.scala"],
+)
+
+scala_library(
+    name = "scala_ignored_unused_dep",
+    srcs = ["IgnoredScalaDep.scala"],
+)
+
+scala_specs2_junit_test(
+    name = "sdeps_test",
+    srcs = ["SdepsTest.scala"],
+    resources = [
+        ":mixed_sources.sdeps",
+        ":only_scala_sources.sdeps",
+    ],
+    suffixes = ["Test"],
+    deps = [
+        "//src/java/io/bazel/rulesscala/scalac/reporter:scala_deps_java_proto",
+        "@com_google_protobuf//:protobuf_java",
+    ],
+)

test_expect_failure/compiler_dependency_tracker/sdeps/IgnoredScalaDep.scala

@@ -0,0 +1,3 @@
+package sdeps
+
+class IgnoredScalaDep

test_expect_failure/compiler_dependency_tracker/sdeps/ScalaDep.scala

@@ -0,0 +1,5 @@
+package sdeps
+
+object ScalaDep {
+  val foo = "bar"
+}

test_expect_failure/compiler_dependency_tracker/sdeps/SdepsTest.scala

@@ -0,0 +1,65 @@
+package sdeps
+
+import org.specs2.mutable.SpecWithJUnit
+import io.bazel.rulesscala.deps.proto.ScalaDeps
+import io.bazel.rulesscala.deps.proto.ScalaDeps.Dependency.Kind._
+import org.specs2.matcher.Matcher
+
+import java.util
+
+class SdepsTest extends SpecWithJUnit {
+
+  "Mixed Scala/Java source targets" should {
+    val deps = loadSdeps("mixed_sources.sdeps")
+
+    "mark unused deps as ignored" in {
+      val unusedDep = findDep(deps, ":scala_unused_dep")
+
+      unusedDep must (beIgnored and beUnused)
+    }
+
+    "mark ignored deps as ignored" in {
+      val ignoredDep = findDep(deps, ":scala_ignored_unused_dep")
+
+      ignoredDep must (beIgnored and beUnused)
+    }
+  }
+
+  "Scala-only source targets" should {
+    val deps = loadSdeps("only_scala_sources.sdeps")
+
+    "mark unused deps as not ignored" in {
+      val unusedDep = findDep(deps, ":scala_unused_dep")
+
+      unusedDep must (not(beIgnored) and beUnused)
+    }
+
+    "mark ignored deps as ignored" in {
+      val ignoredDep = findDep(deps, ":scala_ignored_unused_dep")
+
+      ignoredDep must (beIgnored and beUnused)
+    }
+  }
+
+  def beIgnored: Matcher[ScalaDeps.Dependency] = {
+    beTrue ^^ { (_: ScalaDeps.Dependency).getIgnored }
+  }
+
+  def beUnused: Matcher[ScalaDeps.Dependency] = {
+    be_==(UNUSED) ^^ { (_: ScalaDeps.Dependency).getKind }
+  }
+
+  def loadSdeps(resource: String): util.List[ScalaDeps.Dependency] = {
+    val prefix = "/test_expect_failure/compiler_dependency_tracker/sdeps/"
+    val is = getClass.getResourceAsStream(prefix + resource)
+    ScalaDeps.Dependencies.parseFrom(is).getDependencyList
+  }
+
+  def findDep(deps: util.List[ScalaDeps.Dependency], byLabelSuffix: String): ScalaDeps.Dependency =
+    deps.stream()
+      .filter(_.getLabel.endsWith(byLabelSuffix))
+      .findFirst()
+      .orElseThrow(
+        () => new RuntimeException(byLabelSuffix + " dep not reported in the sdeps file")
+      )
+}