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

6 files changed, 136 insertions, 15 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java
index 7e7291a14e..36507cb19c 100644
--- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java
@@ -61,7 +61,20 @@ public final class SkylarkCallableProcessorTest {
         .processedWith(new SkylarkCallableProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "@SkylarkCallable annotated methods with structField=true must have zero arguments.");
+            "@SkylarkCallable annotated methods with structField=true must have 0 user-supplied "
+                + "parameters. Expected 0 extra interpreter parameters, "
+                + "but found 1 total parameters.");
+  }
+
+  @Test
+  public void testStructFieldWithInvalidInfo() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("StructFieldWithInvalidInfo.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "@SkylarkCallable-annotated methods with structField=true may not also specify "
+                + "useAst, useEnvironment, or useLocation");
   }
 
   @Test
@@ -101,9 +114,9 @@ public final class SkylarkCallableProcessorTest {
   }
 
   @Test
-  public void testSkylarkInfoWrongOrder() throws Exception {
+  public void testSkylarkInfoBeforeParams() throws Exception {
     assertAbout(javaSource())
-        .that(getFile("SkylarkInfoWrongOrder.java"))
+        .that(getFile("SkylarkInfoBeforeParams.java"))
         .processedWith(new SkylarkCallableProcessor())
         .failsToCompile()
         .withErrorContaining(
@@ -113,6 +126,19 @@ public final class SkylarkCallableProcessorTest {
   }
 
   @Test
+  public void testSkylarkInfoParamsWrongOrder() throws Exception {
+    assertAbout(javaSource())
+        .that(getFile("SkylarkInfoParamsWrongOrder.java"))
+        .processedWith(new SkylarkCallableProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "Expected parameter index 1 to be the "
+                + Location.class.getCanonicalName()
+                + " type, matching useLocation, but was "
+                + Environment.class.getCanonicalName());
+  }
+
+  @Test
   public void testTooManyArguments() throws Exception {
     assertAbout(javaSource())
         .that(getFile("TooManyArguments.java"))
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java
index 1eb207719f..e20c99bbc7 100644
--- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.skylarkinterface.ParamType;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.FuncallExpression;
+import com.google.devtools.build.lib.syntax.SkylarkSemantics;
 
 /**
  * Test source file verifying various proper uses of SkylarkCallable.
@@ -35,6 +36,16 @@ public class GoldenCase {
   }
 
   @SkylarkCallable(
+    name = "struct_field_method_with_info",
+    doc = "",
+    structField = true,
+    useSkylarkSemantics = true
+  )
+  public String structFieldMethodWithInfo(SkylarkSemantics semantics) {
+    return "foo";
+  }
+
+  @SkylarkCallable(
     name = "zero_arg_method",
     doc = "")
   public Integer zeroArgMethod() {
@@ -51,9 +62,14 @@ public class GoldenCase {
     doc = "",
     useAst = true,
     useLocation = true,
-    useEnvironment = true
+    useEnvironment = true,
+    useSkylarkSemantics = true
   )
-  public Integer zeroArgMethod(Location location, FuncallExpression ast, Environment environment) {
+  public Integer zeroArgMethod(
+      Location location,
+      FuncallExpression ast,
+      Environment environment,
+      SkylarkSemantics semantics) {
     return 0;
   }
 
@@ -96,7 +112,8 @@ public class GoldenCase {
     },
     useAst = true,
     useLocation = true,
-    useEnvironment = true
+    useEnvironment = true,
+    useSkylarkSemantics = true
   )
   public String threeArgMethodWithParams(
       String one,
@@ -104,7 +121,8 @@ public class GoldenCase {
       String three,
       Location location,
       FuncallExpression ast,
-      Environment environment) {
+      Environment environment,
+      SkylarkSemantics skylarkSemantics) {
     return "baz";
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoWrongOrder.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoBeforeParams.java
index 9e58070c67..d52605a2ab 100644
--- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoWrongOrder.java
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoBeforeParams.java
@@ -22,10 +22,10 @@ import com.google.devtools.build.lib.syntax.Environment;
  * Test case for a SkylarkCallable method which specifies skylark-info parameters (for example
  * Environment) before other parameters.
  */
-public class SkylarkInfoWrongOrder {
+public class SkylarkInfoBeforeParams {
 
   @SkylarkCallable(
-    name = "three_arg_method_missing_location",
+    name = "skylark_info_wrong_order",
     doc = "",
     useLocation = true,
     useEnvironment = true
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoParamsWrongOrder.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoParamsWrongOrder.java
new file mode 100644
index 0000000000..e82b1a1c6c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/SkylarkInfoParamsWrongOrder.java
@@ -0,0 +1,38 @@
+// 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.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.syntax.Environment;
+
+/**
+ * Test case for a SkylarkCallable method which specifies skylark-info parameters in the incorrect
+ * relative order.
+ */
+public class SkylarkInfoParamsWrongOrder {
+
+  @SkylarkCallable(
+    name = "skylark_info_params_wrong_order",
+    doc = "",
+    useLocation = true,
+    useEnvironment = true
+  )
+  public String threeArgMethod(
+      // Note environment should come after location.
+      String someParam, Environment environment, Location location) {
+    return "bar";
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithInvalidInfo.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithInvalidInfo.java
new file mode 100644
index 0000000000..0c5b9267fe
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithInvalidInfo.java
@@ -0,0 +1,32 @@
+// 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.skylarkinterface.processor.testsources;
+
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.syntax.Environment;
+
+/** Test case which verifies a struct field method cannot specify useEnvironment. */
+public class StructFieldWithInvalidInfo {
+
+  @SkylarkCallable(
+    name = "struct_field_method_with_info",
+    doc = "",
+    structField = true,
+    useEnvironment = true
+  )
+  public String structFieldMethodWithInfo(Environment environment) {
+    return "dragon";
+  }
+}
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 d49d0d59cf..4a1a09dee7 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
@@ -244,16 +244,19 @@ public class SkylarkEvaluationTest extends EvaluationTest {
       doc = "",
       useLocation = true,
       useAst = true,
-      useEnvironment = true
+      useEnvironment = true,
+      useSkylarkSemantics = true
     )
     public String withExtraInterpreterParams(
-        Location location, FuncallExpression func, Environment env) {
+        Location location, FuncallExpression func, Environment env, SkylarkSemantics sem) {
       return "with_extra("
           + location.getStartLine()
           + ", "
           + func.getArguments().size()
           + ", "
           + env.isGlobal()
+          + ", "
+          + (sem != null)
           + ")";
     }
 
@@ -308,7 +311,8 @@ public class SkylarkEvaluationTest extends EvaluationTest {
       },
       useAst = true,
       useLocation = true,
-      useEnvironment = true
+      useEnvironment = true,
+      useSkylarkSemantics = true
     )
     public String withParamsAndExtraInterpreterParams(
         Integer pos1,
@@ -321,7 +325,8 @@ public class SkylarkEvaluationTest extends EvaluationTest {
         Object multi,
         Location location,
         FuncallExpression func,
-        Environment env) {
+        Environment env,
+        SkylarkSemantics sem) {
       return "with_params_and_extra("
           + pos1
           + ", "
@@ -342,6 +347,8 @@ public class SkylarkEvaluationTest extends EvaluationTest {
           + func.getArguments().size()
           + ", "
           + env.isGlobal()
+          + ", "
+          + (sem != null)
           + ")";
     }
 
@@ -1034,7 +1041,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     new SkylarkTest()
         .update("mock", new Mock())
         .setUp("v = mock.with_extra()")
-        .testLookup("v", "with_extra(1, 0, true)");
+        .testLookup("v", "with_extra(1, 0, true, true)");
   }
 
   @Test
@@ -1042,7 +1049,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     new SkylarkTest()
         .update("mock", new Mock())
         .setUp("b = mock.with_params_and_extra(1, True, named=True)")
-        .testLookup("b", "with_params_and_extra(1, true, false, true, false, a, 1, 3, true)");
+        .testLookup("b", "with_params_and_extra(1, true, false, true, false, a, 1, 3, true, true)");
   }
 
   @Test