Allow expansion flags to have values.
This lets us change what it expands based on the argument passed to the flag.
RELNOTES: Allows flags that expand to take values.
PiperOrigin-RevId: 160298412
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
index 3f7a193..9d28331 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.ExpansionContext;
import com.google.devtools.common.options.ExpansionFunction;
import com.google.devtools.common.options.IsolatedOptionsData;
import com.google.devtools.common.options.Option;
@@ -153,11 +154,11 @@
}
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
// Grab all registered options that are identified as incompatible changes by either name or
// by category. Ensure they satisfy our requirements.
ArrayList<String> incompatibleChanges = new ArrayList<>();
- for (Map.Entry<String, Field> entry : optionsData.getAllNamedFields()) {
+ for (Map.Entry<String, Field> entry : context.getOptionsData().getAllNamedFields()) {
Field field = entry.getValue();
Option annotation = field.getAnnotation(Option.class);
if (annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
index 18455b1..53540e6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
@@ -165,8 +165,8 @@
// Print out the canonical invocation policy if requested.
if (canonicalizeOptions.canonicalizePolicy) {
- List<FlagPolicy> effectiveFlagPolicies =
- InvocationPolicyEnforcer.getEffectivePolicy(policy, parser, commandName);
+ ImmutableList<FlagPolicy> effectiveFlagPolicies =
+ InvocationPolicyEnforcer.getEffectivePolicies(policy, parser, commandName);
InvocationPolicy effectivePolicy =
InvocationPolicy.newBuilder().addAllFlagPolicies(effectiveFlagPolicies).build();
env.getReporter().getOutErr().printOutLn(effectivePolicy.toString());
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionContext.java b/src/main/java/com/google/devtools/common/options/ExpansionContext.java
new file mode 100644
index 0000000..74dac97
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/ExpansionContext.java
@@ -0,0 +1,54 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.common.options;
+
+import java.lang.reflect.Field;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Encapsulates the data given to {@link ExpansionFunction} objects. This lets {@link
+ * ExpansionFunction} objects change how it expands flags based on the arguments given to the {@link
+ * OptionsParser}.
+ */
+@ThreadSafe
+public final class ExpansionContext {
+ private final IsolatedOptionsData optionsData;
+ private final Field field;
+ @Nullable private final String unparsedValue;
+
+ public ExpansionContext(
+ IsolatedOptionsData optionsData, Field field, @Nullable String unparsedValue) {
+ this.optionsData = optionsData;
+ this.field = field;
+ this.unparsedValue = unparsedValue;
+ }
+
+ /** Metadata for the option that is being expanded. */
+ public IsolatedOptionsData getOptionsData() {
+ return optionsData;
+ }
+
+ /** {@link Field} object for option that is being expanded. */
+ public Field getField() {
+ return field;
+ }
+
+ /** Argument given to this flag during options parsing. Will be null if no argument was given. */
+ @Nullable
+ public String getUnparsedValue() {
+ return unparsedValue;
+ }
+}
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
index be4773e..1031125 100644
--- a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
+++ b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java
@@ -27,7 +27,7 @@
*
* @param optionsData the parser's indexed information about its own options, before expansion
* information is computed
- * @return An expansion to use for all occurrences of this option in this parser
+ * @return An expansion to use on an empty list
*/
- ImmutableList<String> getExpansion(IsolatedOptionsData optionsData);
+ ImmutableList<String> getExpansion(ExpansionContext context) throws OptionsParsingException;
}
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java b/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java
new file mode 100644
index 0000000..d63b988
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java
@@ -0,0 +1,25 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.common.options;
+
+/**
+ * Exception specific to evaluating {@link ExpansionFunction} objects. Used when expansion isn't
+ * possible because of a missing input.
+ */
+public final class ExpansionNeedsValueException extends OptionsParsingException {
+
+ public ExpansionNeedsValueException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
index 651cd78..97be190 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -99,9 +99,9 @@
// The effective policy returned is expanded, filtered for applicable commands, and cleaned of
// redundancies and conflicts.
- List<FlagPolicy> effectivePolicy = getEffectivePolicy(invocationPolicy, parser, command);
+ List<FlagPolicy> effectivePolicies = getEffectivePolicies(invocationPolicy, parser, command);
- for (FlagPolicy flagPolicy : effectivePolicy) {
+ for (FlagPolicy flagPolicy : effectivePolicies) {
String flagName = flagPolicy.getFlagName();
OptionValueDescription valueDescription;
@@ -190,7 +190,7 @@
*
* <p>Expands any policies on expansion flags.
*/
- public static List<FlagPolicy> getEffectivePolicy(
+ public static ImmutableList<FlagPolicy> getEffectivePolicies(
InvocationPolicy invocationPolicy, OptionsParser parser, String command)
throws OptionsParsingException {
if (invocationPolicy == null) {
@@ -220,7 +220,71 @@
effectivePolicy.put(flagName, expandedPolicy);
}
- return new ArrayList<>(effectivePolicy.values());
+ return ImmutableList.copyOf(effectivePolicy.values());
+ }
+
+ private static void throwAllowValuesOnExpansionFlagException(String flagName)
+ throws OptionsParsingException {
+ throw new OptionsParsingException(
+ String.format("Allow_Values on expansion flags like %s is not allowed.", flagName));
+ }
+
+ private static void throwDisallowValuesOnExpansionFlagException(String flagName)
+ throws OptionsParsingException {
+ throw new OptionsParsingException(
+ String.format("Disallow_Values on expansion flags like %s is not allowed.", flagName));
+ }
+
+ private static ImmutableList<OptionValueDescription> getExpansionsFromFlagPolicy(
+ FlagPolicy expansionPolicy, OptionDescription optionDescription, OptionsParser parser)
+ throws OptionsParsingException {
+ if (!optionDescription.isExpansion()) {
+ return ImmutableList.of();
+ }
+
+ String expansionFlagName = expansionPolicy.getFlagName();
+
+ ImmutableList.Builder<OptionValueDescription> resultsBuilder =
+ ImmutableList.<OptionValueDescription>builder();
+ switch (expansionPolicy.getOperationCase()) {
+ case SET_VALUE:
+ {
+ SetValue setValue = expansionPolicy.getSetValue();
+ if (setValue.getFlagValueCount() > 0) {
+ for (String value : setValue.getFlagValueList()) {
+ resultsBuilder.addAll(
+ parser.getExpansionOptionValueDescriptions(expansionFlagName, value));
+ }
+ } else {
+ resultsBuilder.addAll(
+ parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
+ }
+ }
+ break;
+ case USE_DEFAULT:
+ resultsBuilder.addAll(parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
+ break;
+ case ALLOW_VALUES:
+ // All expansions originally given to the parser have been expanded by now, so these two
+ // cases aren't necessary (the values given in the flag policy shouldn't need to be
+ // checked). If you care about blocking specific flag values you should block the behavior
+ // on the specific ones, not the expansion that contains them.
+ throwAllowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
+ break;
+ case DISALLOW_VALUES:
+ throwDisallowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
+ break;
+ case OPERATION_NOT_SET:
+ throw new PolicyOperationNotSetException(expansionPolicy.getFlagName());
+ default:
+ log.warning(
+ String.format(
+ "Unknown operation '%s' from invocation policy for flag '%s'",
+ expansionPolicy.getOperationCase(), expansionFlagName));
+ break;
+ }
+
+ return resultsBuilder.build();
}
/**
@@ -234,21 +298,24 @@
FlagPolicy originalPolicy,
OptionsParser parser)
throws OptionsParsingException {
- List<FlagPolicy> expandedPolicy = new ArrayList<>();
+ List<FlagPolicy> expandedPolicies = new ArrayList<>();
- OptionDescription originalDesc = parser.getOptionDescription(originalPolicy.getFlagName());
- if (originalDesc == null) {
+ OptionDescription originalOptionDescription =
+ parser.getOptionDescription(originalPolicy.getFlagName());
+ if (originalOptionDescription == null) {
// InvocationPolicy ignores policy on non-existing flags by design, for version compatibility.
- return expandedPolicy;
+ return expandedPolicies;
}
+ ImmutableList<OptionValueDescription> expansions =
+ getExpansionsFromFlagPolicy(originalPolicy, originalOptionDescription, parser);
ImmutableList.Builder<OptionValueDescription> subflagBuilder = new ImmutableList.Builder<>();
ImmutableList<OptionValueDescription> subflags =
subflagBuilder
- .addAll(originalDesc.getImplicitRequirements())
- .addAll(originalDesc.getExpansions())
+ .addAll(originalOptionDescription.getImplicitRequirements())
+ .addAll(expansions)
.build();
- boolean isExpansion = !originalDesc.getExpansions().isEmpty();
+ boolean isExpansion = originalOptionDescription.isExpansion();
if (!subflags.isEmpty() && log.isLoggable(Level.FINE)) {
// Log the expansion. Since this is logged regardless of user provided command line, it is
@@ -283,10 +350,10 @@
&& originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE)) {
repeatableSubflagsInSetValues.put(currentSubflag.getName(), currentSubflag);
} else {
- FlagPolicy subflagAsPolicy = getSubflagAsPolicy(
- currentSubflag, originalPolicy, originalDesc);
+ FlagPolicy subflagAsPolicy =
+ getSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
// In case any of the expanded flags are themselves expansions, recurse.
- expandedPolicy.addAll(expandPolicy(subflagAsPolicy, parser));
+ expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser));
}
}
@@ -299,22 +366,18 @@
for (OptionValueDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) {
newValues.add(setValue.getOriginalValueString());
}
- expandedPolicy.add(
+ expandedPolicies.add(
getSetValueSubflagAsPolicy(
- repeatableFlag,
- newValues,
- /* allowMultiple */ true,
- originalPolicy));
-
+ repeatableFlag, newValues, /* allowMultiple */ true, originalPolicy));
}
// Don't add the original policy if it was an expansion flag, which have no value, but do add
// it if there was either no expansion or if it was a valued flag with implicit requirements.
if (!isExpansion) {
- expandedPolicy.add(originalPolicy);
+ expandedPolicies.add(originalPolicy);
}
- return expandedPolicy;
+ return expandedPolicies;
}
/**
@@ -365,19 +428,18 @@
* corresponding policy.
*/
private static FlagPolicy getSubflagAsPolicy(
- OptionValueDescription currentSubflag,
- FlagPolicy originalPolicy,
- OptionDescription originalDesc) throws OptionsParsingException {
- boolean isExpansion = !originalDesc.getExpansions().isEmpty();
+ OptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion)
+ throws OptionsParsingException {
FlagPolicy subflagAsPolicy = null;
switch (originalPolicy.getOperationCase()) {
case SET_VALUE:
- assert(!currentSubflag.getAllowMultiple());
- subflagAsPolicy = getSetValueSubflagAsPolicy(
- currentSubflag.getName(),
- ImmutableList.of(currentSubflag.getOriginalValueString()),
- /* allowMultiple */ false,
- originalPolicy);
+ assert (!currentSubflag.getAllowMultiple());
+ subflagAsPolicy =
+ getSetValueSubflagAsPolicy(
+ currentSubflag.getName(),
+ ImmutableList.of(currentSubflag.getOriginalValueString()),
+ /*allowMultiple=*/ false,
+ originalPolicy);
break;
case USE_DEFAULT:
@@ -394,10 +456,7 @@
case ALLOW_VALUES:
if (isExpansion) {
- throw new OptionsParsingException(
- String.format(
- "Allow_Values on expansion flags like %s is not allowed.",
- originalPolicy.getFlagName()));
+ throwAllowValuesOnExpansionFlagException(originalPolicy.getFlagName());
}
// If this flag is an implicitRequirement, and some values for the parent flag are
// allowed, nothing needs to happen on the implicitRequirement that is set for all
@@ -406,10 +465,7 @@
case DISALLOW_VALUES:
if (isExpansion) {
- throw new OptionsParsingException(
- String.format(
- "Disallow_Values on expansion flags like %s is not allowed.",
- originalPolicy.getFlagName()));
+ throwDisallowValuesOnExpansionFlagException(originalPolicy.getFlagName());
}
// If this flag is an implicitRequirement, and some values for the parent flag are
// disallowed, that implies that all others are allowed, so nothing needs to happen
@@ -684,7 +740,7 @@
OptionDescription optionDescription,
Set<Object> convertedPolicyValues)
throws OptionsParsingException {
-
+
if (optionDescription.getAllowMultiple()) {
// allowMultiple requires that the type of the option be List<T>, so cast from Object
// to List<?>.
diff --git a/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java
index ba53172..48f47ff 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -14,14 +14,15 @@
package com.google.devtools.common.options;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Map;
+import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
@@ -32,38 +33,100 @@
@Immutable
final class OptionsData extends IsolatedOptionsData {
- /** Mapping from each Option-annotated field to expansion strings, if it has any. */
- private final ImmutableMap<Field, ImmutableList<String>> evaluatedExpansions;
+ /**
+ * Keeps track of all the information needed to calculate expansion flags, whether they come from
+ * a static list or a @{link ExpansionFunction} object.
+ */
+ static class ExpansionData {
+ private final ImmutableList<String> staticExpansion;
+ @Nullable private final ExpansionFunction dynamicExpansions;
+
+ ExpansionData(ImmutableList<String> staticExpansion) {
+ Preconditions.checkArgument(staticExpansion != null);
+ this.staticExpansion = staticExpansion;
+ this.dynamicExpansions = null;
+ }
+
+ ExpansionData(ExpansionFunction dynamicExpansions) {
+ Preconditions.checkArgument(dynamicExpansions != null);
+ this.staticExpansion = EMPTY_EXPANSION;
+ this.dynamicExpansions = dynamicExpansions;
+ }
+
+ ImmutableList<String> getExpansion(ExpansionContext context) throws OptionsParsingException {
+ Preconditions.checkArgument(context != null);
+ if (dynamicExpansions != null) {
+ ImmutableList<String> result = dynamicExpansions.getExpansion(context);
+ if (result == null) {
+ String valueString =
+ context.getUnparsedValue() != null ? context.getUnparsedValue() : "(null)";
+ String name = context.getField().getAnnotation(Option.class).name();
+ throw new OptionsParsingException(
+ "Error expanding option '"
+ + name
+ + "': no expansions defined for value: "
+ + valueString,
+ name);
+ }
+ return result;
+ } else {
+ return staticExpansion;
+ }
+ }
+
+ boolean isEmpty() {
+ return staticExpansion.isEmpty() && (dynamicExpansions == null);
+ }
+ }
+
+ /**
+ * Mapping from each Option-annotated field with expansion information to the {@link
+ * ExpansionData} needed to caclulate it.
+ */
+ private final ImmutableMap<Field, ExpansionData> expansionDataForFields;
/** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */
- private OptionsData(
- IsolatedOptionsData base, Map<Field, ImmutableList<String>> evaluatedExpansions) {
+ private OptionsData(IsolatedOptionsData base, Map<Field, ExpansionData> expansionDataForFields) {
super(base);
- this.evaluatedExpansions = ImmutableMap.copyOf(evaluatedExpansions);
+ this.expansionDataForFields = ImmutableMap.copyOf(expansionDataForFields);
}
private static final ImmutableList<String> EMPTY_EXPANSION = ImmutableList.<String>of();
+ private static final ExpansionData EMPTY_EXPANSION_DATA = new ExpansionData(EMPTY_EXPANSION);
/**
* Returns the expansion of an options field, regardless of whether it was defined using {@link
* Option#expansion} or {@link Option#expansionFunction}. If the field is not an expansion option,
* returns an empty array.
*/
- public ImmutableList<String> getEvaluatedExpansion(Field field) {
- ImmutableList<String> result = evaluatedExpansions.get(field);
- return result != null ? result : EMPTY_EXPANSION;
+ public ImmutableList<String> getEvaluatedExpansion(Field field, @Nullable String unparsedValue)
+ throws OptionsParsingException {
+ ExpansionData expansionData = expansionDataForFields.get(field);
+ if (expansionData == null) {
+ return EMPTY_EXPANSION;
+ }
+
+ return expansionData.getExpansion(new ExpansionContext(this, field, unparsedValue));
+ }
+
+ ExpansionData getExpansionDataForField(Field field) {
+ ExpansionData result = expansionDataForFields.get(field);
+ return result != null ? result : EMPTY_EXPANSION_DATA;
}
/**
* Constructs an {@link OptionsData} object for a parser that knows about the given {@link
* OptionsBase} classes. In addition to the work done to construct the {@link
- * IsolatedOptionsData}, this also computes expansion information.
+ * IsolatedOptionsData}, this also computes expansion information. If an option has static
+ * expansions or uses an expansion function that takes a Void object, try to precalculate the
+ * expansion here.
*/
- public static OptionsData from(Collection<Class<? extends OptionsBase>> classes) {
+ static OptionsData from(Collection<Class<? extends OptionsBase>> classes) {
IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes);
// All that's left is to compute expansions.
- Map<Field, ImmutableList<String>> evaluatedExpansionsBuilder = Maps.newHashMap();
+ ImmutableMap.Builder<Field, ExpansionData> expansionDataBuilder =
+ ImmutableMap.<Field, ExpansionData>builder();
for (Map.Entry<String, Field> entry : isolatedData.getAllNamedFields()) {
Field field = entry.getValue();
Option annotation = field.getAnnotation(Option.class);
@@ -74,7 +137,7 @@
throw new AssertionError(
"Cannot set both expansion and expansionFunction for option --" + annotation.name());
} else if (constExpansion.length > 0) {
- evaluatedExpansionsBuilder.put(field, ImmutableList.copyOf(constExpansion));
+ expansionDataBuilder.put(field, new ExpansionData(ImmutableList.copyOf(constExpansion)));
} else if (usesExpansionFunction(annotation)) {
if (Modifier.isAbstract(expansionFunctionClass.getModifiers())) {
throw new AssertionError(
@@ -90,11 +153,25 @@
// time it is used.
throw new AssertionError(e);
}
- ImmutableList<String> expansion = instance.getExpansion(isolatedData);
- evaluatedExpansionsBuilder.put(field, expansion);
+
+ ImmutableList<String> staticExpansion = null;
+ try {
+ staticExpansion = instance.getExpansion(new ExpansionContext(isolatedData, field, null));
+ Preconditions.checkState(
+ staticExpansion != null,
+ "Error calling expansion function for option: %s",
+ annotation.name());
+ expansionDataBuilder.put(field, new ExpansionData(staticExpansion));
+ } catch (ExpansionNeedsValueException e) {
+ // This expansion function needs data that isn't available yet. Save the instance and call
+ // it later.
+ expansionDataBuilder.put(field, new ExpansionData(instance));
+ } catch (OptionsParsingException e) {
+ throw new IllegalStateException("Error expanding void expansion function: ", e);
+ }
}
}
- return new OptionsData(isolatedData, evaluatedExpansionsBuilder);
+ return new OptionsData(isolatedData, expansionDataBuilder.build());
}
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index 728c490..c007db8 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -239,7 +239,7 @@
private final Converter<?> converter;
private final boolean allowMultiple;
- private final ImmutableList<OptionValueDescription> expansions;
+ private final OptionsData.ExpansionData expansionData;
private final ImmutableList<OptionValueDescription> implicitRequirements;
OptionDescription(
@@ -247,13 +247,13 @@
Object defaultValue,
Converter<?> converter,
boolean allowMultiple,
- ImmutableList<OptionValueDescription> expansions,
+ OptionsData.ExpansionData expansionData,
ImmutableList<OptionValueDescription> implicitRequirements) {
this.name = name;
this.defaultValue = defaultValue;
this.converter = converter;
this.allowMultiple = allowMultiple;
- this.expansions = expansions;
+ this.expansionData = expansionData;
this.implicitRequirements = implicitRequirements;
}
@@ -277,8 +277,14 @@
return implicitRequirements;
}
- public ImmutableList<OptionValueDescription> getExpansions() {
- return expansions;
+ public boolean isExpansion() {
+ return !expansionData.isEmpty();
+ }
+
+ /** Return a list of flags that this option expands to. */
+ public ImmutableList<String> getExpansion(ExpansionContext context)
+ throws OptionsParsingException {
+ return expansionData.getExpansion(context);
}
}
@@ -658,21 +664,33 @@
* @return The {@link OptionDescription} for the option, or null if there is no option by the
* given name.
*/
- public OptionDescription getOptionDescription(String name) throws OptionsParsingException {
+ OptionDescription getOptionDescription(String name) throws OptionsParsingException {
return impl.getOptionDescription(name);
}
/**
- * Returns a description of the option value set by the last previous call to
- * {@link #parse(OptionPriority, String, List)} that successfully set the given
- * option. If the option is of type {@link List}, the description will
- * correspond to any one of the calls, but not necessarily the last.
+ * Returns a description of the options values that get expanded from this flag with the given
+ * flag value.
+ *
+ * @return The {@link ImmutableList<OptionValueDescription>} for the option, or null if there is
+ * no option by the given name.
+ */
+ ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions(
+ String flagName, @Nullable String flagValue) throws OptionsParsingException {
+ return impl.getExpansionOptionValueDescriptions(flagName, flagValue);
+ }
+
+ /**
+ * Returns a description of the option value set by the last previous call to {@link
+ * #parse(OptionPriority, String, List)} that successfully set the given option. If the option is
+ * of type {@link List}, the description will correspond to any one of the calls, but not
+ * necessarily the last.
*
* @return The {@link OptionValueDescription} for the option, or null if the value has not been
- * set.
+ * set.
* @throws IllegalArgumentException if there is no option by the given name.
*/
- public OptionValueDescription getOptionValueDescription(String name) {
+ OptionValueDescription getOptionValueDescription(String name) {
return impl.getOptionValueDescription(name);
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index e0fc062..f599cea 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -41,6 +41,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import javax.annotation.Nullable;
/**
* The implementation of the options parser. This is intentionally package
@@ -350,41 +351,66 @@
optionsData.getDefaultValue(field),
optionsData.getConverter(field),
optionsData.getAllowMultiple(field),
- getExpansionDescriptions(
- optionsData.getEvaluatedExpansion(field),
- /* expandedFrom */ name,
- /* implicitDependant */ null),
- getExpansionDescriptions(
- ImmutableList.copyOf(optionAnnotation.implicitRequirements()),
- /* expandedFrom */ null,
- /* implicitDependant */ name));
+ optionsData.getExpansionDataForField(field),
+ getImplicitDependantDescriptions(
+ ImmutableList.copyOf(optionAnnotation.implicitRequirements()), name));
}
/**
- * @return A list of the descriptions corresponding to the list of unparsed flags passed in. These
- * descriptions are are divorced from the command line - there is no correct priority or
+ * @return A list of the descriptions corresponding to the implicit dependant flags passed in.
+ * These descriptions are are divorced from the command line - there is no correct priority or
* source for these, as they are not actually set values. The value itself is also a string,
* no conversion has taken place.
*/
- private ImmutableList<OptionValueDescription> getExpansionDescriptions(
- ImmutableList<String> optionStrings, String expandedFrom, String implicitDependant)
- throws OptionsParsingException {
+ private ImmutableList<OptionValueDescription> getImplicitDependantDescriptions(
+ ImmutableList<String> options, String implicitDependant) throws OptionsParsingException {
ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
- ImmutableList<String> options = ImmutableList.copyOf(optionStrings);
Iterator<String> optionsIterator = options.iterator();
while (optionsIterator.hasNext()) {
String unparsedFlagExpression = optionsIterator.next();
ParseOptionResult parseResult = parseOption(unparsedFlagExpression, optionsIterator);
- builder.add(new OptionValueDescription(
- parseResult.option.name(),
- parseResult.value,
- /* value */ null,
- /* priority */ null,
- /* source */null,
- implicitDependant,
- expandedFrom,
- optionsData.getAllowMultiple(parseResult.field)));
+ builder.add(
+ new OptionValueDescription(
+ parseResult.option.name(),
+ parseResult.value,
+ /* value */ null,
+ /* priority */ null,
+ /* source */ null,
+ implicitDependant,
+ /* expendedFrom */ null,
+ optionsData.getAllowMultiple(parseResult.field)));
+ }
+ return builder.build();
+ }
+
+ /**
+ * @return A list of the descriptions corresponding to options expanded from the flag for the
+ * given value. These descriptions are are divorced from the command line - there is no
+ * correct priority or source for these, as they are not actually set values. The value itself
+ * is also a string, no conversion has taken place.
+ */
+ ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions(
+ String flagName, @Nullable String flagValue) throws OptionsParsingException {
+ ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
+ Field field = optionsData.getFieldFromName(flagName);
+
+ ImmutableList<String> options = optionsData.getEvaluatedExpansion(field, flagValue);
+ Iterator<String> optionsIterator = options.iterator();
+
+ while (optionsIterator.hasNext()) {
+ String unparsedFlagExpression = optionsIterator.next();
+ ParseOptionResult parseResult = parseOption(unparsedFlagExpression, optionsIterator);
+ builder.add(
+ new OptionValueDescription(
+ parseResult.option.name(),
+ parseResult.value,
+ /* value */ null,
+ /* priority */ null,
+ /* source */ null,
+ /* implicitDependant */ null,
+ flagName,
+ optionsData.getAllowMultiple(parseResult.field)));
}
return builder.build();
}
@@ -444,7 +470,7 @@
ParseOptionResult parseOptionResult = parseOption(arg, argsIterator);
Field field = parseOptionResult.field;
Option option = parseOptionResult.option;
- String value = parseOptionResult.value;
+ @Nullable String value = parseOptionResult.value;
final String originalName = option.name();
@@ -498,8 +524,9 @@
}
// Handle expansion options.
- ImmutableList<String> expansion = optionsData.getEvaluatedExpansion(field);
- if (!expansion.isEmpty()) {
+ if (OptionsData.isExpansionOption(field.getAnnotation(Option.class))) {
+ ImmutableList<String> expansion = optionsData.getEvaluatedExpansion(field, value);
+
Function<Object, String> expansionSourceFunction =
Functions.constant(
"expanded from option --"
@@ -580,9 +607,9 @@
private static final class ParseOptionResult {
final Field field;
final Option option;
- final String value;
+ @Nullable final String value;
- ParseOptionResult(Field field, Option option, String value) {
+ ParseOptionResult(Field field, Option option, @Nullable String value) {
this.field = field;
this.option = option;
this.value = value;
diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
index 742e9f9..5f7c48a 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -81,15 +81,22 @@
/**
* Returns the expansion for an option, to the extent known. Precisely, if an {@link OptionsData}
- * object is supplied, the expansion is read from that. Otherwise, the annotation is inspected: If
- * the annotation uses {@link Option#expansion} it is returned, and if it uses {@link
- * Option#expansionFunction} null is returned, indicating a lack of definite information. In all
- * cases, when the option is not an expansion option, an empty list is returned.
+ * object is supplied, the expansion is read from that if the expansion function doesn't take an
+ * argument. Otherwise, the annotation is inspected: If the annotation uses {@link
+ * Option#expansion} it is returned, and if it uses {@link Option#expansionFunction} null is
+ * returned, indicating a lack of definite information. In all cases, when the option is not an
+ * expansion option, an empty list is returned.
*/
private static @Nullable ImmutableList<String> getExpansionIfKnown(
Field optionField, Option annotation, @Nullable OptionsData optionsData) {
if (optionsData != null) {
- return optionsData.getEvaluatedExpansion(optionField);
+ try {
+ return optionsData.getEvaluatedExpansion(optionField, null);
+ } catch (ExpansionNeedsValueException e) {
+ return null;
+ } catch (OptionsParsingException e) {
+ throw new IllegalStateException("Error expanding void expansion function: ", e);
+ }
} else {
if (OptionsData.usesExpansionFunction(annotation)) {
return null;
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java b/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java
index 7f91d17..a5ff150 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java
@@ -21,9 +21,9 @@
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.UseDefault;
import com.google.devtools.common.options.Converters;
+import com.google.devtools.common.options.ExpansionContext;
import com.google.devtools.common.options.ExpansionFunction;
import com.google.devtools.common.options.InvocationPolicyEnforcer;
-import com.google.devtools.common.options.IsolatedOptionsData;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionsBase;
@@ -110,7 +110,7 @@
/** Dummy comment (linter suppression) */
public static class YExpansion implements ExpansionFunction {
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
return ImmutableList.of("--noY");
}
}
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
index beb0005..4303665 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java
@@ -201,14 +201,14 @@
TEST_STRING_POLICY_VALUE_2)
.inOrder();
}
-
+
@Test
public void testSetValueWithExpansionFlags() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
- invocationPolicyBuilder.addFlagPoliciesBuilder()
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
.setFlagName("test_expansion")
- .getSetValueBuilder()
- .addFlagValue("true"); // this value is arbitrary, the value for a Void flag is ignored
+ .getSetValueBuilder(); // This value must be empty for a Void flag.
InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
// Unrelated flag, but --test_expansion is not set
@@ -232,6 +232,80 @@
}
@Test
+ public void testSetValueWithExpansionFunctionFlags() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getSetValueBuilder()
+ .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE);
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion_function is not set
+ parser.parse("--test_string=throwaway value");
+
+ // The flags that --test_expansion_function expands into should still be their default values
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ // After policy enforcement, the flags should be the values from
+ // --test_expansion_function=valueA
+ testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_EXPANSION_FUNCTION_VALUE);
+ }
+
+ @Test
+ public void testSetValueWithExpansionFunctionFlagsDefault() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getSetValueBuilder();
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion_function is not set
+ parser.parse("--test_string=throwaway value");
+
+ // The flags that --test_expansion_function expands into should still be their default values
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
+
+ try {
+ enforcer.enforce(parser, BUILD_COMMAND);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertThat(e).hasMessage("Expansion value not set.");
+ }
+ }
+
+ @Test
+ public void testSetValueWithExpansionFunctionFlagsWrongValue() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getSetValueBuilder()
+ .addFlagValue("unknown_value");
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion_function is not set
+ parser.parse("--test_string=throwaway value");
+
+ // The flags that --test_expansion_function expands into should still be their default values
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
+
+ try {
+ enforcer.enforce(parser, BUILD_COMMAND);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertThat(e).hasMessage("Unrecognized expansion value: unknown_value");
+ }
+ }
+
+ @Test
public void testOverridableSetValueWithExpansionFlags() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
invocationPolicyBuilder
@@ -265,13 +339,39 @@
}
@Test
+ public void testOverridableSetValueWithExpansionFunction() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getSetValueBuilder()
+ .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE)
+ .setOverridable(true);
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion_function is not set
+ parser.parse("--expanded_d=value that overrides");
+
+ // The flags that --test_expansion_function expands into should still be their default values
+ // except for the explicitly marked flag.
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo("value that overrides");
+
+ enforcer.enforce(parser, "build");
+
+ // After policy enforcement, the flags should be the values from --test_expansion_function,
+ // except for the user-set value, since the expansion flag was set to overridable.
+ testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo("value that overrides");
+ }
+
+ @Test
public void testOverridableSetValueWithExpansionToRepeatingFlag() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
invocationPolicyBuilder
.addFlagPoliciesBuilder()
.setFlagName("test_expansion_to_repeatable")
- .getSetValueBuilder()
- .addFlagValue("") // this value is arbitrary, the value for a Void flag is ignored
+ .getSetValueBuilder() // This value must be empty for a Void flag.
.setOverridable(true);
InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
@@ -298,8 +398,7 @@
invocationPolicyBuilder
.addFlagPoliciesBuilder()
.setFlagName("test_expansion")
- .getSetValueBuilder()
- .addFlagValue("") // this value is arbitrary, the value for a Void flag is ignored
+ .getSetValueBuilder() // This value must be empty for a Void flag.
.setOverridable(false);
InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
@@ -327,13 +426,40 @@
}
@Test
+ public void testNonoverridableSetValueWithExpansionFlags() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getSetValueBuilder()
+ .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE)
+ .setOverridable(false);
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ // Unrelated flag, but --test_expansion_function is not set
+ parser.parse("--expanded_d=value to override");
+
+ // The flags that --test_expansion_function expands into should still be their default values
+ // except for the explicitly marked flag.
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo("value to override");
+
+ enforcer.enforce(parser, "build");
+
+ // After policy enforcement, the flags should be the values from --test_expansion_function,
+ // including the value that the user tried to set, since the expansion flag was set
+ // non-overridably.
+ testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_EXPANSION_FUNCTION_VALUE);
+ }
+
+ @Test
public void testNonOverridableSetValueWithExpansionToRepeatingFlag() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
invocationPolicyBuilder
.addFlagPoliciesBuilder()
.setFlagName("test_expansion_to_repeatable")
- .getSetValueBuilder()
- .addFlagValue("") // this value is arbitrary, the value for a Void flag is ignored
+ .getSetValueBuilder() // This value must be empty for a Void flag.
.setOverridable(false);
InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java
index 0a046e5..975befb 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.common.options;
import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import org.junit.Test;
@@ -101,6 +102,50 @@
}
@Test
+ public void testUseDefaultWithVoidExpansionFunction() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_void_expansion_function")
+ .getUseDefaultBuilder();
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ parser.parse("--expanded_d=value to override");
+
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo("value to override");
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ // After policy enforcement, all the flags that --test_void_expansion_function expanded into
+ // should be back to their default values.
+ testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
+ }
+
+ @Test
+ public void testUseDefaultWithExpansionFunction() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_expansion_function")
+ .getUseDefaultBuilder();
+
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+ parser.parse("--expanded_d=value to override");
+
+ TestOptions testOptions = getTestOptions();
+ assertThat(testOptions.expandedD).isEqualTo("value to override");
+
+ try {
+ enforcer.enforce(parser, BUILD_COMMAND);
+ fail();
+ } catch (OptionsParsingException e) {
+ assertThat(e).hasMessage("Expansion value not set.");
+ }
+ }
+
+ @Test
public void testUseDefaultWithExpansionFlagAndLaterOverride() throws Exception {
InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
invocationPolicyBuilder
diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
index d2e2fae..18b6da9 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -868,7 +868,7 @@
/** ExpFunc */
public static class ExpFunc implements ExpansionFunction {
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
return ImmutableList.of("--yyy");
}
}
@@ -900,7 +900,7 @@
/** ExpFunc */
public static class ExpFunc implements ExpansionFunction {
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
return null;
}
}
@@ -917,8 +917,35 @@
newOptionsParser(NullExpansionsOptions.class);
fail("Should have failed due to null expansion function result");
} catch (OptionsParser.ConstructionException e) {
- assertThat(e).hasCauseThat().isInstanceOf(NullPointerException.class);
- assertThat(e).hasCauseThat().hasMessageThat().contains("null value in entry");
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ }
+ }
+
+ /** NullExpansionOptions */
+ public static class NullExpansionsWithArgumentOptions extends OptionsBase {
+
+ /** ExpFunc */
+ public static class ExpFunc implements ExpansionFunction {
+ @Override
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
+ return null;
+ }
+ }
+
+ @Option(name = "badness", expansionFunction = ExpFunc.class, defaultValue = "null")
+ public String badness;
+ }
+
+ @Test
+ public void nullExpansionsWithArgument() throws Exception {
+ try {
+ // When an expansion takes a value, this exception should still happen at parse time.
+ newOptionsParser(NullExpansionsWithArgumentOptions.class);
+ fail("Should have failed due to null expansion function result");
+ } catch (OptionsParser.ConstructionException e) {
+ assertThat(e)
+ .hasMessageThat()
+ .isEqualTo("Error calling expansion function for option: badness");
}
}
@@ -937,7 +964,7 @@
/** ExpFunc */
public static class ExpFunc implements ExpansionFunction {
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
return ImmutableList.of("--expands");
}
}
@@ -946,6 +973,29 @@
public Void expandsByFunction;
}
+ /** ExpansionMultipleOptions */
+ public static class ExpansionMultipleOptions extends OptionsBase {
+ @Option(name = "underlying", defaultValue = "null", allowMultiple = true)
+ public List<String> underlying;
+
+ /** ExpFunc */
+ public static class ExpFunc implements ExpansionFunction {
+ @Override
+ public ImmutableList<String> getExpansion(ExpansionContext context)
+ throws OptionsParsingException {
+ String value = context.getUnparsedValue();
+ if (value == null) {
+ throw new ExpansionNeedsValueException("No value given to 'expands_by_function'");
+ }
+
+ return ImmutableList.of("--underlying=pre_" + value, "--underlying=post_" + value);
+ }
+ }
+
+ @Option(name = "expands_by_function", defaultValue = "null", expansionFunction = ExpFunc.class)
+ public Void expandsByFunction;
+ }
+
@Test
public void describeOptionsWithExpansion() throws Exception {
// We have to test this here rather than in OptionsTest because expansion functions require
@@ -976,6 +1026,19 @@
assertThat(options.underlying).isEqualTo("from_expansion");
}
+ // Makes sure the expansion options are expanded in the right order if they affect flags that
+ // allow multiples.
+ @Test
+ public void multipleExpansionOptionsWithValue() throws Exception {
+ OptionsParser parser = OptionsParser.newOptionsParser(ExpansionMultipleOptions.class);
+ parser.parse(
+ OptionPriority.COMMAND_LINE,
+ null,
+ Arrays.asList("--expands_by_function=a", "--expands_by_function=b"));
+ ExpansionMultipleOptions options = parser.getOptions(ExpansionMultipleOptions.class);
+ assertThat(options.underlying).containsExactly("pre_a", "post_a", "pre_b", "post_b").inOrder();
+ }
+
@Test
public void overrideWithHigherPriority() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NullTestOptions.class);
diff --git a/src/test/java/com/google/devtools/common/options/OptionsTest.java b/src/test/java/com/google/devtools/common/options/OptionsTest.java
index dbe680d..bb0a22e 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsTest.java
@@ -71,9 +71,9 @@
/** SpecialExpansion */
public static class SpecialExpansion implements ExpansionFunction {
@Override
- public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) {
+ public ImmutableList<String> getExpansion(ExpansionContext context) {
TreeSet<String> flags = new TreeSet<>();
- for (Map.Entry<String, ?> entry : optionsData.getAllNamedFields()) {
+ for (Map.Entry<String, ?> entry : context.getOptionsData().getAllNamedFields()) {
if (entry.getKey().startsWith("specialexp_")) {
flags.add("--" + entry.getKey());
}
@@ -82,6 +82,23 @@
}
}
+ /** VariableExpansion */
+ public static class VariableExpansion implements ExpansionFunction {
+ @Override
+ public ImmutableList<String> getExpansion(ExpansionContext context)
+ throws OptionsParsingException {
+ String value = context.getUnparsedValue();
+ if (value == null) {
+ throw new ExpansionNeedsValueException("Expansion value not set.");
+ }
+ if (value.equals("foo_bar")) {
+ return ImmutableList.<String>of("--specialexp_foo", "--specialexp_bar");
+ }
+
+ throw new OptionsParsingException("Unexpected expansion argument: " + value);
+ }
+ }
+
@Option(name = "specialexp_foo", defaultValue = "false")
public boolean specialExpFoo;
@@ -90,6 +107,9 @@
@Option(name = "specialexp", defaultValue = "null", expansionFunction = SpecialExpansion.class)
public Void specialExp;
+
+ @Option(name = "dynamicexp", defaultValue = "null", expansionFunction = VariableExpansion.class)
+ public Void variableExpansion;
}
@Test
@@ -539,4 +559,23 @@
Options.parse(HttpOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"});
assertThat(options1.getOptions()).isEqualTo(options2.getOptions());
}
+
+ @Test
+ public void dynamicExpansionFunctionWorks() throws Exception {
+ Options<HttpOptions> options1 =
+ Options.parse(HttpOptions.class, new String[] {"--dynamicexp=foo_bar"});
+ Options<HttpOptions> options2 =
+ Options.parse(HttpOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"});
+ assertThat(options1.getOptions()).isEqualTo(options2.getOptions());
+ }
+
+ @Test
+ public void dynamicExpansionFunctionUnknowValue() throws Exception {
+ try {
+ Options.parse(HttpOptions.class, new String[] {"--dynamicexp=foo"});
+ fail("Unknown expansion argument should cause a failure.");
+ } catch (OptionsParsingException e) {
+ assertThat(e).hasMessage("Unexpected expansion argument: foo");
+ }
+ }
}
diff --git a/src/test/java/com/google/devtools/common/options/TestOptions.java b/src/test/java/com/google/devtools/common/options/TestOptions.java
index ce532bb..c9d606c 100644
--- a/src/test/java/com/google/devtools/common/options/TestOptions.java
+++ b/src/test/java/com/google/devtools/common/options/TestOptions.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.common.options;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.common.options.InvocationPolicyEnforcerTestBase.ToListConverter;
import java.util.List;
@@ -174,4 +175,47 @@
implicitRequirements = {"--test_implicit_requirement=" + TEST_IMPLICIT_REQUIREMENT_REQUIRED}
)
public String testRecursiveImplicitRequirement;
+
+ public static final String TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE = "valueA";
+ public static final String EXPANDED_D_EXPANSION_FUNCTION_VALUE = "expanded valueA";
+
+ /** Used for testing an expansion flag that requires a value. */
+ public static class TestExpansionFunction implements ExpansionFunction {
+ @Override
+ public ImmutableList<String> getExpansion(ExpansionContext expansionContext)
+ throws OptionsParsingException {
+ String value = expansionContext.getUnparsedValue();
+ if (value == null) {
+ throw new ExpansionNeedsValueException("Expansion value not set.");
+ } else if (value.equals(TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE)) {
+ return ImmutableList.of("--expanded_d", EXPANDED_D_EXPANSION_FUNCTION_VALUE);
+ } else {
+ throw new OptionsParsingException("Unrecognized expansion value: " + value);
+ }
+ }
+ }
+
+ @Option(
+ name = "test_expansion_function",
+ defaultValue = "null",
+ expansionFunction = TestExpansionFunction.class
+ )
+ public Void testExpansionFunction;
+
+ public static final String EXPANDED_D_VOID_EXPANSION_FUNCTION_VALUE = "void expanded";
+
+ /** Used for testing an expansion flag that doesn't requires a value. */
+ public static class TestVoidExpansionFunction implements ExpansionFunction {
+ @Override
+ public ImmutableList<String> getExpansion(ExpansionContext expansionContext) {
+ return ImmutableList.of("--expanded_d", EXPANDED_D_VOID_EXPANSION_FUNCTION_VALUE);
+ }
+ }
+
+ @Option(
+ name = "test_void_expansion_function",
+ defaultValue = "null",
+ expansionFunction = TestVoidExpansionFunction.class
+ )
+ public Void testVoidExpansionFunction;
}
