Add SubruleContext as the first positional parameter to subrule imp…

…lementation functions. PiperOrigin-RevId: 561258193 Change-Id: Ic1743532d5b88de56558965bf4cd96d7752c1e99
bazelbuild · Aug 30, 2023 · 0e633a6 · 0e633a6
1 parent 0c5c892
commit 0e633a6
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.analysis.starlark;
 
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.BazelRuleAnalysisThreadContext;
 import com.google.devtools.build.lib.starlarkbuildapi.StarlarkSubruleApi;
 import net.starlark.java.eval.Dict;
@@ -43,7 +44,11 @@ public String getName() {
   public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
       throws EvalException, InterruptedException {
     BazelRuleAnalysisThreadContext.fromOrFail(thread, getName());
-    // TODO(hvd): inject SubruleContext as first positional arg
-    return Starlark.call(thread, implementation, args, kwargs);
+    SubruleContext subruleContext = new SubruleContext();
+    ImmutableList<Object> positionals =
+        ImmutableList.builder().add(subruleContext).addAll(args).build();
+    return Starlark.call(thread, implementation, positionals, kwargs);
   }
+
+  private static class SubruleContext {}
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis.starlark;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -70,11 +71,35 @@ public void testSubrule_isCallableOnlyFromRuleOrAspectImplementation() throws Ex
         "subrule(lambda) can only be called from a rule or aspect implementation", "x()");
   }
 
+  @Test
+  public void testSubrule_implementationMustAcceptSubruleContext() throws Exception {
+    scratch.file(
+        "subrule_testing/myrule.bzl",
+        "_my_subrule = subrule(implementation = lambda : '')",
+        "",
+        "def _rule_impl(ctx):",
+        "  _my_subrule()",
+        "",
+        "my_rule = rule(implementation = _rule_impl)");
+    scratch.file(
+        "subrule_testing/BUILD",
+        //
+        "load('myrule.bzl', 'my_rule')",
+        "my_rule(name = 'foo')");
+
+    AssertionError error =
+        assertThrows(AssertionError.class, () -> getConfiguredTarget("//subrule_testing:foo"));
+
+    assertThat(error)
+        .hasMessageThat()
+        .contains("Error: lambda() does not accept positional arguments, but got 1");
+  }
+
   @Test
   public void testSubrule_isCallableFromRule() throws Exception {
     scratch.file(
         "subrule_testing/myrule.bzl",
-        "_my_subrule = subrule(implementation = lambda : 'dummy rule result')",
+        "_my_subrule = subrule(implementation = lambda ctx: 'dummy rule result')",
         "",
         "MyInfo = provider()",
         "def _rule_impl(ctx):",
@@ -99,7 +124,7 @@ public void testSubrule_isCallableFromRule() throws Exception {
   public void testSubrule_isCallableFromAspect() throws Exception {
     scratch.file(
         "subrule_testing/myrule.bzl",
-        "_my_subrule = subrule(implementation = lambda : 'dummy aspect result')",
+        "_my_subrule = subrule(implementation = lambda ctx: 'dummy aspect result')",
         "",
         "MyInfo = provider()",
         "def _aspect_impl(ctx,target):",