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

1 files changed, 46 insertions, 15 deletions

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 236bcc2576..bf43266b21 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 @@ public final class FuncallExpression extends Expression {
   /**
    * 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 final class FuncallExpression extends Expression {
   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 @@ public final class FuncallExpression extends Expression {
     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 @@ public final class FuncallExpression extends Expression {
   }
 
   /**
+   * 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 @@ public final class FuncallExpression extends Expression {
     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());
   }