Require the "command" param of run_shell is a string

This constitutes an incompatible change guarded by flag --incompatible_run_shell_command_string.

Progress toward bazelbuild#5903.

RELNOTES: The `command` parameter of the `actions.run_shell()` function will be restricted to only accept strings (and not string sequences). This check is attached to flag `--incompatible_run_shell_command_string`. One may migrate by using the `arguments` parameter of `actions.run()` instead. See bazelbuild#5903 for more info.
PiperOrigin-RevId: 253122136

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java

@@ -310,7 +310,8 @@ public void runShell(
       Object envUnchecked,
       Object executionRequirementsUnchecked,
       Object inputManifestsUnchecked,
-      Location location)
+      Location location,
+      StarlarkSemantics semantics)
       throws EvalException {
     context.checkMutable("actions.run_shell");
 
@@ -338,6 +339,13 @@ public void runShell(
         }
       }
     } else if (commandUnchecked instanceof SkylarkList) {
+      if (semantics.incompatibleRunShellCommandString()) {
+        throw new EvalException(
+            location,
+            "'command' must be of type string. passing a sequence of strings as 'command'"
+                + " is deprecated. To temporarily disable this check,"
+                + " set --incompatible_objc_framework_cleanup=false.");
+      }
       SkylarkList commandList = (SkylarkList) commandUnchecked;
       if (argumentList.size() > 0) {
         throw new EvalException(location,

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java

@@ -919,7 +919,8 @@ public Runtime.NoneType action(
               envUnchecked,
               executionRequirementsUnchecked,
               inputManifestsUnchecked,
-              loc);
+              loc,
+              env.getSemantics());
     }
     return Runtime.NONE;
   }

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -503,6 +503,18 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "will be available")
   public boolean incompatibleRemoveNativeMavenJar;
 
+  @Option(
+      name = "incompatible_run_shell_command_string",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help = "If set to true, the command parameter of actions.run_shell will only accept string")
+  public boolean incompatibleRunShellCommandString;
+
   /** Used in an integration test to confirm that flags are visible to the interpreter. */
   @Option(
       name = "internal_skylark_flag_test_canary",
@@ -625,6 +637,7 @@ public StarlarkSemantics toSkylarkSemantics() {
         .incompatibleRemapMainRepo(incompatibleRemapMainRepo)
         .incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
         .incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)
+        .incompatibleRunShellCommandString(incompatibleRunShellCommandString)
         .incompatibleStringJoinRequiresStrings(incompatibleStringJoinRequiresStrings)
         .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
         .incompatibleDoNotSplitLinkingCmdline(incompatibleDoNotSplitLinkingCmdline)

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 
 /** Module providing functions to create actions. */
 @SkylarkModule(
@@ -409,7 +410,9 @@ public void run(
                     + "<p><b>(Deprecated)</b> If <code>command</code> is a sequence of strings, "
                     + "the first item is the executable to run and the remaining items are its "
                     + "arguments. If this form is used, the <code>arguments</code> parameter must "
-                    + "not be supplied."
+                    + "not be supplied. <i>Note that this form is deprecated and will soon "
+                    + "be removed. It is disabled with `--incompatible_run_shell_command_string`. "
+                    + "Use this flag to verify your code is compatible. </i>"
                     + ""
                     + "<p>Bazel uses the same shell to execute the command as it does for "
                     + "genrules."),
@@ -463,9 +466,10 @@ public void run(
                 "(Experimental) sets the input runfiles metadata; "
                     + "they are typically generated by resolve_command.")
       },
+      useStarlarkSemantics = true,
       useLocation = true)
   public void runShell(
-      SkylarkList outputs,
+      SkylarkList<?> outputs,
       Object inputs,
       Object toolsUnchecked,
       Object arguments,
@@ -476,7 +480,8 @@ public void runShell(
       Object envUnchecked,
       Object executionRequirementsUnchecked,
       Object inputManifestsUnchecked,
-      Location location)
+      Location location,
+      StarlarkSemantics semantics)
       throws EvalException;
 
   @SkylarkCallable(

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -191,6 +191,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleRestrictNamedParams();
 
+  public abstract boolean incompatibleRunShellCommandString();
+
   public abstract boolean incompatibleStringJoinRequiresStrings();
 
   public abstract boolean internalSkylarkFlagTestCanary();
@@ -264,6 +266,7 @@ public static Builder builderWithDefaults() {
           .incompatibleObjcFrameworkCleanup(true)
           .incompatibleRemapMainRepo(false)
           .incompatibleRemoveNativeMavenJar(false)
+          .incompatibleRunShellCommandString(false)
           .incompatibleRestrictNamedParams(false)
           .incompatibleStringJoinRequiresStrings(true)
           .internalSkylarkFlagTestCanary(false)
@@ -348,6 +351,8 @@ public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllo
 
     public abstract Builder incompatibleRestrictNamedParams(boolean value);
 
+    public abstract Builder incompatibleRunShellCommandString(boolean value);
+
     public abstract Builder incompatibleStringJoinRequiresStrings(boolean value);
 
     public abstract Builder internalSkylarkFlagTestCanary(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -161,6 +161,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--incompatible_remap_main_repo=" + rand.nextBoolean(),
         "--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
         "--incompatible_restrict_named_params=" + rand.nextBoolean(),
+        "--incompatible_run_shell_command_string=" + rand.nextBoolean(),
         "--incompatible_string_join_requires_strings=" + rand.nextBoolean(),
         "--incompatible_restrict_string_escapes=" + rand.nextBoolean(),
         "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
@@ -212,6 +213,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleRemapMainRepo(rand.nextBoolean())
         .incompatibleRemoveNativeMavenJar(rand.nextBoolean())
         .incompatibleRestrictNamedParams(rand.nextBoolean())
+        .incompatibleRunShellCommandString(rand.nextBoolean())
         .incompatibleStringJoinRequiresStrings(rand.nextBoolean())
         .incompatibleRestrictStringEscapes(rand.nextBoolean())
         .internalSkylarkFlagTestCanary(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java

@@ -2955,6 +2955,26 @@ public void testExecutableNotInRunfiles() throws Exception {
     assertContainsEvent("exe not included in runfiles");
   }
 
+  @Test
+  public void testCommandStringList() throws Exception {
+    setSkylarkSemanticsOptions("--incompatible_run_shell_command_string");
+    scratch.file(
+        "test/skylark/test_rule.bzl",
+        "def _my_rule_impl(ctx):",
+        "  exe = ctx.actions.declare_file('exe')",
+        "  ctx.actions.run_shell(outputs=[exe], command=['touch', 'exe'])",
+        "  return []",
+        "my_rule = rule(implementation = _my_rule_impl)");
+    scratch.file(
+        "test/skylark/BUILD",
+        "load('//test/skylark:test_rule.bzl', 'my_rule')",
+        "my_rule(name = 'target')");
+
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//test/skylark:target");
+    assertContainsEvent("'command' must be of type string");
+  }
+
   /**
    * Skylark integration test that forces inlining.
    */