Skip to content

Commit 59fadb8

Browse files
chaorenittaiz
chaoren
authored and
ittaiz
committed
Organize implicit dependencies necessary for IDEs.
Add a separate attribute whose sole purpose is to propagate dependency information via aspects. This will allow the IntelliJ aspect to get every necessary implicit dependency by just following _scala_toolchain. This also allows us to avoid the issue of certain dependencies being required to be used as files instead of java targets.
1 parent be2caba commit 59fadb8

File tree

4 files changed

+133
-47
lines changed

4 files changed

+133
-47
lines changed

‎scala/scala.bzl

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,28 @@ _implicit_deps = {
620620
"_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True),
621621
}
622622

623+
# Single dep to allow IDEs to pickup all the implicit dependencies.
624+
_resolve_deps = {
625+
"_scala_toolchain" : attr.label_list(default=[
626+
Label("//external:io_bazel_rules_scala/dependency/scala/scala_library"),
627+
], allow_files=False),
628+
}
629+
630+
_test_resolve_deps = {
631+
"_scala_toolchain" : attr.label_list(default=[
632+
Label("//external:io_bazel_rules_scala/dependency/scala/scala_library"),
633+
Label("//external:io_bazel_rules_scala/dependency/scalatest/scalatest"),
634+
], allow_files=False),
635+
}
636+
637+
_junit_resolve_deps = {
638+
"_scala_toolchain" : attr.label_list(default=[
639+
Label("//external:io_bazel_rules_scala/dependency/scala/scala_library"),
640+
Label("//external:io_bazel_rules_scala/dependency/junit/junit"),
641+
Label("//external:io_bazel_rules_scala/dependency/hamcrest/hamcrest_core"),
642+
], allow_files=False),
643+
}
644+
623645
# Common attributes reused across multiple rules.
624646
_common_attrs = {
625647
"srcs": attr.label_list(
@@ -644,7 +666,7 @@ scala_library = rule(
644666
attrs={
645667
"main_class": attr.string(),
646668
"exports": attr.label_list(allow_files=False),
647-
} + _implicit_deps + _common_attrs,
669+
} + _implicit_deps + _common_attrs + _resolve_deps,
648670
outputs={
649671
"jar": "%{name}.jar",
650672
"deploy_jar": "%{name}_deploy.jar",
@@ -658,7 +680,7 @@ scala_macro_library = rule(
658680
attrs={
659681
"main_class": attr.string(),
660682
"exports": attr.label_list(allow_files=False),
661-
} + _implicit_deps + _common_attrs,
683+
} + _implicit_deps + _common_attrs + _resolve_deps,
662684
outputs={
663685
"jar": "%{name}.jar",
664686
"deploy_jar": "%{name}_deploy.jar",
@@ -670,7 +692,7 @@ scala_binary = rule(
670692
implementation=_scala_binary_impl,
671693
attrs={
672694
"main_class": attr.string(mandatory=True),
673-
} + _launcher_template + _implicit_deps + _common_attrs,
695+
} + _launcher_template + _implicit_deps + _common_attrs + _resolve_deps,
674696
outputs={
675697
"jar": "%{name}.jar",
676698
"deploy_jar": "%{name}_deploy.jar",
@@ -687,7 +709,7 @@ scala_test = rule(
687709
"_scalatest": attr.label(default=Label("//external:io_bazel_rules_scala/dependency/scalatest/scalatest"), allow_files=True),
688710
"_scalatest_runner": attr.label(executable=True, cfg="host", default=Label("//src/java/io/bazel/rulesscala/scala_test:runner.jar"), allow_files=True),
689711
"_scalatest_reporter": attr.label(default=Label("//scala/support:test_reporter")),
690-
} + _launcher_template + _implicit_deps + _common_attrs,
712+
} + _launcher_template + _implicit_deps + _common_attrs + _test_resolve_deps,
691713
outputs={
692714
"jar": "%{name}.jar",
693715
"deploy_jar": "%{name}_deploy.jar",
@@ -699,7 +721,7 @@ scala_test = rule(
699721

700722
scala_repl = rule(
701723
implementation=_scala_repl_impl,
702-
attrs= _launcher_template + _implicit_deps + _common_attrs,
724+
attrs= _launcher_template + _implicit_deps + _common_attrs + _resolve_deps,
703725
outputs={
704726
"jar": "%{name}.jar",
705727
"deploy_jar": "%{name}_deploy.jar",
@@ -865,7 +887,7 @@ def scala_library_suite(name,
865887

866888
scala_junit_test = rule(
867889
implementation=_scala_junit_test_impl,
868-
attrs= _launcher_template + _implicit_deps + _common_attrs + {
890+
attrs= _launcher_template + _implicit_deps + _common_attrs + _junit_resolve_deps + {
869891
"prefixes": attr.string_list(default=[]),
870892
"suffixes": attr.string_list(default=[]),
871893
"print_discovered_classes": attr.bool(default=False, mandatory=False),

‎test/aspect/BUILD

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,34 @@
11
load(":aspect.bzl", "aspect_test")
2-
load("//scala:scala.bzl", "scala_test")
2+
load(
3+
"//scala:scala.bzl",
4+
"scala_library",
5+
"scala_test",
6+
"scala_junit_test",
7+
"scala_specs2_junit_test",
8+
)
39

410
aspect_test(
5-
name = "test",
6-
scala_rule = ":dummy",
11+
name = "aspect_test",
12+
targets = [
13+
":scala_library",
14+
":scala_test",
15+
":scala_junit_test",
16+
":scala_specs2_junit_test",
17+
],
18+
)
19+
20+
scala_library(name = "scala_library")
21+
22+
scala_test(name = "scala_test")
23+
24+
scala_junit_test(
25+
name = "scala_junit_test",
26+
srcs = ["FakeJunitTest.scala"],
27+
suffixes = ["Test"],
728
)
829

9-
scala_test(name = "dummy")
30+
scala_specs2_junit_test(
31+
name = "scala_specs2_junit_test",
32+
srcs = ["FakeJunitTest.scala"],
33+
suffixes = ["Test"],
34+
)

‎test/aspect/FakeJunitTest.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package test.aspect
2+
3+
import org.junit.Test
4+
import org.junit.runner.RunWith
5+
import org.junit.runners.JUnit4
6+
7+
@RunWith(classOf[JUnit4])
8+
class FakeJunitTest {
9+
@Test
10+
def fakeTest(): Unit = {}
11+
}

‎test/aspect/aspect.bzl

Lines changed: 65 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
1-
attr_aspects = [
2-
"deps",
3-
"runtime_deps",
4-
"_scalalib",
5-
"_scalareflect",
6-
"_scalatest_reporter",
7-
]
1+
"""
2+
This test makes sure that the implicit rule dependencies are discoverable by
3+
an IDE. We stuff all dependencies into _scala_toolchain so we just need to make
4+
sure the targets we expect are there.
5+
"""
6+
attr_aspects = ["_scala_toolchain", "deps"]
87

98
def _aspect_impl(target, ctx):
10-
visited = [target.label.name]
11-
for name in attr_aspects:
12-
if hasattr(ctx.rule.attr, name):
13-
attr = getattr(ctx.rule.attr, name)
14-
# Need to handle whether attribute is label or label list
15-
children = attr if type(attr) == "list" else [attr]
16-
for child in children:
17-
if hasattr(child, "visited"):
18-
visited += child.visited
9+
visited = [str(target.label)]
10+
for attr_name in attr_aspects:
11+
if hasattr(ctx.rule.attr, attr_name):
12+
for dep in getattr(ctx.rule.attr, attr_name):
13+
if hasattr(dep, "visited"):
14+
visited += dep.visited
1915
return struct(visited = visited)
2016

2117
test_aspect = aspect(
@@ -24,26 +20,55 @@ test_aspect = aspect(
2420
)
2521

2622
def _rule_impl(ctx):
27-
expected = [
28-
"dummy",
29-
"jar", # This is scalatest since @scalatest/jar
30-
"scala-library",
31-
"scala-reflect",
32-
"scala-xml", # dependency of test_reporter
33-
"test_reporter",
34-
]
35-
# Remove duplicates and sort so can do simple comparison
36-
visited = sorted(depset(ctx.attr.scala_rule.visited).to_list())
37-
if visited == expected:
38-
content = "true"
39-
else:
40-
content = """
41-
echo Expected these rules to be visited by the aspect: 1>&2
42-
echo %s, 1>&2
43-
echo but got these instead: 1>&2
44-
echo %s 1>&2
45-
false
46-
""" % (', '.join(expected), ', '.join(visited))
23+
expected_deps = {
24+
"scala_library" : [
25+
"//test/aspect:scala_library",
26+
"@scala//:scala-library",
27+
],
28+
"scala_test" : [
29+
"//test/aspect:scala_test",
30+
"@scala//:scala-library",
31+
"@scalatest//jar:jar",
32+
],
33+
"scala_junit_test" : [
34+
"//test/aspect:scala_junit_test",
35+
"@scala//:scala-library",
36+
"@io_bazel_rules_scala_junit_junit//jar:jar",
37+
"@io_bazel_rules_scala_org_hamcrest_hamcrest_core//jar:jar",
38+
],
39+
"scala_specs2_junit_test" : [
40+
"//test/aspect:scala_specs2_junit_test",
41+
"@scala//:scala-library",
42+
"@io_bazel_rules_scala_junit_junit//jar:jar",
43+
"@io_bazel_rules_scala_org_hamcrest_hamcrest_core//jar:jar",
44+
# From specs2/specs2.bzl:specs2_dependencies()
45+
"@io_bazel_rules_scala_org_specs2_specs2_core//jar:jar",
46+
"@io_bazel_rules_scala_org_specs2_specs2_common//jar:jar",
47+
"@io_bazel_rules_scala_org_specs2_specs2_matcher//jar:jar",
48+
"@io_bazel_rules_scala_org_scalaz_scalaz_effect//jar:jar",
49+
"@io_bazel_rules_scala_org_scalaz_scalaz_core//jar:jar",
50+
"@scala//:scala-xml",
51+
"@scala//:scala-parser-combinators",
52+
"@scala//:scala-library",
53+
"@scala//:scala-reflect",
54+
# From specs2/specs2_junit.bzl:specs2_junit_dependencies()
55+
"@io_bazel_rules_scala_org_specs2_specs2_junit_2_11//jar:jar",
56+
],
57+
}
58+
content = ""
59+
for target in ctx.attr.targets:
60+
visited = sorted(target.visited)
61+
expected = sorted(expected_deps[target.label.name])
62+
if visited != expected:
63+
content += """
64+
echo Expected these deps from {name}: 1>&2
65+
echo {expected}, 1>&2
66+
echo but got these instead: 1>&2
67+
echo {visited} 1>&2
68+
false # test returns 1 (and fails) if this is the final line
69+
""".format(name=target.label.name,
70+
expected=', '.join(expected),
71+
visited=', '.join(visited))
4772
ctx.file_action(
4873
output = ctx.outputs.executable,
4974
content = content,
@@ -52,7 +77,10 @@ def _rule_impl(ctx):
5277

5378
aspect_test = rule(
5479
implementation = _rule_impl,
55-
attrs = { "scala_rule" : attr.label(aspects = [test_aspect]) },
80+
attrs = {
81+
# The targets whose dependencies we want to verify.
82+
"targets" : attr.label_list(aspects = [test_aspect]),
83+
},
5684
test = True,
5785
)
5886

0 commit comments

Comments
 (0)