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 "