diff --git a/src/main/java/com/google/devtools/build/docgen/DocLinkMap.java b/src/main/java/com/google/devtools/build/docgen/DocLinkMap.java index c367d6ca6a5bf1..d993b55cc308bf 100644 --- a/src/main/java/com/google/devtools/build/docgen/DocLinkMap.java +++ b/src/main/java/com/google/devtools/build/docgen/DocLinkMap.java @@ -32,18 +32,18 @@ public class DocLinkMap { // For SourceUrlMapper final String sourceUrlRoot; - final Map labelRewrites; // Gson#fromJson ensures the map is of an ordered type + final Map repoPathRewrites; // Gson#fromJson ensures the map is of an ordered type @VisibleForTesting DocLinkMap( String beRoot, Map beReferences, String sourceUrlRoot, - Map labelRewrites) { + Map repoPathRewrites) { this.beRoot = beRoot; this.beReferences = beReferences; this.sourceUrlRoot = sourceUrlRoot; - this.labelRewrites = labelRewrites; + this.repoPathRewrites = repoPathRewrites; } public static DocLinkMap createFromFile(String filePath) { diff --git a/src/main/java/com/google/devtools/build/docgen/SourceUrlMapper.java b/src/main/java/com/google/devtools/build/docgen/SourceUrlMapper.java index 831389d0f044b9..db5f580b184772 100644 --- a/src/main/java/com/google/devtools/build/docgen/SourceUrlMapper.java +++ b/src/main/java/com/google/devtools/build/docgen/SourceUrlMapper.java @@ -26,29 +26,27 @@ * for the corresponding source file in the source code repository. */ class SourceUrlMapper { - // TODO(arostovtsev): eventually, we will want to support multiple URL roots (e.g. for - // language-specific rules). private final String sourceUrlRoot; private final Path inputRoot; - private final ImmutableMap labelRewrites; + private final ImmutableMap repoPathsRewrites; /** * @param sourceUrlRoot root URL for the source code repository * @param inputRoot input directory corresponding to the source tree root - * @param labelRewrites an ordered map of label string prefixes; a map entry of the form "foo" -> - * "bar" indicates that if a label string starts with "foo", that prefix should be replaced - * with "bar". The intended use case is transforming labels of .bzl files in @_builtins, whose - * corresponding source files live elsewhere in the repo. + * @param repoPathsRewrites an ordered map of repo path prefixes; A repo path is a string formed + * by concatenation of @repo// and a path. This handles labels from @builtins as well as from + * external * repositories. A map entry of the form "foo" -> "bar" indicates that if a repo + * path starts with "foo", that prefix should be replaced with "bar" to form a full url. */ SourceUrlMapper( - String sourceUrlRoot, String inputRoot, ImmutableMap labelRewrites) { + String sourceUrlRoot, String inputRoot, ImmutableMap repoPathsRewrites) { this.sourceUrlRoot = sourceUrlRoot; this.inputRoot = new File(inputRoot).toPath(); - this.labelRewrites = labelRewrites; + this.repoPathsRewrites = repoPathsRewrites; } SourceUrlMapper(DocLinkMap linkMap, String inputRoot) { - this(linkMap.sourceUrlRoot, inputRoot, ImmutableMap.copyOf(linkMap.labelRewrites)); + this(linkMap.sourceUrlRoot, inputRoot, ImmutableMap.copyOf(linkMap.repoPathRewrites)); } /** @@ -68,10 +66,10 @@ String urlOfFile(File file) { * Returns the source code repository URL of a .bzl file label which was passed as an input to the * Build Encyclopedia generator tool. * - *

A label is first rewritten via {@link labelRewrites}: an entry of the form "foo" -> "bar" - * means that if {@code labelString} starts with "foo", the "foo" prefix is replaced with "bar". - * Rewrite rules in {@link labelRewrites} are examined in order, and only the first matching - * rewrite is applied. + *

A label is first rewritten via {@link repoPathsRewrites}: an entry of the form "foo" -> + * "bar" means that if {@code labelString} starts with "foo", the "foo" prefix is replaced with + * "bar". Rewrite rules in {@link repoPathsRewrites} are examined in order, and only the first + * matching rewrite is applied. * *

If the result is a label in the main repo, the (possibly rewritten) label is transformed * into a URL. @@ -79,26 +77,29 @@ String urlOfFile(File file) { * @throws InvalidArgumentException if the URL could not be produced. */ String urlOfLabel(String labelString) { - String originalLabelString = labelString; - for (Map.Entry entry : labelRewrites.entrySet()) { - if (labelString.startsWith(entry.getKey())) { - labelString = entry.getValue() + labelString.substring(entry.getKey().length()); - break; - } - } Label label; try { label = Label.parseCanonical(labelString); } catch (LabelSyntaxException e) { String message = String.format("Failed to parse label '%s'", labelString); - if (!labelString.equals(originalLabelString)) { - message = String.format("%s (rewritten; originally '%s')", message, originalLabelString); - } throw new IllegalArgumentException(message, e); } - Preconditions.checkArgument( - label.getRepository().isMain(), "Label '%s' is not in the main repository", labelString); + return urlOfLabel(label); + } + + private String urlOfLabel(Label label) { + String path = + "@" + + label.getPackageIdentifier().getRepository().getName() + + "//" + + label.toPathFragment().getPathString(); + for (Map.Entry entry : repoPathsRewrites.entrySet()) { + if (path.startsWith(entry.getKey())) { + path = entry.getValue() + path.substring(entry.getKey().length()); + break; + } + } - return sourceUrlRoot + label.toPathFragment().getPathString(); + return path; } } diff --git a/src/main/java/com/google/devtools/build/docgen/bazel_link_map.json b/src/main/java/com/google/devtools/build/docgen/bazel_link_map.json index 3ca4cbc341e2f0..bc3e85944b7097 100644 --- a/src/main/java/com/google/devtools/build/docgen/bazel_link_map.json +++ b/src/main/java/com/google/devtools/build/docgen/bazel_link_map.json @@ -23,7 +23,9 @@ "visibility": "/concepts/visibility" }, "sourceUrlRoot": "https://github.com/bazelbuild/bazel/blob/master/", - "labelRewrites": { - "@@_builtins//:": "//src/main/starlark/builtins_bzl:" + "repoPathRewrites": { + "@//": "https://github.com/bazelbuild/bazel/blob/master/", + "@_builtins//": "https://github.com/bazelbuild/bazel/blob/master/src/main/starlark/builtins_bzl/", + "@rules_python//": "https://github.com/bazelbuild/rules_python/blob/main/" } } diff --git a/src/test/java/com/google/devtools/build/docgen/BuildDocCollectorTest.java b/src/test/java/com/google/devtools/build/docgen/BuildDocCollectorTest.java index 40d96178f4338e..df3566f0f7d071 100644 --- a/src/test/java/com/google/devtools/build/docgen/BuildDocCollectorTest.java +++ b/src/test/java/com/google/devtools/build/docgen/BuildDocCollectorTest.java @@ -48,7 +48,7 @@ public final class BuildDocCollectorTest { new SourceUrlMapper( /* sourceUrlRoot= */ "https://example.com/", /* inputRoot= */ "/tmp/io_bazel/", - ImmutableMap.of("@_builtins//:", "//src/main/starlark/builtins_bzl:")); + ImmutableMap.of("@_builtins//", "https://example.com/src/main/starlark/builtins_bzl/")); @Before public void setUpCollectorState() { diff --git a/src/test/java/com/google/devtools/build/docgen/DocLinkMapTest.java b/src/test/java/com/google/devtools/build/docgen/DocLinkMapTest.java index 70197dc6fe23e1..06c2eaf02b2a95 100644 --- a/src/test/java/com/google/devtools/build/docgen/DocLinkMapTest.java +++ b/src/test/java/com/google/devtools/build/docgen/DocLinkMapTest.java @@ -56,10 +56,10 @@ public void testCreateFromFile() throws IOException { + " \"build-ref3\": \"build-ref3.html\"" + "}," + "\"sourceUrlRoot\": \"https://example.com/\"," - + "\"labelRewrites\": {" - + " \"@_builtins//:common/\": \"//common/builtins_bzl:\"," - + " \"@_builtins//:uncommon/\": \"//uncommon/builtins_bzl:\"," - + " \"@_builtins//:\": \"//main/builtins_bzl:\"" + + "\"repoPathRewrites\": {" + + " \"@_builtins//common/\": \"https://example.com/common/builtins_bzl\"," + + " \"@_builtins//uncommon/\": \"https://example.com/uncommon/builtins_bzl\"," + + " \"@_builtins//\": \"https://example.com/main/builtins_bzl\"" + "}" + "}"; @@ -78,14 +78,14 @@ public void testCreateFromFile() throws IOException { "build-ref3.html") .inOrder(); assertThat(map.sourceUrlRoot).isEqualTo("https://example.com/"); - assertThat(map.labelRewrites) + assertThat(map.repoPathRewrites) .containsExactly( - "@_builtins//:common/", - "//common/builtins_bzl:", - "@_builtins//:uncommon/", - "//uncommon/builtins_bzl:", - "@_builtins//:", - "//main/builtins_bzl:") + "@_builtins//common/", + "https://example.com/common/builtins_bzl", + "@_builtins//uncommon/", + "https://example.com/uncommon/builtins_bzl", + "@_builtins//", + "https://example.com/main/builtins_bzl") .inOrder(); } } diff --git a/src/test/java/com/google/devtools/build/docgen/SourceUrlMapperTest.java b/src/test/java/com/google/devtools/build/docgen/SourceUrlMapperTest.java index efb58758a649bf..9199eda5329f52 100644 --- a/src/test/java/com/google/devtools/build/docgen/SourceUrlMapperTest.java +++ b/src/test/java/com/google/devtools/build/docgen/SourceUrlMapperTest.java @@ -32,10 +32,10 @@ public final class SourceUrlMapperTest { "https://example.com/", "/tmp/io_bazel", ImmutableMap.of( - "@_builtins//:common/", - "//src/common/starlark/builtins_bzl:", - "@_builtins//:", - "//src/main/starlark/builtins_bzl:")); + "@//", + "https://example.com/", + "@_builtins//", + "https://example.com/src/main/starlark/builtins_bzl/")); @Test public void urlOfFile() { @@ -56,21 +56,9 @@ public void urlOfFile_throwsIfFileNotUnderSourceRoot() { @Test public void urlOfLabel() { - assertThat(mapper.urlOfLabel("@_builtins//:common/foo/bar.bzl")) - .isEqualTo("https://example.com/src/common/starlark/builtins_bzl/foo/bar.bzl"); assertThat(mapper.urlOfLabel("@_builtins//:foo/bar.bzl")) .isEqualTo("https://example.com/src/main/starlark/builtins_bzl/foo/bar.bzl"); assertThat(mapper.urlOfLabel("//not/in/builtins/foo:bar.bzl")) .isEqualTo("https://example.com/not/in/builtins/foo/bar.bzl"); } - - @Test - public void urlOfLabel_throwsIfLabelNotInMainRepo() { - IllegalArgumentException e = - assertThrows( - IllegalArgumentException.class, () -> mapper.urlOfLabel("@other_repo//foo:bar.bzl")); - assertThat(e) - .hasMessageThat() - .contains("Label '@other_repo//foo:bar.bzl' is not in the main repository"); - } }