Migrate SkylarkNativeModule methods to use @SkylarkCallable instead of @SkylarkSignature

Most notably, this involves introduction of a new function abstraction, BuiltinMethod, which can wrap a {objc, SkylarkCallable} pair into a BaseFunction for later calling. (This is required due to the current layer of indirection on the end "native" module)

RELNOTES: None.
PiperOrigin-RevId: 191642467

4 files changed, 226 insertions, 14 deletions

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 e445e46cf8..bad903c3df 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
@@ -487,7 +487,7 @@ public abstract class BaseFunction implements SkylarkValue {
     Preconditions.checkState(!isConfigured()); // must not be configured yet
 
     this.paramDoc = new ArrayList<>();
-    this.signature = SkylarkSignatureProcessor.getSignature(
+    this.signature = SkylarkSignatureProcessor.getSignatureForCallable(
         getName(), annotation, unconfiguredDefaultValues, paramDoc, getEnforcedArgumentTypes());
     this.objectType = annotation.objectType().equals(Object.class)
         ? null : annotation.objectType();
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
new file mode 100644
index 0000000000..8419e7fc64
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java
@@ -0,0 +1,137 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+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.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.syntax.Environment.LexicalFrame;
+import com.google.devtools.build.lib.syntax.FuncallExpression.MethodDescriptor;
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * A function-object abstraction on object methods exposed to skylark using {@link SkylarkCallable}.
+ */
+public class BuiltinCallable extends BaseFunction {
+
+  /** Represents a required interpreter parameter as dictated by {@link SkylarkCallable} */
+  public enum ExtraArgKind {
+    LOCATION, // SkylarkCallable.useLocation
+    SYNTAX_TREE, // SkylarkCallable.useAst
+    ENVIRONMENT, // SkylarkCallable.useEnvironment
+    SEMANTICS; // SkylarkCallable.useSemantics
+  }
+
+  // Builtins cannot create or modify variable bindings. So it's sufficient to use a shared
+  // instance.
+  private static final LexicalFrame SHARED_LEXICAL_FRAME_FOR_BUILTIN_METHOD_CALLS =
+      LexicalFrame.create(Mutability.IMMUTABLE);
+
+  private final Object obj;
+  private final MethodDescriptor descriptor;
+  private final List<ExtraArgKind> extraArgs;
+  private int innerArgumentCount;
+
+  public BuiltinCallable(String name, Object obj, MethodDescriptor descriptor) {
+    super(name);
+    this.obj = obj;
+    this.descriptor = descriptor;
+    this.extraArgs = getExtraArgs(descriptor.getAnnotation());
+    configure(obj, descriptor);
+  }
+
+  @Override
+  protected int getArgArraySize () {
+    return innerArgumentCount;
+  }
+
+  private static List<ExtraArgKind> getExtraArgs(SkylarkCallable annotation) {
+    ImmutableList.Builder<ExtraArgKind> extraArgs = ImmutableList.builder();
+    if (annotation.useLocation()) {
+      extraArgs.add(ExtraArgKind.LOCATION);
+    }
+    if (annotation.useAst()) {
+      extraArgs.add(ExtraArgKind.SYNTAX_TREE);
+    }
+    if (annotation.useEnvironment()) {
+      extraArgs.add(ExtraArgKind.ENVIRONMENT);
+    }
+    if (annotation.useSkylarkSemantics()) {
+      extraArgs.add(ExtraArgKind.SEMANTICS);
+    }
+    return extraArgs.build();
+  }
+
+  /** Configure a BaseFunction from a @SkylarkCallable-annotated method */
+  private void configure(Object obj, MethodDescriptor descriptor) {
+    Preconditions.checkState(!isConfigured()); // must not be configured yet
+
+    this.paramDoc = new ArrayList<>();
+    this.signature = SkylarkSignatureProcessor.getSignatureForCallable(
+        getName(), descriptor, paramDoc, getEnforcedArgumentTypes());
+    this.objectType = obj.getClass();
+    this.innerArgumentCount = signature.getSignature().getShape().getArguments() + extraArgs.size();
+    configure();
+  }
+
+  @Override
+  @Nullable
+  public Object call(Object[] args,
+      FuncallExpression ast, Environment env)
+      throws EvalException, InterruptedException {
+    Preconditions.checkNotNull(env);
+
+    // ast is null when called from Java (as there's no Skylark call site).
+    Location loc = ast == null ? Location.BUILTIN : ast.getLocation();
+
+    // Add extra arguments, if needed
+    int index = args.length - extraArgs.size();
+    for (ExtraArgKind extraArg : extraArgs) {
+      switch(extraArg) {
+        case LOCATION:
+          args[index] = loc;
+          break;
+
+        case SYNTAX_TREE:
+          args[index] = ast;
+          break;
+
+        case ENVIRONMENT:
+          args[index] = env;
+          break;
+
+        case SEMANTICS:
+          args[index] = env.getSemantics();
+          break;
+      }
+      index++;
+    }
+
+    Profiler.instance().startTask(ProfilerTask.SKYLARK_BUILTIN_FN, getName());
+
+    try {
+      env.enterScope(this, SHARED_LEXICAL_FRAME_FOR_BUILTIN_METHOD_CALLS, ast, env.getGlobals());
+      return FuncallExpression.callMethod(
+          descriptor, getName(), obj, args, ast.getLocation(), env);
+    } finally {
+      Profiler.instance().completeTask(ProfilerTask.SKYLARK_BUILTIN_FN);
+      env.exitScope();
+    }
+  }
+}
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 7a807f5e20..597508f04d 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
@@ -318,6 +318,21 @@ public final class FuncallExpression extends Expression {
   }
 
   /**
+   * Returns a {@link BuiltinCallable} representing a {@link SkylarkCallable}-annotated instance
+   * method of a given object with the given method name.
+   */
+  public static BuiltinCallable getBuiltinCallable(Object obj, String methodName) {
+    Class<?> objClass = obj.getClass();
+    List<MethodDescriptor> methodDescriptors = getMethods(objClass, methodName);
+    if (methodDescriptors.size() != 1) {
+      throw new IllegalStateException(String.format(
+          "Expected exactly 1 method named '%s' in %s, but found %s",
+          methodName, objClass, methodDescriptors.size()));
+    }
+    return new BuiltinCallable(methodName, obj, methodDescriptors.get(0));
+  }
+
+  /**
    * Invokes the given structField=true method and returns the result.
    *
    * @param methodDescriptor the descriptor of the method to invoke
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
index 6424a47e44..a2dc39a377 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
@@ -14,10 +14,13 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.primitives.Booleans;
 import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.BuiltinFunction.ExtraArgKind;
+import com.google.devtools.build.lib.syntax.FuncallExpression.MethodDescriptor;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -31,8 +34,50 @@ import javax.annotation.Nullable;
  * to configure a given field.
  */
 public class SkylarkSignatureProcessor {
+
   /**
-   * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from an annotation
+   * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a
+   * {@link SkylarkCallable}-annotated method.
+   *
+   * @param name the name of the function
+   * @param descriptor the method descriptor
+   * @param paramDoc an optional list into which to store documentation strings
+   * @param enforcedTypesList an optional list into which to store effective types to enforce
+   */
+  public static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
+      String name, MethodDescriptor descriptor,
+      @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
+
+    SkylarkCallable annotation = descriptor.getAnnotation();
+
+    // TODO(cparsons): Validate these properties with the annotation processor instead.
+    Preconditions.checkArgument(annotation.name().isEmpty() || name.equals(annotation.name()),
+        "%s != %s", name, annotation.name());
+    boolean documented = annotation.documented();
+    if (annotation.doc().isEmpty() && documented) {
+      throw new RuntimeException(String.format("function %s is undocumented", name));
+    }
+    ImmutableList.Builder<Parameter<Object, SkylarkType>> parameters = ImmutableList.builder();
+
+    Class<?>[] javaMethodSignatureParams = descriptor.getMethod().getParameterTypes();
+
+    for (int paramIndex = 0; paramIndex < annotation.mandatoryPositionals(); paramIndex++) {
+      Parameter<Object, SkylarkType> parameter =
+          new Parameter.Mandatory<Object, SkylarkType>("arg" + paramIndex,
+              SkylarkType.of(javaMethodSignatureParams[paramIndex]));
+      parameters.add(parameter);
+    }
+
+    return getSignatureForCallable(name, documented, parameters.build(), annotation.parameters(),
+        /*extraPositionals=*/null,
+        /*extraKeywords=*/null, /*defaultValues=*/null, paramDoc, enforcedTypesList);
+  }
+
+
+  /**
+   * Extracts a {@code FunctionSignature.WithValues<Object, SkylarkType>} from a
+   * {@link SkylarkSignature} annotation.
+   *
    * @param name the name of the function
    * @param annotation the annotation
    * @param defaultValues an optional list of default values
@@ -42,35 +87,50 @@ public class SkylarkSignatureProcessor {
   // NB: the two arguments paramDoc and enforcedTypesList are used to "return" extra values via
   // side-effects, and that's ugly
   // TODO(bazel-team): use AutoValue to declare a value type to use as return value?
-  public static FunctionSignature.WithValues<Object, SkylarkType> getSignature(
+  public static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
       String name, SkylarkSignature annotation,
       @Nullable Iterable<Object> defaultValues,
       @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
 
     Preconditions.checkArgument(name.equals(annotation.name()),
         "%s != %s", name, annotation.name());
+    boolean documented = annotation.documented();
+    if (annotation.doc().isEmpty() && documented) {
+      throw new RuntimeException(String.format("function %s is undocumented", name));
+    }
+    return getSignatureForCallable(name, documented,
+        /*mandatoryPositionals=*/ImmutableList.<Parameter<Object, SkylarkType>>of(),
+        annotation.parameters(),
+        annotation.extraPositionals(),
+        annotation.extraKeywords(), defaultValues, paramDoc, enforcedTypesList);
+  }
+
+  private static FunctionSignature.WithValues<Object, SkylarkType> getSignatureForCallable(
+      String name, boolean documented,
+      ImmutableList<Parameter<Object, SkylarkType>> mandatoryPositionals,
+      Param[] parameters,
+      @Nullable Param extraPositionals, @Nullable Param extraKeywords,
+      @Nullable Iterable<Object> defaultValues,
+      @Nullable List<String> paramDoc, @Nullable List<SkylarkType> enforcedTypesList) {
     ArrayList<Parameter<Object, SkylarkType>> paramList = new ArrayList<>();
+    paramList.addAll(mandatoryPositionals);
     HashMap<String, SkylarkType> enforcedTypes =
         enforcedTypesList == null ? null : new HashMap<>();
 
     HashMap<String, String> doc = new HashMap<>();
-    boolean documented = annotation.documented();
-    if (annotation.doc().isEmpty() && documented) {
-      throw new RuntimeException(String.format("function %s is undocumented", name));
-    }
 
     Iterator<Object> defaultValuesIterator = defaultValues == null
         ? null : defaultValues.iterator();
     try {
       boolean named = false;
-      for (Param param : annotation.parameters()) {
+      for (Param param : parameters) {
         boolean mandatory = param.defaultValue() != null && param.defaultValue().isEmpty();
         Object defaultValue = mandatory ? null : getDefaultValue(param, defaultValuesIterator);
         if (param.named() && !param.positional() && !named) {
           named = true;
           @Nullable Param starParam = null;
-          if (!annotation.extraPositionals().name().isEmpty()) {
-            starParam = annotation.extraPositionals();
+          if (extraPositionals != null && !extraPositionals.name().isEmpty()) {
+            starParam = extraPositionals;
           }
           paramList.add(getParameter(name, starParam, enforcedTypes, doc, documented,
                 /*mandatory=*/false, /*star=*/true, /*starStar=*/false, /*defaultValue=*/null));
@@ -78,14 +138,14 @@ public class SkylarkSignatureProcessor {
         paramList.add(getParameter(name, param, enforcedTypes, doc, documented,
                 mandatory, /*star=*/false, /*starStar=*/false, defaultValue));
       }
-      if (!annotation.extraPositionals().name().isEmpty() && !named) {
-        paramList.add(getParameter(name, annotation.extraPositionals(), enforcedTypes, doc,
+      if (extraPositionals != null && !extraPositionals.name().isEmpty() && !named) {
+        paramList.add(getParameter(name, extraPositionals, enforcedTypes, doc,
             documented, /*mandatory=*/false, /*star=*/true, /*starStar=*/false,
             /*defaultValue=*/null));
       }
-      if (!annotation.extraKeywords().name().isEmpty()) {
+      if (extraKeywords != null && !extraKeywords.name().isEmpty()) {
         paramList.add(
-            getParameter(name, annotation.extraKeywords(), enforcedTypes, doc, documented,
+            getParameter(name, extraKeywords, enforcedTypes, doc, documented,
                 /*mandatory=*/false, /*star=*/false, /*starStar=*/true, /*defaultValue=*/null));
       }
       FunctionSignature.WithValues<Object, SkylarkType> signature =