Migrate tests to ctx.actions.run/run_shell.

RELNOTES: None.
PiperOrigin-RevId: 160817326

4 files changed, 26 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 39f435cdb6..4474a76cd3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -566,10 +566,11 @@ public abstract class AbstractAction implements Action, SkylarkValue {
 
   @SkylarkCallable(
       name = "argv",
-      doc = "For actions created by <a href=\"ctx.html#action\">ctx.action()</a>, an immutable "
-          + "list of the arguments for the command line to be executed. Note that when using the "
-          + "shell (i.e., when <a href=\"ctx.html#action.executable\">executable</a> is not set), "
-          + "the first two arguments will be the shell path and <code>\"-c\"</code>.",
+      doc = "For actions created by <a href=\"actions.html#run\">ctx.actions.run()</a> "
+          + "or <a href=\"actions.html#run_shell\">ctx.actions.run_shell()</a>  an immutable "
+          + "list of the arguments for the command line to be executed. Note that "
+          + "for shell actions the first two arguments will be the shell path "
+          + "and <code>\"-c\"</code>.",
       structField = true,
       allowReturnNones = true)
   public SkylarkList<String> getSkylarkArgv() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
index fb85a7e2e5..2fad61119a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkActionFactory.java
@@ -514,7 +514,6 @@ public class SkylarkActionFactory implements SkylarkValue {
     context.checkMutable("actions.run_shell");
 
     // TODO(bazel-team): builder still makes unnecessary copies of inputs, outputs and args.
-    boolean hasCommand = true;
     SpawnAction.Builder builder = new SpawnAction.Builder();
     if (arguments.size() > 0) {
       // When we use a shell command, add an empty argument before other arguments.
@@ -523,7 +522,10 @@ public class SkylarkActionFactory implements SkylarkValue {
       // arg1 and arg2 will be $1 and $2, as a user expects.
       builder.addArgument("");
     }
-    builder.addArguments(arguments.getContents(String.class, "arguments"));
+
+    @SuppressWarnings("unchecked")
+    List<String> argumentsContents = arguments.getContents(String.class, "arguments");
+    builder.addArguments(argumentsContents);
 
     if (commandUnchecked instanceof String) {
       builder.setShellCommand((String) commandUnchecked);
@@ -532,7 +534,9 @@ public class SkylarkActionFactory implements SkylarkValue {
       if (commandList.size() < 3) {
         throw new EvalException(null, "'command' list has to be of size at least 3");
       }
-      builder.setShellCommand(commandList.getContents(String.class, "command"));
+      @SuppressWarnings("unchecked")
+      List<String> command = commandList.getContents(String.class, "command");
+      builder.setShellCommand(command);
     } else {
       throw new EvalException(
           null,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
index 943906d9a8..1be40eb9d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
@@ -72,8 +72,10 @@ public class SkylarkRuleImplementationFunctions {
   @SkylarkSignature(
     name = "action",
     doc =
-        "Creates an action that runs an executable or a shell command. You must specify either "
-            + "<code>command</code> or <code>executable</code>.\n"
+        "DEPRECATED. Use <a href=\"acttions.html#run\">ctx.actions.run()</a> or"
+            + " <a href=\"acttions.html#run_shell\">ctx.actions.run_shell()</a>. <br>"
+            + "Creates an action that runs an executable or a shell command."
+            + " You must specify either <code>command</code> or <code>executable</code>.\n"
             + "Actions and genrules are very similar, but have different use cases. Actions are "
             + "used inside rules, and genrules are used inside macros. Genrules also have make "
             + "variable expansion.",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 682b15f629..6caf43fc2f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -21,6 +21,7 @@ import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.skylarkinterface.Param;
@@ -39,8 +40,8 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -462,7 +463,7 @@ public final class FuncallExpression extends Expression {
     }
 
     // Then the parameters specified in callable.parameters()
-    Set<String> keys = new HashSet<>(kwargs.keySet());
+    Set<String> keys = new LinkedHashSet<>(kwargs.keySet());
     for (Param param : callable.parameters()) {
       SkylarkType type = getType(param);
       if (param.noneable()) {
@@ -502,9 +503,15 @@ public final class FuncallExpression extends Expression {
             String.format("parameter '%s' cannot be None", param.name()));
       }
     }
-    if (i < args.size() || !keys.isEmpty()) {
+    if (i < args.size()) {
       return ArgumentListConversionResult.fromError("too many arguments");
     }
+    if (!keys.isEmpty()) {
+      return ArgumentListConversionResult.fromError(
+          String.format("unexpected keyword%s %s",
+          keys.size() > 1 ? "s" : "",
+          Joiner.on(",").join(Iterables.transform(keys, s -> "'" + s + "'"))));
+    }
     return ArgumentListConversionResult.fromArgumentList(builder.build());
   }