Create useSkylarkSemantics for @SkylarkCallable, so annotated methods can specifically get a semantics object

This is slightly redundant with useEnvironment, yes (as one can easily obtain the semantics object with Environment), but we intend on restricting useEnvironment so that structField=true methods cannot specify useEnvironment, but *can* specify useSkylarkSemantics.

In general, we can also ween off non structField methods to use useSkylarkSemantics instead of useEnvironment in cases where this is feasible.

RELNOTES: None.
PiperOrigin-RevId: 190082547

2 files changed, 63 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java
index 3ce0f70d9d..aed52245f9 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java
@@ -21,20 +21,20 @@ import java.lang.annotation.Target;
 /**
  * A marker interface for Java methods which can be called from Skylark.
  *
- * <p>Methods annotated with this annotation are expected to meet certain requirements which
- * are enforced by an annotation processor:</p>
+ * <p>Methods annotated with this annotation are expected to meet certain requirements which are
+ * enforced by an annotation processor:
+ *
  * <ul>
- * <li>The method must be public.</li>
- * <li>If structField=true, there must be zero user-supplied parameters.</li>
- * <li>Method parameters must be supplied in the following order:
- *   <pre>method([positionals]*[other user-args](Location)(FuncallExpression)(Envrionment))</pre>
- *   where Location, FuncallExpression, and Environment are supplied by the interpreter if and
- *   only if useLocation, useAst, and useEnvironment are specified, respectively.
-*  </li>
- * <li>
- *   The number of method parameters much match the number of annotation-declared parameters
- *   plus the number of interpreter-supplied parameters.
- * </li>
+ *   <li>The method must be public.
+ *   <li>If structField=true, there must be zero user-supplied parameters.
+ *   <li>Method parameters must be supplied in the following order:
+ *       <pre>method([positionals][other user-args]
+ *       (Location)(FuncallExpression)(Envrionment)(SkylarkSemantics))</pre>
+ *       where Location, FuncallExpression, Environment, and SkylarkSemantics are supplied by the
+ *       interpreter if and only if useLocation, useAst, useEnvironment, and useSkylarkSemantics are
+ *       specified, respectively.
+ *   <li>The number of method parameters much match the number of annotation-declared parameters
+ *       plus the number of interpreter-supplied parameters.
  * </ul>
  */
 @Target({ElementType.METHOD})
@@ -91,6 +91,8 @@ public @interface SkylarkCallable {
    * If true, the location of the call site will be passed as an argument of the annotated function.
    * (Thus, the annotated method signature must contain Location as a parameter. See the
    * interface-level javadoc for details.)
+   *
+   * <p>This is incompatible with structField=true. If structField is true, this must be false.
    */
   boolean useLocation() default false;
 
@@ -98,6 +100,8 @@ public @interface SkylarkCallable {
    * If true, the AST of the call site will be passed as an argument of the annotated function.
    * (Thus, the annotated method signature must contain FuncallExpression as a parameter. See the
    * interface-level javadoc for details.)
+   *
+   * <p>This is incompatible with structField=true. If structField is true, this must be false.
    */
   boolean useAst() default false;
 
@@ -105,6 +109,16 @@ public @interface SkylarkCallable {
    * If true, the Skylark Environment will be passed as an argument of the annotated function.
    * (Thus, the annotated method signature must contain Environment as a parameter. See the
    * interface-level javadoc for details.)
+   *
+   * <p>This is incompatible with structField=true. If structField is true, this must be false.
    */
   boolean useEnvironment() default false;
+
+  /**
+   * If true, the Skylark semantics will be passed as an argument of the annotated function. (Thus,
+   * the annotated method signature must contain SkylarkSemantics as a parameter. See the
+   * interface-level javadoc for details.)
+   */
+  // TODO(cparsons): This field should work with structField=true.
+  boolean useSkylarkSemantics() default false;
 }
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 ef1dc953b7..f867a32291 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
@@ -65,6 +65,8 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
   private static final String LOCATION = "com.google.devtools.build.lib.events.Location";
   private static final String AST = "com.google.devtools.build.lib.syntax.FuncallExpression";
   private static final String ENVIRONMENT = "com.google.devtools.build.lib.syntax.Environment";
+  private static final String SKYLARK_SEMANTICS =
+      "com.google.devtools.build.lib.syntax.SkylarkSemantics";
 
   @Override
   public synchronized void init(ProcessingEnvironment processingEnv) {
@@ -86,6 +88,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
       }
 
       try {
+        verifyNotStructFieldWithInvalidExtraParams(methodElement, annotation);
         verifyParamSemantics(methodElement, annotation);
         verifyNumberOfParameters(methodElement, annotation);
         verifyExtraInterpreterParams(methodElement, annotation);
@@ -97,6 +100,19 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
     return true;
   }
 
+  private void verifyNotStructFieldWithInvalidExtraParams(
+      ExecutableElement methodElement, SkylarkCallable annotation)
+      throws SkylarkCallableProcessorException {
+    if (annotation.structField()) {
+      if (annotation.useAst() || annotation.useEnvironment() || annotation.useAst()) {
+        throw new SkylarkCallableProcessorException(
+            methodElement,
+            "@SkylarkCallable-annotated methods with structField=true may not also specify "
+                + "useAst, useEnvironment, or useLocation");
+      }
+    }
+  }
+
   private void verifyParamSemantics(ExecutableElement methodElement, SkylarkCallable annotation)
       throws SkylarkCallableProcessorException {
     for (Param parameter : annotation.parameters()) {
@@ -149,11 +165,14 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
       }
     }
     if (annotation.structField()) {
-      if (methodSignatureParams.size() > 0) {
-        // TODO(cparsons): Allow structField methods to accept interpreter-supplied arguments.
+      if (methodSignatureParams.size() != numExtraInterpreterParams) {
         throw new SkylarkCallableProcessorException(
             methodElement,
-            "@SkylarkCallable annotated methods with structField=true must have zero arguments.");
+            String.format(
+                "@SkylarkCallable annotated methods with structField=true must have "
+                    + "0 user-supplied parameters. Expected %d extra interpreter parameters, "
+                    + "but found %d total parameters.",
+                numExtraInterpreterParams, methodSignatureParams.size()));
       }
     }
   }
@@ -199,6 +218,19 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
                 ENVIRONMENT,
                 methodSignatureParams.get(currentIndex).asType().toString()));
       }
+      currentIndex++;
+    }
+    if (annotation.useSkylarkSemantics()) {
+      if (!SKYLARK_SEMANTICS.equals(methodSignatureParams.get(currentIndex).asType().toString())) {
+        throw new SkylarkCallableProcessorException(
+            methodElement,
+            String.format(
+                "Expected parameter index %d to be the %s type, matching useSkylarkSemantics, "
+                    + "but was %s",
+                currentIndex,
+                SKYLARK_SEMANTICS,
+                methodSignatureParams.get(currentIndex).asType().toString()));
+      }
     }
   }
 
@@ -207,6 +239,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor {
     numExtraInterpreterParams += annotation.useLocation() ? 1 : 0;
     numExtraInterpreterParams += annotation.useAst() ? 1 : 0;
     numExtraInterpreterParams += annotation.useEnvironment() ? 1 : 0;
+    numExtraInterpreterParams += annotation.useSkylarkSemantics() ? 1 : 0;
     return numExtraInterpreterParams;
   }