Provide access to the doc string of starlark defined aspects

Required for new implementation of Stardoc

PiperOrigin-RevId: 511812841
Change-Id: I006a071e668d038fac46a8e2ed9651c5502ed7b1
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 f436d0d..376efe5 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
@@ -598,7 +598,7 @@
       Sequence<?> hostFragments,
       Sequence<?> toolchains,
       boolean useToolchainTransition,
-      String doc,
+      Object doc,
       Boolean applyToGeneratingRules,
       Sequence<?> rawExecCompatibleWith,
       Object rawExecGroups,
@@ -721,6 +721,7 @@
 
     return new StarlarkDefinedAspect(
         implementation,
+        Starlark.toJavaOptional(doc, String.class),
         attrAspects.build(),
         attributes.build(),
         StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"),
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
index f81c78e..7efc005 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
@@ -26,6 +26,8 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.EventHandler;
 import java.util.Objects;
+import java.util.Optional;
+import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Starlark;
@@ -35,6 +37,8 @@
 /** A Starlark value that is a result of an 'aspect(..)' function call. */
 public final class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect {
   private final StarlarkCallable implementation;
+  // @Nullable rather than Optional for the sake of serialization.
+  @Nullable private final String documentation;
   private final ImmutableList<String> attributeAspects;
   private final ImmutableList<Attribute> attributes;
   private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders;
@@ -61,6 +65,7 @@
 
   public StarlarkDefinedAspect(
       StarlarkCallable implementation,
+      Optional<String> documentation,
       ImmutableList<String> attributeAspects,
       ImmutableList<Attribute> attributes,
       ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders,
@@ -74,6 +79,7 @@
       ImmutableSet<Label> execCompatibleWith,
       ImmutableMap<String, ExecGroup> execGroups) {
     this.implementation = implementation;
+    this.documentation = documentation.orElse(null);
     this.attributeAspects = attributeAspects;
     this.attributes = attributes;
     this.requiredProviders = requiredProviders;
@@ -92,6 +98,14 @@
     return implementation;
   }
 
+  /**
+   * Returns the value of the doc parameter passed to aspect() Starlark builtin, or an empty
+   * Optional if a doc string was not provided.
+   */
+  public Optional<String> getDocumentation() {
+    return Optional.ofNullable(documentation);
+  }
+
   public ImmutableList<String> getAttributeAspects() {
     return attributeAspects;
   }
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 0d44034..3215dbc 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
@@ -634,7 +634,11 @@
         @Param(
             name = "doc",
             named = true,
-            defaultValue = "''",
+            allowedTypes = {
+              @ParamType(type = String.class),
+              @ParamType(type = NoneType.class),
+            },
+            defaultValue = "None",
             doc =
                 "A description of the aspect that can be extracted by documentation generating "
                     + "tools."),
@@ -691,7 +695,7 @@
       Sequence<?> hostFragments,
       Sequence<?> toolchains,
       boolean useToolchainTransition,
-      String doc,
+      Object doc,
       Boolean applyToGeneratingRules,
       Sequence<?> execCompatibleWith,
       Object execGroups,
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 f891b9d..5aff436 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
@@ -195,7 +195,7 @@
       Sequence<?> hostFragments,
       Sequence<?> toolchains,
       boolean useToolchainTransition,
-      String doc,
+      Object doc,
       Boolean applyToFiles,
       Sequence<?> execCompatibleWith,
       Object execGroups,
@@ -225,10 +225,8 @@
 
     // Only the Builder is passed to AspectInfoWrapper as the aspect name is not yet available.
     AspectInfo.Builder aspectInfo =
-        AspectInfo.newBuilder()
-            .setDocString(doc)
-            .addAllAttribute(attrInfos)
-            .addAllAspectAttribute(aspectAttrs);
+        AspectInfo.newBuilder().addAllAttribute(attrInfos).addAllAspectAttribute(aspectAttrs);
+    Starlark.toJavaOptional(doc, String.class).ifPresent(aspectInfo::setDocString);
 
     aspectInfoList.add(new AspectInfoWrapper(fakeAspect, thread.getCallerLocation(), aspectInfo));
 
diff --git a/src/main/java/net/starlark/java/eval/Starlark.java b/src/main/java/net/starlark/java/eval/Starlark.java
index 7ee4b90..7522f64 100644
--- a/src/main/java/net/starlark/java/eval/Starlark.java
+++ b/src/main/java/net/starlark/java/eval/Starlark.java
@@ -28,6 +28,7 @@
 import java.time.Duration;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 import javax.annotation.Nullable;
@@ -214,6 +215,23 @@
   }
 
   /**
+   * Converts a Starlark method's bound, non-None parameter value to a Java Optional wrapping that
+   * value, and an unbound or None value to an empty Optional.
+   *
+   * <p>This is typically used in {@link StarlarkMethod} implementations, with a parameter whose
+   * {@link Param#allowedTypes} is set to be {@code {T}} or {@code {NoneType, T}}.
+   *
+   * @throws ClassCastException if value is bound and non-None but is not of the expected class
+   */
+  public static <T> Optional<T> toJavaOptional(Object x, Class<T> expectedClass) {
+    if (x == Starlark.UNBOUND || x == Starlark.NONE) {
+      return Optional.empty();
+    } else {
+      return Optional.of(expectedClass.cast(x));
+    }
+  }
+
+  /**
    * Returns the truth value of a valid Starlark value, as if by the Starlark expression {@code
    * bool(x)}.
    */
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 40d88f2..3774a42 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
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.starlark;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 import static com.google.devtools.build.lib.analysis.testing.ExecGroupSubject.assertThat;
 import static com.google.devtools.build.lib.analysis.testing.RuleClassSubject.assertThat;
 import static com.google.devtools.build.lib.analysis.testing.StarlarkDefinedAspectSubject.assertThat;
@@ -2178,13 +2179,21 @@
         ev,
         "def _impl(target, ctx):", //
         "   pass",
-        "my_aspect = aspect(_impl, doc='foo')");
+        "documented_aspect = aspect(_impl, doc='My doc string')",
+        "undocumented_aspect = aspect(_impl)");
+
+    StarlarkDefinedAspect documentedAspect = (StarlarkDefinedAspect) ev.lookup("documented_aspect");
+    assertThat(documentedAspect.getDocumentation()).hasValue("My doc string");
+    StarlarkDefinedAspect undocumentedAspect =
+        (StarlarkDefinedAspect) ev.lookup("undocumented_aspect");
+    assertThat(undocumentedAspect.getDocumentation()).isEmpty();
   }
 
   @Test
   public void aspectBadTypeForDoc() throws Exception {
     registerDummyStarlarkFunction();
-    ev.checkEvalErrorContains("got value of type 'int', want 'string'", "aspect(impl, doc = 1)");
+    ev.checkEvalErrorContains(
+        "got value of type 'int', want 'string or NoneType'", "aspect(impl, doc = 1)");
   }
 
   @Test