Implement imports (via load()) in Skydoc.

Skydoc will generate documentation for all rule definitions in the transitive dependencies of the given input file.

RELNOTES: None.
PiperOrigin-RevId: 202553088

8 files changed, 212 insertions, 54 deletions

diff --git a/src/test/java/com/google/devtools/build/skydoc/BUILD b/src/test/java/com/google/devtools/build/skydoc/BUILD
index c196fbfd91..af9f14112f 100644
--- a/src/test/java/com/google/devtools/build/skydoc/BUILD
+++ b/src/test/java/com/google/devtools/build/skydoc/BUILD
@@ -58,3 +58,14 @@ skydoc_test(
     input_file = "testdata/apple_basic_test/input.bzl",
     skydoc = "//src/main/java/com/google/devtools/build/skydoc",
 )
+
+skydoc_test(
+    name = "multiple_files_test",
+    golden_file = "testdata/multiple_files_test/golden.txt",
+    input_file = "testdata/multiple_files_test/input.bzl",
+    skydoc = "//src/main/java/com/google/devtools/build/skydoc",
+    deps = [
+        "testdata/multiple_files_test/dep.bzl",
+        "testdata/multiple_files_test/inner_dep.bzl",
+    ],
+)
diff --git a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
index ad524c6df5..0f0d79ffb6 100644
--- a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
+++ b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.skydoc;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -24,9 +25,12 @@ import com.google.devtools.build.lib.syntax.ParserInputSource;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skydoc.rendering.RuleInfo;
+import java.io.IOException;
+import java.nio.file.Paths;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -37,33 +41,47 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public final class SkydocTest extends SkylarkTestCase {
 
+  private SkydocMain skydocMain;
+
+  @Before
+  public void setUp() {
+    skydocMain = new SkydocMain(new SkylarkFileAccessor() {
+
+      @Override
+      public ParserInputSource inputSource(String pathString) throws IOException {
+        Path path = fileSystem.getPath(pathString);
+        byte[] bytes = FileSystemUtils.asByteSource(path).read();
+
+        return ParserInputSource.create(bytes, path.asFragment());
+      }
+    });
+  }
+
   @Test
   public void testRuleInfoAttrs() throws Exception {
-    Path file =
-        scratch.file(
-            "/test/test.bzl",
-            "def rule_impl(ctx):",
-            "  return struct()",
-            "",
-            "my_rule = rule(",
-            "    doc = 'This is my rule. It does stuff.',",
-            "    implementation = rule_impl,",
-            "    attrs = {",
-            "        'first': attr.label(mandatory=True, allow_files=True, single_file=True),",
-            "        'second': attr.string_dict(mandatory=True),",
-            "        'third': attr.output(mandatory=True),",
-            "        'fourth': attr.bool(default=False, mandatory=False),",
-            "    },",
-            ")");
-    byte[] bytes = FileSystemUtils.readWithKnownFileSize(file, file.getFileSize());
-
-    ParserInputSource parserInputSource =
-        ParserInputSource.create(bytes, file.asFragment());
+    scratch.file(
+        "/test/test.bzl",
+        "def rule_impl(ctx):",
+        "  return struct()",
+        "",
+        "my_rule = rule(",
+        "    doc = 'This is my rule. It does stuff.',",
+        "    implementation = rule_impl,",
+        "    attrs = {",
+        "        'first': attr.label(mandatory=True, allow_files=True, single_file=True),",
+        "        'second': attr.string_dict(mandatory=True),",
+        "        'third': attr.output(mandatory=True),",
+        "        'fourth': attr.bool(default=False, mandatory=False),",
+        "    },",
+        ")");
 
     ImmutableMap.Builder<String, RuleInfo> ruleInfoMap = ImmutableMap.builder();
     ImmutableList.Builder<RuleInfo> unexportedRuleInfos = ImmutableList.builder();
 
-    new SkydocMain().eval(parserInputSource, ruleInfoMap, unexportedRuleInfos);
+    skydocMain.eval(
+        Paths.get("/test/test.bzl"),
+        ruleInfoMap,
+        unexportedRuleInfos);
     Map<String, RuleInfo> ruleInfos = ruleInfoMap.build();
     assertThat(ruleInfos).hasSize(1);
 
@@ -77,40 +95,38 @@ public final class SkydocTest extends SkylarkTestCase {
 
   @Test
   public void testMultipleRuleNames() throws Exception {
-    Path file =
-        scratch.file(
-            "/test/test.bzl",
-            "def rule_impl(ctx):",
-            "  return struct()",
-            "",
-            "rule_one = rule(",
-            "    doc = 'Rule one',",
-            "    implementation = rule_impl,",
-            ")",
-            "",
-            "rule(",
-            "    doc = 'This rule is not named',",
-            "    implementation = rule_impl,",
-            ")",
-            "",
-            "rule(",
-            "    doc = 'This rule also is not named',",
-            "    implementation = rule_impl,",
-            ")",
-            "",
-            "rule_two = rule(",
-            "    doc = 'Rule two',",
-            "    implementation = rule_impl,",
-            ")");
-    byte[] bytes = FileSystemUtils.readWithKnownFileSize(file, file.getFileSize());
-
-    ParserInputSource parserInputSource =
-        ParserInputSource.create(bytes, file.asFragment());
+    scratch.file(
+        "/test/test.bzl",
+        "def rule_impl(ctx):",
+        "  return struct()",
+        "",
+        "rule_one = rule(",
+        "    doc = 'Rule one',",
+        "    implementation = rule_impl,",
+        ")",
+        "",
+        "rule(",
+        "    doc = 'This rule is not named',",
+        "    implementation = rule_impl,",
+        ")",
+        "",
+        "rule(",
+        "    doc = 'This rule also is not named',",
+        "    implementation = rule_impl,",
+        ")",
+        "",
+        "rule_two = rule(",
+        "    doc = 'Rule two',",
+        "    implementation = rule_impl,",
+        ")");
 
     ImmutableMap.Builder<String, RuleInfo> ruleInfoMap = ImmutableMap.builder();
     ImmutableList.Builder<RuleInfo> unexportedRuleInfos = ImmutableList.builder();
 
-    new SkydocMain().eval(parserInputSource, ruleInfoMap, unexportedRuleInfos);
+    skydocMain.eval(
+        Paths.get("/test/test.bzl"),
+        ruleInfoMap,
+        unexportedRuleInfos);
 
     assertThat(ruleInfoMap.build().keySet()).containsExactly("rule_one", "rule_two");
 
@@ -119,4 +135,82 @@ public final class SkydocTest extends SkylarkTestCase {
             .collect(Collectors.toList()))
         .containsExactly("This rule is not named", "This rule also is not named");
   }
+
+  @Test
+  public void testRulesAcrossMultipleFiles() throws Exception {
+    scratch.file(
+        "/lib/rule_impl.bzl",
+        "def rule_impl(ctx):",
+        "  return struct()");
+
+    scratch.file(
+        "/deps/foo/docstring.bzl",
+        "doc_string = 'Dep rule'");
+
+    scratch.file(
+        "/deps/foo/dep_rule.bzl",
+        "load('//lib:rule_impl.bzl', 'rule_impl')",
+        "load(':docstring.bzl', 'doc_string')",
+        "",
+        "some_var = 1",
+        "",
+        "dep_rule = rule(",
+        "    doc = doc_string,",
+        "    implementation = rule_impl,",
+        ")");
+
+    scratch.file(
+        "/test/main.bzl",
+        "load('//lib:rule_impl.bzl', 'rule_impl')",
+        "load('//deps/foo:dep_rule.bzl', 'some_var')",
+        "",
+        "main_rule = rule(",
+        "    doc = 'Main rule',",
+        "    implementation = rule_impl,",
+        ")");
+
+    ImmutableMap.Builder<String, RuleInfo> ruleInfoMapBuilder = ImmutableMap.builder();
+
+    skydocMain.eval(
+        Paths.get("/test/main.bzl"),
+        ruleInfoMapBuilder,
+        ImmutableList.builder());
+
+    Map<String, RuleInfo> ruleInfoMap = ruleInfoMapBuilder.build();
+
+    assertThat(ruleInfoMap.keySet()).containsExactly("main_rule", "dep_rule");
+    assertThat(ruleInfoMap.get("main_rule").getDocString()).isEqualTo("Main rule");
+    assertThat(ruleInfoMap.get("dep_rule").getDocString()).isEqualTo("Dep rule");
+  }
+
+  @Test
+  public void testSkydocCrashesOnCycle() throws Exception {
+    scratch.file(
+        "/dep/dep.bzl",
+        "load('//test:main.bzl', 'some_var')",
+        "def rule_impl(ctx):",
+        "  return struct()");
+
+    scratch.file(
+        "/test/main.bzl",
+        "load('//dep:dep.bzl', 'rule_impl')",
+        "",
+        "some_var = 1",
+        "",
+        "main_rule = rule(",
+        "    doc = 'Main rule',",
+        "    implementation = rule_impl,",
+        ")");
+
+    ImmutableMap.Builder<String, RuleInfo> ruleInfoMapBuilder = ImmutableMap.builder();
+
+    IllegalStateException expected =
+        assertThrows(IllegalStateException.class,
+            () -> skydocMain.eval(
+                Paths.get("/test/main.bzl"),
+                ruleInfoMapBuilder,
+                ImmutableList.builder()));
+
+    assertThat(expected).hasMessageThat().contains("cycle with /test/main.bzl");
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/skydoc/skydoc_e2e_test_runner.sh b/src/test/java/com/google/devtools/build/skydoc/skydoc_e2e_test_runner.sh
index c32973c17c..8a9a1c7010 100755
--- a/src/test/java/com/google/devtools/build/skydoc/skydoc_e2e_test_runner.sh
+++ b/src/test/java/com/google/devtools/build/skydoc/skydoc_e2e_test_runner.sh
@@ -24,7 +24,9 @@ golden_file=$3
 
 actual_file="${TEST_TMPDIR}/actual"
 
+set -e
 ${skydoc_bin} ${input_file} ${actual_file}
+set +e
 
 DIFF="$(diff ${actual_file} ${golden_file})"
 
diff --git a/src/test/java/com/google/devtools/build/skydoc/skydoc_test.bzl b/src/test/java/com/google/devtools/build/skydoc/skydoc_test.bzl
index 776eb39fa4..c63cedaa08 100644
--- a/src/test/java/com/google/devtools/build/skydoc/skydoc_test.bzl
+++ b/src/test/java/com/google/devtools/build/skydoc/skydoc_test.bzl
@@ -20,7 +20,7 @@
 #    the golden file if changes are made to skydoc.
 """Convenience macro for skydoc tests."""
 
-def skydoc_test(name, input_file, golden_file, skydoc):
+def skydoc_test(name, input_file, golden_file, skydoc, deps = []):
     """Creates a test target and golden-file regeneration target for skydoc testing.
 
     The test target is named "{name}_e2e_test".
@@ -33,6 +33,7 @@ def skydoc_test(name, input_file, golden_file, skydoc):
       golden_file: The label string of the golden file containing the documentation when skydoc
           is run on the input file.
       skydoc: The label string of the skydoc binary.
+      deps: A list of label strings of skylark file dependencies of the input_file.
     """
     output_golden_file = "%s_output.txt" % name
     native.sh_test(
@@ -47,14 +48,14 @@ def skydoc_test(name, input_file, golden_file, skydoc):
             input_file,
             golden_file,
             skydoc,
-        ],
+        ] + deps,
     )
 
     native.genrule(
         name = "regenerate_%s_golden" % name,
         srcs = [
             input_file,
-        ],
+        ] + deps,
         outs = [output_golden_file],
         cmd = "$(location %s) " % skydoc +
               "$(location %s) $(location %s)" % (input_file, output_golden_file),
diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/dep.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/dep.bzl
new file mode 100644
index 0000000000..f7c5503309
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/dep.bzl
@@ -0,0 +1,5 @@
+load(":inner_dep.bzl", "inner_rule_impl", "prep_work")
+
+prep_work()
+
+my_rule_impl = inner_rule_impl
diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/golden.txt b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/golden.txt
new file mode 100644
index 0000000000..b1a4a66325
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/golden.txt
@@ -0,0 +1,12 @@
+my_rule
+This is my rule. It does stuff.
+first,second
+
+other_rule
+This is another rule.
+third,fourth
+
+yet_another_rule
+This is yet another rule
+fifth
+
diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/inner_dep.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/inner_dep.bzl
new file mode 100644
index 0000000000..bb989dc595
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/inner_dep.bzl
@@ -0,0 +1,6 @@
+def prep_work():
+    return 1
+
+def inner_rule_impl(ctx):
+    _ignore = [ctx]
+    return struct()
diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/input.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/input.bzl
new file mode 100644
index 0000000000..b331e602c2
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skydoc/testdata/multiple_files_test/input.bzl
@@ -0,0 +1,27 @@
+load(":dep.bzl", "my_rule_impl")
+
+my_rule = rule(
+    implementation = my_rule_impl,
+    doc = "This is my rule. It does stuff.",
+    attrs = {
+        "first": attr.label(mandatory = True, allow_files = True, single_file = True),
+        "second": attr.string_dict(mandatory = True),
+    },
+)
+
+other_rule = rule(
+    implementation = my_rule_impl,
+    doc = "This is another rule.",
+    attrs = {
+        "third": attr.label(mandatory = True, allow_files = True, single_file = True),
+        "fourth": attr.string_dict(mandatory = True),
+    },
+)
+
+yet_another_rule = rule(
+    implementation = my_rule_impl,
+    doc = "This is yet another rule",
+    attrs = {
+        "fifth": attr.label(mandatory = True, allow_files = True, single_file = True),
+    },
+)