Allow structField callables to specify useSkylarkSemantics, useLocation, and useEnvironment
Unfortunately this doesn't work for all callers, namely NativeInfo objects, as they may have structField callables invoked from contexts that have no environment available.
RELNOTES[INC]: Skylark structs (using struct()) may no longer have to_json and to_proto overridden.
PiperOrigin-RevId: 201376969
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java b/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java
index 21f8475..4396b43 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java
@@ -38,8 +38,14 @@
@Override
public Info createStruct(SkylarkDict<?, ?> kwargs, Location loc) throws EvalException {
- return SkylarkInfo.createSchemaless(
- this, kwargs.getContents(String.class, Object.class, "kwargs"), loc);
+ Map<String, Object> kwargsMap = kwargs.getContents(String.class, Object.class, "kwargs");
+ if (kwargsMap.containsKey("to_json")) {
+ throw new EvalException(loc, "cannot override built-in struct function 'to_json'");
+ }
+ if (kwargsMap.containsKey("to_proto")) {
+ throw new EvalException(loc, "cannot override built-in struct function 'to_proto'");
+ }
+ return SkylarkInfo.createSchemaless(this, kwargsMap, loc);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java
index 71df625..2ccc6cc 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java
@@ -158,14 +158,12 @@
throws SkylarkCallableProcessorException {
if (annotation.structField()) {
if (annotation.useAst()
- || annotation.useEnvironment()
- || annotation.useAst()
|| !annotation.extraPositionals().name().isEmpty()
|| !annotation.extraKeywords().name().isEmpty()) {
throw new SkylarkCallableProcessorException(
methodElement,
"@SkylarkCallable-annotated methods with structField=true may not also specify "
- + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords");
+ + "useAst, extraPositionals, or extraKeywords");
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index ed45d60..b41f274c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -118,6 +118,32 @@
*/
public static Object eval(Object objValue, String name,
Location loc, Environment env) throws EvalException, InterruptedException {
+
+ Iterable<MethodDescriptor> methods =
+ objValue instanceof Class<?>
+ ? FuncallExpression.getMethods((Class<?>) objValue, name)
+ : FuncallExpression.getMethods(objValue.getClass(), name);
+
+ if (methods != null) {
+ Optional<MethodDescriptor> method =
+ Streams.stream(methods)
+ .filter(methodDescriptor -> methodDescriptor.getAnnotation().structField())
+ .findFirst();
+ if (method.isPresent() && method.get().getAnnotation().structField()) {
+ return FuncallExpression.callMethod(
+ method.get(),
+ name,
+ objValue,
+ FuncallExpression.extraInterpreterArgs(
+ method.get().getAnnotation(),
+ /* ast = */ null,
+ loc,
+ env).toArray(),
+ loc,
+ env);
+ }
+ }
+
if (objValue instanceof SkylarkClassObject) {
try {
return ((SkylarkClassObject) objValue).getValue(name);
@@ -142,22 +168,6 @@
}
}
- Iterable<MethodDescriptor> methods =
- objValue instanceof Class<?>
- ? FuncallExpression.getMethods((Class<?>) objValue, name)
- : FuncallExpression.getMethods(objValue.getClass(), name);
-
- if (methods != null) {
- Optional<MethodDescriptor> method =
- Streams.stream(methods)
- .filter(methodDescriptor -> methodDescriptor.getAnnotation().structField())
- .findFirst();
- if (method.isPresent() && method.get().getAnnotation().structField()) {
- return FuncallExpression.callMethod(
- method.get(), name, objValue, new Object[] {}, loc, env);
- }
- }
-
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 236bcc2..bf43266 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -411,6 +411,9 @@
/**
* Invokes the given structField=true method and returns the result.
*
+ * <p>The given method must <b>not</b> require extra-interpreter parameters, such as
+ * {@link Environment}. This method throws {@link IllegalArgumentException} for violations.</p>
+ *
* @param methodDescriptor the descriptor of the method to invoke
* @param fieldName the name of the struct field
* @param obj the object on which to invoke the method
@@ -420,7 +423,12 @@
public static Object invokeStructField(
MethodDescriptor methodDescriptor, String fieldName, Object obj)
throws EvalException, InterruptedException {
- Preconditions.checkArgument(methodDescriptor.getAnnotation().structField());
+ Preconditions.checkArgument(methodDescriptor.getAnnotation().structField(),
+ "Can only be invoked on structField callables");
+ Preconditions.checkArgument(!methodDescriptor.getAnnotation().useEnvironment()
+ || !methodDescriptor.getAnnotation().useSkylarkSemantics()
+ || !methodDescriptor.getAnnotation().useLocation(),
+ "Cannot be invoked on structField callables with extra interpreter params");
return callMethod(methodDescriptor, fieldName, obj, new Object[0], Location.BUILTIN, null);
}
@@ -494,8 +502,12 @@
if (methods != null) {
for (MethodDescriptor method : methods) {
if (method.getAnnotation().structField()) {
- // TODO(cparsons): Allow structField methods to accept interpreter-supplied arguments.
- return new Pair<>(method, null);
+ // This indicates a built-in structField which returns a function which may have
+ // one or more arguments itself. For example, foo.bar('baz'), where foo.bar is a
+ // structField returning a function. Calling the "bar" callable of foo should
+ // not have 'baz' propagated, though extra interpreter arguments should be supplied.
+ return new Pair<>(method,
+ extraInterpreterArgs(method.getAnnotation(), null, getLocation(), environment));
} else {
argumentListConversionResult = convertArgumentList(args, kwargs, method, environment);
if (argumentListConversionResult.getArguments() != null) {
@@ -567,6 +579,36 @@
}
/**
+ * Returns the extra interpreter arguments for the given {@link SkylarkCallable}, to be added
+ * at the end of the argument list for the callable.
+ *
+ * <p>This method accepts null {@code ast} only if {@code callable.useAst()} is false. It is
+ * up to the caller to validate this invariant.</p>
+ */
+ public static List<Object> extraInterpreterArgs(SkylarkCallable callable,
+ @Nullable FuncallExpression ast, Location loc, Environment env) {
+
+ ImmutableList.Builder<Object> builder = ImmutableList.builder();
+
+ if (callable.useLocation()) {
+ builder.add(loc);
+ }
+ if (callable.useAst()) {
+ if (ast == null) {
+ throw new IllegalArgumentException("Callable expects to receive ast: " + callable.name());
+ }
+ builder.add(ast);
+ }
+ if (callable.useEnvironment()) {
+ builder.add(env);
+ }
+ if (callable.useSkylarkSemantics()) {
+ builder.add(env.getSemantics());
+ }
+ return builder.build();
+ }
+
+ /**
* Constructs the parameters list to actually pass to the method, filling with default values if
* any. If there is a type or argument mismatch, returns a result containing an error message.
*/
@@ -702,18 +744,7 @@
if (acceptsExtraKwargs) {
builder.add(SkylarkDict.copyOf(environment, extraKwargsBuilder.build()));
}
- if (callable.useLocation()) {
- builder.add(getLocation());
- }
- if (callable.useAst()) {
- builder.add(this);
- }
- if (callable.useEnvironment()) {
- builder.add(environment);
- }
- if (callable.useSkylarkSemantics()) {
- builder.add(environment.getSemantics());
- }
+ builder.addAll(extraInterpreterArgs(callable, this, getLocation(), environment));
return ArgumentListConversionResult.fromArgumentList(builder.build());
}
