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
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 89319e9..7a3a79d 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.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 @@
)
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 @@
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 @@
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 @@
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 @@
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 @@
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 (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,
+ 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 (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 @@
SkylarkList skylarkList = (SkylarkList) value;
vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(skylarkList);
}
- vectorArg.setLocation(loc);
- if (format != Runtime.NONE) {
- vectorArg.setFormat((String) format);
+ if (mapEach != null) {
+ validateMapEach(mapEach, loc);
}
- if (beforeEach != Runtime.NONE) {
- vectorArg.setBeforeEach((String) beforeEach);
- }
- if (joinWith != Runtime.NONE) {
- vectorArg.setJoinWith((String) joinWith);
- }
- if (mapFn != Runtime.NONE) {
- vectorArg.setMapFn((BaseFunction) mapFn);
- }
+ 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 @@
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 @@
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 ded9563..3fbb5db 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 @@
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.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 @@
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 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 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 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 @@
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 @@
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++));
- }
+ int count = (Integer) arguments.get(argi++);
+ originalValues = arguments.subList(argi, argi + count);
+ argi += count;
}
- final Location location = hasLocation ? (Location) arguments.get(argi++) : null;
- if (hasMapFn) {
+ 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
return this;
}
- public Builder setMapFn(BaseFunction mapFn) {
+ Builder setMapFn(BaseFunction mapFn) {
this.mapFn = mapFn;
return this;
}
@@ -425,11 +518,13 @@
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 @@
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 @@
}
}
+ 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 21d5c49..43975c1 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 @@
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 @@
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 864bec1..d97cf33 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 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 @@
.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 bad903c3..fea0b8d 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 @@
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 @@
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 52a466b..697248a 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 @@
* @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 46f3f1a..ea90d37 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 boolean incompatibleDisallowDictPlus();
+ public abstract boolean incompatibleDisallowOldStyleArgsAdd();
+
public abstract boolean incompatibleDisallowThreeArgVardef();
public abstract boolean incompatibleDisallowToplevelIfStatement();
@@ -88,6 +90,7 @@
.incompatibleDisableGlobTracking(true)
.incompatibleDisableObjcProviderResources(false)
.incompatibleDisallowDictPlus(false)
+ .incompatibleDisallowOldStyleArgsAdd(false)
.incompatibleDisallowThreeArgVardef(false)
.incompatibleDisallowToplevelIfStatement(true)
.incompatibleNewActionsApi(false)
@@ -115,6 +118,8 @@
public abstract Builder incompatibleDisallowDictPlus(boolean value);
+ public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
+
public abstract Builder incompatibleDisallowThreeArgVardef(boolean value);
public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 7cfedfe..4256ab8 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -125,6 +125,7 @@
"--incompatible_disable_glob_tracking=" + rand.nextBoolean(),
"--incompatible_disable_objc_provider_resources=" + rand.nextBoolean(),
"--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
+ "--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_three_arg_vardef=" + rand.nextBoolean(),
"--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
@@ -148,6 +149,7 @@
.incompatibleDisableGlobTracking(rand.nextBoolean())
.incompatibleDisableObjcProviderResources(rand.nextBoolean())
.incompatibleDisallowDictPlus(rand.nextBoolean())
+ .incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowThreeArgVardef(rand.nextBoolean())
.incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index a1bbf220..6734c79 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -1749,7 +1749,249 @@
}
@Test
- public void testLazyArgs() throws Exception {
+ public void testArgsScalarAdd() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "args = ruleContext.actions.args()",
+ "args.add('--foo')",
+ "args.add('-')",
+ "args.add('foo', format='format%s')",
+ "args.add('-')",
+ "args.add(arg_name='--foo', value='val')",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly("foo/t.exe", "--foo", "-", "formatfoo", "-", "--foo", "val")
+ .inOrder();
+ }
+
+ @Test
+ public void testArgsScalarAddThrowsWithVectorArg() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_old_style_args_add=true");
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ checkErrorContains(
+ ruleContext,
+ "Args#add no longer accepts vectorized",
+ "args = ruleContext.actions.args()",
+ "args.add([1, 2])",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ }
+
+ @Test
+ public void testArgsAddAll() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "args = ruleContext.actions.args()",
+ "args.add_all([1, 2])",
+ "args.add('-')",
+ "args.add_all(arg_name='--foo', values=[1, 2])",
+ "args.add('-')",
+ "args.add_all([1, 2], before_each='-before')",
+ "args.add('-')",
+ "args.add_all([1, 2], format_each='format/%s')",
+ "args.add('-')",
+ "args.add_all(ruleContext.files.srcs)",
+ "args.add('-')",
+ "args.add_all(ruleContext.files.srcs, format_each='format/%s')",
+ "args.add('-')",
+ "args.add_all([1, 2], terminate_with='--terminator')",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly(
+ "foo/t.exe",
+ "1",
+ "2",
+ "-",
+ "--foo",
+ "1",
+ "2",
+ "-",
+ "-before",
+ "1",
+ "-before",
+ "2",
+ "-",
+ "format/1",
+ "format/2",
+ "-",
+ "foo/a.txt",
+ "foo/b.img",
+ "-",
+ "format/foo/a.txt",
+ "format/foo/b.img",
+ "-",
+ "1",
+ "2",
+ "--terminator")
+ .inOrder();
+ }
+
+ @Test
+ public void testArgsAddAllWithMapEach() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "def add_one(val): return str(val + 1)",
+ "def expand_to_many(val): return ['hey', 'hey']",
+ "args = ruleContext.actions.args()",
+ "args.add_all([1, 2], map_each=add_one)",
+ "args.add('-')",
+ "args.add_all([1, 2], map_each=expand_to_many)",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly("foo/t.exe", "2", "3", "-", "hey", "hey", "hey", "hey")
+ .inOrder();
+ }
+
+ @Test
+ public void testOmitIfEmpty() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "def add_one(val): return str(val + 1)",
+ "def filter(val): return None",
+ "args = ruleContext.actions.args()",
+ "args.add_joined([], join_with=',')",
+ "args.add('-')",
+ "args.add_joined([], join_with=',', omit_if_empty=False)",
+ "args.add('-')",
+ "args.add_all(arg_name='--foo', values=[])",
+ "args.add('-')",
+ "args.add_all(arg_name='--foo', values=[], omit_if_empty=False)",
+ "args.add('-')",
+ "args.add_all(arg_name='--foo', values=[1], map_each=filter, terminate_with='hello')",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly(
+ "foo/t.exe",
+ // Nothing
+ "-",
+ "", // Empty string was joined and added
+ "-",
+ // Nothing
+ "-",
+ "--foo", // Arg added regardless
+ "-"
+ // Nothing, all values were filtered
+ )
+ .inOrder();
+ }
+
+ @Test
+ public void testUniquify() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "def add_one(val): return str(val + 1)",
+ "args = ruleContext.actions.args()",
+ "args.add_all(['a', 'b', 'a'])",
+ "args.add('-')",
+ "args.add_all(['a', 'b', 'a', 'c', 'b'], uniquify=True)",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly("foo/t.exe", "a", "b", "a", "-", "a", "b", "c")
+ .inOrder();
+ }
+
+ @Test
+ public void testArgsAddJoined() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "def add_one(val): return str(val + 1)",
+ "args = ruleContext.actions.args()",
+ "args.add_joined([1, 2], join_with=':')",
+ "args.add('-')",
+ "args.add_joined([1, 2], join_with=':', format_each='format/%s')",
+ "args.add('-')",
+ "args.add_joined([1, 2], join_with=':', format_each='format/%s', format_joined='--foo=%s')",
+ "args.add('-')",
+ "args.add_joined([1, 2], join_with=':', map_each=add_one)",
+ "args.add('-')",
+ "args.add_joined(ruleContext.files.srcs, join_with=':')",
+ "args.add('-')",
+ "args.add_joined(ruleContext.files.srcs, join_with=':', format_each='format/%s')",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ assertThat(action.getArguments())
+ .containsExactly(
+ "foo/t.exe",
+ "1:2",
+ "-",
+ "format/1:format/2",
+ "-",
+ "--foo=format/1:format/2",
+ "-",
+ "2:3",
+ "-",
+ "foo/a.txt:foo/b.img",
+ "-",
+ "format/foo/a.txt:format/foo/b.img")
+ .inOrder();
+ }
+
+ @Test
+ public void testLazyArgsLegacy() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_old_style_args_add=false");
SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
evalRuleContextCode(
ruleContext,
@@ -1803,6 +2045,35 @@
}
@Test
+ public void testLegacyLazyArgMapFnReturnsWrongArgumentCount() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_disallow_old_style_args_add=false");
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ evalRuleContextCode(
+ ruleContext,
+ "args = ruleContext.actions.args()",
+ "def bad_fn(args): return [0]",
+ "args.add([1, 2], map_fn=bad_fn)",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ SpawnAction action =
+ (SpawnAction)
+ Iterables.getOnlyElement(
+ ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+ try {
+ action.getArguments();
+ fail();
+ } catch (CommandLineExpansionException e) {
+ assertThat(e)
+ .hasMessageThat()
+ .contains("map_fn must return a list of the same length as the input");
+ }
+ }
+
+ @Test
public void testMultipleLazyArgsMixedWithStrings() throws Exception {
SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
evalRuleContextCode(
@@ -1913,7 +2184,7 @@
evalRuleContextCode(
ruleContext,
"args = ruleContext.actions.args()",
- "args.add([1, 2], format='format/%s%s')", // Expects two args, will only be given one
+ "args.add('foo', format='format/%s%s')", // Expects two args, will only be given one
"ruleContext.actions.run(",
" inputs = depset(ruleContext.files.srcs),",
" outputs = ruleContext.files.srcs,",
@@ -1933,13 +2204,30 @@
}
@Test
- public void testLazyArgBadMapFn() throws Exception {
+ public void testLazyArgMapEachWrongArgCount() throws Exception {
+ SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+ checkErrorContains(
+ ruleContext,
+ "map_each must be a function that accepts a single",
+ "args = ruleContext.actions.args()",
+ "def bad_fn(val, val2): return str(val)",
+ "args.add_all([1, 2], map_each=bad_fn)",
+ "ruleContext.actions.run(",
+ " inputs = depset(ruleContext.files.srcs),",
+ " outputs = ruleContext.files.srcs,",
+ " arguments = [args],",
+ " executable = ruleContext.files.tools[0],",
+ ")");
+ }
+
+ @Test
+ public void testLazyArgMapEachThrowsError() throws Exception {
SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
evalRuleContextCode(
ruleContext,
"args = ruleContext.actions.args()",
- "def bad_fn(args): 'hello'.nosuchmethod()",
- "args.add([1, 2], map_fn=bad_fn)",
+ "def bad_fn(val): 'hello'.nosuchmethod()",
+ "args.add_all([1, 2], map_each=bad_fn)",
"ruleContext.actions.run(",
" inputs = depset(ruleContext.files.srcs),",
" outputs = ruleContext.files.srcs,",
@@ -1959,13 +2247,13 @@
}
@Test
- public void testLazyArgMapFnReturnsWrongType() throws Exception {
+ public void testLazyArgMapEachReturnsNone() throws Exception {
SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
evalRuleContextCode(
ruleContext,
"args = ruleContext.actions.args()",
- "def bad_fn(args): return None",
- "args.add([1, 2], map_fn=bad_fn)",
+ "def none_fn(val): return None if val == 'nokeep' else val",
+ "args.add_all(['keep', 'nokeep'], map_each=none_fn)",
"ruleContext.actions.run(",
" inputs = depset(ruleContext.files.srcs),",
" outputs = ruleContext.files.srcs,",
@@ -1976,22 +2264,17 @@
(SpawnAction)
Iterables.getOnlyElement(
ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
- try {
- action.getArguments();
- fail();
- } catch (CommandLineExpansionException e) {
- assertThat(e.getMessage()).contains("map_fn must return a list, got NoneType");
- }
+ assertThat(action.getArguments()).containsExactly("foo/t.exe", "keep").inOrder();
}
@Test
- public void testLazyArgMapFnReturnsWrongArgumentCount() throws Exception {
+ public void testLazyArgMapEachReturnsWrongType() throws Exception {
SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
evalRuleContextCode(
ruleContext,
"args = ruleContext.actions.args()",
- "def bad_fn(args): return [0]",
- "args.add([1, 2], map_fn=bad_fn)",
+ "def bad_fn(val): return 1",
+ "args.add_all([1, 2], map_each=bad_fn)",
"ruleContext.actions.run(",
" inputs = depset(ruleContext.files.srcs),",
" outputs = ruleContext.files.srcs,",
@@ -2007,7 +2290,7 @@
fail();
} catch (CommandLineExpansionException e) {
assertThat(e.getMessage())
- .contains("map_fn must return a list of the same length as the input");
+ .contains("Expected map_each to return string, None, or list of strings, found Integer");
}
}
@@ -2088,7 +2371,6 @@
AssertionError expected =
assertThrows(AssertionError.class, () -> getConfiguredTarget("//test:main"));
-
assertThat(expected).hasMessageThat()
.contains("invalid configuration fragment name 'notarealfragment'");
}
