Skip to content

Commit

Permalink
Add default_repo_name attribute to starlark_doc_extract
Browse files Browse the repository at this point in the history 
The specified repository name is used to make repo-relative labels
repo-absolute. This applies to default values that are labels as well
as the locations of `.bzl` files.
  • Loading branch information
@fmeum
fmeum committed Jul 20, 2023
1 parent 710a964 commit e1cec58
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 11 deletions.
36 changes: 31 additions & 5 deletions ...main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
final class ModuleInfoExtractor {
private final Predicate<String> isWantedQualifiedName;
private final RepositoryMapping repositoryMapping;
private final String defaultRepoName;

@VisibleForTesting
static final AttributeInfo IMPLICIT_NAME_ATTRIBUTE_INFO =
Expand Down Expand Up @@ -109,19 +110,32 @@ final class ModuleInfoExtractor {
* predicate.
* @param repositoryMapping the repository mapping for the repo in which we want to render labels
* as strings
* @param defaultRepoName the repository name to use for labels pointing inside the current
* repository. If this is the empty string, such labels will be emitted in repository-relative
* form.
*/
public ModuleInfoExtractor(
Predicate<String> isWantedQualifiedName, RepositoryMapping repositoryMapping) {
Predicate<String> isWantedQualifiedName,
RepositoryMapping repositoryMapping,
String defaultRepoName) {
this.isWantedQualifiedName = isWantedQualifiedName;
this.repositoryMapping = repositoryMapping;
this.defaultRepoName = defaultRepoName;
}

/** Extracts structured documentation for the loadable symbols of a given module. */
public ModuleInfo extractFrom(Module module) throws ExtractionException {
ModuleInfo.Builder builder = ModuleInfo.newBuilder();
Optional.ofNullable(module.getDocumentation()).ifPresent(builder::setModuleDocstring);
Optional.ofNullable(BazelModuleContext.of(module))
.map(bazelModuleContext -> bazelModuleContext.label().getDisplayForm(repositoryMapping))
.map(bazelModuleContext -> {
String labelString = bazelModuleContext.label().getDisplayForm(repositoryMapping);
if (labelString.startsWith("//") && !defaultRepoName.isEmpty()) {
return "@" + defaultRepoName + labelString;
} else {
return labelString;
}
})
.ifPresent(builder::setFile);

// We do two traversals over the module's globals: (1) find qualified names (including any
Expand All @@ -136,7 +150,8 @@ public ModuleInfo extractFrom(Module module) throws ExtractionException {
builder,
isWantedQualifiedName,
repositoryMapping,
providerQualifiedNameCollector.buildQualifiedNames());
providerQualifiedNameCollector.buildQualifiedNames(),
defaultRepoName);
documentationExtractor.traverse(module);
return builder.build();
}
Expand Down Expand Up @@ -306,6 +321,7 @@ private static final class DocumentationExtractor extends GlobalsVisitor {
private final Predicate<String> isWantedQualifiedName;
private final RepositoryMapping repositoryMapping;
private final ImmutableMap<StarlarkProvider.Key, String> providerQualifiedNames;
private final String defaultRepoName;

/**
* @param moduleInfoBuilder builder to which {@link #traverse} adds extracted documentation
Expand All @@ -318,16 +334,21 @@ private static final class DocumentationExtractor extends GlobalsVisitor {
* @param providerQualifiedNames a map from the keys of documentable Starlark providers loadable
* from this module to the qualified names (including structure namespaces) under which
* those providers are accessible to a user of this module
* @param defaultRepoName the repository name to use for labels pointing inside the current
* repository. If this is the empty string, such labels will be emitted in
* repository-relative form.
*/
DocumentationExtractor(
ModuleInfo.Builder moduleInfoBuilder,
Predicate<String> isWantedQualifiedName,
RepositoryMapping repositoryMapping,
ImmutableMap<StarlarkProvider.Key, String> providerQualifiedNames) {
ImmutableMap<StarlarkProvider.Key, String> providerQualifiedNames,
String defaultRepoName) {
this.moduleInfoBuilder = moduleInfoBuilder;
this.isWantedQualifiedName = isWantedQualifiedName;
this.repositoryMapping = repositoryMapping;
this.providerQualifiedNames = providerQualifiedNames;
this.defaultRepoName = defaultRepoName;
}

@Override
Expand Down Expand Up @@ -498,7 +519,12 @@ protected void visitRepositoryRule(
*/
private Object stringifyLabels(Object o) {
if (o instanceof Label) {
return ((Label) o).getShorthandDisplayForm(repositoryMapping);
String labelString = ((Label) o).getShorthandDisplayForm(repositoryMapping);
if (labelString.startsWith("//") && !defaultRepoName.isEmpty()) {
return "@" + defaultRepoName + labelString;
} else {
return labelString;
}
} else if (o instanceof Map) {
return stringifyLabelsOfMap((Map<?, ?>) o);
} else if (o instanceof List) {
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.base.Verify.verifyNotNull;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fromTemplates;
import static com.google.devtools.build.lib.packages.Type.STRING;
import static com.google.devtools.build.lib.packages.Type.STRING_LIST;
import static java.util.stream.Collectors.partitioningBy;

Expand Down Expand Up @@ -67,6 +68,7 @@ public class StarlarkDocExtract implements RuleConfiguredTargetFactory {
static final String SRC_ATTR = "src";
static final String DEPS_ATTR = "deps";
static final String SYMBOL_NAMES_ATTR = "symbol_names";
static final String DEFAULT_REPO_NAME_ATTR = "default_repo_name";
static final SafeImplicitOutputsFunction BINARYPROTO_OUT = fromTemplates("%{name}.binaryproto");
static final SafeImplicitOutputsFunction TEXTPROTO_OUT = fromTemplates("%{name}.textproto");

Expand Down Expand Up @@ -295,7 +297,10 @@ private static ModuleInfo getModuleInfo(
ModuleInfo moduleInfo;
try {
moduleInfo =
new ModuleInfoExtractor(getWantedSymbolPredicate(ruleContext), repositoryMapping)
new ModuleInfoExtractor(
getWantedSymbolPredicate(ruleContext),
repositoryMapping,
(String) ruleContext.getRule().getAttr(DEFAULT_REPO_NAME_ATTR, STRING))
.extractFrom(module);
} catch (ModuleInfoExtractor.ExtractionException e) {
ruleContext.ruleError(e.getMessage());
Expand Down
7 changes: 7 additions & 0 deletions ...n/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fromFunctions;
import static com.google.devtools.build.lib.packages.Type.STRING;
import static com.google.devtools.build.lib.packages.Type.STRING_LIST;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -93,6 +94,12 @@ An optional list of qualified names of exported functions, rules, providers, or
.add(
attr(StarlarkDocExtract.SYMBOL_NAMES_ATTR, STRING_LIST)
.value(ImmutableList.<String>of()))
/*<!-- #BLAZE_RULE(starlark_doc_extract).ATTRIBUTE(symbol_names) -->
The repository name to emit for labels that reference targets in the same repository as this
rule. If set to the empty string (the default), such labels are rendered in
repository-relative form.
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
.add(attr(StarlarkDocExtract.DEFAULT_REPO_NAME_ATTR, STRING).value(""))
/*<!-- #BLAZE_RULE(starlark_doc_extract).IMPLICIT_OUTPUTS -->
<ul>
<li><code><var>name</var>.binaryproto</code> (the default output): A
Expand Down
43 changes: 38 additions & 5 deletions .../java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,16 @@ private Module execWithOptions(ImmutableList<String> options, String... lines) t
}

private static ModuleInfoExtractor getExtractor() {
return new ModuleInfoExtractor(name -> true, RepositoryMapping.ALWAYS_FALLBACK);
return new ModuleInfoExtractor(name -> true, RepositoryMapping.ALWAYS_FALLBACK, "");
}

private static ModuleInfoExtractor getExtractor(Predicate<String> isWantedQualifiedName) {
return new ModuleInfoExtractor(isWantedQualifiedName, RepositoryMapping.ALWAYS_FALLBACK);
return new ModuleInfoExtractor(isWantedQualifiedName, RepositoryMapping.ALWAYS_FALLBACK, "");
}

private static ModuleInfoExtractor getExtractor(RepositoryMapping repositoryMapping) {
return new ModuleInfoExtractor(name -> true, repositoryMapping);
private static ModuleInfoExtractor getExtractor(
RepositoryMapping repositoryMapping, String defaultRepoName) {
return new ModuleInfoExtractor(name -> true, repositoryMapping, defaultRepoName);
}

@Test
Expand Down Expand Up @@ -681,7 +682,7 @@ public void labelStringification() throws Exception {
RepositoryName canonicalName = RepositoryName.create("canonical");
RepositoryMapping repositoryMapping =
RepositoryMapping.create(ImmutableMap.of("local", canonicalName), RepositoryName.MAIN);
ModuleInfo moduleInfo = getExtractor(repositoryMapping).extractFrom(module);
ModuleInfo moduleInfo = getExtractor(repositoryMapping, "").extractFrom(module);
assertThat(
moduleInfo.getRuleInfoList().get(0).getAttributeList().stream()
.filter(attr -> !attr.equals(ModuleInfoExtractor.IMPLICIT_NAME_ATTRIBUTE_INFO))
Expand All @@ -692,6 +693,38 @@ public void labelStringification() throws Exception {
"{\"//x\": \"label_in_main\", \"@local//y\": \"label_in_dep\"}");
}

@Test
public void labelStringification_defaultRepoName() throws Exception {
Module module =
exec(
"def _my_impl(ctx):",
" pass",
"my_lib = rule(",
" implementation = _my_impl,",
" attrs = {",
" 'label': attr.label(default = '//test:foo'),",
" 'label_list': attr.label_list(",
" default = ['//x', '@canonical//y', '@canonical//y:z'],",
" ),",
" 'label_keyed_string_dict': attr.label_keyed_string_dict(",
" default = {'//x': 'label_in_main', '@canonical//y': 'label_in_dep'}",
" ),",
" }",
")");
RepositoryName canonicalName = RepositoryName.create("canonical");
RepositoryMapping repositoryMapping =
RepositoryMapping.create(ImmutableMap.of("local", canonicalName), RepositoryName.MAIN);
ModuleInfo moduleInfo = getExtractor(repositoryMapping, "my_repo").extractFrom(module);
assertThat(
moduleInfo.getRuleInfoList().get(0).getAttributeList().stream()
.filter(attr -> !attr.equals(ModuleInfoExtractor.IMPLICIT_NAME_ATTRIBUTE_INFO))
.map(AttributeInfo::getDefaultValue))
.containsExactly(
"\"@my_repo//test:foo\"",
"[\"@my_repo//x\", \"@local//y\", \"@local//y:z\"]",
"{\"@my_repo//x\": \"label_in_main\", \"@local//y\": \"label_in_dep\"}");
}

@Test
public void aspectDocstring() throws Exception {
Module module =
Expand Down
84 changes: 84 additions & 0 deletions ...t/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@ public void moduleExtension() throws Exception {
" deps = ['dep_bzl'],",
")");
ModuleInfo moduleInfo = protoFromConfiguredTarget("//:extract");
assertThat(moduleInfo.getFile()).isEqualTo("//:foo.bzl");
assertThat(moduleInfo.getModuleExtensionInfoList())
.containsExactly(
ModuleExtensionInfo.newBuilder()
Expand Down Expand Up @@ -948,4 +949,87 @@ public void moduleExtension() throws Exception {
.build())
.build());
}

@Test
public void moduleExtension_defaultRepoName() throws Exception {
scratch.file(
"dep.bzl",
"_install = tag_class(",
" doc = 'Install',",
" attrs = {",
" 'artifacts': attr.label_list(doc = 'Artifacts', default = ['//:target']),",
" '_hidden': attr.bool(),",
" },",
")",
"",
"_artifact = tag_class(",
" attrs = {",
" 'group': attr.string(),",
" 'artifact': attr.string(default = 'foo'),",
" },",
")",
"",
"def _impl(ctx):",
" pass",
"",
"my_ext = module_extension(",
" doc = 'My extension',",
" tag_classes = {",
" 'install': _install,",
" 'artifact': _artifact,",
" },",
" implementation = _impl,",
")");
scratch.file(
"foo.bzl", //
"load('//:dep.bzl', 'my_ext')",
"foo = struct(ext = my_ext)");
scratch.file(
"BUILD", //
"load('bzl_library.bzl', 'bzl_library')",
"bzl_library(",
" name = 'dep_bzl',",
" srcs = ['dep.bzl'],",
")",
"starlark_doc_extract(",
" name = 'extract',",
" src = 'foo.bzl',",
" default_repo_name = 'my_module',",
" deps = ['dep_bzl'],",
")");
ModuleInfo moduleInfo = protoFromConfiguredTarget("//:extract");
assertThat(moduleInfo.getFile()).isEqualTo("@my_module//:foo.bzl");
assertThat(moduleInfo.getModuleExtensionInfoList())
.containsExactly(
ModuleExtensionInfo.newBuilder()
.setExtensionName("foo.ext")
.setDocString("My extension")
.setOriginKey(OriginKey.newBuilder().setFile("//:dep.bzl").build())
.addTagClass(
ModuleExtensionTagClassInfo.newBuilder()
.setTagName("install")
.setDocString("Install")
.addAttribute(
AttributeInfo.newBuilder()
.setName("artifacts")
.setType(AttributeType.LABEL_LIST)
.setDocString("Artifacts")
.setDefaultValue("[\"@my_module//:target\"]"))
.build())
.addTagClass(
ModuleExtensionTagClassInfo.newBuilder()
.setTagName("artifact")
.addAttribute(
AttributeInfo.newBuilder()
.setName("group")
.setType(AttributeType.STRING)
.setDefaultValue("\"\""))
.addAttribute(
AttributeInfo.newBuilder()
.setName("artifact")
.setType(AttributeType.STRING)
.setDefaultValue("\"foo\""))
.build())
.build());
}
}

0 comments on commit e1cec58

Please sign in to comment.