bazel syntax: make MethodDescriptor and ParamDescriptor private

The only external uses are docgen and Kythe,
which need only SkylarkCallable annotation.

PiperOrigin-RevId: 282839886
diff --git a/src/main/java/com/google/devtools/build/docgen/ApiExporter.java b/src/main/java/com/google/devtools/build/docgen/ApiExporter.java
index 4e8c74c..b32886b 100644
--- a/src/main/java/com/google/devtools/build/docgen/ApiExporter.java
+++ b/src/main/java/com/google/devtools/build/docgen/ApiExporter.java
@@ -31,8 +31,6 @@
 import com.google.devtools.build.lib.syntax.BuiltinCallable;
 import com.google.devtools.build.lib.syntax.CallUtils;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
-import com.google.devtools.build.lib.syntax.MethodDescriptor;
-import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.common.options.OptionsParser;
 import java.io.BufferedOutputStream;
@@ -96,10 +94,7 @@
       if (obj instanceof BaseFunction) {
         value = valueFromFunction((BaseFunction) obj);
       } else if (obj instanceof BuiltinCallable) {
-        BuiltinCallable builtinCallable = (BuiltinCallable) obj;
-        MethodDescriptor descriptor =
-            builtinCallable.getMethodDescriptor(StarlarkSemantics.DEFAULT_SEMANTICS);
-        value = valueFromAnnotation(descriptor.getAnnotation());
+        value = valueFromAnnotation(((BuiltinCallable) obj).getAnnotation());
       } else {
         value.setName(entry.getKey());
       }
@@ -119,11 +114,9 @@
       } else {
         SkylarkModule typeModule = SkylarkInterfaceUtils.getSkylarkModule(obj.getClass());
         if (typeModule != null) {
-          MethodDescriptor selfCall =
-              CallUtils.getSelfCallMethodDescriptor(
-                  StarlarkSemantics.DEFAULT_SEMANTICS, obj.getClass());
+          SkylarkCallable selfCall = CallUtils.getSelfCallAnnotation(obj.getClass());
           if (selfCall != null) {
-            value = valueFromAnnotation(selfCall.getAnnotation());
+            value = valueFromAnnotation(selfCall);
           } else {
             value.setName(entry.getKey());
             value.setType(entry.getKey());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java
index 292139a..26901de 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java
@@ -17,13 +17,14 @@
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
- * A BuiltinCallable is a callable Starlark value that reflectively invokes a method of a Java
- * object.
+ * A BuiltinCallable is a callable Starlark value that reflectively invokes a
+ * SkylarkCallable-annotated method of a Java object.
  */
 // TODO(adonovan): make this private. Most users would be content with StarlarkCallable; the rest
 // need only a means of querying the function's parameters.
@@ -32,7 +33,7 @@
   private final Object obj;
   private final String methodName;
 
-  public BuiltinCallable(Object obj, String methodName) {
+  BuiltinCallable(Object obj, String methodName) {
     this.obj = obj;
     this.methodName = methodName;
   }
@@ -44,6 +45,8 @@
       FuncallExpression ast,
       StarlarkThread thread)
       throws EvalException, InterruptedException {
+    // Even though all callers of 'new BuiltinCallable' have a MethodDescriptor,
+    // we have to look it up again with the correct semantics from the thread.
     MethodDescriptor methodDescriptor = getMethodDescriptor(thread.getSemantics());
     Class<?> clazz;
     Object objValue;
@@ -67,10 +70,20 @@
     }
   }
 
-  public MethodDescriptor getMethodDescriptor(StarlarkSemantics semantics) {
+  private MethodDescriptor getMethodDescriptor(StarlarkSemantics semantics) {
     return CallUtils.getMethod(semantics, obj.getClass(), methodName);
   }
 
+  /**
+   * Returns the SkylarkCallable annotation of this Starlark-callable Java method.
+   *
+   * @deprecated This method is intended only for docgen, and uses the default semantics.
+   */
+  @Deprecated
+  public SkylarkCallable getAnnotation() {
+    return getMethodDescriptor(StarlarkSemantics.DEFAULT_SEMANTICS).getAnnotation();
+  }
+
   @Override
   public Location getLocation() {
     return Location.BUILTIN;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/CallUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/CallUtils.java
index 4c7514e..539dfdd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/CallUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/CallUtils.java
@@ -197,14 +197,23 @@
    * method of the given object (the {@link SkylarkCallable} method with {@link
    * SkylarkCallable#selfCall()} set to true). Returns null if no such method exists.
    */
-  // TODO(adonovan): eliminate sole use in docgen.
   @Nullable
-  public static MethodDescriptor getSelfCallMethodDescriptor(
+  static MethodDescriptor getSelfCallMethodDescriptor(
       StarlarkSemantics semantics, Class<?> objClass) {
     return getCacheValue(objClass, semantics).selfCall;
   }
 
   /**
+   * Returns the annotation from the SkylarkCallable-annotated self-call method of the specified
+   * class, or null if not found.
+   */
+  public static SkylarkCallable getSelfCallAnnotation(Class<?> objClass) {
+    MethodDescriptor selfCall =
+        getSelfCallMethodDescriptor(StarlarkSemantics.DEFAULT_SEMANTICS, objClass);
+    return selfCall == null ? null : selfCall.getAnnotation();
+  }
+
+  /**
    * Returns a {@link BuiltinCallable} representing a {@link SkylarkCallable}-annotated instance
    * method of a given object with the given Starlark field name (not necessarily the same as the
    * Java method name).
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
index 978075d..2e31842 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
@@ -30,9 +30,7 @@
  * <p>The annotation metadata is duplicated in this class to avoid usage of Java dynamic proxies
  * which are ~7X slower.
  */
-// TODO(adonovan): make this private. All external uses either want parameter types, or want to
-// "invoke" a struct field, both of which need better abstractions.
-public final class MethodDescriptor {
+final class MethodDescriptor {
   private final Method method;
   private final SkylarkCallable annotation;
 
@@ -84,12 +82,12 @@
   }
 
   /** Returns the SkylarkCallable annotation corresponding to this method. */
-  public SkylarkCallable getAnnotation() {
+  SkylarkCallable getAnnotation() {
     return annotation;
   }
 
   /** @return Skylark method descriptor for provided Java method and signature annotation. */
-  public static MethodDescriptor of(
+  static MethodDescriptor of(
       Method method, SkylarkCallable annotation, StarlarkSemantics semantics) {
     // This happens when the interface is public but the implementation classes
     // have reduced visibility.
@@ -120,7 +118,7 @@
    * <p>{@code obj} may be {@code null} in case this method is static. Methods with {@code void}
    * return type return {@code None} following Python convention.
    */
-  public Object call(Object obj, Object[] args, Location loc, StarlarkThread thread)
+  Object call(Object obj, Object[] args, Location loc, StarlarkThread thread)
       throws EvalException, InterruptedException {
     Preconditions.checkNotNull(obj);
     Object result;
@@ -166,7 +164,7 @@
   }
 
   /** @see SkylarkCallable#name() */
-  public String getName() {
+  String getName() {
     return name;
   }
 
@@ -175,12 +173,12 @@
   }
 
   /** @see SkylarkCallable#structField() */
-  public boolean isStructField() {
+  boolean isStructField() {
     return structField;
   }
 
   /** @see SkylarkCallable#useStarlarkThread() */
-  public boolean isUseStarlarkThread() {
+  boolean isUseStarlarkThread() {
     return useStarlarkThread;
   }
 
@@ -190,26 +188,26 @@
   }
 
   /** @see SkylarkCallable#useLocation() */
-  public boolean isUseLocation() {
+  boolean isUseLocation() {
     return useLocation;
   }
 
   /** @see SkylarkCallable#allowReturnNones() */
-  public boolean isAllowReturnNones() {
+  boolean isAllowReturnNones() {
     return allowReturnNones;
   }
 
   /** @see SkylarkCallable#useAst() */
-  public boolean isUseAst() {
+  boolean isUseAst() {
     return useAst;
   }
 
   /** @see SkylarkCallable#extraPositionals() */
-  public ParamDescriptor getExtraPositionals() {
+  ParamDescriptor getExtraPositionals() {
     return extraPositionals;
   }
 
-  public ParamDescriptor getExtraKeywords() {
+  ParamDescriptor getExtraKeywords() {
     return extraKeywords;
   }
 
@@ -224,22 +222,22 @@
   }
 
   /** @see SkylarkCallable#parameters() */
-  public ImmutableList<ParamDescriptor> getParameters() {
+  ImmutableList<ParamDescriptor> getParameters() {
     return parameters;
   }
 
   /** @see SkylarkCallable#documented() */
-  public boolean isDocumented() {
+  boolean isDocumented() {
     return documented;
   }
 
   /** @see SkylarkCallable#doc() */
-  public String getDoc() {
+  String getDoc() {
     return doc;
   }
 
   /** @see SkylarkCallable#selfCall() */
-  public boolean isSelfCall() {
+  boolean isSelfCall() {
     return selfCall;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java b/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java
index ca90184..4c8260b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java
@@ -22,7 +22,7 @@
 import javax.annotation.Nullable;
 
 /** A value class for storing {@link Param} metadata to avoid using Java proxies. */
-public final class ParamDescriptor {
+final class ParamDescriptor {
 
   private final String name;
   private final String defaultValue;
@@ -77,7 +77,7 @@
    * Returns a {@link ParamDescriptor} representing the given raw {@link Param} annotation and the
    * given semantics.
    */
-  public static ParamDescriptor of(Param param, StarlarkSemantics starlarkSemantics) {
+  static ParamDescriptor of(Param param, StarlarkSemantics starlarkSemantics) {
     ImmutableList<ParamTypeDescriptor> allowedTypes =
         Arrays.stream(param.allowedTypes())
             .map(ParamTypeDescriptor::of)
@@ -106,32 +106,26 @@
         allowedTypes,
         generic,
         noneable,
-        isNamed(param, starlarkSemantics),
+        param.named()
+            || (param.legacyNamed() && !starlarkSemantics.incompatibleRestrictNamedParams()),
         param.positional(),
         getType(type, generic, allowedTypes, noneable),
         valueOverride,
         flagResponsibleForDisable);
   }
 
-  private static boolean isNamed(Param param, StarlarkSemantics starlarkSemantics) {
-    if (param.named()) {
-      return true;
-    }
-    return param.legacyNamed() && !starlarkSemantics.incompatibleRestrictNamedParams();
-  }
-
   /** @see Param#name() */
-  public String getName() {
+  String getName() {
     return name;
   }
 
   /** @see Param#allowedTypes() */
-  public ImmutableList<ParamTypeDescriptor> getAllowedTypes() {
+  ImmutableList<ParamTypeDescriptor> getAllowedTypes() {
     return allowedTypes;
   }
 
   /** @see Param#type() */
-  public Class<?> getType() {
+  Class<?> getType() {
     return type;
   }
 
@@ -161,27 +155,27 @@
   }
 
   /** @see Param#generic1() */
-  public Class<?> getGeneric1() {
+  Class<?> getGeneric1() {
     return generic1;
   }
 
   /** @see Param#noneable() */
-  public boolean isNoneable() {
+  boolean isNoneable() {
     return noneable;
   }
 
   /** @see Param#positional() */
-  public boolean isPositional() {
+  boolean isPositional() {
     return positional;
   }
 
   /** @see Param#named() */
-  public boolean isNamed() {
+  boolean isNamed() {
     return named;
   }
 
   /** @see Param#defaultValue() */
-  public String getDefaultValue() {
+  String getDefaultValue() {
     return defaultValue;
   }
 
@@ -190,7 +184,7 @@
   }
 
   /** Returns true if this parameter is disabled under the current skylark semantic flags. */
-  public boolean isDisabledInCurrentSemantics() {
+  boolean isDisabledInCurrentSemantics() {
     return valueOverride != null;
   }
 
@@ -200,7 +194,7 @@
    *
    * @throws IllegalStateException if invoked when {@link #isDisabledInCurrentSemantics()} is false
    */
-  public String getValueOverride() {
+  String getValueOverride() {
     Preconditions.checkState(
         isDisabledInCurrentSemantics(),
         "parameter is not disabled under the current semantic flags. getValueOverride should be "
@@ -214,7 +208,7 @@
    *
    * @throws IllegalStateException if invoked when {@link #isDisabledInCurrentSemantics()} is false
    */
-  public FlagIdentifier getFlagResponsibleForDisable() {
+  FlagIdentifier getFlagResponsibleForDisable() {
     Preconditions.checkState(
         isDisabledInCurrentSemantics(),
         "parameter is not disabled under the current semantic flags. getFlagResponsibleForDisable "