Render *args / * / **kwargs properly in function summary line

.bazelci/presubmit.yml

@@ -98,5 +98,15 @@ tasks:
     test_flags: *common_flags
     test_targets:
     - "//test:proto_format_test_e2e_test"
+    - "//test:macro_kwargs_legacy_test_e2e_test"
+
+  bazel_8_tests:
+    name: Stardoc golden tests requiring Bazel HEAD
+    platform: ubuntu2004
+    bazel: last_green
+    build_flags: *common_flags
+    test_flags: *common_flags
+    test_targets:
+    - "//test:macro_kwargs_test_e2e_test"
 
 buildifier: latest

docs/stardoc_rule.md

@@ -7,10 +7,10 @@ Starlark rule for stardoc: a documentation generator tool written in Java.
 ## stardoc
 
 <pre>
-stardoc(<a href="#stardoc-name">name</a>, <a href="#stardoc-input">input</a>, <a href="#stardoc-out">out</a>, <a href="#stardoc-deps">deps</a>, <a href="#stardoc-format">format</a>, <a href="#stardoc-symbol_names">symbol_names</a>, <a href="#stardoc-renderer">renderer</a>, <a href="#stardoc-aspect_template">aspect_template</a>, <a href="#stardoc-func_template">func_template</a>,
+stardoc(*, <a href="#stardoc-name">name</a>, <a href="#stardoc-input">input</a>, <a href="#stardoc-out">out</a>, <a href="#stardoc-deps">deps</a>, <a href="#stardoc-format">format</a>, <a href="#stardoc-symbol_names">symbol_names</a>, <a href="#stardoc-renderer">renderer</a>, <a href="#stardoc-aspect_template">aspect_template</a>, <a href="#stardoc-func_template">func_template</a>,
         <a href="#stardoc-header_template">header_template</a>, <a href="#stardoc-table_of_contents_template">table_of_contents_template</a>, <a href="#stardoc-provider_template">provider_template</a>, <a href="#stardoc-rule_template">rule_template</a>,
         <a href="#stardoc-repository_rule_template">repository_rule_template</a>, <a href="#stardoc-module_extension_template">module_extension_template</a>, <a href="#stardoc-footer_template">footer_template</a>, <a href="#stardoc-render_main_repo_name">render_main_repo_name</a>,
-        <a href="#stardoc-stamp">stamp</a>, <a href="#stardoc-kwargs">kwargs</a>)
+        <a href="#stardoc-stamp">stamp</a>, <a href="#stardoc-kwargs">**kwargs</a>)
 </pre>
 
 Generates documentation for exported starlark rule definitions in a target starlark file.

src/main/java/com/google/devtools/build/stardoc/rendering/MarkdownUtil.java

@@ -16,6 +16,9 @@
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.devtools.build.lib.starlarkdocextract.StardocOutputProtos.FunctionParamRole.PARAM_ROLE_KEYWORD_ONLY;
+import static com.google.devtools.build.lib.starlarkdocextract.StardocOutputProtos.FunctionParamRole.PARAM_ROLE_KWARGS;
+import static com.google.devtools.build.lib.starlarkdocextract.StardocOutputProtos.FunctionParamRole.PARAM_ROLE_VARARGS;
 import static java.util.Comparator.naturalOrder;
 import static java.util.stream.Collectors.joining;
 
@@ -38,6 +41,7 @@
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -247,9 +251,7 @@ private String providerSummaryImpl(
       String providerName, ProviderInfo providerInfo, String paramAnchorPrefix) {
     ImmutableList<Param> params =
         providerInfo.hasInit()
-            ? providerInfo.getInit().getParameterList().stream()
-                .map(paramInfo -> new Param(paramInfo, paramAnchorPrefix))
-                .collect(toImmutableList())
+            ? getFunctionParamsInDeclarationOrder(providerInfo.getInit(), paramAnchorPrefix)
             : providerInfo.getFieldInfoList().stream()
                 .map(fieldInfo -> new Param(fieldInfo, paramAnchorPrefix))
                 .collect(toImmutableList());
@@ -324,11 +326,9 @@ public String moduleExtensionSummary(String extensionName, ModuleExtensionInfo e
    */
   @SuppressWarnings("unused") // Used by markdown template.
   public String funcSummary(StarlarkFunctionInfo funcInfo) {
-    ImmutableList<Param> params =
-        funcInfo.getParameterList().stream()
-            .map(paramInfo -> new Param(paramInfo, funcInfo.getFunctionName()))
-            .collect(toImmutableList());
-    return summary(funcInfo.getFunctionName(), params);
+    return summary(
+        funcInfo.getFunctionName(),
+        getFunctionParamsInDeclarationOrder(funcInfo, funcInfo.getFunctionName()));
   }
 
   /**
@@ -350,15 +350,30 @@ private static String summary(String functionName, ImmutableList<Param> params)
 
   /** Representation of a callable's parameter in a summary line. */
   private static final class Param {
-    // Parameter name for use in summary line
+    // User-visible name, including the leading "*" or "**" for residuals
     final String name;
     // HTML anchor for the parameter's detailed documentation elsewhere on the page
-    final String anchorName;
+    final Optional<String> anchorName;
+
+    public static final Param STAR_SEPARATOR = new Param("*", Optional.empty());
+
+    private Param(String name, Optional<String> anchorName) {
+      this.name = name;
+      this.anchorName = anchorName;
+    }
 
     Param(FunctionParamInfo paramInfo, String anchorPrefix) {
-      // TODO(https://github.com/bazelbuild/stardoc/issues/225): prepend "*" or "**" to this.name
-      // for residual params.
-      this.name = paramInfo.getName();
+      switch (paramInfo.getRole()) {
+        case PARAM_ROLE_VARARGS:
+          this.name = "*" + paramInfo.getName();
+          break;
+        case PARAM_ROLE_KWARGS:
+          this.name = "**" + paramInfo.getName();
+          break;
+        default:
+          this.name = paramInfo.getName();
+          break;
+      }
       this.anchorName = formatAnchorName(paramInfo.getName(), anchorPrefix);
     }
 
@@ -372,17 +387,65 @@ private static final class Param {
       this.anchorName = formatAnchorName(fieldInfo.getName(), anchorPrefix);
     }
 
-    private static String formatAnchorName(String name, String anchorPrefix) {
-      return String.format("%s-%s", anchorPrefix, name);
+    private static Optional<String> formatAnchorName(String name, String anchorPrefix) {
+      return Optional.of(String.format("%s-%s", anchorPrefix, name));
     }
 
     String getName() {
       return this.name;
     }
 
     String renderHtml() {
-      return String.format("<a href=\"#%s\">%s</a>", anchorName, name);
+      if (anchorName.isPresent()) {
+        return String.format("<a href=\"#%s\">%s</a>", anchorName.get(), name);
+      } else {
+        return name;
+      }
+    }
+  }
+
+  private static ImmutableList<Param> getFunctionParamsInDeclarationOrder(
+      StarlarkFunctionInfo funcInfo, String anchorPrefix) {
+    List<FunctionParamInfo> paramInfos = funcInfo.getParameterList();
+    int nparams = paramInfos.size();
+    Optional<Param> kwargs;
+    if (nparams > 0 && paramInfos.get(nparams - 1).getRole() == PARAM_ROLE_KWARGS) {
+      kwargs = Optional.of(new Param(paramInfos.get(nparams - 1), anchorPrefix));
+      nparams--;
+    } else {
+      kwargs = Optional.empty();
+    }
+    Optional<Param> varargs;
+    if (nparams > 0 && paramInfos.get(nparams - 1).getRole() == PARAM_ROLE_VARARGS) {
+      varargs = Optional.of(new Param(paramInfos.get(nparams - 1), anchorPrefix));
+      nparams--;
+    } else {
+      varargs = Optional.empty();
+    }
+    // Invariant: nparams is now the number of non-residual parameters.
+
+    ImmutableList.Builder<Param> paramsBuilder = new ImmutableList.Builder();
+    int numKwonly = 0;
+    // Add ordinary or positional-only params
+    for (int i = 0; i < nparams; i++) {
+      FunctionParamInfo paramInfo = paramInfos.get(i);
+      if (paramInfo.getRole() == PARAM_ROLE_KEYWORD_ONLY) {
+        numKwonly = nparams - i;
+        break;
+      }
+      paramsBuilder.add(new Param(paramInfo, anchorPrefix));
+    }
+    // Add *args or (if needed) the "*" separator
+    if (varargs.isPresent() || numKwonly > 0) {
+      paramsBuilder.add(varargs.orElse(Param.STAR_SEPARATOR));
+    }
+    // Add kwonly params (if any)
+    for (int i = nparams - numKwonly; i < nparams; i++) {
+      paramsBuilder.add(new Param(paramInfos.get(i), anchorPrefix));
     }
+    // Add **kwargs
+    kwargs.ifPresent(paramsBuilder::add);
+    return paramsBuilder.build();
   }
 
   /**

stardoc/proto/stardoc_output.proto

@@ -13,7 +13,8 @@
 // limitations under the License.
 //
 // Vendored from src/main/protobuf/stardoc_output.proto
-// in the Bazel source tree at commit f4cfc846dbdf5f6c19d0a716fccd2ddcdae0d609n
+// in the Bazel source tree at commit bd1c3af2ea14e81268e940d2b8ba5ad00c3f08d7n
+
 //
 // Protos for Stardoc data.
 //
@@ -193,7 +194,16 @@ message StarlarkFunctionInfo {
   // "foo.frobnicate".
   string function_name = 1;
 
-  // The parameters for the function.
+  // The parameters for the function, in the following order:
+  // - positional parameters
+  // - keyword-only parameters
+  // - residual varargs parameter (`*args`)
+  // - residual keyword arguments parameter (`**kwargs`)
+  // This order differs from the order in which parameters are listed in the
+  // function's declaration (where positional parameters and keyword-only
+  // parameters are separated either by `*` or `*args`). The declaration order
+  // can be recovered by looking for the transition from ordinary/positional to
+  // keyword-only.
   repeated FunctionParamInfo parameter = 2;
 
   // The documented description of the function (if specified in the function's
@@ -213,9 +223,28 @@ message StarlarkFunctionInfo {
   OriginKey origin_key = 6;
 }
 
+// Representation of the syntactic role of a given function parameter.
+enum FunctionParamRole {
+  PARAM_ROLE_UNSPECIFIED = 0;
+  // An ordinary parameter which may be used as a positional or by keyword.
+  PARAM_ROLE_ORDINARY = 1;
+  // A positional-only parameter; such parameters cannot be defined in pure
+  // Starlark code, but exist in some natively-defined functions.
+  PARAM_ROLE_POSITIONAL_ONLY = 2;
+  // A keyword-only parameter, i.e. a non-vararg/kwarg parameter that follows
+  // `*` or `*args` in the function's declaration.
+  PARAM_ROLE_KEYWORD_ONLY = 3;
+  // Residual varargs, typically `*args` in the function's declaration.
+  PARAM_ROLE_VARARGS = 4;
+  // Residual keyword arguments, typically `**kwargs` in the function's
+  // declaration.
+  PARAM_ROLE_KWARGS = 5;
+}
+
 // Representation of a Starlark function parameter definition.
 message FunctionParamInfo {
-  // The name of the parameter.
+  // The name of the parameter. This does *not* include the `*` or `**` prefix
+  // for varargs or residual keyword argument parameters.
   string name = 1;
 
   // The documented description of the parameter (if specified in the function's
@@ -230,6 +259,9 @@ message FunctionParamInfo {
   // parameter. This might be false even if defaultValue is empty in the case of
   // special parameter such as *args and **kwargs"
   bool mandatory = 4;
+
+  // The parameter's syntactic role.
+  FunctionParamRole role = 5;
 }
 
 message FunctionReturnInfo {

test/BUILD

@@ -13,6 +13,11 @@ self_gen_test(
     name = "stardoc_self_gen_test",
     golden_file = "//stardoc:stardoc_doc.md",
     stardoc_doc = "//:stardoc_rule_doc",
+    # stardoc_doc.md output was generated with Bazel HEAD on 2024-06-12 and may differ in other versions
+    tags = [
+        "bazel_8",
+        "manual",
+    ],
 )
 
 exports_files(["testdata/fakedeps/dep.bzl"])
@@ -199,10 +204,26 @@ stardoc_test(
     ],
 )
 
+stardoc_test(
+    name = "macro_kwargs_legacy_test",
+    golden_file = "testdata/macro_kwargs_test/legacy_golden.md",
+    input_file = "testdata/macro_kwargs_test/input.bzl",
+    # Golden output was generated with Bazel 7.1 and may differ in other versions
+    tags = [
+        "bazel_7_1",
+        "manual",
+    ],
+)
+
 stardoc_test(
     name = "macro_kwargs_test",
     golden_file = "testdata/macro_kwargs_test/golden.md",
     input_file = "testdata/macro_kwargs_test/input.bzl",
+    # Golden output was generated with Bazel HEAD on 2024-06-12 and may differ in other versions
+    tags = [
+        "bazel_8",
+        "manual",
+    ],
 )
 
 stardoc_test(