bazel syntax: eliminate all but one calls to Runtime.setupStarlarkFunctions
This functionality has moved to class Starlark, in two variants
that reflect the two different intentions of their callers:
- addModule adds a single name to the environment (e.g. attr, native)
- addMethods adds all the methods of a value to the environment (e.g. glob, exports_files, etc)
setupStarlarkFunctions must remain in place temporarily until the last
remaining use, in the copybara code base, is removed. This is trivial but
cannot be done in the same change.
Also, make these CallUtils methods private:
getMethods
getMethodNames (1 overload)
getBuiltinCallable
PiperOrigin-RevId: 279746790
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
index 48740de..2697035 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -33,7 +33,6 @@
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkNativeModuleApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
-import com.google.devtools.build.lib.syntax.CallUtils;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Mutability;
@@ -43,6 +42,7 @@
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.SkylarkUtils;
+import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.Tuple;
import java.io.IOException;
@@ -69,11 +69,8 @@
public static final ImmutableMap<String, Object> BINDINGS_FOR_BUILD_FILES = initializeBindings();
private static ImmutableMap<String, Object> initializeBindings() {
- SkylarkNativeModule nativeModule = new SkylarkNativeModule();
ImmutableMap.Builder<String, Object> bindings = ImmutableMap.builder();
- for (String methodName : CallUtils.getMethodNames(SkylarkNativeModule.class)) {
- bindings.put(methodName, CallUtils.getBuiltinCallable(nativeModule, methodName));
- }
+ Starlark.addMethods(bindings, new SkylarkNativeModule());
return bindings.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java
index e6a1f9e..74a1562 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkBuildLibrary.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import java.util.List;
import java.util.Set;
@@ -44,9 +45,9 @@
public static final ImmutableMap<String, Object> BINDINGS = initializeBindings();
private static ImmutableMap<String, Object> initializeBindings() {
- ImmutableMap.Builder<String, Object> bindings = ImmutableMap.builder();
- Runtime.setupSkylarkLibrary(bindings, new StarlarkBuildLibrary());
- return bindings.build();
+ ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
+ Starlark.addMethods(env, new StarlarkBuildLibrary());
+ return env.build();
}
private StarlarkBuildLibrary() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 921a037..9e4f0a2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.BuiltinFunction;
-import com.google.devtools.build.lib.syntax.CallUtils;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -39,7 +38,6 @@
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.ParserInput;
-import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkUtils;
import com.google.devtools.build.lib.syntax.SkylarkUtils.Phase;
import com.google.devtools.build.lib.syntax.Starlark;
@@ -312,20 +310,20 @@
private static ImmutableMap<String, Object> createWorkspaceFunctions(
boolean allowOverride, RuleFactory ruleFactory, WorkspaceGlobals workspaceGlobals) {
- ImmutableMap.Builder<String, Object> map = ImmutableMap.builder();
- Runtime.setupSkylarkLibrary(map, workspaceGlobals);
+ ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
+ Starlark.addMethods(env, workspaceGlobals);
- Map<String, BaseFunction> ruleFunctions = new HashMap<>();
for (String ruleClass : ruleFactory.getRuleClassNames()) {
// There is both a "bind" WORKSPACE function and a "bind" rule. In workspace files,
// the non-rule function takes precedence.
// TODO(cparsons): Rule functions should not be added to WORKSPACE files.
- if (!"bind".equals(ruleClass)) {
+ if (!ruleClass.equals("bind")) {
BaseFunction ruleFunction = newRuleFunction(ruleFactory, ruleClass, allowOverride);
- ruleFunctions.put(ruleClass, ruleFunction);
+ env.put(ruleClass, ruleFunction);
}
}
- return map.putAll(ruleFunctions).build();
+
+ return env.build();
}
private ImmutableMap<String, Object> getDefaultEnvironment() {
@@ -352,23 +350,19 @@
private static ClassObject newNativeModule(
ImmutableMap<String, Object> workspaceFunctions, String version) {
- ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();
- SkylarkNativeModule nativeModuleInstance = new SkylarkNativeModule();
- for (String nativeFunction : CallUtils.getMethodNames(SkylarkNativeModule.class)) {
- builder.put(
- nativeFunction, CallUtils.getBuiltinCallable(nativeModuleInstance, nativeFunction));
- }
+ ImmutableMap.Builder<String, Object> env = new ImmutableMap.Builder<>();
+ Starlark.addMethods(env, new SkylarkNativeModule());
for (Map.Entry<String, Object> function : workspaceFunctions.entrySet()) {
// "workspace" is explicitly omitted from the native module, as it must only occur at the
// top of a WORKSPACE file.
// TODO(cparsons): Clean up separation between environments.
- if (!"workspace".equals(function.getKey())) {
- builder.put(function.getKey(), function.getValue());
+ if (!function.getKey().equals("workspace")) {
+ env.put(function);
}
}
- builder.put("bazel_version", version);
- return StructProvider.STRUCT.create(builder.build(), "no native function or rule '%s'");
+ env.put("bazel_version", version);
+ return StructProvider.STRUCT.create(env.build(), "no native function or rule '%s'");
}
static ClassObject newNativeModule(RuleClassProvider ruleClassProvider, String version) {
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/TopLevelBootstrap.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/TopLevelBootstrap.java
index 27060a2..304c8b3 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/TopLevelBootstrap.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/TopLevelBootstrap.java
@@ -17,7 +17,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.skylarkbuildapi.DefaultInfoApi.DefaultInfoApiProvider;
import com.google.devtools.build.lib.skylarkbuildapi.OutputGroupInfoApi.OutputGroupInfoApiProvider;
-import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.Starlark;
/**
* A {@link Bootstrap} for top-level libraries of the build API.
@@ -56,11 +56,11 @@
@Override
public void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) {
- Runtime.setupSkylarkLibrary(builder, skylarkAttrApi);
- Runtime.setupSkylarkLibrary(builder, skylarkBuildApiGlobals);
- Runtime.setupSkylarkLibrary(builder, skylarkCommandLineApi);
- Runtime.setupSkylarkLibrary(builder, skylarkNativeModuleApi);
- Runtime.setupSkylarkLibrary(builder, skylarkRuleFunctionsApi);
+ Starlark.addMethods(builder, skylarkBuildApiGlobals);
+ Starlark.addMethods(builder, skylarkRuleFunctionsApi);
+ Starlark.addModule(builder, skylarkAttrApi); // "attr"
+ Starlark.addModule(builder, skylarkCommandLineApi); // "cmd_helper"
+ Starlark.addModule(builder, skylarkNativeModuleApi); // "native"
builder.put("struct", structProvider);
builder.put("OutputGroupInfo", outputGroupInfoProvider);
builder.put("Actions", actionsInfoProviderApi);
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigBootstrap.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigBootstrap.java
index 752a74d..9533c8e 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigBootstrap.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigBootstrap.java
@@ -17,7 +17,7 @@
import com.google.common.collect.ImmutableMap.Builder;
import com.google.devtools.build.lib.skylarkbuildapi.Bootstrap;
import com.google.devtools.build.lib.skylarkbuildapi.StarlarkConfigApi;
-import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.Starlark;
/**
* A {@link Bootstrap} for config-related libraries of the build API.
@@ -41,6 +41,6 @@
public void addBindingsToBuilder(Builder<String, Object> builder) {
builder.put("config_common", configSkylarkCommonApi);
builder.put("config", starlarkConfigApi);
- Runtime.setupSkylarkLibrary(builder, configGlobalLibrary);
+ Starlark.addMethods(builder, configGlobalLibrary);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryBootstrap.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryBootstrap.java
index e982bf9..a6e1541 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryBootstrap.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryBootstrap.java
@@ -16,7 +16,7 @@
import com.google.common.collect.ImmutableMap.Builder;
import com.google.devtools.build.lib.skylarkbuildapi.Bootstrap;
-import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.Starlark;
/**
* A {@link Bootstrap} for repository-related libraries of the build API.
@@ -31,6 +31,6 @@
@Override
public void addBindingsToBuilder(Builder<String, Object> builder) {
- Runtime.setupSkylarkLibrary(builder, repositoryModuleApi);
+ Starlark.addMethods(builder, repositoryModuleApi);
}
}
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 69037ce..3a4857e 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
@@ -191,12 +191,12 @@
* @deprecated use {@link #getMethods(StarlarkSemantics, Class, String)} instead
*/
@Deprecated
- public static MethodDescriptor getMethod(Class<?> objClass, String methodName) {
+ private static MethodDescriptor getMethod(Class<?> objClass, String methodName) {
return getMethod(StarlarkSemantics.DEFAULT_SEMANTICS, objClass, methodName);
}
/** Returns the list of Skylark callable Methods of objClass with the given name. */
- public static MethodDescriptor getMethod(
+ static MethodDescriptor getMethod(
StarlarkSemantics semantics, Class<?> objClass, String methodName) {
return getCacheValue(objClass, semantics).methods.get(methodName);
}
@@ -208,7 +208,7 @@
* @deprecated use {@link #getMethodNames(StarlarkSemantics, Class)} instead
*/
@Deprecated
- public static Set<String> getMethodNames(Class<?> objClass) {
+ static Set<String> getMethodNames(Class<?> objClass) {
return getMethodNames(StarlarkSemantics.DEFAULT_SEMANTICS, objClass);
}
@@ -236,9 +236,9 @@
* method of a given object with the given Starlark field name (not necessarily the same as the
* Java method name).
*/
- // TODO(adonovan): replace with EvalUtils.getAttr, once the latter doesn't require
- // a Thread and Location.
- public static BuiltinCallable getBuiltinCallable(Object obj, String methodName) {
+ static BuiltinCallable getBuiltinCallable(Object obj, String methodName) {
+ // TODO(adonovan): implement by EvalUtils.getAttr, once the latter doesn't require
+ // a Thread and Location.
Class<?> objClass = obj.getClass();
MethodDescriptor methodDescriptor = getMethod(objClass, methodName);
if (methodDescriptor == null) {
@@ -613,7 +613,7 @@
// java method 'bar()', this avoids evaluating 'foo.bar' in isolation (which would require
// creating a throwaway function-like object).
MethodDescriptor methodDescriptor =
- CallUtils.getMethod(thread.getSemantics(), object.getClass(), methodName);
+ getMethod(thread.getSemantics(), object.getClass(), methodName);
if (methodDescriptor != null && !methodDescriptor.isStructField()) {
Object[] javaArguments =
convertStarlarkArgumentsToJavaMethodArguments(
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java b/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java
index be0d65c..0e9e5a2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Starlark.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.util.Pair;
import java.util.Map;
@@ -45,7 +47,7 @@
.put("False", false)
.put("True", true)
.put("None", Runtime.NONE);
- Runtime.setupSkylarkLibrary(env, new MethodLibrary());
+ addMethods(env, new MethodLibrary());
return env.build();
}
@@ -67,6 +69,36 @@
}
}
+ /**
+ * Adds to the environment {@code env} all {@code StarlarkCallable}-annotated fields and methods
+ * of value {@code v}. The class of {@code v} must have or inherit a {@code SkylarkModule} or
+ * {@code SkylarkGlobalLibrary} annotation.
+ */
+ public static void addMethods(ImmutableMap.Builder<String, Object> env, Object v) {
+ Class<?> cls = v.getClass();
+ if (!SkylarkInterfaceUtils.hasSkylarkGlobalLibrary(cls)
+ && SkylarkInterfaceUtils.getSkylarkModule(cls) == null) {
+ throw new IllegalArgumentException(
+ cls.getName() + " is annotated with neither @SkylarkGlobalLibrary nor @SkylarkModule");
+ }
+ for (String name : CallUtils.getMethodNames(v.getClass())) {
+ env.put(name, CallUtils.getBuiltinCallable(v, name));
+ }
+ }
+
+ /**
+ * Adds to the environment {@code env} the value {@code v}, under its annotated name. The class of
+ * {@code v} must have or inherit a {@code SkylarkModule} annotation.
+ */
+ public static void addModule(ImmutableMap.Builder<String, Object> env, Object v) {
+ Class<?> cls = v.getClass();
+ SkylarkModule annot = SkylarkInterfaceUtils.getSkylarkModule(cls);
+ if (annot == null) {
+ throw new IllegalArgumentException(cls.getName() + " is not annotated with @SkylarkModule");
+ }
+ env.put(annot.name(), v);
+ }
+
// TODO(adonovan):
//
// The code below shows the API that is the destination toward which all of the recent
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 04b7b45..8e50a2f 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -58,7 +58,6 @@
import com.google.devtools.build.lib.skylarkinterface.Param;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkGlobalLibrary;
-import com.google.devtools.build.lib.syntax.CallUtils;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Printer;
@@ -66,6 +65,7 @@
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -186,13 +186,18 @@
return (StructImpl) configuredTarget.get(key);
}
- private void setupSkylarkFunction(String line) throws Exception {
- update("mock", CallUtils.getBuiltinCallable(this, "mock"));
- exec(line);
+ // Defines all @SkylarkCallable-annotated methods (mock, throw, ...) in the environment.
+ private void defineTestMethods() throws Exception {
+ ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
+ Starlark.addMethods(env, this);
+ for (Map.Entry<String, Object> entry : env.build().entrySet()) {
+ update(entry.getKey(), entry.getValue());
+ }
}
private void checkSkylarkFunctionError(String errorSubstring, String line) throws Exception {
- EvalException e = assertThrows(EvalException.class, () -> setupSkylarkFunction(line));
+ defineTestMethods();
+ EvalException e = assertThrows(EvalException.class, () -> exec(line));
assertThat(e).hasMessageThat().contains(errorSubstring);
}
@@ -200,7 +205,8 @@
@Test
public void testSkylarkFunctionPosArgs() throws Exception {
- setupSkylarkFunction("a = mock('a', 'b', mandatory_key='c')");
+ defineTestMethods();
+ exec("a = mock('a', 'b', mandatory_key='c')");
Map<?, ?> params = (Map<?, ?>) lookup("a");
assertThat(params.get("mandatory")).isEqualTo("a");
assertThat(params.get("optional")).isEqualTo("b");
@@ -210,7 +216,8 @@
@Test
public void testSkylarkFunctionKwArgs() throws Exception {
- setupSkylarkFunction("a = mock(optional='b', mandatory='a', mandatory_key='c')");
+ defineTestMethods();
+ exec("a = mock(optional='b', mandatory='a', mandatory_key='c')");
Map<?, ?> params = (Map<?, ?>) lookup("a");
assertThat(params.get("mandatory")).isEqualTo("a");
assertThat(params.get("optional")).isEqualTo("b");
@@ -1893,10 +1900,9 @@
@Test
public void testStackTraceWithoutOriginalMessage() throws Exception {
- update("throw", CallUtils.getBuiltinCallable(this, "throw1"));
+ defineTestMethods();
checkEvalErrorContains(
- "There Is No Message: SkylarkRuleImplementationFunctionsTest",
- "throw()");
+ "There Is No Message: SkylarkRuleImplementationFunctionsTest", "throw1()");
}
@SkylarkCallable(name = "throw2", documented = false)
@@ -1906,8 +1912,8 @@
@Test
public void testNoStackTraceOnInterrupt() throws Exception {
- update("throw", CallUtils.getBuiltinCallable(this, "throw2"));
- assertThrows(InterruptedException.class, () -> eval("throw()"));
+ defineTestMethods();
+ assertThrows(InterruptedException.class, () -> eval("throw2()"));
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 672fee8..f840c7c 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -414,9 +414,7 @@
allowReturnNones = true)
public ClassObject proxyMethodsObject() {
ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();
- for (String nativeFunction : CallUtils.getMethodNames(Mock.class)) {
- builder.put(nativeFunction, CallUtils.getBuiltinCallable(this, nativeFunction));
- }
+ Starlark.addMethods(builder, this);
return StructProvider.STRUCT.create(builder.build(), "no native callable '%s'");
}
