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 "