Add documented flag to Starlark Param annotation.

This is expected to be used only rarely.

PiperOrigin-RevId: 345010973

src/main/java/com/google/devtools/build/docgen/ApiExporter.java

@@ -335,6 +335,10 @@ private static Signature getSignature(StarlarkMethod annot) {
     ArrayList<String> defaults = new ArrayList<>();
 
     for (net.starlark.java.annot.Param param : annot.parameters()) {
+      // Ignore undocumented parameters
+      if (!param.documented()) {
+        continue;
+      }
       // Implicit * or *args parameter separates transition from positional to named.
       // f (..., *, ... )  or  f(..., *args, ...)
       // TODO(adonovan): this logic looks fishy. Clean it up.

src/main/java/com/google/devtools/build/docgen/starlark/StarlarkDocUtils.java

@@ -44,7 +44,9 @@ static ImmutableList<StarlarkParamDoc> determineParams(
       Param extraKeywords) {
     ImmutableList.Builder<StarlarkParamDoc> paramsBuilder = ImmutableList.builder();
     for (Param param : userSuppliedParams) {
-      paramsBuilder.add(new StarlarkParamDoc(methodDoc, param));
+      if (param.documented()) {
+        paramsBuilder.add(new StarlarkParamDoc(methodDoc, param));
+      }
     }
     if (!extraPositionals.name().isEmpty()) {
       paramsBuilder.add(new StarlarkParamDoc(methodDoc, extraPositionals));

src/main/java/com/google/devtools/build/docgen/starlark/StarlarkMethodDoc.java

@@ -75,6 +75,9 @@ private String getParameterString(Method method) {
 
     boolean named = false;
     for (Param param : withoutSelfParam(annotation, method)) {
+      if (!param.documented()) {
+        continue;
+      }
       if (param.named() && !param.positional() && !named) {
         named = true;
         if (!argList.isEmpty()) {

src/main/java/net/starlark/java/annot/Param.java

@@ -31,6 +31,15 @@
    */
   String doc() default "";
 
+  /**
+   * Determines whether the parameter appears in generated documentation. Set this to false to
+   * suppress parameters whose use is intentionally restricted.
+   *
+   * <p>An undocumented parameter must be {@link #named} and may not be followed by positional
+   * parameters or {@code **kwargs}.
+   */
+  boolean documented() default true;
+
   /**
    * Default value for the parameter, written as a Starlark expression (e.g. "False", "True", "[]",
    * "None").

src/main/java/net/starlark/java/annot/processor/StarlarkMethodProcessor.java

@@ -245,6 +245,7 @@ private void checkParameters(ExecutableElement method, StarlarkMethod annot) {
     boolean allowPositionalNext = true;
     boolean allowPositionalOnlyNext = true;
     boolean allowNonDefaultPositionalNext = true;
+    boolean hasUndocumentedMethods = false;
 
     // Check @Param annotations match parameters.
     Param[] paramAnnots = annot.parameters();
@@ -274,7 +275,8 @@ private void checkParameters(ExecutableElement method, StarlarkMethod annot) {
         if (!paramAnnot.named() && !allowPositionalOnlyNext) {
           errorf(
               param,
-              "Positional-only parameter '%s' is specified after one or more named parameters",
+              "Positional-only parameter '%s' is specified after one or more named or undocumented"
+                  + " parameters",
               paramAnnot.name());
         }
         if (paramAnnot.defaultValue().isEmpty()) { // There is no default value.
@@ -297,12 +299,21 @@ private void checkParameters(ExecutableElement method, StarlarkMethod annot) {
           errorf(param, "Parameter '%s' must be either positional or named", paramAnnot.name());
         }
       }
-      if (paramAnnot.named()) {
+      if (!paramAnnot.documented()) {
+        hasUndocumentedMethods = true;
+      }
+      if (paramAnnot.named() || !paramAnnot.documented()) {
         // No positional-only parameters can come after this parameter.
         allowPositionalOnlyNext = false;
       }
     }
 
+    if (hasUndocumentedMethods && !annot.extraKeywords().name().isEmpty()) {
+      errorf(
+          method,
+          "Method '%s' has undocumented parameters but also allows extra keyword parameters",
+          annot.name());
+    }
     checkSpecialParams(method, annot);
   }
 
@@ -379,6 +390,12 @@ private void checkParameter(Element param, Param paramAnnot) {
               : "Parameter '%s' has valueWhenDisabled set, but is always enabled",
           paramAnnot.name());
     }
+
+    // Ensure positional arguments are documented.
+    if (!paramAnnot.documented() && paramAnnot.positional()) {
+      errorf(
+          param, "Parameter '%s' must be documented because it is positional.", paramAnnot.name());
+    }
   }
 
   private static boolean hasPlusMinusPrefix(String s) {

src/test/java/com/google/devtools/build/docgen/StarlarkDocumentationTest.java

@@ -193,6 +193,30 @@ public Integer test(int a, int b, int c, int d, Sequence<?> args, Dict<?, ?> kwa
     }
   }
 
+  /** MockClassI */
+  @StarlarkBuiltin(name = "MockClassI", doc = "MockClassI")
+  private static class MockClassI implements StarlarkValue {
+    @StarlarkMethod(
+        name = "test",
+        doc = "MockClassI#test",
+        parameters = {
+          @Param(name = "a", named = false, positional = true),
+          @Param(name = "b", named = true, positional = true),
+          @Param(name = "c", named = true, positional = false),
+          @Param(name = "d", named = true, positional = false, defaultValue = "1"),
+          @Param(
+              name = "e",
+              named = true,
+              positional = false,
+              documented = false,
+              defaultValue = "2"),
+        },
+        extraPositionals = @Param(name = "myArgs"))
+    public Integer test(int a, int b, int c, int d, int e, Sequence<?> args) {
+      return 0;
+    }
+  }
+
   /**
    * MockGlobalLibrary. While nothing directly depends on it, a test method in
    * StarlarkDocumentationTest checks all of the classes under a wide classpath and ensures this one
@@ -387,6 +411,21 @@ public void testStarlarkCallableParametersAndArgsAndKwargs() throws Exception {
     assertThat(methodDoc.getParams()).hasSize(6);
   }
 
+  @Test
+  public void testStarlarkUndocumentedParameters() throws Exception {
+    Map<String, StarlarkBuiltinDoc> objects = collect(MockClassI.class);
+    StarlarkBuiltinDoc moduleDoc = objects.get("MockClassI");
+    assertThat(moduleDoc.getDocumentation()).isEqualTo("MockClassI");
+    assertThat(moduleDoc.getMethods()).hasSize(1);
+    StarlarkMethodDoc methodDoc = moduleDoc.getMethods().iterator().next();
+    assertThat(methodDoc.getDocumentation()).isEqualTo("MockClassI#test");
+    assertThat(methodDoc.getSignature())
+        .isEqualTo(
+            "<a class=\"anchor\" href=\"int.html\">int</a> "
+                + "MockClassI.test(a, b, *, c, d=1, *myArgs)");
+    assertThat(methodDoc.getParams()).hasSize(5);
+  }
+
   @Test
   public void testStarlarkGlobalLibraryCallable() throws Exception {
     StarlarkBuiltinDoc topLevel =

src/test/java/net/starlark/java/annot/processor/StarlarkMethodProcessorTest.java

@@ -198,7 +198,8 @@ public void testPositionalOnlyParamAfterNamed() throws Exception {
         .processedWith(new StarlarkMethodProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "Positional-only parameter 'two' is specified after one or more named parameters");
+            "Positional-only parameter 'two' is specified after one or more named or undocumented"
+                + " parameters");
   }
 
   @Test
@@ -323,4 +324,24 @@ public void testSpecifiedGenericType() throws Exception {
             "parameter 'one' has generic type "
                 + "net.starlark.java.eval.Sequence<java.lang.String>");
   }
+
+  @Test
+  public void testKwargsWithUndocumentedParam() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("KwargsWithUndocumentedParams.java"))
+        .processedWith(new StarlarkMethodProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "Method 'undocumented_with_kwargs' has undocumented parameters but also allows extra"
+                + " keyword parameters");
+  }
+
+  @Test
+  public void testUndocumentedPositionalParam() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("UndocumentedPositionalParam.java"))
+        .processedWith(new StarlarkMethodProcessor())
+        .failsToCompile()
+        .withErrorContaining("Parameter 'one' must be documented because it is positional");
+  }
 }

src/test/java/net/starlark/java/annot/processor/testsources/KwargsWithUndocumentedParams.java

@@ -0,0 +1,43 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package net.starlark.java.annot.processor.testsources;
+
+import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.StarlarkValue;
+
+/**
+ * Test case for a StarlarkMethod method which specifies extraKeywords whilst also specifying an
+ * undocumented parameter.
+ */
+public class KwargsWithUndocumentedParams implements StarlarkValue {
+
+  @StarlarkMethod(
+      name = "undocumented_with_kwargs",
+      documented = false,
+      parameters = {
+        @Param(
+            name = "one",
+            named = true,
+            positional = false,
+            documented = false,
+            defaultValue = "test")
+      },
+      extraKeywords = @Param(name = "kwargs"))
+  public String threeArgMethod(String one, Dict<?, ?> kwargs) {
+    return "bar";
+  }
+}

src/test/java/net/starlark/java/annot/processor/testsources/UndocumentedPositionalParam.java

@@ -0,0 +1,32 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package net.starlark.java.annot.processor.testsources;
+
+import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.StarlarkValue;
+
+/** Test case for a StarlarkMethod method which specifies an undocumented positional parameter. */
+public class UndocumentedPositionalParam implements StarlarkValue {
+
+  @StarlarkMethod(
+      name = "undocumented_positional",
+      documented = false,
+      parameters = {@Param(name = "one", documented = false)})
+  public String threeArgMethod(String one, Dict<?, ?> kwargs) {
+    return "bar";
+  }
+}