diff --git a/scala/private/rules/scala_doc.bzl b/scala/private/rules/scala_doc.bzl index 74bd06482..e40e6220c 100644 --- a/scala/private/rules/scala_doc.bzl +++ b/scala/private/rules/scala_doc.bzl @@ -3,9 +3,9 @@ load("@io_bazel_rules_scala//scala/private:common.bzl", "collect_plugin_paths") ScaladocAspectInfo = provider(fields = [ - "src_files", - "compile_jars", - "plugins", + "src_files", #depset[File] + "compile_jars", #depset[File] + "plugins", #depset[Target] ]) def _scaladoc_intransitive_aspect_impl(target, ctx): @@ -15,40 +15,39 @@ def _scaladoc_intransitive_aspect_impl(target, ctx): def _scaladoc_aspect_impl(target, ctx, transitive = True): """Collect source files and compile_jars from JavaInfo-returning deps.""" + src_files = depset() + plugins = depset() + compile_jars = depset() + # We really only care about visited targets with srcs, so only look at those. if hasattr(ctx.rule.attr, "srcs"): # Collect only Java and Scala sources enumerated in visited targets, including src_files in deps. - direct_deps = [file for file in ctx.rule.files.srcs if file.extension.lower() in ["java", "scala"]] - - # Sometimes we only want to generate scaladocs for a single target and not all of its - # dependencies - if transitive: - transitive_deps = [dep[ScaladocAspectInfo].src_files for dep in ctx.rule.attr.deps if ScaladocAspectInfo in dep] - else: - transitive_deps = [] - - src_files = depset(direct = direct_deps, transitive = transitive_deps) - - # Collect compile_jars from visited targets' deps. - compile_jars = depset( - direct = [file for file in ctx.rule.files.deps], - transitive = ( - [dep[JavaInfo].compile_jars for dep in ctx.rule.attr.deps if JavaInfo in dep] + - [dep[ScaladocAspectInfo].compile_jars for dep in ctx.rule.attr.deps if ScaladocAspectInfo in dep] - ), - ) + src_files = depset([file for file in ctx.rule.files.srcs if file.extension.lower() in ["java", "scala"]]) + + compile_jars = target[JavaInfo].transitive_compile_time_jars - plugins = depset() if hasattr(ctx.rule.attr, "plugins"): - plugins = depset(direct = ctx.rule.attr.plugins) - - return [ScaladocAspectInfo( - src_files = src_files, - compile_jars = compile_jars, - plugins = plugins, - )] - else: - return [] + plugins = depset(ctx.rule.attr.plugins) + + # Sometimes we only want to generate scaladocs for a single target and not all of its + # dependencies + transitive_srcs = depset() + transitive_compile_jars = depset() + transitive_plugins = depset() + + if transitive: + for dep in ctx.rule.attr.deps: + if ScaladocAspectInfo in dep: + aspec_info = dep[ScaladocAspectInfo] + transitive_srcs = aspec_info.src_files + transitive_compile_jars = aspec_info.compile_jars + transitive_plugins = aspec_info.plugins + + return [ScaladocAspectInfo( + src_files = depset(transitive = [src_files, transitive_srcs]), + compile_jars = depset(transitive = [compile_jars, transitive_compile_jars]), + plugins = depset(transitive = [plugins, transitive_plugins]), + )] _scaladoc_transitive_aspect = aspect( implementation = _scaladoc_aspect_impl, diff --git a/test/shell/test_scaladoc.sh b/test/shell/test_scaladoc.sh new file mode 100644 index 000000000..a886f7f3a --- /dev/null +++ b/test/shell/test_scaladoc.sh @@ -0,0 +1,63 @@ +# shellcheck source=./test_runner.sh + +dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) +. "${dir}"/test_runner.sh +. "${dir}"/test_helper.sh +runner=$(get_test_runner "${1:-local}") + +_check_file_existence() { + local filepath=$1 + local expect=$2 #1 for yes, 0 for no + + if [[ -f "$filepath" ]] ; then + if [[ $expect == 0 ]]; then + echo "Error: Unexpected scaladoc file found: ${filepath} " + exit 1; + fi + else + if [[ $expect == 1 ]]; then + echo "Error: Expected scaladoc file not found: ${filepath} " + exit 1; + fi + fi +} + +test_scaladoc_transitive() { + #Verify that docs are generated for B AND A. (B depends on A). + set -e + + bazel build //test_expect_failure/scaladoc:scaladoc_transitive --extra_toolchains=//test/toolchains:ast_plus_one_deps_unused_deps_warn + + local scaladoc_outpath="$(bazel cquery //test_expect_failure/scaladoc:scaladoc_transitive --extra_toolchains=//test/toolchains:ast_plus_one_deps_unused_deps_warn --output=files)" + + _check_file_existence ${scaladoc_outpath}/"B$.html" 1 + _check_file_existence ${scaladoc_outpath}/"A$.html" 1 +} + +test_scaladoc_intransitive() { + #Verify that docs only generated for B. (B depends on A) + + set -e + + bazel build //test_expect_failure/scaladoc:scaladoc_intransitive --extra_toolchains=//test/toolchains:ast_plus_one_deps_unused_deps_warn + + local scaladoc_outpath="$(bazel cquery //test_expect_failure/scaladoc:scaladoc_intransitive --extra_toolchains=//test/toolchains:ast_plus_one_deps_unused_deps_warn --output=files)" + + _check_file_existence ${scaladoc_outpath}/"B$.html" 1 + _check_file_existence ${scaladoc_outpath}/"A$.html" 0 +} + +test_scaladoc_works_with_transitive_external_deps() { + #Tests absense of a bug where scaladoc rule wasn't handling transitive dependencies that aren't scala_xxxx (i.e. don't hav a srcs attribute) + #Note: need to use a plus-one toolchain to enable transitive deps. + + set -e + + #Just make sure it builds correctly + bazel build //test_expect_failure/scaladoc:scaladoc_C --extra_toolchains=//test/toolchains:ast_plus_one_deps_unused_deps_warn + +} + +$runner test_scaladoc_intransitive +$runner test_scaladoc_transitive +$runner test_scaladoc_works_with_transitive_external_deps diff --git a/test_expect_failure/scaladoc/A.scala b/test_expect_failure/scaladoc/A.scala new file mode 100644 index 000000000..8c725bf5c --- /dev/null +++ b/test_expect_failure/scaladoc/A.scala @@ -0,0 +1,6 @@ + +object A { + def main() { + println("hi") + } +} diff --git a/test_expect_failure/scaladoc/B.scala b/test_expect_failure/scaladoc/B.scala new file mode 100644 index 000000000..04ac4a950 --- /dev/null +++ b/test_expect_failure/scaladoc/B.scala @@ -0,0 +1,5 @@ +object B { + def myfunc() { + A.main() + } +} diff --git a/test_expect_failure/scaladoc/BUILD b/test_expect_failure/scaladoc/BUILD new file mode 100644 index 000000000..a60aae390 --- /dev/null +++ b/test_expect_failure/scaladoc/BUILD @@ -0,0 +1,55 @@ +load("scaladoc.bzl", "scala_doc_intransitive") +load("//scala:scala.bzl", "scala_doc", "scala_library") +load("//scala:scala_import.bzl", "scala_import") + +package(default_testonly = 1) + +scala_library( + name = "A", + srcs = ["A.scala"], + deps = [], +) + +#B depends on A +scala_library( + name = "B", + srcs = ["B.scala"], + deps = ["A"], +) + +#Simulate A & B as if they were imported libraries (in particular they are targets that do not have srcs attribute) +scala_import( + name = "ImportedA", + jars = ["A"], +) + +scala_import( + name = "ImportedB", + jars = ["B"], + deps = ["ImportedA"], +) + +#Setup this scala_library target so that it has dependency on a transitive external lib. +scala_library( + name = "C", + srcs = ["C.scala"], + deps = [ + "ImportedB", + #For the test, don't include ImportedA here... we want it to be loaded transitively + ], +) + +scala_doc( + name = "scaladoc_transitive", + deps = ["B"], +) + +scala_doc_intransitive( + name = "scaladoc_intransitive", + deps = ["B"], +) + +scala_doc( + name = "scaladoc_C", + deps = ["C"], +) diff --git a/test_expect_failure/scaladoc/C.scala b/test_expect_failure/scaladoc/C.scala new file mode 100644 index 000000000..c78a89bfd --- /dev/null +++ b/test_expect_failure/scaladoc/C.scala @@ -0,0 +1,7 @@ +//import cats.syntax.all._ + +object C { + def myfunc() = { + A.main() //Call into A, which is a transitive dependency of this lib + } +} diff --git a/test_expect_failure/scaladoc/scaladoc.bzl b/test_expect_failure/scaladoc/scaladoc.bzl new file mode 100644 index 000000000..ff1bee656 --- /dev/null +++ b/test_expect_failure/scaladoc/scaladoc.bzl @@ -0,0 +1,3 @@ +load("//scala:scala.bzl", "make_scala_doc_rule", "scaladoc_intransitive_aspect") + +scala_doc_intransitive = make_scala_doc_rule(scaladoc_intransitive_aspect) #Only scaladoc specified deps diff --git a/test_rules_scala.sh b/test_rules_scala.sh index 3d962dedb..05600140c 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -57,3 +57,4 @@ $runner bazel build //test_statsfile:SimpleNoStatsFile_statsfile --extra_toolcha . "${test_dir}"/test_inherited_environment.sh . "${test_dir}"/test_persistent_worker.sh . "${test_dir}"/test_semanticdb.sh +. "${test_dir}"/test_scaladoc.sh