diff options
12 files changed, 152 insertions, 48 deletions
diff --git a/src/main/java/com/google/devtools/build/skydoc/BUILD b/src/main/java/com/google/devtools/build/skydoc/BUILD index b0357f2140..d5b485fe7e 100644 --- a/src/main/java/com/google/devtools/build/skydoc/BUILD +++ b/src/main/java/com/google/devtools/build/skydoc/BUILD @@ -43,6 +43,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:syntax", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple", diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java index 01fff20892..2a0bf3d475 100644 --- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java +++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java @@ -18,6 +18,8 @@ import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.skylarkbuildapi.TopLevelBootstrap; import com.google.devtools.build.lib.skylarkbuildapi.android.AndroidBootstrap; @@ -38,7 +40,6 @@ import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkImport; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skydoc.fakebuildapi.FakeActionsInfoProvider; import com.google.devtools.build.skydoc.fakebuildapi.FakeBuildApiGlobals; import com.google.devtools.build.skydoc.fakebuildapi.FakeDefaultInfoProvider; @@ -67,6 +68,7 @@ import com.google.devtools.build.skydoc.rendering.MarkdownRenderer; import com.google.devtools.build.skydoc.rendering.RuleInfo; import java.io.IOException; import java.io.PrintWriter; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -88,7 +90,7 @@ import java.util.stream.Collectors; * * <p>Usage:</p> * <pre> - * skydoc {target_skylark_file} {output_file} [symbol_name]... + * skydoc {target_skylark_file_label} {output_file} [symbol_name]... * </pre> * <p> * Generates documentation for all exported symbols of the target skylark file that are @@ -107,22 +109,25 @@ public class SkydocMain { this.fileAccessor = fileAccessor; } - public static void main(String[] args) throws IOException, InterruptedException { + public static void main(String[] args) + throws IOException, InterruptedException, LabelSyntaxException { if (args.length < 2) { throw new IllegalArgumentException("Expected two or more arguments. Usage:\n" - + "{skydoc_bin} {target_skylark_file} {output_file} [symbol_names]..."); + + "{skydoc_bin} {target_skylark_file_label} {output_file} [symbol_names]..."); } - String bzlPath = args[0]; + String targetFileLabelString = args[0]; String outputPath = args[1]; - ImmutableSet<String> symbolNames = getSymbolNames(args); - Path path = Paths.get(bzlPath); + Label targetFileLabel = + Label.parseAbsolute(targetFileLabelString, ImmutableMap.of()); + ImmutableSet<String> symbolNames = getSymbolNames(args); ImmutableMap.Builder<String, RuleInfo> ruleInfoMap = ImmutableMap.builder(); ImmutableList.Builder<RuleInfo> unknownNamedRules = ImmutableList.builder(); - new SkydocMain(new FilesystemFileAccessor()).eval(path, ruleInfoMap, unknownNamedRules); + new SkydocMain(new FilesystemFileAccessor()) + .eval(targetFileLabel, ruleInfoMap, unknownNamedRules); MarkdownRenderer renderer = new MarkdownRenderer(); @@ -175,7 +180,7 @@ public class SkydocMain { * using a fake build API and collects information about all rule definitions made in the * root skylark file. * - * @param path the path of the skylark file to evaluate + * @param label the label of the skylark file to evaluate * @param ruleInfoMap a map builder to be populated with rule definition information for * named rules. Keys are exported names of rules, and values are their {@link RuleInfo} * rule descriptions. For example, 'my_rule = rule(...)' has key 'my_rule' @@ -184,16 +189,13 @@ public class SkydocMain { * @throws InterruptedException if evaluation is interrupted */ public Environment eval( - Path path, + Label label, ImmutableMap.Builder<String, RuleInfo> ruleInfoMap, ImmutableList.Builder<RuleInfo> unknownNamedRules) - throws InterruptedException, IOException { - if (path.isAbsolute()) { - path = Paths.get(PathFragment.create(path.toString()).toRelative().getPathString()); - } + throws InterruptedException, IOException, LabelSyntaxException { List<RuleInfo> ruleInfoList = new ArrayList<>(); - Environment env = recursiveEval(path, ruleInfoList); + Environment env = recursiveEval(label, ruleInfoList); Map<BaseFunction, RuleInfo> ruleFunctions = ruleInfoList.stream() .collect(Collectors.toMap( @@ -220,14 +222,15 @@ public class SkydocMain { * dependencies using a fake build API and collects information about all rule definitions made * in those files. * - * @param path the path of the skylark file to evaluate + * @param label the label of the skylark file to evaluate * @param ruleInfoList a collection of all rule definitions made so far (using rule()); this * method will add to this list as it evaluates additional files * @throws InterruptedException if evaluation is interrupted */ private Environment recursiveEval( - Path path, List<RuleInfo> ruleInfoList) - throws InterruptedException, IOException { + Label label, List<RuleInfo> ruleInfoList) + throws InterruptedException, IOException, LabelSyntaxException { + Path path = Paths.get(label.toPathFragment().toString()); if (pending.contains(path)) { throw new IllegalStateException("cycle with " + path); } else if (loaded.containsKey(path)) { @@ -240,11 +243,17 @@ public class SkydocMain { Map<String, Extension> imports = new HashMap<>(); for (SkylarkImport anImport : buildFileAST.getImports()) { - Path importPath = fromPathFragment(path, anImport.asPathFragment()); - - Environment importEnv = recursiveEval(importPath, ruleInfoList); - - imports.put(anImport.getImportString(), new Extension(importEnv)); + Label relativeLabel = label.getRelative(anImport.getImportString()); + Path importPath = Paths.get(relativeLabel.toPathFragment().toString()); + + try { + Environment importEnv = recursiveEval(relativeLabel, ruleInfoList); + imports.put(anImport.getImportString(), new Extension(importEnv)); + } catch (NoSuchFileException noSuchFileException) { + throw new IllegalStateException( + String.format("File %s imported '%s', yet %s was not found.", + path, anImport.getImportString(), importPath)); + } } Environment env = evalSkylarkBody(buildFileAST, imports, ruleInfoList); @@ -255,12 +264,6 @@ public class SkydocMain { return env; } - private static Path fromPathFragment(Path fromPath, PathFragment pathFragment) { - return pathFragment.isAbsolute() - ? Paths.get(pathFragment.toRelative().getPathString()) - : fromPath.resolveSibling(pathFragment.getPathString()); - } - /** * Evaluates the AST from a single skylark file, given the already-resolved imports. */ diff --git a/src/test/java/com/google/devtools/build/skydoc/BUILD b/src/test/java/com/google/devtools/build/skydoc/BUILD index f36e9dbdc8..1df4b52d0e 100644 --- a/src/test/java/com/google/devtools/build/skydoc/BUILD +++ b/src/test/java/com/google/devtools/build/skydoc/BUILD @@ -8,7 +8,9 @@ package( filegroup( name = "srcs", testonly = 0, - srcs = glob(["**"]), + srcs = glob(["**"]) + [ + "//src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test:srcs", + ], visibility = ["//src:__pkg__"], ) @@ -20,6 +22,7 @@ java_test( visibility = ["//devtools/blaze/main:__pkg__"], deps = [ "//src/main/java/com/google/devtools/build/lib:syntax", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/skydoc:skydoc_lib", "//src/main/java/com/google/devtools/build/skydoc/fakebuildapi", 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 0047e820b4..f9f4d6203e 100644 --- a/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java +++ b/src/test/java/com/google/devtools/build/skydoc/SkydocTest.java @@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skylark.util.SkylarkTestCase; import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -27,7 +28,6 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skydoc.fakebuildapi.FakeDescriptor.Type; 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; @@ -79,7 +79,7 @@ public final class SkydocTest extends SkylarkTestCase { ImmutableList.Builder<RuleInfo> unexportedRuleInfos = ImmutableList.builder(); skydocMain.eval( - Paths.get("/test/test.bzl"), + Label.parseAbsoluteUnchecked("//test:test.bzl"), ruleInfoMap, unexportedRuleInfos); Map<String, RuleInfo> ruleInfos = ruleInfoMap.build(); @@ -140,7 +140,7 @@ public final class SkydocTest extends SkylarkTestCase { ImmutableList.Builder<RuleInfo> unexportedRuleInfos = ImmutableList.builder(); skydocMain.eval( - Paths.get("/test/test.bzl"), + Label.parseAbsoluteUnchecked("//test:test.bzl"), ruleInfoMap, unexportedRuleInfos); @@ -191,7 +191,7 @@ public final class SkydocTest extends SkylarkTestCase { ImmutableMap.Builder<String, RuleInfo> ruleInfoMapBuilder = ImmutableMap.builder(); skydocMain.eval( - Paths.get("/test/main.bzl"), + Label.parseAbsoluteUnchecked("//test:main.bzl"), ruleInfoMapBuilder, ImmutableList.builder()); @@ -228,7 +228,7 @@ public final class SkydocTest extends SkylarkTestCase { IllegalStateException expected = assertThrows(IllegalStateException.class, () -> skydocMain.eval( - Paths.get("/test/main.bzl"), + Label.parseAbsoluteUnchecked("//test:main.bzl"), ruleInfoMapBuilder, ImmutableList.builder())); 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 455b45d859..b2d657edb2 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 @@ -39,12 +39,17 @@ def skydoc_test(name, input_file, golden_file, skydoc, deps = [], whitelisted_sy will be generated. """ output_golden_file = "%s_output.txt" % name + + # Skydoc requires an absolute input file label to both load the target file and + # track what its target is for the purpose of resolving relative labels. + abs_input_file_label = str(Label("//%s" % native.package_name()).relative(input_file)) + native.sh_test( name = "%s_e2e_test" % name, srcs = ["skydoc_e2e_test_runner.sh"], args = [ "$(location %s)" % skydoc, - "$(location %s)" % input_file, + abs_input_file_label, "$(location %s)" % golden_file, ] + whitelisted_symbols, data = [ @@ -60,8 +65,9 @@ def skydoc_test(name, input_file, golden_file, skydoc, deps = [], whitelisted_sy input_file, ] + deps, outs = [output_golden_file], + heuristic_label_expansion = 0, cmd = "$(location %s) " % skydoc + - "$(location %s) $(location %s) " % (input_file, output_golden_file) + + "%s $(location %s) " % (abs_input_file_label, output_golden_file) + " ".join(whitelisted_symbols), tools = [skydoc], ) diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/filter_rules_test/input.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/filter_rules_test/input.bzl index 3ec3530279..1b7ff1aae6 100644 --- a/src/test/java/com/google/devtools/build/skydoc/testdata/filter_rules_test/input.bzl +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/filter_rules_test/input.bzl @@ -1,6 +1,8 @@ -load(":dep.bzl", - "my_rule_impl", - dep_rule = "my_rule") +load( + ":testdata/filter_rules_test/dep.bzl", + "my_rule_impl", + dep_rule = "my_rule", +) def my_rule_impl(ctx): return struct() @@ -9,8 +11,12 @@ my_rule = rule( implementation = my_rule_impl, doc = "This is my rule. It does stuff.", attrs = { - "first": attr.label(mandatory = True, doc = "first my_rule doc string", - allow_files = True, single_file = True), + "first": attr.label( + mandatory = True, + doc = "first my_rule doc string", + allow_files = True, + single_file = True, + ), "second": attr.string_dict(mandatory = True), }, ) 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 index f7c5503309..129c1dd70c 100644 --- 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 @@ -1,4 +1,4 @@ -load(":inner_dep.bzl", "inner_rule_impl", "prep_work") +load(":testdata/multiple_files_test/inner_dep.bzl", "inner_rule_impl", "prep_work") prep_work() 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 index efd5ed460c..2c73da21d5 100644 --- 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 @@ -1,11 +1,15 @@ -load(":dep.bzl", "my_rule_impl") +load(":testdata/multiple_files_test/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, doc = "first my_rule doc string", - allow_files = True, single_file = True), + "first": attr.label( + mandatory = True, + doc = "first my_rule doc string", + allow_files = True, + single_file = True, + ), "second": attr.string_dict(mandatory = True), }, ) @@ -14,8 +18,12 @@ other_rule = rule( implementation = my_rule_impl, doc = "This is another rule.", attrs = { - "third": attr.label(mandatory = True, doc = "third other_rule doc string", - allow_files = True, single_file = True), + "third": attr.label( + mandatory = True, + doc = "third other_rule doc string", + allow_files = True, + single_file = True, + ), "fourth": attr.string_dict(mandatory = True), }, ) diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/BUILD b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/BUILD new file mode 100644 index 0000000000..97c2e468d2 --- /dev/null +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/BUILD @@ -0,0 +1,14 @@ +filegroup( + name = "srcs", + testonly = 0, + srcs = glob(["**"]), + visibility = ["//src:__subpackages__"], +) + +exports_files( + [ + "dep.bzl", + "golden.txt", + "input.bzl", + ], +) diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/dep.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/dep.bzl new file mode 100644 index 0000000000..db2cb089ac --- /dev/null +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/dep.bzl @@ -0,0 +1,3 @@ +def my_rule_impl(ctx): + _ignore = [ctx] + return struct() diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/golden.txt b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/golden.txt new file mode 100644 index 0000000000..72b0116063 --- /dev/null +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/golden.txt @@ -0,0 +1,45 @@ +<a name="#my_rule"></a> +## my_rule + +<pre> +my_rule(name, first, second) +</pre> + +This is my rule. It does stuff. + +### Attributes + +<table class="params-table"> + <colgroup> + <col class="col-param" /> + <col class="col-description" /> + </colgroup> + <tbody> + <tr id="#my_rule_name"> + <td><code>name</code></td> + <td> + String; required + <p> + A unique name for this rule. + </p> + </td> + </tr> + <tr id="#my_rule_first"> + <td><code>first</code></td> + <td> + Label; required + <p> + first my_rule doc string + </p> + </td> + </tr> + <tr id="#my_rule_second"> + <td><code>second</code></td> + <td> + Dictionary: String -> String; required + </td> + </tr> + </tbody> +</table> + + diff --git a/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/input.bzl b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/input.bzl new file mode 100644 index 0000000000..f0e989f3e9 --- /dev/null +++ b/src/test/java/com/google/devtools/build/skydoc/testdata/same_level_file_test/input.bzl @@ -0,0 +1,15 @@ +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, + doc = "first my_rule doc string", + allow_files = True, + single_file = True, + ), + "second": attr.string_dict(mandatory = True), + }, +) |