Skip to content

Make java library earlier on classpath #1797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vinnybod wants to merge 4 commits into
base: master
Choose a base branch
from
+70 −3
Open

Make java library earlier on classpath #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
222c1c7
Make java library earlier on classpath
vinnybod Dec 2, 2025
7be0e1e
work around strict deps check
vinnybod Dec 2, 2025
2d6667b
add additional build_tag_filters
vinnybod Dec 2, 2025
8674138
another edit to the filters
vinnybod Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scala/private/phases/phase_compile.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def _compile_or_empty(
full_jars.append(java_jar.jar)
Copy link
Collaborator

@WojciechMazur WojciechMazur Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially we might also align this part as well: currently it would use the old order: Scala -> Java
I'm not sure if can come up with example which would require that, but at least both list would have the same order


if java_jar:
merged_provider = java_common.merge([scala_compilation_provider, java_jar.java_compilation_provider])
merged_provider = java_common.merge([java_jar.java_compilation_provider, scala_compilation_provider])
else:
merged_provider = scala_compilation_provider

Expand Down
28 changes: 28 additions & 0 deletions test/src/main/scala/scalarules/test/java_classpath_order/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
load("//scala:scala.bzl", "scala_library", "scala_test")

# Original library with the class to be shadowed
scala_library(
name = "original",
srcs = ["Original.scala"],
)

# Library that shadows the Overridable class with a Java file
# The Java file (Overridable.java) should take precedence over
# the Overridable class from the :original dependency
scala_library(
name = "with_java_override",
srcs = ["Overridable.java"],
deps = [":original"],
)

# Test that verifies the Java override works correctly
# Note: This test is incompatible with strict deps checking because the checker
# doesn't understand class shadowing - it sees Overridable as coming from :original
# rather than the shadowed version from :with_java_override
scala_test(
name = "verify_override_test",
srcs = ["VerifyOverride.scala"],
tags = ["no-strict-deps"],
unused_dependency_checker_mode = "off",
deps = [":with_java_override"],
)
6 changes: 6 additions & 0 deletions test/src/main/scala/scalarules/test/java_classpath_order/Original.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package scalarules.test.java_classpath_order

class Overridable {
def getValue(): String = "original"
}

8 changes: 8 additions & 0 deletions test/src/main/scala/scalarules/test/java_classpath_order/Overridable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package scalarules.test.java_classpath_order;

public class Overridable {
public String getValue() {
return "overridden";
}
}

25 changes: 25 additions & 0 deletions test/src/main/scala/scalarules/test/java_classpath_order/VerifyOverride.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package scalarules.test.java_classpath_order

import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

/**
* This test verifies that the Java class (Overridable) shadows
* the Scala class from the dependency.
*
* The expected behavior is:
* - The Java file compiled in the scala_library with_java_override should
* appear first on the classpath
* - When instantiating Overridable, we should get the Java version
* which returns "overridden"
*/
class VerifyOverrideTest extends AnyFlatSpec with Matchers {

"Java class in scala_library" should "shadow the dependency's class" in {
val instance = new Overridable()
val value = instance.getValue()

value shouldBe "overridden"
}
}

4 changes: 2 additions & 2 deletions test_rules_scala.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ $runner bazel build src/... test/...
$runner bazel test "${test_output_flag}" src/... test/...
$runner bazel test "${test_output_flag}" third_party/...
$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=//scala:minimal_direct_source_deps --build_tag_filters=-no-strict-deps -- test/...
#$runner bazel build --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error --all_incompatible_changes -- test/...
$runner bazel test "${test_output_flag}" --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/...
$runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps -- test/...
$runner bazel test "${test_output_flag}" --extra_toolchains=//scala:minimal_direct_source_deps --build_tag_filters=-no-strict-deps --test_tag_filters=-no-strict-deps -- test/...
Copy link
Contributor Author

@vinnybod vinnybod Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love that I had to do this, but afaik there isn't a way to disable "strict deps" on a target level like you can with "unused deps"

Copy link
Collaborator

@WojciechMazur WojciechMazur Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this patch should do the job without changes to the testing script. We should be able to revert these changes and the tag from the test (however it would be worth it to add comment linking to the change in the toolchain)

diff --git a/scala/BUILD b/scala/BUILD
index 2f0b40b..ee98b80 100644
--- a/scala/BUILD
+++ b/scala/BUILD
@@ -29,6 +29,11 @@ scala_toolchain(
     dependency_tracking_method = "ast",
     strict_deps_mode = "error",
     unused_dependency_checker_mode = "error",
+    # Exclude the shadowing test package from strict-deps to allow classpath-order coverage without disabling strict-deps globally in CI runs.
+    dependency_tracking_strict_deps_patterns = [
+        "",  # keep default include
+        "-//test/src/main/scala/scalarules/test/java_classpath_order",
+    ],
 )
 
 [

$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_output_flag" //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps"
Expand Down