Split Args#add into three methods.

Args#add(value, *, arg, format)
Args#add_all(value, *, arg, map_each, format_each, before_each, omit_if_empty, uniquify)
Args#add_joined(value, *, arg, join_with, map_each, format_each, format_joined, omit_if_empty, uniquify)

The old Args#add remains backwards compatible, but we add a flag to disable this compatibility mode.

RELNOTES: None
PiperOrigin-RevId: 191804482

7 files changed, 644 insertions, 146 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 89319e92f5..7a3a79d116 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
@@ -50,6 +50,7 @@ import com.google.devtools.build.lib.syntax.BaseFunction;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.FunctionSignature.Shape;
 import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.Runtime.NoneType;
@@ -880,6 +881,7 @@ public class SkylarkActionFactory implements SkylarkValue {
   )
   static class Args extends SkylarkMutable {
     private final Mutability mutability;
+    private final SkylarkSemantics skylarkSemantics;
     private final SkylarkCustomCommandLine.Builder commandLine;
     private ParameterFileType parameterFileType = ParameterFileType.SHELL_QUOTED;
     private String flagFormatString;
@@ -891,13 +893,18 @@ public class SkylarkActionFactory implements SkylarkValue {
       parameters = {
         @Param(
           name = "value",
-          type = Object.class,
+          named = true,
           doc =
               "The object to add to the argument list. "
-                  + "If the object is scalar, the object's string representation is added. "
-                  + "If it's a <a href=\"list.html\">list</a> or "
-                  + "<a href=\"depset.html\">depset</a>, "
-                  + "each element's string representation is added."
+                  + "The object's string representation is added. "
+        ),
+        @Param(
+          name = "arg_name",
+          type = String.class,
+          named = true,
+          defaultValue = "None",
+          noneable = true,
+          doc = "The argument name as a string to add before the value."
         ),
         @Param(
           name = "format",
@@ -909,7 +916,7 @@ public class SkylarkActionFactory implements SkylarkValue {
           doc =
               "A format string used to format the object(s). "
                   + "The format string is as per pattern % tuple. "
-                  + "Limitations: only %d %s %r %% are supported."
+                  + "Limitations: only %s and %% are supported."
         ),
         @Param(
           name = "before_each",
@@ -920,7 +927,9 @@ public class SkylarkActionFactory implements SkylarkValue {
           noneable = true,
           doc =
               "Each object in the list is prepended by this string. "
-                  + "Only supported for vector arguments."
+                  + "Only supported for vector arguments. "
+                  + "Deprecated. Please use <a href=\"Args.html#add_all\">add_all</a>"
+                  + " or <a href=\"Args.html#add_joined\">add_joined</a> instead."
         ),
         @Param(
           name = "join_with",
@@ -931,7 +940,9 @@ public class SkylarkActionFactory implements SkylarkValue {
           noneable = true,
           doc =
               "Each object in the list is joined with this string. "
-                  + "Only supported for vector arguments."
+                  + "Only supported for vector arguments. "
+                  + "Deprecated. Please use <a href=\"Args.html#add_all\">add_all</a>"
+                  + " or <a href=\"Args.html#add_joined\">add_joined</a> instead."
         ),
         @Param(
           name = "map_fn",
@@ -943,36 +954,328 @@ public class SkylarkActionFactory implements SkylarkValue {
           doc =
               "The passed objects are passed through a map function. "
                   + "For vector args the function is given a list and is expected to "
-                  + "return a list of the same length as the input."
+                  + "return a list of the same length as the input. "
+                  + "Deprecated. Please use <a href=\"Args.html#add_all\">add_all</a>"
+                  + " or <a href=\"Args.html#add_joined\">add_joined</a> instead."
         )
       },
       useLocation = true
     )
     public NoneType addArgument(
         Object value,
+        Object argName,
         Object format,
         Object beforeEach,
         Object joinWith,
         Object mapFn,
         Location loc)
         throws EvalException {
-      if (this.isImmutable()) {
+      if (isImmutable()) {
         throw new EvalException(null, "cannot modify frozen value");
       }
+      if (argName != Runtime.NONE) {
+        commandLine.add(argName);
+      }
       if (value instanceof SkylarkNestedSet || value instanceof SkylarkList) {
-        addVectorArg(value, format, beforeEach, joinWith, mapFn, loc);
+        if (skylarkSemantics.incompatibleDisallowOldStyleArgsAdd()) {
+          throw new EvalException(
+              loc,
+              "Args#add no longer accepts vectorized arguments when "
+                  + "--incompatible_disallow_old_style_args_add is set. "
+                  + "Please use Args#add_all or Args#add_joined.");
+        }
+        addVectorArg(
+            value,
+            null /* argName */,
+            mapFn != Runtime.NONE ? (BaseFunction) mapFn : null,
+            null /* mapEach */,
+            format != Runtime.NONE ? (String) format : null,
+            beforeEach != Runtime.NONE ? (String) beforeEach : null,
+            joinWith != Runtime.NONE ? (String) joinWith : null,
+            null /* formatJoined */,
+            false /* omitIfEmpty */,
+            false /* uniquify */,
+            null /* terminateWith */,
+            loc);
       } else {
-        addScalarArg(value, format, beforeEach, joinWith, mapFn, loc);
+        if (mapFn != Runtime.NONE && skylarkSemantics.incompatibleDisallowOldStyleArgsAdd()) {
+          throw new EvalException(
+              loc,
+              "Args#add no longer accepts map_fn when"
+                  + "--incompatible_disallow_old_style_args_add is set. "
+                  + "Please eagerly map the value.");
+        }
+        if (beforeEach != Runtime.NONE) {
+          throw new EvalException(null, "'before_each' is not supported for scalar arguments");
+        }
+        if (joinWith != Runtime.NONE) {
+          throw new EvalException(null, "'join_with' is not supported for scalar arguments");
+        }
+        addScalarArg(
+            value,
+            format != Runtime.NONE ? (String) format : null,
+            mapFn != Runtime.NONE ? (BaseFunction) mapFn : null,
+            loc);
       }
       return Runtime.NONE;
     }
 
-    private void addVectorArg(
-        Object value, Object format, Object beforeEach, Object joinWith, Object mapFn, Location loc)
+    @SkylarkCallable(
+      name = "add_all",
+      doc = "Adds an vector argument to be dynamically expanded at evaluation time.",
+      parameters = {
+        @Param(
+          name = "values",
+          allowedTypes = {
+            @ParamType(type = SkylarkList.class),
+            @ParamType(type = SkylarkNestedSet.class),
+          },
+          named = true,
+          doc = "The sequence to add. Each element's string representation is added."
+        ),
+        @Param(
+          name = "arg_name",
+          type = String.class,
+          named = true,
+          defaultValue = "None",
+          noneable = true,
+          doc = "The argument name as a string to add before the values."
+        ),
+        @Param(
+          name = "map_each",
+          type = BaseFunction.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "Each object is passed through a map function "
+                  + "prior to formatting and joining. "
+                  + "The function is given a single element and is expected to return a string, "
+                  + "a list of strings, or None (in which case the return value is ignored)."
+        ),
+        @Param(
+          name = "format_each",
+          type = String.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "A format string used to format each object in the list. "
+                  + "The format string is as per pattern % tuple. "
+                  + "Limitations: only %s and %% are supported. "
+        ),
+        @Param(
+          name = "before_each",
+          type = String.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "Each object in the list is prepended by this string. "
+                  + "This happens after any mapping and formatting."
+        ),
+        @Param(
+          name = "omit_if_empty",
+          type = Boolean.class,
+          named = true,
+          positional = false,
+          defaultValue = "True",
+          doc =
+              "Omits the arg_name and terminate_with (if passed) if the values end up empty, "
+                  + "either because empty values was passed, "
+                  + "or because map_each filtered out the values."
+        ),
+        @Param(
+          name = "uniquify",
+          type = Boolean.class,
+          named = true,
+          positional = false,
+          defaultValue = "False",
+          doc =
+              "Omits non-unique values. Order is preserved. "
+                  + "This is usually not needed as depsets already omit duplicates."
+        ),
+        @Param(
+          name = "terminate_with",
+          type = String.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc = "Adds a terminating argument last, after all other arguments have been added."
+        ),
+      },
+      useLocation = true
+    )
+    public NoneType addAll(
+        Object value,
+        Object argName,
+        Object mapEach,
+        Object formatEach,
+        Object beforeEach,
+        Boolean omitIfEmpty,
+        Boolean uniquify,
+        Object terminateWith,
+        Location loc)
+        throws EvalException {
+      if (isImmutable()) {
+        throw new EvalException(null, "cannot modify frozen value");
+      }
+      addVectorArg(
+          value,
+          argName != Runtime.NONE ? (String) argName : null,
+          null /* mapAll */,
+          mapEach != Runtime.NONE ? (BaseFunction) mapEach : null,
+          formatEach != Runtime.NONE ? (String) formatEach : null,
+          beforeEach != Runtime.NONE ? (String) beforeEach : null,
+          null /* joinWith */,
+          null /* formatJoined */,
+          omitIfEmpty,
+          uniquify,
+          terminateWith != Runtime.NONE ? (String) terminateWith : null,
+          loc);
+      return Runtime.NONE;
+    }
+
+    @SkylarkCallable(
+      name = "add_joined",
+      doc =
+          "Adds a vector argument to be dynamically expanded at evaluation time "
+              + "and joined with a string.",
+      parameters = {
+        @Param(
+          name = "values",
+          allowedTypes = {
+            @ParamType(type = SkylarkList.class),
+            @ParamType(type = SkylarkNestedSet.class),
+          },
+          named = true,
+          doc = "The sequence to add. Each element's string representation is joined and added."
+        ),
+        @Param(
+          name = "arg_name",
+          type = String.class,
+          named = true,
+          defaultValue = "None",
+          noneable = true,
+          doc = "The argument name as a string to add before the values."
+        ),
+        @Param(
+          name = "join_with",
+          type = String.class,
+          named = true,
+          positional = false,
+          doc = "Each object in the supplied list is joined with this string. "
+        ),
+        @Param(
+          name = "map_each",
+          type = BaseFunction.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "Each object is passed through a map function "
+                  + "prior to formatting and joining. "
+                  + "The function is given a single element and is expected to return a string, "
+                  + "a list of strings, or None (in which case the return value is ignored)."
+        ),
+        @Param(
+          name = "format_each",
+          type = String.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "A format string used to format each object in the list prior to joining. "
+                  + "The format string is as per pattern % tuple. "
+                  + "Limitations: only %s and %% are supported. "
+                  + "Only supported for vector arguments."
+        ),
+        @Param(
+          name = "format_joined",
+          type = String.class,
+          named = true,
+          positional = false,
+          defaultValue = "None",
+          noneable = true,
+          doc =
+              "A format string used to format the final joined string. "
+                  + "The format string is as per pattern % tuple. "
+                  + "Limitations: only %s and %% are supported."
+        ),
+        @Param(
+          name = "omit_if_empty",
+          type = Boolean.class,
+          named = true,
+          positional = false,
+          defaultValue = "True",
+          doc =
+              "Omits the joined value when the passed objects are empty. "
+                  + "In case 'arg_name' was passed it is also omitted. "
+                  + "If false, the joined string is always added, even if it is the empty string."
+        ),
+        @Param(
+          name = "uniquify",
+          type = Boolean.class,
+          named = true,
+          positional = false,
+          defaultValue = "False",
+          doc =
+              "Omits non-unique values. Order is preserved. "
+                  + "This is usually not needed as depsets already omit duplicates."
+        )
+      },
+      useLocation = true
+    )
+    public NoneType addJoined(
+        Object value,
+        Object argName,
+        String joinWith,
+        Object mapEach,
+        Object formatEach,
+        Object formatJoined,
+        Boolean omitIfEmpty,
+        Boolean uniquify,
+        Location loc)
         throws EvalException {
-      if (beforeEach != Runtime.NONE && joinWith != Runtime.NONE) {
-        throw new EvalException(null, "cannot pass both 'before_each' and 'join_with'");
+      if (isImmutable()) {
+        throw new EvalException(null, "cannot modify frozen value");
       }
+      addVectorArg(
+          value,
+          argName != Runtime.NONE ? (String) argName : null,
+          null /* mapAll */,
+          mapEach != Runtime.NONE ? (BaseFunction) mapEach : null,
+          formatEach != Runtime.NONE ? (String) formatEach : null,
+          null /* beforeEach */,
+          joinWith,
+          formatJoined != Runtime.NONE ? (String) formatJoined : null,
+          omitIfEmpty,
+          uniquify,
+          null /* terminateWith */,
+          loc);
+      return Runtime.NONE;
+    }
+
+    private void addVectorArg(
+        Object value,
+        String argName,
+        BaseFunction mapAll,
+        BaseFunction mapEach,
+        String formatEach,
+        String beforeEach,
+        String joinWith,
+        String formatJoined,
+        boolean omitIfEmpty,
+        boolean uniquify,
+        String terminateWith,
+        Location loc)
+        throws EvalException {
       SkylarkCustomCommandLine.VectorArg.Builder vectorArg;
       if (value instanceof SkylarkNestedSet) {
         NestedSet<?> nestedSet = ((SkylarkNestedSet) value).getSet(Object.class);
@@ -981,45 +1284,47 @@ public class SkylarkActionFactory implements SkylarkValue {
         SkylarkList skylarkList = (SkylarkList) value;
         vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(skylarkList);
       }
-      vectorArg.setLocation(loc);
-      if (format != Runtime.NONE) {
-        vectorArg.setFormat((String) format);
-      }
-      if (beforeEach != Runtime.NONE) {
-        vectorArg.setBeforeEach((String) beforeEach);
-      }
-      if (joinWith != Runtime.NONE) {
-        vectorArg.setJoinWith((String) joinWith);
-      }
-      if (mapFn != Runtime.NONE) {
-        vectorArg.setMapFn((BaseFunction) mapFn);
+      if (mapEach != null) {
+        validateMapEach(mapEach, loc);
       }
+      vectorArg
+          .setLocation(loc)
+          .setArgName(argName)
+          .setMapAll(mapAll)
+          .setFormatEach(formatEach)
+          .setBeforeEach(beforeEach)
+          .setJoinWith(joinWith)
+          .setFormatJoined(formatJoined)
+          .omitIfEmpty(omitIfEmpty)
+          .uniquify(uniquify)
+          .setTerminateWith(terminateWith)
+          .setMapEach(mapEach);
       commandLine.add(vectorArg);
     }
 
-    private void addScalarArg(
-        Object value, Object format, Object beforeEach, Object joinWith, Object mapFn, Location loc)
+    private void validateMapEach(BaseFunction mapEach, Location loc) throws EvalException {
+      Shape shape = mapEach.getSignature().getSignature().getShape();
+      boolean valid =
+          shape.getMandatoryPositionals() == 1
+              && shape.getOptionalPositionals() == 0
+              && shape.getMandatoryNamedOnly() == 0
+              && shape.getOptionalPositionals() == 0;
+      if (!valid) {
+        throw new EvalException(
+            loc, "map_each must be a function that accepts a single positional argument");
+      }
+    }
+
+    private void addScalarArg(Object value, String format, BaseFunction mapFn, Location loc)
         throws EvalException {
       if (!EvalUtils.isImmutable(value)) {
         throw new EvalException(null, "arg must be an immutable type");
       }
-      if (beforeEach != Runtime.NONE) {
-        throw new EvalException(null, "'before_each' is not supported for scalar arguments");
-      }
-      if (joinWith != Runtime.NONE) {
-        throw new EvalException(null, "'join_with' is not supported for scalar arguments");
-      }
-      if (format == Runtime.NONE && mapFn == Runtime.NONE) {
+      if (format == null && mapFn == null) {
         commandLine.add(value);
       } else {
-        ScalarArg.Builder scalarArg = new ScalarArg.Builder(value);
-        scalarArg.setLocation(loc);
-        if (format != Runtime.NONE) {
-          scalarArg.setFormat((String) format);
-        }
-        if (mapFn != Runtime.NONE) {
-          scalarArg.setMapFn((BaseFunction) mapFn);
-        }
+        ScalarArg.Builder scalarArg =
+            new ScalarArg.Builder(value).setLocation(loc).setFormat(format).setMapFn(mapFn);
         commandLine.add(scalarArg);
       }
     }
@@ -1104,6 +1409,7 @@ public class SkylarkActionFactory implements SkylarkValue {
 
     private Args(@Nullable Mutability mutability, SkylarkSemantics skylarkSemantics) {
       this.mutability = mutability != null ? mutability : Mutability.IMMUTABLE;
+      this.skylarkSemantics = skylarkSemantics;
       this.commandLine = new SkylarkCustomCommandLine.Builder(skylarkSemantics);
     }
 
@@ -1128,7 +1434,7 @@ public class SkylarkActionFactory implements SkylarkValue {
     useEnvironment = true
   )
   public Args args(Environment env) {
-    return new Args(env.mutability(), env.getSemantics());
+    return new Args(env.mutability(), skylarkSemantics);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
index ded956388b..3fbb5dbaa8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
@@ -15,10 +15,10 @@ package com.google.devtools.build.lib.analysis.skylark;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Interner;
-import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.actions.CommandLineItem;
@@ -32,9 +32,11 @@ import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkSemantics;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.IllegalFormatException;
 import java.util.List;
 import javax.annotation.Nullable;
@@ -52,54 +54,55 @@ class SkylarkCustomCommandLine extends CommandLine {
   static final class VectorArg {
     private static Interner<VectorArg> interner = BlazeInterners.newStrongInterner();
 
-    private final boolean isNestedSet;
-    private final boolean hasFormat;
-    private final boolean hasBeforeEach;
-    private final boolean hasJoinWith;
-    private final boolean hasMapFn;
-    private final boolean hasLocation;
-
-    private VectorArg(
-        boolean isNestedSet,
-        boolean hasFormat,
-        boolean hasBeforeEach,
-        boolean hasJoinWith,
-        boolean hasMapFn,
-        boolean hasLocation) {
-      this.isNestedSet = isNestedSet;
-      this.hasFormat = hasFormat;
-      this.hasBeforeEach = hasBeforeEach;
-      this.hasJoinWith = hasJoinWith;
-      this.hasMapFn = hasMapFn;
-      this.hasLocation = hasLocation;
+    private static final int IS_NESTED_SET = 1;
+    private static final int HAS_LOCATION = 1 << 1;
+    private static final int HAS_ARG_NAME = 1 << 2;
+    private static final int HAS_MAP_ALL = 1 << 3;
+    private static final int HAS_MAP_EACH = 1 << 4;
+    private static final int HAS_FORMAT_EACH = 1 << 5;
+    private static final int HAS_BEFORE_EACH = 1 << 6;
+    private static final int HAS_JOIN_WITH = 1 << 7;
+    private static final int HAS_FORMAT_JOINED = 1 << 8;
+    private static final int OMIT_IF_EMPTY = 1 << 9;
+    private static final int UNIQUIFY = 1 << 10;
+    private static final int HAS_TERMINATE_WITH = 1 << 11;
+
+    private final int features;
+
+    private VectorArg(int features) {
+      this.features = features;
     }
 
     @AutoCodec.VisibleForSerialization
     @AutoCodec.Instantiator
-    static VectorArg create(
-        boolean isNestedSet,
-        boolean hasFormat,
-        boolean hasBeforeEach,
-        boolean hasJoinWith,
-        boolean hasMapFn,
-        boolean hasLocation) {
-      return interner.intern(
-          new VectorArg(isNestedSet, hasFormat, hasBeforeEach, hasJoinWith, hasMapFn, hasLocation));
+    static VectorArg create(int features) {
+      return interner.intern(new VectorArg(features));
     }
 
     private static void push(ImmutableList.Builder<Object> arguments, Builder arg) {
-      boolean wantsLocation = arg.format != null || arg.mapFn != null;
-      boolean hasLocation = arg.location != null && wantsLocation;
-      VectorArg vectorArg =
-          VectorArg.create(
-              arg.nestedSet != null,
-              arg.format != null,
-              arg.beforeEach != null,
-              arg.joinWith != null,
-              arg.mapFn != null,
-              hasLocation);
+      int features = 0;
+      features |= arg.nestedSet != null ? IS_NESTED_SET : 0;
+      features |= arg.argName != null ? HAS_ARG_NAME : 0;
+      features |= arg.mapAll != null ? HAS_MAP_ALL : 0;
+      features |= arg.mapEach != null ? HAS_MAP_EACH : 0;
+      features |= arg.formatEach != null ? HAS_FORMAT_EACH : 0;
+      features |= arg.beforeEach != null ? HAS_BEFORE_EACH : 0;
+      features |= arg.joinWith != null ? HAS_JOIN_WITH : 0;
+      features |= arg.formatJoined != null ? HAS_FORMAT_JOINED : 0;
+      features |= arg.omitIfEmpty ? OMIT_IF_EMPTY : 0;
+      features |= arg.uniquify ? UNIQUIFY : 0;
+      features |= arg.terminateWith != null ? HAS_TERMINATE_WITH : 0;
+      boolean hasLocation =
+          arg.location != null
+              && (features & (HAS_FORMAT_EACH | HAS_FORMAT_JOINED | HAS_MAP_ALL | HAS_MAP_EACH))
+                  != 0;
+      features |= hasLocation ? HAS_LOCATION : 0;
+      Preconditions.checkState(
+          (features & (HAS_MAP_ALL | HAS_MAP_EACH)) != (HAS_MAP_ALL | HAS_MAP_EACH),
+          "Cannot use both map_all and map_each");
+      VectorArg vectorArg = VectorArg.create(features);
       arguments.add(vectorArg);
-      if (vectorArg.isNestedSet) {
+      if (arg.nestedSet != null) {
         arguments.add(arg.nestedSet);
       } else {
         ImmutableList<?> list = arg.list.getImmutableList();
@@ -112,18 +115,30 @@ class SkylarkCustomCommandLine extends CommandLine {
       if (hasLocation) {
         arguments.add(arg.location);
       }
-      if (vectorArg.hasMapFn) {
-        arguments.add(arg.mapFn);
+      if (arg.argName != null) {
+        arguments.add(arg.argName);
       }
-      if (vectorArg.hasFormat) {
-        arguments.add(arg.format);
+      if (arg.mapAll != null) {
+        arguments.add(arg.mapAll);
       }
-      if (vectorArg.hasBeforeEach) {
+      if (arg.mapEach != null) {
+        arguments.add(arg.mapEach);
+      }
+      if (arg.formatEach != null) {
+        arguments.add(arg.formatEach);
+      }
+      if (arg.beforeEach != null) {
         arguments.add(arg.beforeEach);
       }
-      if (vectorArg.hasJoinWith) {
+      if (arg.joinWith != null) {
         arguments.add(arg.joinWith);
       }
+      if (arg.formatJoined != null) {
+        arguments.add(arg.formatJoined);
+      }
+      if (arg.terminateWith != null) {
+        arguments.add(arg.terminateWith);
+      }
     }
 
     private int eval(
@@ -132,23 +147,26 @@ class SkylarkCustomCommandLine extends CommandLine {
         ImmutableList.Builder<String> builder,
         SkylarkSemantics skylarkSemantics)
         throws CommandLineExpansionException {
-      final List<Object> mutatedValues;
-      final int count;
-      if (isNestedSet) {
-        NestedSet<?> nestedSet = (NestedSet<?>) arguments.get(argi++);
-        mutatedValues = Lists.newArrayList(nestedSet);
-        count = mutatedValues.size();
+      final List<Object> originalValues;
+      if ((features & IS_NESTED_SET) != 0) {
+        NestedSet<Object> nestedSet = (NestedSet<Object>) arguments.get(argi++);
+        originalValues = nestedSet.toList();
       } else {
-        count = (Integer) arguments.get(argi++);
-        mutatedValues = new ArrayList<>(count);
-        for (int i = 0; i < count; ++i) {
-          mutatedValues.add(arguments.get(argi++));
-        }
-      }
-      final Location location = hasLocation ? (Location) arguments.get(argi++) : null;
-      if (hasMapFn) {
+        int count = (Integer) arguments.get(argi++);
+        originalValues = arguments.subList(argi, argi + count);
+        argi += count;
+      }
+      List<String> stringValues;
+      final Location location =
+          ((features & HAS_LOCATION) != 0) ? (Location) arguments.get(argi++) : null;
+      final String argName =
+          ((features & HAS_ARG_NAME) != 0) ? (String) arguments.get(argi++) : null;
+      if ((features & HAS_MAP_EACH) != 0) {
+        BaseFunction mapEach = (BaseFunction) arguments.get(argi++);
+        stringValues = applyMapEach(mapEach, originalValues, location, skylarkSemantics);
+      } else if ((features & HAS_MAP_ALL) != 0) {
         BaseFunction mapFn = (BaseFunction) arguments.get(argi++);
-        Object result = applyMapFn(mapFn, mutatedValues, location, skylarkSemantics);
+        Object result = applyMapFn(mapFn, originalValues, location, skylarkSemantics);
         if (!(result instanceof List)) {
           throw new CommandLineExpansionException(
               errorMessage(
@@ -157,45 +175,90 @@ class SkylarkCustomCommandLine extends CommandLine {
                   null));
         }
         List resultAsList = (List) result;
-        if (resultAsList.size() != count) {
+        if (resultAsList.size() != originalValues.size()) {
           throw new CommandLineExpansionException(
               errorMessage(
                   String.format(
                       "map_fn must return a list of the same length as the input. "
                           + "Found list of length %d, expected %d.",
-                      resultAsList.size(), count),
+                      resultAsList.size(), originalValues.size()),
                   location,
                   null));
         }
-        mutatedValues.clear();
-        mutatedValues.addAll(resultAsList);
+        int count = resultAsList.size();
+        stringValues = new ArrayList<>(count);
+        // map_fn contract doesn't guarantee that the values returned are strings,
+        // so convert here
+        for (int i = 0; i < count; ++i) {
+          stringValues.add(CommandLineItem.expandToCommandLine(resultAsList.get(i)));
+        }
+      } else {
+        int count = originalValues.size();
+        stringValues = new ArrayList<>(originalValues.size());
+        for (int i = 0; i < count; ++i) {
+          stringValues.add(CommandLineItem.expandToCommandLine(originalValues.get(i)));
+        }
       }
-      for (int i = 0; i < count; ++i) {
-        mutatedValues.set(i, CommandLineItem.expandToCommandLine(mutatedValues.get(i)));
+      // It's safe to uniquify at this stage, any transformations after this
+      // will ensure continued uniqueness of the values
+      if ((features & UNIQUIFY) != 0) {
+        HashSet<String> seen = new HashSet<>(stringValues.size());
+        int count = stringValues.size();
+        int addIndex = 0;
+        for (int i = 0; i < count; ++i) {
+          String val = stringValues.get(i);
+          if (seen.add(val)) {
+            stringValues.set(addIndex++, val);
+          }
+        }
+        stringValues = stringValues.subList(0, addIndex);
       }
-      if (hasFormat) {
+      boolean isEmptyAndShouldOmit = stringValues.isEmpty() && (features & OMIT_IF_EMPTY) != 0;
+      if (argName != null && !isEmptyAndShouldOmit) {
+        builder.add(argName);
+      }
+      if ((features & HAS_FORMAT_EACH) != 0) {
         String formatStr = (String) arguments.get(argi++);
-        Formatter formatter = new Formatter(formatStr, location);
+        Formatter formatter = new Formatter(formatStr, skylarkSemantics, location);
         try {
+          int count = stringValues.size();
           for (int i = 0; i < count; ++i) {
-            mutatedValues.set(i, formatter.format(mutatedValues.get(i)));
+            stringValues.set(i, formatter.format(stringValues.get(i)));
           }
         } catch (IllegalFormatException e) {
           throw new CommandLineExpansionException(errorMessage(e.getMessage(), location, null));
         }
       }
-      if (hasBeforeEach) {
+      if ((features & HAS_BEFORE_EACH) != 0) {
         String beforeEach = (String) arguments.get(argi++);
+        int count = stringValues.size();
         for (int i = 0; i < count; ++i) {
           builder.add(beforeEach);
-          builder.add((String) mutatedValues.get(i));
+          builder.add(stringValues.get(i));
         }
-      } else if (hasJoinWith) {
+      } else if ((features & HAS_JOIN_WITH) != 0) {
         String joinWith = (String) arguments.get(argi++);
-        builder.add(Joiner.on(joinWith).join(mutatedValues));
+        String formatJoined =
+            ((features & HAS_FORMAT_JOINED) != 0) ? (String) arguments.get(argi++) : null;
+        if (!isEmptyAndShouldOmit) {
+          String result = Joiner.on(joinWith).join(stringValues);
+          if (formatJoined != null) {
+            Formatter formatter = new Formatter(formatJoined, skylarkSemantics, location);
+            try {
+              result = formatter.format(result);
+            } catch (IllegalFormatException e) {
+              throw new CommandLineExpansionException(errorMessage(e.getMessage(), location, null));
+            }
+          }
+          builder.add(result);
+        }
       } else {
-        for (int i = 0; i < count; ++i) {
-          builder.add((String) mutatedValues.get(i));
+        builder.addAll(stringValues);
+      }
+      if ((features & HAS_TERMINATE_WITH) != 0) {
+        String terminateWith = (String) arguments.get(argi++);
+        if (!isEmptyAndShouldOmit) {
+          builder.add(terminateWith);
         }
       }
       return argi;
@@ -204,18 +267,24 @@ class SkylarkCustomCommandLine extends CommandLine {
     static class Builder {
       @Nullable private final SkylarkList<?> list;
       @Nullable private final NestedSet<?> nestedSet;
-      private String format;
+      private Location location;
+      public String argName;
+      private BaseFunction mapAll;
+      private BaseFunction mapEach;
+      private String formatEach;
       private String beforeEach;
       private String joinWith;
-      private Location location;
-      private BaseFunction mapFn;
+      private String formatJoined;
+      private boolean omitIfEmpty;
+      private boolean uniquify;
+      private String terminateWith;
 
-      public Builder(SkylarkList<?> list) {
+      Builder(SkylarkList<?> list) {
         this.list = list;
         this.nestedSet = null;
       }
 
-      public Builder(NestedSet<?> nestedSet) {
+      Builder(NestedSet<?> nestedSet) {
         this.list = null;
         this.nestedSet = nestedSet;
       }
@@ -225,8 +294,23 @@ class SkylarkCustomCommandLine extends CommandLine {
         return this;
       }
 
-      Builder setFormat(String format) {
-        this.format = format;
+      Builder setArgName(String argName) {
+        this.argName = argName;
+        return this;
+      }
+
+      Builder setMapAll(BaseFunction mapAll) {
+        this.mapAll = mapAll;
+        return this;
+      }
+
+      Builder setMapEach(BaseFunction mapEach) {
+        this.mapEach = mapEach;
+        return this;
+      }
+
+      Builder setFormatEach(String format) {
+        this.formatEach = format;
         return this;
       }
 
@@ -235,13 +319,28 @@ class SkylarkCustomCommandLine extends CommandLine {
         return this;
       }
 
-      public Builder setJoinWith(String joinWith) {
+      Builder setJoinWith(String joinWith) {
         this.joinWith = joinWith;
         return this;
       }
 
-      public Builder setMapFn(BaseFunction mapFn) {
-        this.mapFn = mapFn;
+      Builder setFormatJoined(String formatJoined) {
+        this.formatJoined = formatJoined;
+        return this;
+      }
+
+      Builder omitIfEmpty(boolean omitIfEmpty) {
+        this.omitIfEmpty = omitIfEmpty;
+        return this;
+      }
+
+      Builder uniquify(boolean uniquify) {
+        this.uniquify = uniquify;
+        return this;
+      }
+
+      Builder setTerminateWith(String terminateWith) {
+        this.terminateWith = terminateWith;
         return this;
       }
     }
@@ -255,18 +354,12 @@ class SkylarkCustomCommandLine extends CommandLine {
         return false;
       }
       VectorArg vectorArg = (VectorArg) o;
-      return isNestedSet == vectorArg.isNestedSet
-          && hasFormat == vectorArg.hasFormat
-          && hasBeforeEach == vectorArg.hasBeforeEach
-          && hasJoinWith == vectorArg.hasJoinWith
-          && hasMapFn == vectorArg.hasMapFn
-          && hasLocation == vectorArg.hasLocation;
+      return features == vectorArg.features;
     }
 
     @Override
     public int hashCode() {
-      return Objects.hashCode(
-          isNestedSet, hasFormat, hasBeforeEach, hasJoinWith, hasMapFn, hasLocation);
+      return Objects.hashCode(features);
     }
   }
 
@@ -322,7 +415,7 @@ class SkylarkCustomCommandLine extends CommandLine {
       object = CommandLineItem.expandToCommandLine(object);
       if (hasFormat) {
         String formatStr = (String) arguments.get(argi++);
-        Formatter formatter = new Formatter(formatStr, location);
+        Formatter formatter = new Formatter(formatStr, skylarkSemantics, location);
         object = formatter.format(object);
       }
       builder.add((String) object);
@@ -349,7 +442,7 @@ class SkylarkCustomCommandLine extends CommandLine {
         return this;
       }
 
-      public Builder setMapFn(BaseFunction mapFn) {
+      Builder setMapFn(BaseFunction mapFn) {
         this.mapFn = mapFn;
         return this;
       }
@@ -425,11 +518,13 @@ class SkylarkCustomCommandLine extends CommandLine {
 
   private static class Formatter {
     private final String formatStr;
+    private final SkylarkSemantics skylarkSemantics;
     @Nullable private final Location location;
     private final ArrayList<Object> args;
 
-    public Formatter(String formatStr, Location location) {
+    public Formatter(String formatStr, SkylarkSemantics skylarkSemantics, Location location) {
       this.formatStr = formatStr;
+      this.skylarkSemantics = skylarkSemantics;
       this.location = location;
       this.args = new ArrayList<>(1); // Reused arg list to reduce GC
       this.args.add(null);
@@ -438,7 +533,9 @@ class SkylarkCustomCommandLine extends CommandLine {
     String format(Object object) throws CommandLineExpansionException {
       try {
         args.set(0, object);
-        return Printer.getPrinter().formatWithList(formatStr, args).toString();
+        return Printer.getPrinter(skylarkSemantics.incompatibleDisallowOldStyleArgsAdd())
+            .formatWithList(formatStr, args)
+            .toString();
       } catch (IllegalFormatException e) {
         throw new CommandLineExpansionException(errorMessage(e.getMessage(), location, null));
       }
@@ -465,6 +562,53 @@ class SkylarkCustomCommandLine extends CommandLine {
     }
   }
 
+  private static ArrayList<String> applyMapEach(
+      BaseFunction mapFn,
+      List<Object> originalValues,
+      Location location,
+      SkylarkSemantics skylarkSemantics)
+      throws CommandLineExpansionException {
+    try (Mutability mutability = Mutability.create("map_each")) {
+      Environment env =
+          Environment.builder(mutability)
+              .setSemantics(skylarkSemantics)
+              // TODO(b/77140311): Error if we issue print statements
+              .setEventHandler(NullEventHandler.INSTANCE)
+              .build();
+      Object[] args = new Object[1];
+      int count = originalValues.size();
+      ArrayList<String> stringValues = new ArrayList<>(count);
+      for (int i = 0; i < count; ++i) {
+        args[0] = originalValues.get(i);
+        Object ret = mapFn.callWithArgArray(args, null, env, location);
+        if (ret instanceof String) {
+          stringValues.add((String) ret);
+        } else if (ret instanceof SkylarkList) {
+          for (Object val : ((SkylarkList) ret)) {
+            if (!(val instanceof String)) {
+              throw new CommandLineExpansionException(
+                  "Expected map_each to return string, None, or list of strings, "
+                      + "found list containing "
+                      + val.getClass().getSimpleName());
+            }
+            stringValues.add((String) val);
+          }
+        } else if (ret != Runtime.NONE) {
+          throw new CommandLineExpansionException(
+              "Expected map_each to return string, None, or list of strings, found "
+                  + ret.getClass().getSimpleName());
+        }
+      }
+      return stringValues;
+    } catch (EvalException e) {
+      throw new CommandLineExpansionException(errorMessage(e.getMessage(), location, e.getCause()));
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new CommandLineExpansionException(
+          errorMessage("Thread was interrupted", location, null));
+    }
+  }
+
   private static String errorMessage(
       String message, @Nullable Location location, @Nullable Throwable cause) {
     return LINE_JOINER.join(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
index 21d5c49aad..43975c1f56 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
@@ -49,6 +49,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     codedOut.writeBoolNoTag(semantics.incompatibleDisableGlobTracking());
     codedOut.writeBoolNoTag(semantics.incompatibleDisableObjcProviderResources());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
+    codedOut.writeBoolNoTag(semantics.incompatibleDisallowOldStyleArgsAdd());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowThreeArgVardef());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
@@ -71,6 +72,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     builder.incompatibleDisableGlobTracking(codedIn.readBool());
     builder.incompatibleDisableObjcProviderResources(codedIn.readBool());
     builder.incompatibleDisallowDictPlus(codedIn.readBool());
+    builder.incompatibleDisallowOldStyleArgsAdd(codedIn.readBool());
     builder.incompatibleDisallowThreeArgVardef(codedIn.readBool());
     builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 864bec1712..d97cf33449 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -144,6 +144,21 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatibleDisallowDictPlus;
 
+  /** Controls legacy arguments to ctx.actions.Args#add. */
+  @Option(
+    name = "incompatible_disallow_old_style_args_add",
+    defaultValue = "false",
+    category = "incompatible changes",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    metadataTags = {
+      OptionMetadataTag.INCOMPATIBLE_CHANGE,
+      OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+    },
+    help = "If set to true, vectorized calls to Args#add are disallowed."
+  )
+  public boolean incompatibleDisallowOldStyleArgsAdd;
+
   @Option(
     name = "incompatible_disallow_three_arg_vardef",
     defaultValue = "false",
@@ -266,6 +281,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
         .incompatibleDisableGlobTracking(incompatibleDisableGlobTracking)
         .incompatibleDisableObjcProviderResources(incompatibleDisableObjcProviderResources)
         .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
+        .incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
         .incompatibleDisallowThreeArgVardef(incompatibleDisallowThreeArgVardef)
         .incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index bad903c3df..fea0b8d458 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -393,12 +393,12 @@ public abstract class BaseFunction implements SkylarkValue {
     if (types == null) {
       return;
     }
-    List<String> names = signature.getSignature().getNames();
     int length = types.size();
     for (int i = 0; i < length; i++) {
       Object value = arguments[i];
       SkylarkType type = types.get(i);
       if (value != null && type != null && !type.contains(value)) {
+        List<String> names = signature.getSignature().getNames();
         throw new EvalException(loc,
             String.format("expected %s for '%s' while calling %s but got %s instead: %s",
                 type, names.get(i), getName(), EvalUtils.getDataTypeName(value, true), value));
@@ -437,6 +437,22 @@ public abstract class BaseFunction implements SkylarkValue {
     Location loc = ast == null ? Location.BUILTIN : ast.getLocation();
 
     Object[] arguments = processArguments(args, kwargs, loc, env);
+    return callWithArgArray(arguments, ast, env, location);
+  }
+
+  /**
+   * The outer calling convention to a BaseFunction. This function expects all arguments to have
+   * been resolved into positional ones.
+   *
+   * @param ast the expression for this function's definition
+   * @param env the Environment in the function is called
+   * @return the value resulting from evaluating the function with the given arguments
+   * @throws EvalException-s containing source information.
+   */
+  public Object callWithArgArray(
+      Object[] arguments, @Nullable FuncallExpression ast, Environment env, Location loc)
+      throws EvalException, InterruptedException {
+    Preconditions.checkState(isConfigured(), "Function %s was not configured", getName());
     canonicalizeArguments(arguments, loc);
 
     try {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index 52a466b759..697248a8fb 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -67,6 +67,15 @@ public class Printer {
    * @return new {@link BasePrinter}
    */
   public static BasePrinter getPrinter() {
+    return new BasePrinter(new StringBuilder());
+  }
+
+  /**
+   * Creates an instance of {@link BasePrinter} with an empty buffer.
+   *
+   * @param simplifiedFormatStrings if true, format strings will allow only %s and %%
+   */
+  public static BasePrinter getPrinter(boolean simplifiedFormatStrings) {
     return getPrinter(new StringBuilder());
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 46f3f1ae94..ea90d37d86 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -51,6 +51,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean incompatibleDisallowDictPlus();
 
+  public abstract boolean incompatibleDisallowOldStyleArgsAdd();
+
   public abstract boolean incompatibleDisallowThreeArgVardef();
 
   public abstract boolean incompatibleDisallowToplevelIfStatement();
@@ -88,6 +90,7 @@ public abstract class SkylarkSemantics {
           .incompatibleDisableGlobTracking(true)
           .incompatibleDisableObjcProviderResources(false)
           .incompatibleDisallowDictPlus(false)
+          .incompatibleDisallowOldStyleArgsAdd(false)
           .incompatibleDisallowThreeArgVardef(false)
           .incompatibleDisallowToplevelIfStatement(true)
           .incompatibleNewActionsApi(false)
@@ -115,6 +118,8 @@ public abstract class SkylarkSemantics {
 
     public abstract Builder incompatibleDisallowDictPlus(boolean value);
 
+    public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
+
     public abstract Builder incompatibleDisallowThreeArgVardef(boolean value);
 
     public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);