Don't output unused dependency warning and errors when compilation fails. (#1548)

When a scala source is malformed, the AST parsing dependency checking step comes
back with no/very little dependencies used. It then prints out a bunch of these
warnings which are not necessarily true. As a workaround, let's not print the
unused dependency warnings if the build has already failed before hand.

Author: laurynaslubys

src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java

@@ -146,20 +146,22 @@ public void prepareReport() throws IOException {
     }
 
     Set<Dependency> unusedDeps = new HashSet<>();
-    for (int i = 0; i < ops.directTargets.length; i++) {
-      String directTarget = ops.directTargets[i];
-      if (usedTargets.contains(directTarget)) {
-        continue;
-      }
+    if (!hasErrors()) {
+      for (int i = 0; i < ops.directTargets.length; i++) {
+        String directTarget = ops.directTargets[i];
+        if (usedTargets.contains(directTarget)) {
+          continue;
+        }
 
-      unusedDeps.add(
+        unusedDeps.add(
           buildDependency(
-              ops.directJars[i],
-              directTarget,
-              Kind.UNUSED,
-              ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
+            ops.directJars[i],
+            directTarget,
+            Kind.UNUSED,
+            ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
           )
-      );
+        );
+      }
     }
 
     writeSdepsFile(usedDeps, unusedDeps);

src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_13_12/DepsTrackingReporter.java

@@ -137,20 +137,22 @@ public void prepareReport() throws IOException {
     }
 
     Set<Dependency> unusedDeps = new HashSet<>();
-    for (int i = 0; i < ops.directTargets.length; i++) {
-      String directTarget = ops.directTargets[i];
-      if (usedTargets.contains(directTarget)) {
-        continue;
-      }
+    if (!hasErrors()) {
+      for (int i = 0; i < ops.directTargets.length; i++) {
+        String directTarget = ops.directTargets[i];
+        if (usedTargets.contains(directTarget)) {
+          continue;
+        }
 
-      unusedDeps.add(
+        unusedDeps.add(
           buildDependency(
-              ops.directJars[i],
-              directTarget,
-              Kind.UNUSED,
-              ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
+            ops.directJars[i],
+            directTarget,
+            Kind.UNUSED,
+            ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
           )
-      );
+        );
+      }
     }
 
     writeSdepsFile(usedDeps, unusedDeps);

src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/before_2_12_13/DepsTrackingReporter.java

@@ -133,20 +133,22 @@ public void prepareReport() throws IOException {
     }
 
     Set<Dependency> unusedDeps = new HashSet<>();
-    for (int i = 0; i < ops.directTargets.length; i++) {
-      String directTarget = ops.directTargets[i];
-      if (usedTargets.contains(directTarget)) {
-        continue;
-      }
+    if (!hasErrors()) {
+      for (int i = 0; i < ops.directTargets.length; i++) {
+        String directTarget = ops.directTargets[i];
+        if (usedTargets.contains(directTarget)) {
+          continue;
+        }
 
-      unusedDeps.add(
+        unusedDeps.add(
           buildDependency(
-              ops.directJars[i],
-              directTarget,
-              Kind.UNUSED,
-              ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
+            ops.directJars[i],
+            directTarget,
+            Kind.UNUSED,
+            ignoredTargets.contains(directTarget) || "off".equals(ops.unusedDependencyCheckerMode)
           )
-      );
+        );
+      }
     }
 
     writeSdepsFile(usedDeps, unusedDeps);

test/shell/test_compiler_dependency_tracking.sh

@@ -28,7 +28,35 @@ test_sdeps() {
   bazel test --extra_toolchains=//test_expect_failure/compiler_dependency_tracker:ast_plus_warn //test_expect_failure/compiler_dependency_tracker/sdeps/...
 }
 
+test_fails_without_warning() {
+  cmd=$1
+  expected=$2
+
+  local output
+  output=$($cmd 2>&1)
+
+  if [ $? -eq 0 ]; then
+    echo "Expected build to fail"
+    echo "$output"
+    exit 1
+  fi
+
+  echo "$output" | grep "$expected"
+  if [ $? -eq 0 ]; then
+    echo "Expected output:[$output] to not contain [$expected]"
+    exit 1
+  fi
+}
+
+test_no_unused_warn_when_broken() {
+  action_should_fail_without_message \
+    "remove deps" \
+    build //test_expect_failure/compiler_dependency_tracker:F
+}
+
+
 $runner test_fails_for_unused_dep
 $runner test_fails_for_missing_compile_dep
 $runner test_fails_for_strict_dep
 $runner test_sdeps
+$runner test_no_unused_warn_when_broken

test/shell/test_helper.sh

@@ -73,6 +73,27 @@ action_should_fail_with_message() {
   fi
 }
 
+action_should_fail_without_message() {
+  set +e
+  MSG=$1
+  TEST_ARG=${@:2}
+  RES=$(bazel $TEST_ARG 2>&1)
+  RESPONSE_CODE=$?
+  echo $RES | grep -- "$MSG"
+  GREP_RES=$?
+  if [ $RESPONSE_CODE -eq 0 ]; then
+    echo $RES
+    echo -e "${RED} \"bazel $TEST_ARG\" should have failed but passed. $NC"
+    exit 1
+  elif [ $GREP_RES -eq 0 ]; then
+    echo $RES
+    echo -e "${RED} \"bazel $TEST_ARG\" should have failed with message not containing \"$MSG\" but it did. $NC"
+    exit 1
+  else
+    exit 0
+  fi
+}
+
 test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message() {
   set +e
 
@@ -133,4 +154,4 @@ jar_contains_files() {
       return 1
     fi
   done
-}
+}

test_expect_failure/compiler_dependency_tracker/BUILD

@@ -69,3 +69,9 @@ scala_library(
     name = "E",
     srcs = ["E.scala"],
 )
+
+scala_library(
+    name = "F",
+    srcs = ["F.scala"],
+    deps = [":E"],
+)

test_expect_failure/compiler_dependency_tracker/F.scala

@@ -0,0 +1,2 @@
+class F {
+