Document symbolic macro attributes in starlark_doc_extract ... and add a test to verify that we properly ignore unexported macro functions. PiperOrigin-RevId: 627454342 Change-Id: Iad6cfec3cb653d26dda65d548869a500c81b6f3e
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index db6c14f..1a26818 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -1128,6 +1128,15 @@ this.starlarkLabel = starlarkLabel; } + /** + * Returns an exported macro's MacroClass (representing its schema and implementation function), + * or null if the macro has not been exported yet. + */ + @Nullable + public MacroClass getMacroClass() { + return macroClass; + } + @Override public boolean isExported() { return macroClass != null;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java index 7bea06f..1a39607 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java +++ b/src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.BuiltinProvider; +import com.google.devtools.build.lib.packages.MacroClass; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.StarlarkDefinedAspect; @@ -76,6 +77,18 @@ .build(); @VisibleForTesting + static final AttributeInfo IMPLICIT_MACRO_NAME_ATTRIBUTE_INFO = + AttributeInfo.newBuilder() + .setName("name") + .setType(AttributeType.NAME) + .setMandatory(true) + .setDocString( + "A unique name for this macro instance. Normally, this is also the name for the" + + " macro's main or only target. The names of any other targets that this macro" + + " might create will be this name with a string suffix.") + .build(); + + @VisibleForTesting static final ImmutableList<AttributeInfo> IMPLICIT_REPOSITORY_RULE_ATTRIBUTES = ImmutableList.of( AttributeInfo.newBuilder() @@ -227,7 +240,8 @@ protected void visitMacroFunction( @SuppressWarnings("unused") String qualifiedName, - @SuppressWarnings("unused") MacroFunction value) {} + @SuppressWarnings("unused") MacroFunction value) + throws ExtractionException {} protected void visitProvider( @SuppressWarnings("unused") String qualifiedName, @@ -397,7 +411,12 @@ } @Override - protected void visitMacroFunction(String qualifiedName, MacroFunction macroFunction) { + protected void visitMacroFunction(String qualifiedName, MacroFunction macroFunction) + throws ExtractionException { + if (!macroFunction.isExported()) { + // No point in documenting unexported macroFunctions - they cannot be used as macros. + return; + } MacroInfo.Builder macroInfoBuilder = MacroInfo.newBuilder(); // Record the name under which this symbol is made accessible, which may differ from the // symbol's exported name @@ -408,7 +427,15 @@ .setName(macroFunction.getName()) .setFile(labelRenderer.render(macroFunction.getExtensionLabel()))); macroFunction.getDocumentation().ifPresent(macroInfoBuilder::setDocString); - // TODO(#19922): add and document macro attributes + + MacroClass macroClass = macroFunction.getMacroClass(); + // inject the name attribute; addDocumentableAttributes skips non-Starlark-defined attributes. + macroInfoBuilder.addAttribute(IMPLICIT_MACRO_NAME_ATTRIBUTE_INFO); + addDocumentableAttributes( + macroClass.getAttributes().values(), + macroInfoBuilder::addAttribute, + "macro " + qualifiedName); + moduleInfoBuilder.addMacroInfo(macroInfoBuilder); }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java index 9e7e4a2..52cf956 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java
@@ -759,15 +759,74 @@ .setDocString("My doc") .setOriginKey( OriginKey.newBuilder().setName("documented_macro").setFile(fakeLabelString)) + .addAttribute(ModuleInfoExtractor.IMPLICIT_MACRO_NAME_ATTRIBUTE_INFO) .build(), MacroInfo.newBuilder() .setMacroName("undocumented_macro") .setOriginKey( OriginKey.newBuilder().setName("undocumented_macro").setFile(fakeLabelString)) + .addAttribute(ModuleInfoExtractor.IMPLICIT_MACRO_NAME_ATTRIBUTE_INFO) .build()); } @Test + public void macroAttributes() throws Exception { + Module module = + execWithOptions( + ImmutableList.of("--experimental_enable_first_class_macros"), + """ + def _my_impl(name): + pass + + my_macro = macro( + attrs = { + "some_attr": attr.label(mandatory = True), + "another_attr": attr.int(doc = "An integer", default = 42), + "_implicit_attr": attr.string(default = "IMPLICIT"), + }, + implementation = _my_impl, + ) + """); + ModuleInfo moduleInfo = getExtractor().extractFrom(module); + assertThat(moduleInfo.getMacroInfoList().get(0).getAttributeList()) + .containsExactly( + ModuleInfoExtractor.IMPLICIT_MACRO_NAME_ATTRIBUTE_INFO, // name comes first + AttributeInfo.newBuilder() + .setName("some_attr") + .setType(AttributeType.LABEL) + .setMandatory(true) + .build(), + AttributeInfo.newBuilder() + .setName("another_attr") + .setType(AttributeType.INT) + .setDocString("An integer") + .setDefaultValue("42") + .build() + // note that implicit attributes don't get documented + ); + } + + @Test + public void unexportedMacro_notDocumented() throws Exception { + Module module = + execWithOptions( + ImmutableList.of("--experimental_enable_first_class_macros"), + """ + def _my_impl(name): + pass + + s = struct( + my_macro = macro( + doc = "Unexported macro", + implementation = _my_impl, + ) + ) + """); + ModuleInfo moduleInfo = getExtractor().extractFrom(module); + assertThat(moduleInfo.getMacroInfoList()).isEmpty(); + } + + @Test public void providerNameGroups_useFirstDocumentableProviderName() throws Exception { Module module = exec(
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index 2ef8bab..187cd6d 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
@@ -611,11 +611,19 @@ exported = macro( implementation=_impl, doc = "Exported macro", + attrs = { + "abc": attr.int(), + "xyz": attr.string(), + } ) s = struct( unexported = macro( implementation=_impl, doc = "Unexported macro", + attrs = { + "abc": attr.int(), + "xyz": attr.string(), + } ), ) """); @@ -637,6 +645,17 @@ assertThat(exported.getExtensionLabel()).isEqualTo(FAKE_LABEL); assertThat(unexported.getExtensionLabel()).isNull(); + + assertThat(exported.getMacroClass().getName()).isEqualTo("exported"); + assertThat(exported.getMacroClass().getAttributes()) + .containsExactly( + "name", + RuleClass.NAME_ATTRIBUTE, + "abc", + Attribute.attr("abc", Type.INTEGER).starlarkDefined().build(), + "xyz", + Attribute.attr("xyz", Type.STRING).starlarkDefined().build()); + assertThat(unexported.getMacroClass()).isNull(); } private RuleClass getRuleClass(String name) throws Exception {