bazel syntax: eliminate two subclasses of BuiltinFunction
This change replaces two test-only subclasses of BuiltinFunction with
StarlarkCallables. (The remaining two subclasses, rule and
repository_rule, will be dealt with in a follow-up.)
Also, change type of rule's and repository_rule's "implementation" parameter
to StarlarkFunction, and eliminate casts that already made this assumption.
Also, give StarlarkCallable.getLocation a default value of BUILTIN.
PiperOrigin-RevId: 285022081
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index ec1c2fd..904e177 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -36,9 +36,9 @@
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
import com.google.devtools.build.lib.skyframe.BlacklistedPackagePrefixesValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
-import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.vfs.Path;
@@ -84,7 +84,7 @@
new SkylarkRepositoryDefinitionLocationEvent(
rule.getName(), rule.getDefinitionInformation()));
}
- BaseFunction function = rule.getRuleClassObject().getConfiguredTargetFunction();
+ StarlarkFunction function = rule.getRuleClassObject().getConfiguredTargetFunction();
if (declareEnvironmentDependencies(markerData, env, getEnviron(rule)) == null) {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
index 4fdc01c..fdf1b9e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.SkylarkUtils;
import com.google.devtools.build.lib.syntax.Starlark;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import java.util.Map;
@@ -61,7 +62,7 @@
@Override
public BaseFunction repositoryRule(
- BaseFunction implementation,
+ StarlarkFunction implementation,
Object attrs,
Boolean local,
Sequence<?> environ, // <String> expected
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index f087893..29a0e35 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -55,10 +55,10 @@
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
-import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.Starlark;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -640,7 +640,7 @@
PredicatesWithMessage.<Rule>alwaysTrue();
private Predicate<String> preferredDependencyPredicate = Predicates.alwaysFalse();
private AdvertisedProviderSet.Builder advertisedProviders = AdvertisedProviderSet.builder();
- private BaseFunction configuredTargetFunction = null;
+ private StarlarkFunction configuredTargetFunction = null;
private BuildSetting buildSetting = null;
private Function<? super Rule, Map<String, Label>> externalBindingsFunction =
NO_EXTERNAL_BINDINGS;
@@ -1150,10 +1150,8 @@
return attributes.containsKey(name);
}
- /**
- * Sets the rule implementation function. Meant for Skylark usage.
- */
- public Builder setConfiguredTargetFunction(BaseFunction func) {
+ /** Sets the rule implementation function. Meant for Skylark usage. */
+ public Builder setConfiguredTargetFunction(StarlarkFunction func) {
this.configuredTargetFunction = func;
return this;
}
@@ -1443,7 +1441,7 @@
/**
* The Skylark rule implementation of this RuleClass. Null for non Skylark executable RuleClasses.
*/
- @Nullable private final BaseFunction configuredTargetFunction;
+ @Nullable private final StarlarkFunction configuredTargetFunction;
/**
* The BuildSetting associated with this rule. Null for all RuleClasses except Skylark-defined
@@ -1530,7 +1528,7 @@
PredicateWithMessage<Rule> validityPredicate,
Predicate<String> preferredDependencyPredicate,
AdvertisedProviderSet advertisedProviders,
- @Nullable BaseFunction configuredTargetFunction,
+ @Nullable StarlarkFunction configuredTargetFunction,
Function<? super Rule, Map<String, Label>> externalBindingsFunction,
Function<? super Rule, ? extends Set<String>> optionReferenceFunction,
@Nullable Label ruleDefinitionEnvironmentLabel,
@@ -2369,10 +2367,9 @@
return binaryOutput;
}
- /**
- * Returns this RuleClass's custom Skylark rule implementation.
- */
- @Nullable public BaseFunction getConfiguredTargetFunction() {
+ /** Returns this RuleClass's custom Skylark rule implementation. */
+ @Nullable
+ public StarlarkFunction getConfiguredTargetFunction() {
return configuredTargetFunction;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryModuleApi.java
index 33afe11..8f7840c 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/RepositoryModuleApi.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Sequence;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier;
import com.google.devtools.build.lib.syntax.StarlarkThread;
@@ -41,12 +42,13 @@
parameters = {
@Param(
name = "implementation",
- type = BaseFunction.class,
+ type = StarlarkFunction.class,
named = true,
doc =
- "the function implementing this rule, has to have exactly one parameter: <code><a"
- + " href=\"repository_ctx.html\">repository_ctx</a></code>. The function is"
- + " called during loading phase for each instance of the rule."),
+ "the Starlark function that implements this rule. Must have a single parameter,"
+ + " <code><a href=\"repository_ctx.html\">repository_ctx</a></code>. The"
+ + " function is called during the loading phase for each instance of the"
+ + " rule."),
@Param(
name = "attrs",
type = Dict.class,
@@ -110,7 +112,7 @@
useAst = true,
useStarlarkThread = true)
BaseFunction repositoryRule(
- BaseFunction implementation,
+ StarlarkFunction implementation,
Object attrs,
Boolean local,
Sequence<?> environ, // <String> expected
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index 90325f0..a248503 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -277,6 +277,7 @@
* @return the value resulting from evaluating the function with the given arguments
* @throws EvalException-s containing source information.
*/
+ @Override
public Object call(
List<Object> args,
@Nullable Map<String, Object> kwargs,
@@ -358,9 +359,4 @@
public void repr(Printer printer) {
printer.append("<function " + getName() + ">");
}
-
- @Override
- public Location getLocation() {
- return Location.BUILTIN;
- }
}
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 8faf014..0cdd461 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
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -104,11 +103,6 @@
}
@Override
- public Location getLocation() {
- return Location.BUILTIN;
- }
-
- @Override
public String getName() {
return methodName;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkCallable.java
index b4d488d..a8372d1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkCallable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkCallable.java
@@ -55,5 +55,7 @@
* Returns the location of the definition of this callable value, or BUILTIN if it was not defined
* in Starlark code.
*/
- Location getLocation();
+ default Location getLocation() {
+ return Location.BUILTIN;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java
index 4486139..773fab5 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.syntax.FunctionSignature;
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.Starlark;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.skydoc.fakebuildapi.FakeDescriptor;
import com.google.devtools.build.skydoc.fakebuildapi.FakeSkylarkRuleFunctionsApi.AttributeNameComparator;
@@ -51,7 +52,7 @@
@Override
public BaseFunction repositoryRule(
- BaseFunction implementation,
+ StarlarkFunction implementation,
Object attrs,
Boolean local,
Sequence<?> environ, // <String> expected
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
index 667e582..644934f 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
@@ -39,14 +39,12 @@
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
-import com.google.devtools.build.lib.syntax.BuiltinFunction;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Expression;
-import com.google.devtools.build.lib.syntax.FuncallExpression;
-import com.google.devtools.build.lib.syntax.FunctionSignature;
+import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.ParserInput;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkList;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
@@ -61,7 +59,6 @@
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.Before;
@@ -70,11 +67,9 @@
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
-/**
- * Unit tests for complex function of SkylarkRepositoryContext.
- */
+/** Unit tests for complex function of SkylarkRepositoryContext. */
@RunWith(JUnit4.class)
-public class SkylarkRepositoryContextTest {
+public final class SkylarkRepositoryContextTest {
private Scratch scratch;
private Path outputDirectory;
@@ -100,18 +95,19 @@
}
ruleClassBuilder.setWorkspaceOnly();
ruleClassBuilder.setConfiguredTargetFunction(
- new BuiltinFunction(FunctionSignature.ANY) {
- @Override
- public String getName() {
- return "test";
- }
-
- public void invoke(
- List<Object> args, Map<String, Object> kwargs, StarlarkThread thread) {}
- });
+ (StarlarkFunction) execAndEval("def test(ctx): pass", "test"));
return ruleClassBuilder.build();
}
+ private static Object execAndEval(String... lines) {
+ try (Mutability mu = Mutability.create("impl")) {
+ return EvalUtils.execAndEvalOptionalFinalExpression(
+ ParserInput.fromLines(lines), StarlarkThread.builder(mu).useDefaultSemantics().build());
+ } catch (Exception ex) { // SyntaxError | EvalException | InterruptedException
+ throw new AssertionError("exec failed", ex);
+ }
+ }
+
protected void setUpContextForRule(
Map<String, Object> kwargs,
ImmutableSet<PathFragment> ignoredPathFragments,
@@ -126,11 +122,9 @@
"runfiles",
starlarkSemantics);
ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class);
- ParserInput input = ParserInput.fromLines("test()");
- FuncallExpression ast = (FuncallExpression) Expression.parse(input);
Rule rule =
WorkspaceFactoryHelper.createAndAddRepositoryRule(
- packageBuilder, buildRuleClass(attributes), null, kwargs, ast.getLocation());
+ packageBuilder, buildRuleClass(attributes), null, kwargs, Location.BUILTIN);
HttpDownloader downloader = Mockito.mock(HttpDownloader.class);
SkyFunction.Environment environment = Mockito.mock(SkyFunction.Environment.class);
when(environment.getListener()).thenReturn(listener);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index fbc4177..ceb1586 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -56,7 +56,7 @@
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
-import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.StarlarkFunction;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.vfs.Path;
@@ -873,7 +873,7 @@
PredicateWithMessage<Rule> validityPredicate,
Predicate<String> preferredDependencyPredicate,
AdvertisedProviderSet advertisedProviders,
- @Nullable BaseFunction configuredTargetFunction,
+ @Nullable StarlarkFunction configuredTargetFunction,
Function<? super Rule, Map<String, Label>> externalBindingsFunction,
@Nullable StarlarkThread ruleDefinitionStarlarkThread,
Set<Class<?>> allowedConfigurationFragments,
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 dcb9f0f..9fb50eb 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
@@ -2088,34 +2088,34 @@
new NativeProvider<SkylarkClassObjectWithSkylarkCallables>(
SkylarkClassObjectWithSkylarkCallables.class, "struct_with_skylark_callables") {};
+ // A function that returns "fromValues".
+ Object returnFromValues =
+ new StarlarkCallable() {
+ @Override
+ public String getName() {
+ return "returnFromValues";
+ }
+
+ @Override
+ public Object call(
+ List<Object> args,
+ Map<String, Object> kwargs,
+ FuncallExpression call,
+ StarlarkThread thread) {
+ return "fromValues";
+ }
+ };
+
final Map<String, Object> fields =
ImmutableMap.of(
"values_only_field",
"fromValues",
"values_only_method",
- new BuiltinFunction(FunctionSignature.of()) {
- @Override
- public String getName() {
- return "values_only_method";
- }
-
- public String invoke() {
- return "fromValues";
- }
- },
+ returnFromValues,
"collision_field",
"fromValues",
"collision_method",
- new BuiltinFunction(FunctionSignature.of()) {
- @Override
- public String getName() {
- return "collision_method";
- }
-
- public String invoke() {
- return "fromValues";
- }
- });
+ returnFromValues);
SkylarkClassObjectWithSkylarkCallables() {
super(CONSTRUCTOR, Location.BUILTIN);
