Remove strict_java_deps backwards compatibility (#1033)

* Remove strict_java_deps backwards compatibility

* fix

* fix

Author: Jamie5

README.md

@@ -266,6 +266,8 @@ buildozer 'add deps //some_package:transitive_dependency' //some_other_package:t
 ```
 Note that if you have `buildozer` installed you can just run the last line and have it automatically apply the fix for you.
 
+Note that this option only applies to scala code. Any java code, even that within `scala_library` and other rules_scala rules, is still controlled by the `--strict_java_deps` command-line flag.
+
 ### [Experimental] Unused dependency checking
 To allow for better caching and faster builds we want to minimize the direct dependencies of our targets. Unused dependency checking
 makes sure that all targets specified as direct dependencies are actually used. If `unused_dependency_checker_mode` is set to either
@@ -302,12 +304,6 @@ It can be daunting to turn on strict deps checking or unused dependency mode che
 
 We recommend turning on strict_deps_mode first, as rule `A` might have an entry `B` in its `deps`, and `B` in turn depends on `C`. Meanwhile, the code of `A` only uses `C` but not `B`. Hence, the unused dependency checker, if on, will request that `B` be removed from `A`'s deps. But this will lead to a compile error as `A` can no longer depend on `C`. However, if strict dependency checking was on, then `A`'s deps is guaranteed to have `C` in it.
 
-### [Experimental] Migrating from deprecated configurations
-
-There are a few deprecated configuration methods which we will be removing in the near future.
-
-- The command line argument `--strict_java_deps=WARN/ERROR`. Instead, set `dependency_mode = "transitive"` on the scala toolchain, and if only a warning is desired set `strict_deps_mode = "warn"` on the toolchain. In the future, `strict_java_deps` will no longer affect how scala files are compiled. Note that `strict_java_deps` will still control java compilation.
-
 ## Advanced configurable rules
 To make the ruleset more flexible and configurable, we introduce a phase architecture. By using a phase architecture, where rule implementations are defined as a list of phases that are executed sequentially, functionality can easily be added (or modified) by adding (or swapping) phases.
 

scala/private/common_attributes.bzl

@@ -47,9 +47,6 @@ common_attrs = {}
 common_attrs.update(common_attrs_for_plugin_bootstrapping)
 
 common_attrs.update({
-    # using stricts scala deps is done by using command line flag called 'strict_java_deps'
-    # switching mode to "on" means that ANY API change in a target's transitive dependencies will trigger a recompilation of that target,
-    # on the other hand any internal change (i.e. on code that ijar omits) WON’T trigger recompilation by transitive dependencies
     "_dependency_analyzer_plugin": attr.label(
         default = Label(
             "@io_bazel_rules_scala//third_party/dependency_analyzer/src/main:dependency_analyzer",

scala/scala_toolchain.bzl

@@ -19,20 +19,14 @@ def _compute_dependency_tracking_method(input_dependency_tracking_method):
     return input_dependency_tracking_method
 
 def _scala_toolchain_impl(ctx):
-    if ctx.fragments.java.strict_java_deps != "default" and ctx.fragments.java.strict_java_deps != "off":
-        dependency_mode = "transitive"
-        strict_deps_mode = ctx.fragments.java.strict_java_deps
-        unused_dependency_checker_mode = "off"
-        dependency_tracking_method = "high-level"
-    else:
-        dependency_mode = ctx.attr.dependency_mode
-        strict_deps_mode = _compute_strict_deps_mode(
-            ctx.attr.strict_deps_mode,
-            dependency_mode,
-        )
+    dependency_mode = ctx.attr.dependency_mode
+    strict_deps_mode = _compute_strict_deps_mode(
+        ctx.attr.strict_deps_mode,
+        dependency_mode,
+    )
 
-        unused_dependency_checker_mode = ctx.attr.unused_dependency_checker_mode
-        dependency_tracking_method = _compute_dependency_tracking_method(ctx.attr.dependency_tracking_method)
+    unused_dependency_checker_mode = ctx.attr.unused_dependency_checker_mode
+    dependency_tracking_method = _compute_dependency_tracking_method(ctx.attr.dependency_tracking_method)
 
     # Final quality checks to possibly detect buggy code above
     if dependency_mode not in ("direct", "plus-one", "transitive"):

test/shell/test_deps.sh

@@ -39,7 +39,7 @@ test_scala_import_expect_failure_on_missing_direct_deps_warn_mode() {
   local expected_message1="buildozer 'add deps $dependency_target1' //$test_target"
   local expected_message2="buildozer 'add deps $dependency_target2' //$test_target"
 
-  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message1}" ${test_target} "--strict_java_deps=warn" "ne" "${expected_message2}"
+  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message1}" ${test_target} "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_warn" "ne" "${expected_message2}"
 }
 
 test_plus_one_ast_analyzer_strict_deps() {

test/shell/test_helper.sh

@@ -108,5 +108,5 @@ test_scala_library_expect_failure_on_missing_direct_deps() {
 
   local expected_message="buildozer 'add deps $dependenecy_target' //$test_target"
 
-  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--strict_java_deps=error"
+  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error"
 }

test/shell/test_scala_library.sh

@@ -85,7 +85,7 @@ test_scala_library_expect_failure_on_missing_direct_deps_warn_mode() {
 
   expected_message="warning: Target '$dependenecy_target' is used but isn't explicitly declared, please add it to the deps"
 
-  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--strict_java_deps=warn" "ne"
+  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_warn" "ne"
 }
 
 test_scala_library_expect_failure_on_missing_direct_deps_warn_mode_java() {
@@ -101,13 +101,13 @@ test_scala_library_expect_failure_on_missing_direct_deps_off_mode() {
   expected_message="test_expect_failure/missing_direct_deps/internal_deps/A.scala:[0-9+]: error: not found: value C"
   test_target='test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_user'
 
-  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--strict_java_deps=off"
+  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--extra_toolchains=//test/toolchains:high_level_direct_deps"
 }
 
 test_scala_library_expect_no_recompilation_on_internal_change_of_transitive_dependency() {
   set +e
   no_recompilation_path="test/src/main/scala/scalarules/test/strict_deps/no_recompilation"
-  build_command="bazel build //$no_recompilation_path/... --subcommands --strict_java_deps=error"
+  build_command="bazel build //$no_recompilation_path/... --subcommands --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error"
 
   echo "running initial build"
   $build_command
@@ -166,7 +166,7 @@ test_scala_library_expect_better_failure_message_on_missing_transitive_dependenc
 
   expected_message="Unknown label of file $transitive_target which came from $direct_target"
 
-  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--strict_java_deps=error"
+  test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" $test_target "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error"
 }
 
 $runner test_scala_library_suite

test/shell/test_unused_dependency.sh

@@ -39,11 +39,11 @@ test_succeeds_with_warning() {
 test_unused_dependency_checker_mode_warn() {
   # this is a hack to invalidate the cache, so that the target actually gets built and outputs warnings.
   bazel build \
-    --strict_java_deps=warn \
+    --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_warn \
     //test:UnusedDependencyCheckerWarn
 
   test_succeeds_with_warning \
-    "bazel build --strict_java_deps=off //test:UnusedDependencyCheckerWarn" \
+    "bazel build --extra_toolchains=//test/toolchains:high_level_direct_deps //test:UnusedDependencyCheckerWarn" \
     "warning: Target '//test:UnusedLib' is specified as a dependency to //test:UnusedDependencyCheckerWarn but isn't used, please remove it from the deps."
 }
 

test/toolchains/BUILD.bazel

@@ -59,3 +59,48 @@ toolchain(
     toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
     visibility = ["//visibility:public"],
 )
+
+scala_toolchain(
+    name = "high_level_transitive_deps_strict_deps_warn_impl",
+    dependency_mode = "transitive",
+    dependency_tracking_method = "high-level",
+    strict_deps_mode = "warn",
+    visibility = ["//visibility:public"],
+)
+
+toolchain(
+    name = "high_level_transitive_deps_strict_deps_warn",
+    toolchain = "high_level_transitive_deps_strict_deps_warn_impl",
+    toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
+    visibility = ["//visibility:public"],
+)
+
+scala_toolchain(
+    name = "high_level_transitive_deps_strict_deps_error_impl",
+    dependency_mode = "transitive",
+    dependency_tracking_method = "high-level",
+    strict_deps_mode = "error",
+    visibility = ["//visibility:public"],
+)
+
+toolchain(
+    name = "high_level_transitive_deps_strict_deps_error",
+    toolchain = "high_level_transitive_deps_strict_deps_error_impl",
+    toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
+    visibility = ["//visibility:public"],
+)
+
+scala_toolchain(
+    name = "high_level_direct_deps_impl",
+    dependency_mode = "direct",
+    dependency_tracking_method = "high-level",
+    strict_deps_mode = "off",
+    visibility = ["//visibility:public"],
+)
+
+toolchain(
+    name = "high_level_direct_deps",
+    toolchain = "high_level_direct_deps_impl",
+    toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
+    visibility = ["//visibility:public"],
+)

test_rules_scala.sh

@@ -16,13 +16,12 @@ $runner bazel build test/...
 #$runner bazel build "test/... --all_incompatible_changes"
 $runner bazel test test/...
 $runner bazel test third_party/...
-# UnusedDependencyChecker doesn't work with strict_java_deps
-$runner bazel build "--strict_java_deps=ERROR -- test/... -test:UnusedDependencyChecker"
-$runner bazel build "--extra_toolchains=//scala:minimal_direct_source_deps -- test/... -test:UnusedDependencyChecker"
-#$runner bazel build "--strict_java_deps=ERROR --all_incompatible_changes -- test/... -test:UnusedDependencyChecker"
-$runner bazel test "--strict_java_deps=ERROR -- test/... -test:UnusedDependencyChecker"
-$runner bazel test "--extra_toolchains=//scala:minimal_direct_source_deps -- test/... -test:UnusedDependencyChecker"
-$runner bazel build "test_expect_failure/missing_direct_deps/internal_deps/... --strict_java_deps=warn"
+$runner bazel build "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/..."
+$runner bazel build "--extra_toolchains=//scala:minimal_direct_source_deps -- test/..."
+#$runner bazel build "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error --all_incompatible_changes -- test/..."
+$runner bazel test "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/..."
+$runner bazel test "--extra_toolchains=//scala:minimal_direct_source_deps -- test/..."
+$runner bazel build "test_expect_failure/missing_direct_deps/internal_deps/... --strict_java_deps=warn --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_warn"
 $runner bazel build //test_expect_failure/proto_source_root/... --strict_proto_deps=off
 $runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps"
 . "${test_dir}"/test_build_event_protocol.sh