From ea571e9786f868067f31deebbecaeebc418d923c Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 24 Feb 2023 12:47:45 -0800 Subject: [PATCH] Provide access to the doc string of starlark rule functions Required for new implementation of Stardoc PiperOrigin-RevId: 512140817 Change-Id: Id5fc0fd2ac993f059440b966db05b6d240300b2c --- .../starlark/StarlarkRuleClassFunctions.java | 25 ++++++++++++-- .../lib/rules/test/StarlarkTestingModule.java | 33 ++++++++++--------- .../StarlarkRuleFunctionsApi.java | 8 +++-- .../FakeStarlarkRuleFunctionsApi.java | 9 +++-- .../StarlarkRuleClassFunctionsTest.java | 14 ++++++-- 5 files changed, 61 insertions(+), 28 deletions(-) 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 a03c9f15a96161..0101d73fa5c180 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 @@ -98,6 +98,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; import javax.annotation.Nullable; import net.starlark.java.eval.Debug; import net.starlark.java.eval.Dict; @@ -287,7 +288,7 @@ public StarlarkRuleFunction rule( Boolean starlarkTestable, Sequence toolchains, boolean useToolchainTransition, - String doc, + Object doc, Sequence providesArg, Sequence execCompatibleWith, Object analysisTest, @@ -309,6 +310,7 @@ public StarlarkRuleFunction rule( hostFragments, starlarkTestable, toolchains, + doc, providesArg, execCompatibleWith, analysisTest, @@ -331,6 +333,7 @@ public static StarlarkRuleFunction createRule( Sequence hostFragments, boolean starlarkTestable, Sequence toolchains, + Object doc, Sequence providesArg, Sequence execCompatibleWith, Object analysisTest, @@ -502,7 +505,12 @@ public static StarlarkRuleFunction createRule( } StarlarkRuleFunction starlarkRuleFunction = - new StarlarkRuleFunction(builder, type, attributes, thread.getCallerLocation()); + new StarlarkRuleFunction( + builder, + type, + attributes, + thread.getCallerLocation(), + Starlark.toJavaOptional(doc, String.class)); // If a name= parameter is supplied (and we're currently initializing a .bzl module), export the // rule immediately under that name; otherwise the rule will be exported by the postAssignHook // set up in BzlLoadFunction. @@ -750,6 +758,7 @@ public static final class StarlarkRuleFunction implements StarlarkExportable, Ru private final RuleClassType type; private ImmutableList> attributes; private final Location definitionLocation; + @Nullable private final String documentation; private Label starlarkLabel; // TODO(adonovan): merge {Starlark,Builtin}RuleFunction and RuleClass, @@ -760,11 +769,13 @@ public StarlarkRuleFunction( RuleClass.Builder builder, RuleClassType type, ImmutableList> attributes, - Location definitionLocation) { + Location definitionLocation, + Optional documentation) { this.builder = builder; this.type = type; this.attributes = attributes; this.definitionLocation = definitionLocation; + this.documentation = documentation.orElse(null); } @Override @@ -772,6 +783,14 @@ public String getName() { return ruleClass != null ? ruleClass.getName() : "unexported rule"; } + /** + * Returns the value of the doc parameter passed to {@code rule()} in Starlark, or an empty + * Optional if a doc string was not provided. + */ + public Optional getDocumentation() { + return Optional.ofNullable(documentation); + } + @Override public Object call(StarlarkThread thread, Tuple args, Dict kwargs) throws EvalException, InterruptedException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java index 20335fb6f35095..2cad81592cf495 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java @@ -74,23 +74,24 @@ public void analysisTest( StarlarkRuleFunction starlarkRuleFunction = StarlarkRuleClassFunctions.createRule( implementation, - /*test=*/ true, + /* test= */ true, attrs, - /*implicitOutputs=*/ Starlark.NONE, - /*executable=*/ false, - /*outputToGenfiles=*/ false, - /*fragments=*/ fragments, - /*hostFragments=*/ StarlarkList.empty(), - /*starlarkTestable=*/ false, - /*toolchains=*/ toolchains, - /*providesArg=*/ StarlarkList.empty(), - /*execCompatibleWith=*/ StarlarkList.empty(), - /*analysisTest=*/ Boolean.TRUE, - /*buildSetting=*/ Starlark.NONE, - /*cfg=*/ Starlark.NONE, - /*execGroups=*/ Starlark.NONE, - /*compileOneFiletype=*/ Starlark.NONE, - /*name=*/ Starlark.NONE, + /* implicitOutputs= */ Starlark.NONE, + /* executable= */ false, + /* outputToGenfiles= */ false, + /* fragments= */ fragments, + /* hostFragments= */ StarlarkList.empty(), + /* starlarkTestable= */ false, + /* toolchains= */ toolchains, + /* doc= */ Starlark.NONE, + /* providesArg= */ StarlarkList.empty(), + /* execCompatibleWith= */ StarlarkList.empty(), + /* analysisTest= */ Boolean.TRUE, + /* buildSetting= */ Starlark.NONE, + /* cfg= */ Starlark.NONE, + /* execGroups= */ Starlark.NONE, + /* compileOneFiletype= */ Starlark.NONE, + /* name= */ Starlark.NONE, thread); // Export the rule diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java index ceaeb39fb3aead..ca24ba9d6243b0 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java @@ -362,7 +362,11 @@ Object provider(Object doc, Object fields, Object init, StarlarkThread thread) @Param( name = "doc", named = true, - defaultValue = "''", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = NoneType.class), + }, + defaultValue = "None", doc = "A description of the rule that can be extracted by documentation generating " + "tools."), @@ -487,7 +491,7 @@ StarlarkCallable rule( Boolean starlarkTestable, Sequence toolchains, boolean useToolchainTransition, - String doc, + Object doc, Sequence providesArg, Sequence execCompatibleWith, Object analysisTest, diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index 2a5608ac7d61a1..9550f18d772589 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java @@ -138,7 +138,7 @@ public StarlarkCallable rule( Boolean starlarkTestable, Sequence toolchains, boolean useToolchainTransition, - String doc, + Object doc, Sequence providesArg, Sequence execCompatibleWith, Object analysisTest, @@ -165,10 +165,9 @@ public StarlarkCallable rule( RuleDefinitionIdentifier functionIdentifier = new RuleDefinitionIdentifier(); // Only the Builder is passed to RuleInfoWrapper as the rule name may not be available yet. - RuleInfo.Builder ruleInfo = RuleInfo.newBuilder().setDocString(doc).addAllAttribute(attrInfos); - if (name != Starlark.NONE) { - ruleInfo.setRuleName((String) name); - } + RuleInfo.Builder ruleInfo = RuleInfo.newBuilder().addAllAttribute(attrInfos); + Starlark.toJavaOptional(doc, String.class).ifPresent(ruleInfo::setDocString); + Starlark.toJavaOptional(name, String.class).ifPresent(ruleInfo::setRuleName); Location loc = thread.getCallerLocation(); ruleInfoList.add(new RuleInfoWrapper(functionIdentifier, loc, ruleInfo)); 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 2291761a0d22e7..36ddca7508b014 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 @@ -860,7 +860,16 @@ public void testRuleImplementation() throws Exception { @Test public void testRuleDoc() throws Exception { - evalAndExport(ev, "def impl(ctx): return None", "rule1 = rule(impl, doc='foo')"); + evalAndExport( + ev, + "def impl(ctx):", + " return None", + "documented_rule = rule(impl, doc='My doc string')", + "undocumented_rule = rule(impl)"); + StarlarkRuleFunction documentedRule = (StarlarkRuleFunction) ev.lookup("documented_rule"); + StarlarkRuleFunction undocumentedRule = (StarlarkRuleFunction) ev.lookup("undocumented_rule"); + assertThat(documentedRule.getDocumentation()).hasValue("My doc string"); + assertThat(undocumentedRule.getDocumentation()).isEmpty(); } @Test @@ -1111,7 +1120,8 @@ public void testRuleBadTypeInAdd() throws Exception { @Test public void testRuleBadTypeForDoc() throws Exception { registerDummyStarlarkFunction(); - ev.checkEvalErrorContains("got value of type 'int', want 'string'", "rule(impl, doc = 1)"); + ev.checkEvalErrorContains( + "got value of type 'int', want 'string or NoneType'", "rule(impl, doc = 1)"); } @Test