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

5 files changed, 50 insertions, 7 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 23d9142131..36c172b5f7 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -820,6 +820,17 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testStructRestrictedOverrides() throws Exception {
+    checkErrorContains(
+        "cannot override built-in struct function 'to_json'",
+        "struct(to_json='foo')");
+
+    checkErrorContains(
+        "cannot override built-in struct function 'to_proto'",
+        "struct(to_proto='foo')");
+  }
+
+  @Test
   public void testSimpleTextMessages() throws Exception {
     checkTextMessage("struct(name='value').to_proto()", "name: \"value\"");
     checkTextMessage("struct(name=['a', 'b']).to_proto()", "name: \"a\"", "name: \"b\"");
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 208e9a3e66..ae4a449854 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
@@ -74,7 +74,7 @@ public final class SkylarkCallableProcessorTest {
         .failsToCompile()
         .withErrorContaining(
             "@SkylarkCallable-annotated methods with structField=true may not also specify "
-                + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords");
+                + "useAst, extraPositionals, or extraKeywords");
   }
 
   @Test
@@ -85,7 +85,7 @@ public final class SkylarkCallableProcessorTest {
         .failsToCompile()
         .withErrorContaining(
             "@SkylarkCallable-annotated methods with structField=true may not also specify "
-                + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords");
+                 + "useAst, extraPositionals, or extraKeywords");
   }
 
   @Test
@@ -96,7 +96,7 @@ public final class SkylarkCallableProcessorTest {
         .failsToCompile()
         .withErrorContaining(
             "@SkylarkCallable-annotated methods with structField=true may not also specify "
-                + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords");
+                + "useAst, extraPositionals, or extraKeywords");
   }
 
   @Test
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 580be01a71..6df1d649a9 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
@@ -211,4 +211,18 @@ public class GoldenCase {
   public Integer selfCallMethod(String one, Integer two) {
     return 0;
   }
+
+  @SkylarkCallable(
+      name = "struct_field_method_with_extra_args",
+      documented = false,
+      structField = true,
+      useLocation = true,
+      useEnvironment = true,
+      useSkylarkSemantics = true
+  )
+  public String structFieldMethodWithInfo(Location location,
+      Environment environment,
+      SkylarkSemantics skylarkSemantics) {
+    return "dragon";
+  }
 }
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
index 2843928d5c..c66e44b49e 100644
--- 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
@@ -15,7 +15,7 @@
 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;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
 
 /** Test case which verifies a struct field method cannot specify useEnvironment. */
 public class StructFieldWithInvalidInfo {
@@ -24,9 +24,9 @@ public class StructFieldWithInvalidInfo {
     name = "struct_field_method_with_info",
     documented = false,
     structField = true,
-    useEnvironment = true
+    useAst = true
   )
-  public String structFieldMethodWithInfo(Environment environment) {
+  public String structFieldMethodWithInfo(FuncallExpression ast) {
     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 e376a6d693..68a6909304 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
@@ -137,6 +137,15 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     public String structField() {
       return "a";
     }
+    @SkylarkCallable(name = "struct_field_with_extra",
+        documented = false,
+        structField = true,
+        useSkylarkSemantics = true)
+    public String structFieldWithExtra(SkylarkSemantics sem) {
+      return "struct_field_with_extra("
+        + (sem != null)
+        + ")";
+    }
     @SkylarkCallable(name = "struct_field_callable", documented = false, structField = true)
     public BuiltinFunction structFieldCallable() {
       return foobar;
@@ -1249,6 +1258,14 @@ public class SkylarkEvaluationTest extends EvaluationTest {
   }
 
   @Test
+  public void testStructFieldWithExtraInterpreterParams() throws Exception {
+    new SkylarkTest()
+        .update("mock", new Mock())
+        .setUp("v = mock.struct_field_with_extra")
+        .testLookup("v", "struct_field_with_extra(true)");
+  }
+
+  @Test
   public void testJavaFunctionWithParamsAndExtraInterpreterParams() throws Exception {
     new SkylarkTest()
         .update("mock", new Mock())
@@ -1851,6 +1868,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
             "string_list_dict",
             "struct_field",
             "struct_field_callable",
+            "struct_field_with_extra",
             "value_of",
             "voidfunc",
             "with_args_and_env",
@@ -2064,7 +2082,7 @@ public class SkylarkEvaluationTest extends EvaluationTest {
     new SkylarkTest()
         .update("val", new SkylarkClassObjectWithSkylarkCallables())
         .setUp("v = val.collision_field")
-        .testLookup("v", "fromValues");
+        .testLookup("v", "fromSkylarkCallable");
   }
 
   @Test