1 files changed, 29 insertions, 9 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 691a1e23a9..a3878a6300 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.CommandLine;
+import com.google.devtools.build.lib.actions.CommandLineItemSimpleFormatter;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -1144,6 +1145,7 @@ public class SkylarkActionFactory implements SkylarkValue {
             false /* uniquify */,
             null /* terminateWith */,
             loc);
+
       } else {
         if (mapFn != Runtime.NONE && skylarkSemantics.incompatibleDisallowOldStyleArgsAdd()) {
           throw new EvalException(
@@ -1254,7 +1256,8 @@ public class SkylarkActionFactory implements SkylarkValue {
           noneable = true,
           doc =
               "An optional format string pattern, applied to each string returned by the "
-                  + "<code>map_each</code> function."
+                  + "<code>map_each</code> function. "
+                  + "The format string must have exactly one '%s' placeholder."
         ),
         @Param(
           name = "before_each",
@@ -1423,7 +1426,9 @@ public class SkylarkActionFactory implements SkylarkValue {
           positional = false,
           defaultValue = "None",
           noneable = true,
-          doc = "An optional format string pattern applied to the joined string."
+          doc =
+              "An optional format string pattern applied to the joined string. "
+                  + "The format string must have exactly one '%s' placeholder."
         ),
         @Param(
           name = "omit_if_empty",
@@ -1511,9 +1516,9 @@ public class SkylarkActionFactory implements SkylarkValue {
         SkylarkList skylarkList = (SkylarkList) value;
         vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(skylarkList);
       }
-      if (mapEach != null) {
-        validateMapEach(mapEach, loc);
-      }
+      validateMapEach(mapEach, loc);
+      validateFormatString("format_each", formatEach);
+      validateFormatString("format_joined", formatJoined);
       vectorArg
           .setLocation(loc)
           .setArgName(argName)
@@ -1549,7 +1554,11 @@ public class SkylarkActionFactory implements SkylarkValue {
       }
     }
 
-    private void validateMapEach(BaseFunction mapEach, Location loc) throws EvalException {
+    private void validateMapEach(@Nullable BaseFunction mapEach, Location loc)
+        throws EvalException {
+      if (mapEach == null) {
+        return;
+      }
       Shape shape = mapEach.getSignature().getSignature().getShape();
       boolean valid =
           shape.getMandatoryPositionals() == 1
@@ -1562,11 +1571,22 @@ public class SkylarkActionFactory implements SkylarkValue {
       }
     }
 
-    private void addScalarArg(Object value, String format, BaseFunction mapFn, Location loc)
+    private void validateFormatString(String argumentName, @Nullable String formatStr)
         throws EvalException {
-      if (!EvalUtils.isImmutable(value)) {
-        throw new EvalException(null, "arg must be an immutable type");
+      if (formatStr != null
+          && skylarkSemantics.incompatibleDisallowOldStyleArgsAdd()
+          && !CommandLineItemSimpleFormatter.isValid(formatStr)) {
+        throw new EvalException(
+            null,
+            String.format(
+                "Invalid value for parameter \"%s\": Expected string with a single \"%%s\"",
+                argumentName));
       }
+    }
+
+    private void addScalarArg(Object value, String format, BaseFunction mapFn, Location loc)
+        throws EvalException {
+      validateFormatString("format", format);
       if (format == null && mapFn == null) {
         commandLine.add(value);
       } else {