Track Option placement within a priority category.
An option has precedence over previous options at the same enum-valued priority. Track its placement in this ordering explicitly.
This will allow after-the-fact expansion of expansion options such that they correctly take precedence or not compared to other mentions of the same flag. This is needed to fix --config's expansion.
RELNOTES: None.
PiperOrigin-RevId: 172367996
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 cf81b22..b686154 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
@@ -97,7 +97,7 @@
* as this constitutes an internal error in the declaration of the option.
*/
private static void validateIncompatibleChange(OptionDefinition optionDefinition) {
- String prefix = "Incompatible change option '--" + optionDefinition.getOptionName() + "' ";
+ String prefix = String.format("Incompatible change %s ", optionDefinition);
// To avoid ambiguity, and the suggestion of using .isEmpty().
String defaultString = "";
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 86ccfdb..8f46581 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -51,7 +51,7 @@
import com.google.devtools.common.options.InvocationPolicyEnforcer;
import com.google.devtools.common.options.OpaqueOptionsData;
import com.google.devtools.common.options.OptionDefinition;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
@@ -219,8 +219,8 @@
// Explicit command-line options:
List<String> cmdLineAfterCommand = args.subList(1, args.size());
- optionsParser.parseWithSourceFunction(OptionPriority.COMMAND_LINE,
- commandOptionSourceFunction, cmdLineAfterCommand);
+ optionsParser.parseWithSourceFunction(
+ PriorityCategory.COMMAND_LINE, commandOptionSourceFunction, cmdLineAfterCommand);
// Command-specific options from .blazerc passed in via --default_override
// and --rc_source. A no-op if none are provided.
@@ -818,7 +818,7 @@
rcfileNotes.add(source + ":\n"
+ " " + inherited + "'" + commandToParse + "' options: "
+ Joiner.on(' ').join(rcfileOptions));
- optionsParser.parse(OptionPriority.RC_FILE, rcfile, rcfileOptions);
+ optionsParser.parse(PriorityCategory.RC_FILE, rcfile, rcfileOptions);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 2bbdb48..29b9e45 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -76,7 +76,7 @@
import com.google.devtools.common.options.CommandNameCache;
import com.google.devtools.common.options.InvocationPolicyParser;
import com.google.devtools.common.options.OptionDefinition;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsClassProvider;
import com.google.devtools.common.options.OptionsParser;
@@ -874,7 +874,7 @@
// First parse the command line so that we get the option_sources argument
OptionsParser parser = OptionsParser.newOptionsParser(optionClasses);
parser.setAllowResidue(false);
- parser.parse(OptionPriority.COMMAND_LINE, null, args);
+ parser.parse(PriorityCategory.COMMAND_LINE, null, args);
Map<String, String> optionSources =
parser.getOptions(BlazeServerStartupOptions.class).optionSources;
Function<OptionDefinition, String> sourceFunction =
@@ -888,7 +888,7 @@
// Then parse the command line again, this time with the correct option sources
parser = OptionsParser.newOptionsParser(optionClasses);
parser.setAllowResidue(false);
- parser.parseWithSourceFunction(OptionPriority.COMMAND_LINE, sourceFunction, args);
+ parser.parseWithSourceFunction(PriorityCategory.COMMAND_LINE, sourceFunction, args);
return parser;
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java
index 313fb18..154a617 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java
@@ -248,7 +248,8 @@
.stream()
.filter(
parsedOptionDescription ->
- parsedOptionDescription.getPriority() == OptionPriority.COMMAND_LINE)
+ parsedOptionDescription.getPriority().getPriorityCategory()
+ == OptionPriority.PriorityCategory.COMMAND_LINE)
.collect(Collectors.toList());
return CommandLineSection.newBuilder()
.setSectionLabel("command options")
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java
index b31bb24..c5abba2 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CoverageCommand.java
@@ -16,7 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.packages.TestTimeout;
import com.google.devtools.build.lib.runtime.Command;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
@@ -122,10 +122,12 @@
public void editOptions(OptionsParser optionsParser) {
super.editOptions(optionsParser);
try {
- optionsParser.parse(OptionPriority.SOFTWARE_REQUIREMENT,
+ optionsParser.parse(
+ PriorityCategory.SOFTWARE_REQUIREMENT,
"Options required by the coverage command",
ImmutableList.of("--collect_code_coverage"));
- optionsParser.parse(OptionPriority.COMPUTED_DEFAULT,
+ optionsParser.parse(
+ PriorityCategory.COMPUTED_DEFAULT,
"Options suggested for the coverage command",
ImmutableList.of(TestTimeout.COVERAGE_CMD_TIMEOUT));
} catch (OptionsParsingException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
index dd43887..98c76bd 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
@@ -22,7 +22,7 @@
import com.google.devtools.build.lib.runtime.ProjectFile;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
@@ -72,7 +72,7 @@
eventHandler.handle(Event.info("Using " + projectFile.getName()));
optionsParser.parse(
- OptionPriority.RC_FILE, projectFile.getName(), projectFile.getCommandLineFor(command));
+ PriorityCategory.RC_FILE, projectFile.getName(), projectFile.getCommandLineFor(command));
eventHandler.post(new GotProjectFileEvent(projectFile.getName()));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index 06cf1fe..60b35bf 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -38,7 +38,7 @@
import com.google.devtools.build.lib.runtime.TestResultNotifier;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
@@ -70,7 +70,8 @@
TestOutputFormat testOutput = optionsParser.getOptions(ExecutionOptions.class).testOutput;
try {
if (testOutput == TestStrategy.TestOutputFormat.STREAMED) {
- optionsParser.parse(OptionPriority.SOFTWARE_REQUIREMENT,
+ optionsParser.parse(
+ PriorityCategory.SOFTWARE_REQUIREMENT,
"streamed output requires locally run tests, without sharding",
ImmutableList.of("--test_sharding_strategy=disabled", "--test_strategy=exclusive"));
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
index 8a83400..0249403 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
@@ -42,7 +42,7 @@
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
@@ -301,12 +301,12 @@
? "mobile_install_incremental" + INTERNAL_SUFFIX
: "mobile_install_full" + INTERNAL_SUFFIX;
optionsParser.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"Options required by the mobile-install command",
ImmutableList.of("--output_groups=" + outputGroup));
} else {
optionsParser.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"Options required by the skylark implementation of mobile-install command",
ImmutableList.of(
"--aspects=" + options.mobileInstallAspect + "%" + options.mode.getAspectName(),
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 ee0e487..37af8b5 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.SetValue;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.UseDefault;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import java.util.ArrayList;
import java.util.Collections;
@@ -35,7 +36,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -51,8 +51,6 @@
private static final Logger logger = Logger.getLogger(InvocationPolicyEnforcer.class.getName());
private static final String INVOCATION_POLICY_SOURCE = "Invocation policy";
- private static final Function<OptionDefinition, String> INVOCATION_POLICY_SOURCE_FUNCTION =
- o -> INVOCATION_POLICY_SOURCE;
@Nullable private final InvocationPolicy invocationPolicy;
private final Level loglevel;
@@ -82,10 +80,13 @@
private static final class FlagPolicyWithContext {
private final FlagPolicy policy;
private final OptionDescription description;
+ private final OptionInstanceOrigin origin;
- public FlagPolicyWithContext(FlagPolicy policy, OptionDescription description) {
+ public FlagPolicyWithContext(
+ FlagPolicy policy, OptionDescription description, OptionInstanceOrigin origin) {
this.policy = policy;
this.description = description;
+ this.origin = origin;
}
}
@@ -160,6 +161,7 @@
new FilterValueOperation.AllowValueOperation(loglevel);
allowValueOperation.apply(
parser,
+ flagPolicy.origin,
allowValues.getAllowedValuesList(),
allowValues.hasNewValue() ? allowValues.getNewValue() : null,
allowValues.hasUseDefault(),
@@ -173,6 +175,7 @@
new FilterValueOperation.DisallowValueOperation(loglevel);
disallowValueOperation.apply(
parser,
+ flagPolicy.origin,
disallowValues.getDisallowedValuesList(),
disallowValues.hasNewValue() ? disallowValues.getNewValue() : null,
disallowValues.hasUseDefault(),
@@ -242,14 +245,22 @@
// Expand all policies to transfer policies on expansion flags to policies on the child flags.
List<FlagPolicyWithContext> expandedPolicies = new ArrayList<>();
+ OptionPriority nextPriority =
+ OptionPriority.lowestOptionPriorityAtCategory(PriorityCategory.INVOCATION_POLICY);
for (FlagPolicy policy : invocationPolicy.getFlagPoliciesList()) {
+ // These policies are high-level, before expansion, and so are not the implicitDependents or
+ // expansions of any other flag, other than in an obtuse sense from --invocation_policy.
+ OptionPriority currentPriority = nextPriority;
+ OptionInstanceOrigin origin =
+ new OptionInstanceOrigin(currentPriority, INVOCATION_POLICY_SOURCE, null, null);
+ nextPriority = OptionPriority.nextOptionPriority(currentPriority);
if (!policyApplies(policy, commandAndParentCommands)) {
// Only keep and expand policies that are applicable to the current command.
continue;
}
+
OptionDescription optionDescription =
- parser.getOptionDescription(
- policy.getFlagName(), OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE);
+ parser.getOptionDescription(policy.getFlagName(), origin);
if (optionDescription == null) {
// InvocationPolicy ignores policy on non-existing flags by design, for version
// compatibility.
@@ -261,8 +272,9 @@
continue;
}
FlagPolicyWithContext policyWithContext =
- new FlagPolicyWithContext(policy, optionDescription);
- List<FlagPolicyWithContext> policies = expandPolicy(policyWithContext, parser, loglevel);
+ new FlagPolicyWithContext(policy, optionDescription, origin);
+ List<FlagPolicyWithContext> policies =
+ expandPolicy(policyWithContext, currentPriority, parser, loglevel);
expandedPolicies.addAll(policies);
}
@@ -289,7 +301,8 @@
}
private static ImmutableList<ParsedOptionDescription> getExpansionsFromFlagPolicy(
- FlagPolicyWithContext expansionPolicy, OptionsParser parser) throws OptionsParsingException {
+ FlagPolicyWithContext expansionPolicy, OptionPriority priority, OptionsParser parser)
+ throws OptionsParsingException {
if (!expansionPolicy.description.isExpansion()) {
return ImmutableList.of();
}
@@ -311,7 +324,7 @@
parser.getExpansionOptionValueDescriptions(
expansionPolicy.description.getOptionDefinition(),
value,
- OptionPriority.INVOCATION_POLICY,
+ priority,
INVOCATION_POLICY_SOURCE));
}
} else {
@@ -319,7 +332,7 @@
parser.getExpansionOptionValueDescriptions(
expansionPolicy.description.getOptionDefinition(),
null,
- OptionPriority.INVOCATION_POLICY,
+ priority,
INVOCATION_POLICY_SOURCE));
}
}
@@ -329,7 +342,7 @@
parser.getExpansionOptionValueDescriptions(
expansionPolicy.description.getOptionDefinition(),
null,
- OptionPriority.INVOCATION_POLICY,
+ priority,
INVOCATION_POLICY_SOURCE));
break;
case ALLOW_VALUES:
@@ -364,21 +377,50 @@
* <p>None of the flagPolicies returned should be on expansion flags.
*/
private static List<FlagPolicyWithContext> expandPolicy(
- FlagPolicyWithContext originalPolicy, OptionsParser parser, Level loglevel)
+ FlagPolicyWithContext originalPolicy,
+ OptionPriority priority,
+ OptionsParser parser,
+ Level loglevel)
throws OptionsParsingException {
List<FlagPolicyWithContext> expandedPolicies = new ArrayList<>();
- ImmutableList<ParsedOptionDescription> expansions =
- getExpansionsFromFlagPolicy(originalPolicy, parser);
- ImmutableList.Builder<ParsedOptionDescription> subflagBuilder = ImmutableList.builder();
- ImmutableList<ParsedOptionDescription> subflags =
- subflagBuilder
- .addAll(originalPolicy.description.getImplicitRequirements())
- .addAll(expansions)
- .build();
+ OptionInstanceOrigin originOfSubflags;
+ ImmutableList<ParsedOptionDescription> subflags;
+ if (originalPolicy.description.getOptionDefinition().hasImplicitRequirements()) {
+ originOfSubflags =
+ new OptionInstanceOrigin(
+ originalPolicy.origin.getPriority(),
+ String.format(
+ "implicitly required by %s (source: %s)",
+ originalPolicy.description.getOptionDefinition(),
+ originalPolicy.origin.getSource()),
+ originalPolicy.description.getOptionDefinition(),
+ null);
+ subflags = originalPolicy.description.getImplicitRequirements();
+ } else if (originalPolicy.description.getOptionDefinition().isExpansionOption()) {
+ subflags = getExpansionsFromFlagPolicy(originalPolicy, priority, parser);
+ originOfSubflags =
+ new OptionInstanceOrigin(
+ originalPolicy.origin.getPriority(),
+ String.format(
+ "expanded by %s (source: %s)",
+ originalPolicy.description.getOptionDefinition(),
+ originalPolicy.origin.getSource()),
+ null,
+ originalPolicy.description.getOptionDefinition());
+ } else {
+ return ImmutableList.of(originalPolicy);
+ }
boolean isExpansion = originalPolicy.description.isExpansion();
+ // We do not get to this point unless there are flags to expand to.
+ boolean hasFlagsToExpandTo = !subflags.isEmpty();
+ Preconditions.checkArgument(
+ hasFlagsToExpandTo,
+ "The only policies that need expanding are those with expansions or implicit requirements, "
+ + "%s has neither yet was not returned as-is.",
+ originalPolicy.description.getOptionDefinition());
- if (!subflags.isEmpty() && logger.isLoggable(loglevel)) {
+ if (logger.isLoggable(loglevel)) {
// Log the expansion. This is only really useful for understanding the invocation policy
// itself.
List<String> subflagNames = new ArrayList<>(subflags.size());
@@ -409,9 +451,7 @@
for (ParsedOptionDescription currentSubflag : subflags) {
OptionDescription subflagOptionDescription =
parser.getOptionDescription(
- currentSubflag.getOptionDefinition().getOptionName(),
- OptionPriority.INVOCATION_POLICY,
- INVOCATION_POLICY_SOURCE);
+ currentSubflag.getOptionDefinition().getOptionName(), originOfSubflags);
if (currentSubflag.getOptionDefinition().allowsMultiple()
&& originalPolicy.policy.getOperationCase().equals(OperationCase.SET_VALUE)) {
@@ -421,7 +461,7 @@
getSingleValueSubflagAsPolicy(
subflagOptionDescription, currentSubflag, originalPolicy, isExpansion);
// In case any of the expanded flags are themselves expansions, recurse.
- expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser, loglevel));
+ expandedPolicies.addAll(expandPolicy(subflagAsPolicy, priority, parser, loglevel));
}
}
@@ -434,7 +474,8 @@
for (ParsedOptionDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) {
newValues.add(setValue.getUnconvertedValue());
}
- expandedPolicies.add(getSetValueSubflagAsPolicy(repeatableFlag, newValues, originalPolicy));
+ expandedPolicies.add(
+ getSetValueSubflagAsPolicy(repeatableFlag, newValues, originOfSubflags, originalPolicy));
}
// Don't add the original policy if it was an expansion flag, which have no value, but do add
@@ -460,6 +501,7 @@
private static FlagPolicyWithContext getSetValueSubflagAsPolicy(
OptionDescription subflagDesc,
List<String> subflagValue,
+ OptionInstanceOrigin subflagOrigin,
FlagPolicyWithContext originalPolicy) {
// Some sanity checks.
OptionDefinition subflag = subflagDesc.getOptionDefinition();
@@ -487,7 +529,8 @@
.setFlagName(subflag.getOptionName())
.setSetValue(setValueExpansion)
.build(),
- subflagDesc);
+ subflagDesc,
+ subflagOrigin);
}
/**
@@ -516,7 +559,9 @@
} else {
subflagValue = ImmutableList.of(currentSubflag.getUnconvertedValue());
}
- subflagAsPolicy = getSetValueSubflagAsPolicy(subflagContext, subflagValue, originalPolicy);
+ subflagAsPolicy =
+ getSetValueSubflagAsPolicy(
+ subflagContext, subflagValue, currentSubflag.getOrigin(), originalPolicy);
break;
case USE_DEFAULT:
@@ -528,7 +573,8 @@
.setFlagName(currentSubflag.getOptionDefinition().getOptionName())
.setUseDefault(UseDefault.getDefaultInstance())
.build(),
- subflagContext);
+ subflagContext,
+ currentSubflag.getOrigin());
break;
case ALLOW_VALUES:
@@ -582,8 +628,8 @@
if (setValue.getFlagValueCount() == 0) {
throw new OptionsParsingException(
String.format(
- "SetValue operation from invocation policy for flag '%s' does not have a value",
- optionDefinition.getOptionName()));
+ "SetValue operation from invocation policy for %s does not have a value",
+ optionDefinition));
}
// Flag must allow multiple values if multiple values are specified by the policy.
@@ -591,9 +637,9 @@
&& !flagPolicy.description.getOptionDefinition().allowsMultiple()) {
throw new OptionsParsingException(
String.format(
- "SetValue operation from invocation policy sets multiple values for flag '%s' which "
+ "SetValue operation from invocation policy sets multiple values for %s which "
+ "does not allow multiple values",
- optionDefinition.getOptionName()));
+ optionDefinition));
}
if (setValue.getOverridable() && valueDescription != null) {
@@ -601,11 +647,11 @@
// value.
logInApplySetValueOperation(
loglevel,
- "Keeping value '%s' from source '%s' for flag '%s' "
- + "because the invocation policy specifying the value(s) '%s' is overridable",
+ "Keeping value '%s' from source '%s' for %s because the invocation policy specifying "
+ + "the value(s) '%s' is overridable",
valueDescription.getValue(),
valueDescription.getSourceString(),
- optionDefinition.getOptionName(),
+ optionDefinition,
setValue.getFlagValueList());
} else {
@@ -619,22 +665,23 @@
if (valueDescription == null) {
logInApplySetValueOperation(
loglevel,
- "Setting value for flag '%s' from invocation policy to '%s', overriding the "
- + "default value '%s'",
- optionDefinition.getOptionName(),
+ "Setting value for %s from invocation policy to '%s', overriding the default value "
+ + "'%s'",
+ optionDefinition,
flagValue,
optionDefinition.getDefaultValue());
} else {
logInApplySetValueOperation(
loglevel,
- "Setting value for flag '%s' from invocation policy to '%s', overriding "
- + "value '%s' from '%s'",
- optionDefinition.getOptionName(),
+ "Setting value for %s from invocation policy to '%s', overriding value '%s' from "
+ + "'%s'",
+ optionDefinition,
flagValue,
valueDescription.getValue(),
valueDescription.getSourceString());
}
- setFlagValue(parser, optionDefinition, flagValue);
+
+ parser.addOptionValueAtSpecificPriority(flagPolicy.origin, optionDefinition, flagValue);
}
}
}
@@ -708,6 +755,7 @@
void apply(
OptionsParser parser,
+ OptionInstanceOrigin origin,
List<String> policyValues,
String newValue,
boolean useDefault,
@@ -746,11 +794,9 @@
if (!defaultValueAllowed && useDefault) {
throw new OptionsParsingException(
String.format(
- "%sValues policy disallows the default value '%s' for flag '%s' but also "
- + "specifies to use the default value",
- policyType,
- optionDefinition.getDefaultValue(),
- optionDefinition.getOptionName()));
+ "%sValues policy disallows the default value '%s' for %s but also specifies to "
+ + "use the default value",
+ policyType, optionDefinition.getDefaultValue(), optionDefinition));
}
}
@@ -760,10 +806,12 @@
// the flag allowing multiple values, however, flags that allow multiple values cannot have
// default values, and their value is always the empty list if they haven't been specified,
// which is why new_default_value is not a repeated field.
- checkDefaultValue(parser, optionDescription, policyValues, newValue, convertedPolicyValues);
+ checkDefaultValue(
+ parser, origin, optionDescription, policyValues, newValue, convertedPolicyValues);
} else {
checkUserValue(
parser,
+ origin,
optionDescription,
valueDescription,
policyValues,
@@ -775,6 +823,7 @@
void checkDefaultValue(
OptionsParser parser,
+ OptionInstanceOrigin origin,
OptionDescription optionDescription,
List<String> policyValues,
String newValue,
@@ -785,27 +834,27 @@
if (!isFlagValueAllowed(
convertedPolicyValues, optionDescription.getOptionDefinition().getDefaultValue())) {
if (newValue != null) {
- // Use the default value from the policy.
+ // Use the default value from the policy, since the original default is not allowed
logger.log(
loglevel,
String.format(
- "Overriding default value '%s' for flag '%s' with value '%s' specified by "
- + "invocation policy. %sed values are: %s",
+ "Overriding default value '%s' for %s with value '%s' specified by invocation "
+ + "policy. %sed values are: %s",
optionDefinition.getDefaultValue(),
- optionDefinition.getOptionName(),
+ optionDefinition,
newValue,
policyType,
policyValues));
parser.clearValue(optionDefinition);
- setFlagValue(parser, optionDefinition, newValue);
+ parser.addOptionValueAtSpecificPriority(origin, optionDefinition, newValue);
} else {
// The operation disallows the default value, but doesn't supply a new value.
throw new OptionsParsingException(
String.format(
- "Default flag value '%s' for flag '%s' is not allowed by invocation policy, but "
+ "Default flag value '%s' for %s is not allowed by invocation policy, but "
+ "the policy does not provide a new value. %sed values are: %s",
optionDescription.getOptionDefinition().getDefaultValue(),
- optionDefinition.getOptionName(),
+ optionDefinition,
policyType,
policyValues));
}
@@ -814,6 +863,7 @@
void checkUserValue(
OptionsParser parser,
+ OptionInstanceOrigin origin,
OptionDescription optionDescription,
OptionValueDescription valueDescription,
List<String> policyValues,
@@ -833,9 +883,9 @@
} else {
throw new OptionsParsingException(
String.format(
- "Flag value '%s' for flag '%s' is not allowed by invocation policy. "
- + "%sed values are: %s",
- value, option.getOptionName(), policyType, policyValues));
+ "Flag value '%s' for %s is not allowed by invocation policy. %sed values "
+ + "are: %s",
+ value, option, policyType, policyValues));
}
}
}
@@ -847,35 +897,22 @@
logger.log(
loglevel,
String.format(
- "Overriding disallowed value '%s' for flag '%s' with value '%s' "
+ "Overriding disallowed value '%s' for %s with value '%s' "
+ "specified by invocation policy. %sed values are: %s",
- valueDescription.getValue(),
- option.getOptionName(),
- newValue,
- policyType,
- policyValues));
+ valueDescription.getValue(), option, newValue, policyType, policyValues));
parser.clearValue(option);
- setFlagValue(parser, option, newValue);
+ parser.addOptionValueAtSpecificPriority(origin, option, newValue);
} else if (useDefault) {
applyUseDefaultOperation(parser, policyType + "Values", option, loglevel);
} else {
throw new OptionsParsingException(
String.format(
- "Flag value '%s' for flag '%s' is not allowed by invocation policy and the "
+ "Flag value '%s' for %s is not allowed by invocation policy and the "
+ "policy does not specify a new value. %sed values are: %s",
- valueDescription.getValue(), option.getOptionName(), policyType, policyValues));
+ valueDescription.getValue(), option, policyType, policyValues));
}
}
}
}
}
-
- private static void setFlagValue(OptionsParser parser, OptionDefinition flag, String flagValue)
- throws OptionsParsingException {
-
- parser.parseWithSourceFunction(
- OptionPriority.INVOCATION_POLICY,
- INVOCATION_POLICY_SOURCE_FUNCTION,
- ImmutableList.of(String.format("--%s=%s", flag.getOptionName(), flagValue)));
- }
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionDefinition.java b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
index 40da044..1c01932 100644
--- a/src/main/java/com/google/devtools/common/options/OptionDefinition.java
+++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -303,6 +303,11 @@
return field.hashCode();
}
+ @Override
+ public String toString() {
+ return String.format("option '--%s'", getOptionName());
+ }
+
static final Comparator<OptionDefinition> BY_OPTION_NAME =
Comparator.comparing(OptionDefinition::getOptionName);
diff --git a/src/main/java/com/google/devtools/common/options/OptionPriority.java b/src/main/java/com/google/devtools/common/options/OptionPriority.java
index a28f012..96c471e 100644
--- a/src/main/java/com/google/devtools/common/options/OptionPriority.java
+++ b/src/main/java/com/google/devtools/common/options/OptionPriority.java
@@ -13,50 +13,98 @@
// limitations under the License.
package com.google.devtools.common.options;
+import java.util.Objects;
+
/**
- * The priority of option values, in order of increasing priority.
+ * The position of an option in the interpretation order. Options are interpreted using a
+ * last-option-wins system for single valued options, and are listed in that order for
+ * multiple-valued options.
*
- * <p>In general, new values for options can only override values with a lower or
- * equal priority. Option values provided in annotations in an options class are
- * implicitly at the priority {@code DEFAULT}.
- *
- * <p>The ordering of the priorities is the source-code order. This is consistent
- * with the automatically generated {@code compareTo} method as specified by the
- * Java Language Specification. DO NOT change the source-code order of these
- * values, or you will break code that relies on the ordering.
+ * <p>The position of the option is in category order, and within the priority category in index
+ * order.
*/
-public enum OptionPriority {
+public class OptionPriority implements Comparable<OptionPriority> {
+ private final PriorityCategory priorityCategory;
+ private final int index;
+
+ private OptionPriority(PriorityCategory priorityCategory, int index) {
+ this.priorityCategory = priorityCategory;
+ this.index = index;
+ }
+
+ public static OptionPriority lowestOptionPriorityAtCategory(PriorityCategory category) {
+ return new OptionPriority(category, 0);
+ }
+
+ public static OptionPriority nextOptionPriority(OptionPriority priority) {
+ return new OptionPriority(priority.priorityCategory, priority.index + 1);
+ }
+
+ public PriorityCategory getPriorityCategory() {
+ return priorityCategory;
+ }
+
+ @Override
+ public int compareTo(OptionPriority o) {
+ if (priorityCategory.equals(o.priorityCategory)) {
+ return index - o.index;
+ }
+ return priorityCategory.ordinal() - o.priorityCategory.ordinal();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof OptionPriority) {
+ OptionPriority other = (OptionPriority) o;
+ return other.priorityCategory.equals(priorityCategory) && other.index == index;
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(priorityCategory, index);
+ }
/**
- * The priority of values specified in the {@link Option} annotation. This
- * should never be specified in calls to {@link OptionsParser#parse}.
+ * The priority of option values, in order of increasing priority.
+ *
+ * <p>In general, new values for options can only override values with a lower or equal priority.
+ * Option values provided in annotations in an options class are implicitly at the priority {@code
+ * DEFAULT}.
+ *
+ * <p>The ordering of the priorities is the source-code order. This is consistent with the
+ * automatically generated {@code compareTo} method as specified by the Java Language
+ * Specification. DO NOT change the source-code order of these values, or you will break code that
+ * relies on the ordering.
*/
- DEFAULT,
+ public enum PriorityCategory {
- /**
- * Overrides default options at runtime, while still allowing the values to be
- * overridden manually.
- */
- COMPUTED_DEFAULT,
+ /**
+ * The priority of values specified in the {@link Option} annotation. This should never be
+ * specified in calls to {@link OptionsParser#parse}.
+ */
+ DEFAULT,
- /**
- * For options coming from a configuration file or rc file.
- */
- RC_FILE,
+ /**
+ * Overrides default options at runtime, while still allowing the values to be overridden
+ * manually.
+ */
+ COMPUTED_DEFAULT,
- /**
- * For options coming from the command line.
- */
- COMMAND_LINE,
+ /** For options coming from a configuration file or rc file. */
+ RC_FILE,
- /**
- * For options coming from invocation policy.
- */
- INVOCATION_POLICY,
+ /** For options coming from the command line. */
+ COMMAND_LINE,
- /**
- * This priority can be used to unconditionally override any user-provided options.
- * This should be used rarely and with caution!
- */
- SOFTWARE_REQUIREMENT;
+ /** For options coming from invocation policy. */
+ INVOCATION_POLICY,
+
+ /**
+ * This priority can be used to unconditionally override any user-provided options. This should
+ * be used rarely and with caution!
+ */
+ SOFTWARE_REQUIREMENT
+ }
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
index 886e97e..ed03e97 100644
--- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
+++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -18,9 +18,10 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.devtools.common.options.OptionsParser.ConstructionException;
-import java.util.ArrayList;
import java.util.Collection;
+import java.util.Comparator;
import java.util.List;
+import java.util.Map.Entry;
import java.util.stream.Collectors;
/**
@@ -168,40 +169,34 @@
if (!implicitDependent.equals(optionThatDependsOnEffectiveValue)) {
warnings.add(
String.format(
- "Option '%s' is implicitly defined by both option '%s' and option '%s'",
- optionDefinition.getOptionName(),
- optionThatDependsOnEffectiveValue.getOptionName(),
- implicitDependent.getOptionName()));
+ "%s is implicitly defined by both %s and %s",
+ optionDefinition, optionThatDependsOnEffectiveValue, implicitDependent));
}
} else if ((implicitDependent != null)
&& parsedOption.getPriority().equals(effectiveOptionInstance.getPriority())) {
warnings.add(
String.format(
- "Option '%s' is implicitly defined by option '%s'; the implicitly set value "
+ "%s is implicitly defined by %s; the implicitly set value "
+ "overrides the previous one",
- optionDefinition.getOptionName(), implicitDependent.getOptionName()));
+ optionDefinition, implicitDependent));
} else if (optionThatDependsOnEffectiveValue != null) {
warnings.add(
String.format(
- "A new value for option '%s' overrides a previous implicit setting of that "
- + "option by option '%s'",
- optionDefinition.getOptionName(),
- optionThatDependsOnEffectiveValue.getOptionName()));
- } else if ((parsedOption.getPriority() == effectiveOptionInstance.getPriority())
+ "A new value for %s overrides a previous implicit setting of that "
+ + "option by %s",
+ optionDefinition, optionThatDependsOnEffectiveValue));
+ } else if ((parsedOption.getPriority().equals(effectiveOptionInstance.getPriority()))
&& ((optionThatExpandedToEffectiveValue == null) && (expandedFrom != null))) {
// Create a warning if an expansion option overrides an explicit option:
warnings.add(
String.format(
- "The option '%s' was expanded and now overrides a previous explicitly "
- + "specified option '%s'",
- expandedFrom.getOptionName(), optionDefinition.getOptionName()));
+ "%s was expanded and now overrides a previous explicitly specified %s",
+ expandedFrom, optionDefinition));
} else if ((optionThatExpandedToEffectiveValue != null) && (expandedFrom != null)) {
warnings.add(
String.format(
- "The option '%s' was expanded to from both options '%s' and '%s'",
- optionDefinition.getOptionName(),
- optionThatExpandedToEffectiveValue.getOptionName(),
- expandedFrom.getOptionName()));
+ "%s was expanded to from both %s and %s",
+ optionDefinition, optionThatExpandedToEffectiveValue, expandedFrom));
}
}
@@ -247,17 +242,15 @@
@Override
public List<Object> getValue() {
// Sort the results by option priority and return them in a new list. The generic type of
- // the list is not known at runtime, so we can't use it here. It was already checked in
- // the constructor, so this is type-safe.
- List<Object> result = new ArrayList<>();
- for (OptionPriority priority : OptionPriority.values()) {
- // If there is no mapping for this key, this check avoids object creation (because
- // ListMultimap has to return a new object on get) and also an unnecessary addAll call.
- if (optionValues.containsKey(priority)) {
- result.addAll(optionValues.get(priority));
- }
- }
- return result;
+ // the list is not known at runtime, so we can't use it here.
+ return optionValues
+ .asMap()
+ .entrySet()
+ .stream()
+ .sorted(Comparator.comparing(Entry::getKey))
+ .map(Entry::getValue)
+ .flatMap(Collection::stream)
+ .collect(Collectors.toList());
}
@Override
diff --git a/src/main/java/com/google/devtools/common/options/Options.java b/src/main/java/com/google/devtools/common/options/Options.java
index b636c09..0783480 100644
--- a/src/main/java/com/google/devtools/common/options/Options.java
+++ b/src/main/java/com/google/devtools/common/options/Options.java
@@ -51,7 +51,7 @@
public static <O extends OptionsBase> Options<O> parse(Class<O> optionsClass, String... args)
throws OptionsParsingException {
OptionsParser parser = OptionsParser.newOptionsParser(optionsClass);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList(args));
List<String> remainingArgs = parser.getResidue();
return new Options<>(parser.getOptions(optionsClass), remainingArgs.toArray(new String[0]));
}
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 5b9a436..69f360a 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -61,10 +61,9 @@
context.getUnparsedValue() != null ? context.getUnparsedValue() : "(null)";
String name = context.getOptionDefinition().getOptionName();
throw new OptionsParsingException(
- "Error expanding option '"
- + name
- + "': no expansions defined for value: "
- + valueString,
+ String.format(
+ "Error expanding %s: no expansions defined for value: %s",
+ context.getOptionDefinition(), valueString),
name);
}
return result;
@@ -159,9 +158,7 @@
staticExpansion =
instance.getExpansion(new ExpansionContext(isolatedData, optionDefinition, null));
Preconditions.checkState(
- staticExpansion != null,
- "Error calling expansion function for option: %s",
- optionDefinition.getOptionName());
+ staticExpansion != null, "Error calling expansion function for %s", optionDefinition);
expansionDataBuilder.put(optionDefinition, new ExpansionData(staticExpansion));
} catch (ExpansionNeedsValueException e) {
// This expansion function needs data that isn't available yet. Save the instance and call
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 25733c7..48dc9f3 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -56,9 +56,10 @@
* <p>FooOptions and BarOptions would be options specification classes, derived from OptionsBase,
* that contain fields annotated with @Option(...).
*
- * <p>Alternatively, rather than calling {@link #parseAndExitUponError(OptionPriority, String,
- * String[])}, client code may call {@link #parse(OptionPriority,String,List)}, and handle parser
- * exceptions usage messages themselves.
+ * <p>Alternatively, rather than calling {@link
+ * #parseAndExitUponError(OptionPriority.PriorityCategory, String, String[])}, client code may call
+ * {@link #parse(OptionPriority.PriorityCategory,String,List)}, and handle parser exceptions usage
+ * messages themselves.
*
* <p>This options parsing implementation has (at least) one design flaw. It allows both '--foo=baz'
* and '--foo baz' for all options except void, boolean and tristate options. For these, the 'baz'
@@ -216,15 +217,16 @@
}
public void parseAndExitUponError(String[] args) {
- parseAndExitUponError(OptionPriority.COMMAND_LINE, "unknown", args);
+ parseAndExitUponError(OptionPriority.PriorityCategory.COMMAND_LINE, "unknown", args);
}
/**
- * A convenience function for use in main methods. Parses the command line
- * parameters, and exits upon error. Also, prints out the usage message
- * if "--help" appears anywhere within {@code args}.
+ * A convenience function for use in main methods. Parses the command line parameters, and exits
+ * upon error. Also, prints out the usage message if "--help" appears anywhere within {@code
+ * args}.
*/
- public void parseAndExitUponError(OptionPriority priority, String source, String[] args) {
+ public void parseAndExitUponError(
+ OptionPriority.PriorityCategory priority, String source, String[] args) {
for (String arg : args) {
if (arg.equals("--help")) {
System.out.println(
@@ -538,9 +540,9 @@
* @return The {@link OptionDescription} for the option, or null if there is no option by the
* given name.
*/
- OptionDescription getOptionDescription(String name, OptionPriority priority, String source)
+ OptionDescription getOptionDescription(String name, OptionInstanceOrigin origin)
throws OptionsParsingException {
- return impl.getOptionDescription(name, priority, source);
+ return impl.getOptionDescription(name, origin);
}
/**
@@ -558,9 +560,9 @@
/**
* 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.
+ * #parse(OptionPriority.PriorityCategory, 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 com.google.devtools.common.options.OptionValueDescription} for the option,
* or null if the value has not been set.
@@ -571,48 +573,64 @@
}
/**
- * A convenience method, equivalent to
- * {@code parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args))}.
+ * A convenience method, equivalent to {@code parse(PriorityCategory.COMMAND_LINE, null,
+ * Arrays.asList(args))}.
*/
public void parse(String... args) throws OptionsParsingException {
- parse(OptionPriority.COMMAND_LINE, null, Arrays.asList(args));
+ parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList(args));
}
/**
- * A convenience method, equivalent to
- * {@code parse(OptionPriority.COMMAND_LINE, null, args)}.
+ * A convenience method, equivalent to {@code parse(PriorityCategory.COMMAND_LINE, null, args)}.
*/
public void parse(List<String> args) throws OptionsParsingException {
- parse(OptionPriority.COMMAND_LINE, null, args);
+ parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, args);
}
/**
- * Parses {@code args}, using the classes registered with this parser.
- * {@link #getOptions(Class)} and {@link #getResidue()} return the results.
- * May be called multiple times; later options override existing ones if they
- * have equal or higher priority. The source of options is a free-form string
- * that can be used for debugging. Strings that cannot be parsed as options
- * accumulates as residue, if this parser allows it.
+ * Parses {@code args}, using the classes registered with this parser, at the given priority.
*
- * @see OptionPriority
+ * <p>May be called multiple times; later options override existing ones if they have equal or
+ * higher priority. Strings that cannot be parsed as options are accumulated as residue, if this
+ * parser allows it.
+ *
+ * <p>{@link #getOptions(Class)} and {@link #getResidue()} will return the results.
+ *
+ * @param priority the priority at which to parse these options. Within this priority category,
+ * each option will be given an index to track its position. If parse() has already been
+ * called at this priority, the indexing will continue where it left off, to keep ordering.
+ * @param source the source to track for each option parsed.
+ * @param args the arg list to parse. Each element might be an option, a value linked to an
+ * option, or residue.
*/
- public void parse(OptionPriority priority, String source,
- List<String> args) throws OptionsParsingException {
+ public void parse(OptionPriority.PriorityCategory priority, String source, List<String> args)
+ throws OptionsParsingException {
parseWithSourceFunction(priority, o -> source, args);
}
/**
- * Parses {@code args}, using the classes registered with this parser. {@link #getOptions(Class)}
- * and {@link #getResidue()} return the results. May be called multiple times; later options
- * override existing ones if they have equal or higher priority. The source of options is given as
- * a function that maps option names to the source of the option. Strings that cannot be parsed as
- * options accumulates as* residue, if this parser allows it.
+ * Parses {@code args}, using the classes registered with this parser, at the given priority.
+ *
+ * <p>May be called multiple times; later options override existing ones if they have equal or
+ * higher priority. Strings that cannot be parsed as options are accumulated as residue, if this
+ * parser allows it.
+ *
+ * <p>{@link #getOptions(Class)} and {@link #getResidue()} will return the results.
+ *
+ * @param priority the priority at which to parse these options. Within this priority category,
+ * each option will be given an index to track its position. If parse() has already been
+ * called at this priority, the indexing will continue where it left off, to keep ordering.
+ * @param sourceFunction a function that maps option names to the source of the option.
+ * @param args the arg list to parse. Each element might be an option, a value linked to an
+ * option, or residue.
*/
public void parseWithSourceFunction(
- OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args)
+ OptionPriority.PriorityCategory priority,
+ Function<OptionDefinition, String> sourceFunction,
+ List<String> args)
throws OptionsParsingException {
Preconditions.checkNotNull(priority);
- Preconditions.checkArgument(priority != OptionPriority.DEFAULT);
+ Preconditions.checkArgument(priority != OptionPriority.PriorityCategory.DEFAULT);
residue.addAll(impl.parse(priority, sourceFunction, args));
if (!allowResidue && !residue.isEmpty()) {
String errorMsg = "Unrecognized arguments: " + Joiner.on(' ').join(residue);
@@ -621,6 +639,20 @@
}
/**
+ * @param origin the origin of this option instance, it includes the priority of the value. If
+ * other values have already been or will be parsed at a higher priority, they might override
+ * the provided value. If this option already has a value at this priority, this value will
+ * have precedence, but this should be avoided, as it breaks order tracking.
+ * @param option the option to add the value for.
+ * @param value the value to add at the given priority.
+ */
+ void addOptionValueAtSpecificPriority(
+ OptionInstanceOrigin origin, OptionDefinition option, String value)
+ throws OptionsParsingException {
+ impl.addOptionValueAtSpecificPriority(origin, option, value);
+ }
+
+ /**
* Clears the given option.
*
* <p>This will not affect options objects that have already been retrieved from this parser
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 f584991..96f4c16 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -23,6 +23,7 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser.OptionDescription;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
@@ -52,7 +53,8 @@
* OptionDefinition("--port") -> 80
* </pre>
*
- * This map is modified by repeated calls to {@link #parse(OptionPriority,Function,List)}.
+ * This map is modified by repeated calls to {@link #parse(OptionPriority.PriorityCategory,
+ * Function,List)}.
*/
private final Map<OptionDefinition, OptionValueDescription> optionValues = new HashMap<>();
@@ -185,8 +187,9 @@
}
private void addDeprecationWarning(String optionName, String warning) {
- warnings.add("Option '" + optionName + "' is deprecated"
- + (warning.isEmpty() ? "" : ": " + warning));
+ warnings.add(
+ String.format(
+ "Option '%s' is deprecated%s", optionName, (warning.isEmpty() ? "" : ": " + warning)));
}
@@ -205,7 +208,7 @@
return optionValues.get(optionDefinition);
}
- OptionDescription getOptionDescription(String name, OptionPriority priority, String source)
+ OptionDescription getOptionDescription(String name, OptionInstanceOrigin origin)
throws OptionsParsingException {
OptionDefinition optionDefinition = optionsData.getOptionDefinitionFromName(name);
if (optionDefinition == null) {
@@ -218,16 +221,14 @@
getImplicitDependentDescriptions(
ImmutableList.copyOf(optionDefinition.getImplicitRequirements()),
optionDefinition,
- priority,
- source));
+ origin));
}
/** @return A list of the descriptions corresponding to the implicit dependent flags passed in. */
private ImmutableList<ParsedOptionDescription> getImplicitDependentDescriptions(
ImmutableList<String> options,
OptionDefinition implicitDependent,
- OptionPriority priority,
- String source)
+ OptionInstanceOrigin dependentsOrigin)
throws OptionsParsingException {
ImmutableList.Builder<ParsedOptionDescription> builder = ImmutableList.builder();
Iterator<String> optionsIterator = options.iterator();
@@ -235,15 +236,15 @@
Function<OptionDefinition, String> sourceFunction =
o ->
String.format(
- "implicitely required for option %s (source: %s)",
- implicitDependent.getOptionName(), source);
+ "implicitely required for %s (source: %s)",
+ implicitDependent, dependentsOrigin.getSource());
while (optionsIterator.hasNext()) {
String unparsedFlagExpression = optionsIterator.next();
ParsedOptionDescription parsedOption =
identifyOptionAndPossibleArgument(
unparsedFlagExpression,
optionsIterator,
- priority,
+ dependentsOrigin.getPriority(),
sourceFunction,
implicitDependent,
null);
@@ -267,7 +268,7 @@
ImmutableList<String> options = optionsData.getEvaluatedExpansion(expansionFlag, flagValue);
Iterator<String> optionsIterator = options.iterator();
Function<OptionDefinition, String> sourceFunction =
- o -> String.format("expanded from %s (source: %s)", expansionFlag.getOptionName(), source);
+ o -> String.format("expanded from %s (source: %s)", expansionFlag, source);
while (optionsIterator.hasNext()) {
String unparsedFlagExpression = optionsIterator.next();
ParsedOptionDescription parsedOption =
@@ -293,13 +294,18 @@
/**
* Parses the args, and returns what it doesn't parse. May be called multiple times, and may be
- * called recursively. In each call, there may be no duplicates, but separate calls may contain
- * intersecting sets of options; in that case, the arg seen last takes precedence.
+ * called recursively. The option's definition dictates how it reacts to multiple settings. By
+ * default, the arg seen last at the highest priority takes precedence, overriding the early
+ * values. Options that accumulate multiple values will track them in priority and appearance
+ * order.
*/
List<String> parse(
- OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args)
+ OptionPriority.PriorityCategory priority,
+ Function<OptionDefinition, String> sourceFunction,
+ List<String> args)
throws OptionsParsingException {
- return parse(priority, sourceFunction, null, null, args);
+ return parse(
+ OptionPriority.lowestOptionPriorityAtCategory(priority), sourceFunction, null, null, args);
}
/**
@@ -307,8 +313,8 @@
* called recursively. Calls may contain intersecting sets of options; in that case, the arg seen
* last takes precedence.
*
- * <p>The method uses the invariant that if an option has neither an implicit dependent nor an
- * expanded from value, then it must have been explicitly set.
+ * <p>The method treats options that have neither an implicitDependent nor an expandedFrom value
+ * as explicitly set.
*/
private List<String> parse(
OptionPriority priority,
@@ -336,99 +342,7 @@
ParsedOptionDescription parsedOption =
identifyOptionAndPossibleArgument(
arg, argsIterator, priority, sourceFunction, implicitDependent, expandedFrom);
- OptionDefinition optionDefinition = parsedOption.getOptionDefinition();
- // All options can be deprecated; check and warn before doing any option-type specific work.
- maybeAddDeprecationWarning(optionDefinition);
-
- // Track the value, before any remaining option-type specific work that is done outside of
- // the OptionValueDescription.
- OptionValueDescription entry =
- optionValues.computeIfAbsent(
- optionDefinition, OptionValueDescription::createOptionValueDescription);
- entry.addOptionInstance(parsedOption, warnings);
- @Nullable String unconvertedValue = parsedOption.getUnconvertedValue();
-
- // There are 3 types of flags that expand to other flag values. Expansion flags are the
- // accepted way to do this, but two legacy features remain: implicit requirements and wrapper
- // options. We rely on the OptionProcessor compile-time check's guarantee that no option sets
- // multiple of these behaviors. (In Bazel, --config is another such flag, but that expansion
- // is not controlled within the options parser, so we ignore it here)
-
- // As much as possible, we want the behaviors of these different types of flags to be
- // identical, as this minimizes the number of edge cases, but we do not yet track these values
- // in the same way. Wrapper options are replaced by their value and implicit requirements are
- // hidden from the reported lists of parsed options.
- if (implicitDependent == null && !optionDefinition.isWrapperOption()) {
- // Log explicit options and expanded options in the order they are parsed (can be sorted
- // later). This information is needed to correctly canonicalize flags.
- parsedOptions.add(parsedOption);
- if (optionDefinition.allowsMultiple()) {
- canonicalizeValues.put(optionDefinition, parsedOption);
- } else {
- canonicalizeValues.replaceValues(optionDefinition, ImmutableList.of(parsedOption));
- }
- }
-
- if (optionDefinition.isExpansionOption()
- || optionDefinition.hasImplicitRequirements()
- || optionDefinition.isWrapperOption()) {
- StringBuilder sourceMessage = new StringBuilder();
- ImmutableList<String> expansionArgs;
- if (optionDefinition.isExpansionOption()) {
- expansionArgs = optionsData.getEvaluatedExpansion(optionDefinition, unconvertedValue);
- sourceMessage.append("expanded from option ");
- } else if (optionDefinition.hasImplicitRequirements()) {
- expansionArgs = ImmutableList.copyOf(optionDefinition.getImplicitRequirements());
- sourceMessage.append("implicit requirement of option ");
- } else {
- if (!unconvertedValue.startsWith("-")) {
- // Wrapper options are the only "expansion" flag type that expand to other flags
- // according to user input, so a malformed option is a user error in this case.
- throw new OptionsParsingException(
- "Invalid --"
- + optionDefinition.getOptionName()
- + " value format. "
- + "You may have meant --"
- + optionDefinition.getOptionName()
- + "=--"
- + unconvertedValue);
- } else {
- expansionArgs = ImmutableList.of(unconvertedValue);
- sourceMessage.append("unwrapped from wrapper option ");
- }
- }
- String sourceFunctionApplication = sourceFunction.apply(optionDefinition);
- sourceMessage.append(
- (sourceFunctionApplication == null)
- ? String.format("--%s", optionDefinition.getOptionName())
- : String.format(
- "--%s from %s", optionDefinition.getOptionName(), sourceFunctionApplication));
-
- List<String> unparsed =
- parse(
- priority,
- o -> sourceMessage.toString(),
- optionDefinition.hasImplicitRequirements() ? optionDefinition : null,
- optionDefinition.isExpansionOption() ? optionDefinition : null,
- expansionArgs);
- if (!unparsed.isEmpty()) {
- if (optionDefinition.isWrapperOption()) {
- throw new OptionsParsingException(
- "Unparsed options remain after unwrapping "
- + arg
- + ": "
- + Joiner.on(' ').join(unparsed));
- } else {
- // Throw an assertion here, because this indicates an error in the definition of this
- // option's expansion or requirements, not with the input as provided by the user.
- throw new AssertionError(
- "Unparsed options remain after processing "
- + arg
- + ": "
- + Joiner.on(' ').join(unparsed));
- }
- }
- }
+ handleNewParsedOption(parsedOption);
}
// Go through the final values and make sure they are valid values for their option. Unlike any
@@ -441,6 +355,133 @@
return unparsedArgs;
}
+ /**
+ * Implementation of {@link OptionsParser#addOptionValueAtSpecificPriority(OptionInstanceOrigin,
+ * OptionDefinition, String)}
+ */
+ void addOptionValueAtSpecificPriority(
+ OptionInstanceOrigin origin, OptionDefinition option, String unconvertedValue)
+ throws OptionsParsingException {
+ Preconditions.checkNotNull(option);
+ Preconditions.checkNotNull(
+ unconvertedValue,
+ "Cannot set %s to a null value. Pass \"\" if an empty value is required.",
+ option);
+ Preconditions.checkNotNull(
+ origin,
+ "Cannot assign value \'%s\' to %s without a clear origin for this value.",
+ unconvertedValue,
+ option);
+ PriorityCategory priorityCategory = origin.getPriority().getPriorityCategory();
+ boolean isNotDefault = priorityCategory != OptionPriority.PriorityCategory.DEFAULT;
+ Preconditions.checkArgument(
+ isNotDefault,
+ "Attempt to assign value \'%s\' to %s at priority %s failed. Cannot set options at "
+ + "default priority - by definition, that means the option is unset.",
+ unconvertedValue,
+ option,
+ priorityCategory);
+
+ handleNewParsedOption(
+ new ParsedOptionDescription(
+ option,
+ String.format("--%s=%s", option.getOptionName(), unconvertedValue),
+ unconvertedValue,
+ origin));
+ }
+
+ /** Takes care of tracking the parsed option's value in relation to other options. */
+ private void handleNewParsedOption(ParsedOptionDescription parsedOption)
+ throws OptionsParsingException {
+ OptionDefinition optionDefinition = parsedOption.getOptionDefinition();
+ // All options can be deprecated; check and warn before doing any option-type specific work.
+ maybeAddDeprecationWarning(optionDefinition);
+ // Track the value, before any remaining option-type specific work that is done outside of
+ // the OptionValueDescription.
+ OptionValueDescription entry =
+ optionValues.computeIfAbsent(
+ optionDefinition, OptionValueDescription::createOptionValueDescription);
+ entry.addOptionInstance(parsedOption, warnings);
+ @Nullable String unconvertedValue = parsedOption.getUnconvertedValue();
+
+ // There are 3 types of flags that expand to other flag values. Expansion flags are the
+ // accepted way to do this, but two legacy features remain: implicit requirements and wrapper
+ // options. We rely on the OptionProcessor compile-time check's guarantee that no option sets
+ // multiple of these behaviors. (In Bazel, --config is another such flag, but that expansion
+ // is not controlled within the options parser, so we ignore it here)
+
+ // As much as possible, we want the behaviors of these different types of flags to be
+ // identical, as this minimizes the number of edge cases, but we do not yet track these values
+ // in the same way. Wrapper options are replaced by their value and implicit requirements are
+ // hidden from the reported lists of parsed options.
+ if (parsedOption.getImplicitDependent() == null && !optionDefinition.isWrapperOption()) {
+ // Log explicit options and expanded options in the order they are parsed (can be sorted
+ // later). This information is needed to correctly canonicalize flags.
+ parsedOptions.add(parsedOption);
+ if (optionDefinition.allowsMultiple()) {
+ canonicalizeValues.put(optionDefinition, parsedOption);
+ } else {
+ canonicalizeValues.replaceValues(optionDefinition, ImmutableList.of(parsedOption));
+ }
+ }
+
+ if (optionDefinition.isExpansionOption()
+ || optionDefinition.hasImplicitRequirements()
+ || optionDefinition.isWrapperOption()) {
+ StringBuilder sourceMessage = new StringBuilder();
+ ImmutableList<String> expansionArgs;
+ if (optionDefinition.isExpansionOption()) {
+ expansionArgs = optionsData.getEvaluatedExpansion(optionDefinition, unconvertedValue);
+ sourceMessage.append("expanded from option ");
+ } else if (optionDefinition.hasImplicitRequirements()) {
+ expansionArgs = ImmutableList.copyOf(optionDefinition.getImplicitRequirements());
+ sourceMessage.append("implicit requirement of option ");
+ } else {
+ if (!unconvertedValue.startsWith("-")) {
+ // Wrapper options are the only "expansion" flag type that expand to other flags
+ // according to user input, so a malformed option is a user error in this case.
+ throw new OptionsParsingException(
+ String.format(
+ "Invalid value format for %s. You may have meant --%s=--%s",
+ optionDefinition, optionDefinition.getOptionName(), unconvertedValue));
+ } else {
+ expansionArgs = ImmutableList.of(unconvertedValue);
+ sourceMessage.append("unwrapped from wrapper option ");
+ }
+ }
+ String source = parsedOption.getSource();
+ sourceMessage.append(
+ (source == null)
+ ? String.format("--%s", optionDefinition.getOptionName())
+ : String.format("--%s from %s", optionDefinition.getOptionName(), source));
+
+ List<String> unparsed =
+ parse(
+ parsedOption.getPriority(),
+ o -> sourceMessage.toString(),
+ optionDefinition.hasImplicitRequirements() ? optionDefinition : null,
+ optionDefinition.isExpansionOption() ? optionDefinition : null,
+ expansionArgs);
+ if (!unparsed.isEmpty()) {
+ if (optionDefinition.isWrapperOption()) {
+ throw new OptionsParsingException(
+ "Unparsed options remain after unwrapping "
+ + unconvertedValue
+ + ": "
+ + Joiner.on(' ').join(unparsed));
+ } else {
+ // Throw an assertion here, because this indicates an error in the definition of this
+ // option's expansion or requirements, not with the input as provided by the user.
+ throw new AssertionError(
+ "Unparsed options remain after processing "
+ + unconvertedValue
+ + ": "
+ + Joiner.on(' ').join(unparsed));
+ }
+ }
+ }
+ }
+
private ParsedOptionDescription identifyOptionAndPossibleArgument(
String arg,
Iterator<String> nextArgs,
@@ -562,10 +603,7 @@
optionDefinition.getField().set(optionsInstance, value);
} catch (IllegalArgumentException e) {
throw new IllegalStateException(
- String.format(
- "Unable to set option '%s' to value '%s'.",
- optionDefinition.getOptionName(), value),
- e);
+ String.format("Unable to set %s to value '%s'.", optionDefinition, value), e);
} catch (IllegalAccessException e) {
throw new IllegalStateException(
"Could not set the field due to access issues. This is impossible, as the "
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParsingException.java b/src/main/java/com/google/devtools/common/options/OptionsParsingException.java
index 73b48a0..6b82366 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParsingException.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParsingException.java
@@ -17,7 +17,7 @@
/**
* An exception that's thrown when the {@link OptionsParser} fails.
*
- * @see OptionsParser#parse(OptionPriority,String,java.util.List)
+ * @see OptionsParser#parse(OptionPriority.PriorityCategory,String,java.util.List)
*/
public class OptionsParsingException extends Exception {
private final String invalidArgument;
diff --git a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java
index d558263..f55f8ad 100644
--- a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java
+++ b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java
@@ -115,6 +115,10 @@
return unconvertedValue;
}
+ public OptionInstanceOrigin getOrigin() {
+ return origin;
+ }
+
public OptionPriority getPriority() {
return origin.getPriority();
}
@@ -149,7 +153,7 @@
@Override
public String toString() {
StringBuilder result = new StringBuilder();
- result.append("option '").append(optionDefinition.getOptionName()).append("' ");
+ result.append(optionDefinition);
result.append("set to '").append(unconvertedValue).append("' ");
result.append("with priority ").append(origin.getPriority());
if (origin.getSource() != null) {
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java
index 44724f1..aaeec5b 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java
@@ -27,7 +27,7 @@
import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLineSection.SectionTypeCase;
import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList;
import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.TestOptions;
@@ -217,15 +217,15 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"command line",
ImmutableList.of("--test_string=foo", "--test_multiple_string=bar"));
fakeCommandOptions.parse(
- OptionPriority.INVOCATION_POLICY,
+ PriorityCategory.INVOCATION_POLICY,
"fake invocation policy",
ImmutableList.of("--expanded_c=2"));
fakeCommandOptions.parse(
- OptionPriority.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz"));
+ PriorityCategory.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz"));
CommandLine line =
new OriginalCommandLineEvent(
@@ -260,15 +260,15 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"command line",
ImmutableList.of("--test_string=foo", "--test_multiple_string=bar"));
fakeCommandOptions.parse(
- OptionPriority.INVOCATION_POLICY,
+ PriorityCategory.INVOCATION_POLICY,
"fake invocation policy",
ImmutableList.of("--expanded_c=2"));
fakeCommandOptions.parse(
- OptionPriority.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz"));
+ PriorityCategory.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz"));
CommandLine line =
new CanonicalCommandLineEvent(
@@ -303,7 +303,7 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion"));
+ PriorityCategory.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion"));
CommandLine line =
new OriginalCommandLineEvent(
@@ -335,7 +335,7 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion"));
+ PriorityCategory.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion"));
CommandLine line =
new CanonicalCommandLineEvent(
@@ -375,7 +375,7 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"command line",
ImmutableList.of("--test_implicit_requirement=foo"));
@@ -409,7 +409,7 @@
OptionsParser.newOptionsParser(BlazeServerStartupOptions.class);
OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class);
fakeCommandOptions.parse(
- OptionPriority.COMMAND_LINE,
+ PriorityCategory.COMMAND_LINE,
"command line",
ImmutableList.of("--test_implicit_requirement=foo"));
diff --git a/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java b/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java
index aa23d2a..ddf8dad 100644
--- a/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java
@@ -23,7 +23,7 @@
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionMetadataTag;
-import com.google.devtools.common.options.OptionPriority;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import java.util.Arrays;
@@ -96,8 +96,8 @@
@Test
public void asStringOfExplicitOptionsCorrectSortingByPriority() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(IntrospectionExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--alpha=no"));
- parser.parse(OptionPriority.COMPUTED_DEFAULT, null, Arrays.asList("--beta=no"));
+ parser.parse(PriorityCategory.COMMAND_LINE, null, Arrays.asList("--alpha=no"));
+ parser.parse(PriorityCategory.COMPUTED_DEFAULT, null, Arrays.asList("--beta=no"));
assertThat(OptionsUtils.asShellEscapedString(parser)).isEqualTo("--beta=no --alpha=no");
assertThat(OptionsUtils.asArgumentList(parser))
.containsExactly("--beta=no", "--alpha=no")
@@ -127,14 +127,14 @@
@Test
public void asStringOfExplicitOptionsWithBooleans() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(BooleanOpts.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--b_one", "--nob_two"));
+ parser.parse(PriorityCategory.COMMAND_LINE, null, Arrays.asList("--b_one", "--nob_two"));
assertThat(OptionsUtils.asShellEscapedString(parser)).isEqualTo("--b_one --nob_two");
assertThat(OptionsUtils.asArgumentList(parser))
.containsExactly("--b_one", "--nob_two")
.inOrder();
parser = OptionsParser.newOptionsParser(BooleanOpts.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--b_one=true", "--b_two=0"));
+ parser.parse(PriorityCategory.COMMAND_LINE, null, Arrays.asList("--b_one=true", "--b_two=0"));
assertThat(parser.getOptions(BooleanOpts.class).bOne).isTrue();
assertThat(parser.getOptions(BooleanOpts.class).bTwo).isFalse();
assertThat(OptionsUtils.asShellEscapedString(parser)).isEqualTo("--b_one --nob_two");
@@ -146,8 +146,8 @@
@Test
public void asStringOfExplicitOptionsMultipleOptionsAreMultipleTimes() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(IntrospectionExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--alpha=one"));
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--alpha=two"));
+ parser.parse(PriorityCategory.COMMAND_LINE, null, Arrays.asList("--alpha=one"));
+ parser.parse(PriorityCategory.COMMAND_LINE, null, Arrays.asList("--alpha=two"));
assertThat(OptionsUtils.asShellEscapedString(parser)).isEqualTo("--alpha=one --alpha=two");
assertThat(OptionsUtils.asArgumentList(parser))
.containsExactly("--alpha=one", "--alpha=two")
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyAllowValuesTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyAllowValuesTest.java
index d9112d0..b862588 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicyAllowValuesTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyAllowValuesTest.java
@@ -266,7 +266,8 @@
assertThat(e)
.hasMessageThat()
.contains(
- "Flag value 'b' for flag 'test_list_converters' is not allowed by invocation policy");
+ "Flag value 'b' for option '--test_list_converters' is not allowed by invocation "
+ + "policy");
}
}
}
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyDisallowValuesTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyDisallowValuesTest.java
index 9aa2fc9..2cfaccd 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicyDisallowValuesTest.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyDisallowValuesTest.java
@@ -268,7 +268,8 @@
assertThat(e)
.hasMessageThat()
.contains(
- "Flag value 'a' for flag 'test_list_converters' is not allowed by invocation policy");
+ "Flag value 'a' for option '--test_list_converters' is not allowed by invocation "
+ + "policy");
}
}
}
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 dccc44b..79ac532 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -19,9 +19,12 @@
import static java.util.Arrays.asList;
import static org.junit.Assert.fail;
+import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ListMultimap;
import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
import com.google.devtools.common.options.OptionValueDescription.RepeatableOptionValueDescription;
import com.google.devtools.common.options.OptionValueDescription.SingleOptionValueDescription;
import java.io.ByteArrayInputStream;
@@ -36,11 +39,13 @@
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.function.Consumer;
import java.util.function.Predicate;
import org.junit.Test;
@@ -914,7 +919,8 @@
@Test
public void implicitDependencyHasImplicitDependency() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ImplicitDependencyOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first=first"));
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first=first"));
assertThat(parser.getOptions(ImplicitDependencyOptions.class).first).isEqualTo("first");
assertThat(parser.getOptions(ImplicitDependencyOptions.class).second).isEqualTo("second");
assertThat(parser.getOptions(ImplicitDependencyOptions.class).third).isEqualTo("third");
@@ -935,7 +941,8 @@
public void badImplicitDependency() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(BadImplicitDependencyOptions.class);
try {
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first=first"));
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first=first"));
} catch (AssertionError e) {
/* Expected error. */
return;
@@ -958,7 +965,7 @@
public void badExpansionOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(BadExpansionOptions.class);
try {
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first"));
} catch (AssertionError e) {
/* Expected error. */
return;
@@ -1029,7 +1036,7 @@
} catch (OptionsParser.ConstructionException e) {
assertThat(e)
.hasMessageThat()
- .isEqualTo("Error calling expansion function for option: badness");
+ .isEqualTo("Error calling expansion function for option '--badness'");
}
}
@@ -1121,7 +1128,9 @@
public void overrideExpansionWithExplicit() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class);
parser.parse(
- OptionPriority.COMMAND_LINE, null, Arrays.asList("--expands", "--underlying=direct_value"));
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ null,
+ Arrays.asList("--expands", "--underlying=direct_value"));
ExpansionOptions options = parser.getOptions(ExpansionOptions.class);
assertThat(options.underlying).isEqualTo("direct_value");
assertThat(parser.getWarnings()).isEmpty();
@@ -1130,7 +1139,7 @@
@Test
public void testExpansionOriginIsPropagatedToOption() throws OptionsParsingException {
OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--expands"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--expands"));
OptionValueDescription expansionDescription = parser.getOptionValueDescription("expands");
assertThat(expansionDescription).isNotNull();
@@ -1150,7 +1159,9 @@
public void overrideExplicitWithExpansion() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ExpansionOptions.class);
parser.parse(
- OptionPriority.COMMAND_LINE, null, Arrays.asList("--underlying=direct_value", "--expands"));
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ null,
+ Arrays.asList("--underlying=direct_value", "--expands"));
ExpansionOptions options = parser.getOptions(ExpansionOptions.class);
assertThat(options.underlying).isEqualTo("from_expansion");
}
@@ -1161,7 +1172,7 @@
public void multipleExpansionOptionsWithValue() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ExpansionMultipleOptions.class);
parser.parse(
- OptionPriority.COMMAND_LINE,
+ OptionPriority.PriorityCategory.COMMAND_LINE,
null,
Arrays.asList("--expands_by_function=a", "--expands_by_function=b"));
ExpansionMultipleOptions options = parser.getOptions(ExpansionMultipleOptions.class);
@@ -1171,18 +1182,18 @@
@Test
public void overrideWithHigherPriority() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NullTestOptions.class);
- parser.parse(OptionPriority.RC_FILE, null, Arrays.asList("--simple=a"));
+ parser.parse(OptionPriority.PriorityCategory.RC_FILE, null, Arrays.asList("--simple=a"));
assertThat(parser.getOptions(NullTestOptions.class).simple).isEqualTo("a");
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--simple=b"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--simple=b"));
assertThat(parser.getOptions(NullTestOptions.class).simple).isEqualTo("b");
}
@Test
public void overrideWithLowerPriority() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NullTestOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--simple=a"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--simple=a"));
assertThat(parser.getOptions(NullTestOptions.class).simple).isEqualTo("a");
- parser.parse(OptionPriority.RC_FILE, null, Arrays.asList("--simple=b"));
+ parser.parse(OptionPriority.PriorityCategory.RC_FILE, null, Arrays.asList("--simple=b"));
assertThat(parser.getOptions(NullTestOptions.class).simple).isEqualTo("a");
}
@@ -1206,7 +1217,9 @@
@Test
public void getOptionValueDescriptionWithValue() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NullTestOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, "my description",
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "my description",
Arrays.asList("--simple=abc"));
OptionValueDescription result = parser.getOptionValueDescription("simple");
assertThat(result).isNotNull();
@@ -1218,7 +1231,8 @@
// specific to a single-valued option.
SingleOptionValueDescription singleOptionResult = (SingleOptionValueDescription) result;
ParsedOptionDescription singleOptionInstance = singleOptionResult.getEffectiveOptionInstance();
- assertThat(singleOptionInstance.getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(singleOptionInstance.getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(singleOptionInstance.getOptionDefinition().isExpansionOption()).isFalse();
assertThat(singleOptionInstance.getImplicitDependent()).isNull();
assertThat(singleOptionInstance.getExpandedFrom()).isNull();
@@ -1258,7 +1272,7 @@
parser.parse("--second=second", "--first=first");
assertThat(parser.getWarnings())
.containsExactly(
- "Option 'second' is implicitly defined by option 'first'; the implicitly set value "
+ "option '--second' is implicitly defined by option '--first'; the implicitly set value "
+ "overrides the previous one");
}
@@ -1270,8 +1284,8 @@
parser.parse("--second=second");
assertThat(parser.getWarnings())
.containsExactly(
- "A new value for option 'second' overrides a previous implicit setting of that option "
- + "by option 'first'");
+ "A new value for option '--second' overrides a previous implicit setting of that "
+ + "option by option '--first'");
}
@Test
@@ -1280,8 +1294,8 @@
parser.parse("--first=first", "--second=second");
assertThat(parser.getWarnings())
.containsExactly(
- "A new value for option 'second' overrides a previous implicit setting of that "
- + "option by option 'first'");
+ "A new value for option '--second' overrides a previous implicit setting of that "
+ + "option by option '--first'");
}
@Test
@@ -1292,13 +1306,15 @@
parser.parse("--third=third");
assertThat(parser.getWarnings())
.containsExactly(
- "Option 'second' is implicitly defined by both option 'first' and option 'third'");
+ "option '--second' is implicitly defined by both option '--first' and "
+ + "option '--third'");
}
@Test
public void tesDependentOriginIsPropagatedToOption() throws OptionsParsingException {
OptionsParser parser = OptionsParser.newOptionsParser(ImplicitDependencyWarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first=first"));
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first=first"));
OptionValueDescription originalOption = parser.getOptionValueDescription("first");
assertThat(originalOption).isNotNull();
@@ -1357,21 +1373,21 @@
@Test
public void deprecationWarning() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(WarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first"));
assertThat(parser.getWarnings()).isEqualTo(Arrays.asList("Option 'first' is deprecated"));
}
@Test
public void deprecationWarningForListOption() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(WarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--second=a"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--second=a"));
assertThat(parser.getWarnings()).isEqualTo(Arrays.asList("Option 'second' is deprecated"));
}
@Test
public void deprecationWarningForExpansionOption() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(WarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--third"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--third"));
assertThat(parser.getWarnings()).isEqualTo(Arrays.asList("Option 'third' is deprecated"));
assertThat(parser.getOptions(WarningOptions.class).fourth).isTrue();
}
@@ -1379,7 +1395,7 @@
@Test
public void deprecationWarningForAbbreviatedExpansionOption() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(WarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("-t"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("-t"));
assertThat(parser.getWarnings()).isEqualTo(Arrays.asList("Option 'third' is deprecated"));
assertThat(parser.getOptions(WarningOptions.class).fourth).isTrue();
}
@@ -1426,7 +1442,7 @@
@Test
public void newDeprecationWarning() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NewWarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--first"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first"));
assertThat(parser.getWarnings())
.isEqualTo(Arrays.asList("Option 'first' is deprecated: it's gone"));
}
@@ -1434,7 +1450,7 @@
@Test
public void newDeprecationWarningForListOption() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NewWarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--second=a"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--second=a"));
assertThat(parser.getWarnings())
.isEqualTo(Arrays.asList("Option 'second' is deprecated: sorry, no replacement"));
}
@@ -1442,7 +1458,7 @@
@Test
public void newDeprecationWarningForExpansionOption() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(NewWarningOptions.class);
- parser.parse(OptionPriority.COMMAND_LINE, null, Arrays.asList("--third"));
+ parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--third"));
assertThat(parser.getWarnings())
.isEqualTo(Arrays.asList("Option 'third' is deprecated: use --forth instead"));
assertThat(parser.getOptions(NewWarningOptions.class).fourth).isTrue();
@@ -1482,16 +1498,18 @@
parser.parse("--underlying=underlying", "--first");
assertThat(parser.getWarnings())
.containsExactly(
- "The option 'first' was expanded and now overrides a previous explicitly specified "
- + "option 'underlying'");
+ "option '--first' was expanded and now overrides a previous explicitly specified "
+ + "option '--underlying'");
}
@Test
public void warningForTwoConflictingExpansionOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ExpansionWarningOptions.class);
parser.parse("--first", "--second");
- assertThat(parser.getWarnings()).containsExactly(
- "The option 'underlying' was expanded to from both options 'first' " + "and 'second'");
+ assertThat(parser.getWarnings())
+ .containsExactly(
+ "option '--underlying' was expanded to from both option '--first' and option "
+ + "'--second'");
}
// This test is here to make sure that nobody accidentally changes the
@@ -1499,12 +1517,17 @@
// in the code.
@Test
public void optionPrioritiesAreCorrectlyOrdered() throws Exception {
- assertThat(OptionPriority.values()).hasLength(6);
- assertThat(OptionPriority.DEFAULT).isLessThan(OptionPriority.COMPUTED_DEFAULT);
- assertThat(OptionPriority.COMPUTED_DEFAULT).isLessThan(OptionPriority.RC_FILE);
- assertThat(OptionPriority.RC_FILE).isLessThan(OptionPriority.COMMAND_LINE);
- assertThat(OptionPriority.COMMAND_LINE).isLessThan(OptionPriority.INVOCATION_POLICY);
- assertThat(OptionPriority.INVOCATION_POLICY).isLessThan(OptionPriority.SOFTWARE_REQUIREMENT);
+ assertThat(OptionPriority.PriorityCategory.values()).hasLength(6);
+ assertThat(OptionPriority.PriorityCategory.DEFAULT)
+ .isLessThan(OptionPriority.PriorityCategory.COMPUTED_DEFAULT);
+ assertThat(OptionPriority.PriorityCategory.COMPUTED_DEFAULT)
+ .isLessThan(OptionPriority.PriorityCategory.RC_FILE);
+ assertThat(OptionPriority.PriorityCategory.RC_FILE)
+ .isLessThan(OptionPriority.PriorityCategory.COMMAND_LINE);
+ assertThat(OptionPriority.PriorityCategory.COMMAND_LINE)
+ .isLessThan(OptionPriority.PriorityCategory.INVOCATION_POLICY);
+ assertThat(OptionPriority.PriorityCategory.INVOCATION_POLICY)
+ .isLessThan(OptionPriority.PriorityCategory.SOFTWARE_REQUIREMENT);
}
public static class IntrospectionExample extends OptionsBase {
@@ -1555,7 +1578,9 @@
@Test
public void asListOfUnparsedOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(IntrospectionExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, "source",
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "source",
Arrays.asList("--alpha=one", "--gamma=two", "--echo=three"));
List<ParsedOptionDescription> result = parser.asCompleteListOfParsedOptions();
assertThat(result).isNotNull();
@@ -1566,27 +1591,32 @@
assertThat(result.get(0).isHidden()).isFalse();
assertThat(result.get(0).getUnconvertedValue()).isEqualTo("one");
assertThat(result.get(0).getSource()).isEqualTo("source");
- assertThat(result.get(0).getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(result.get(0).getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(result.get(1).getOptionDefinition().getOptionName()).isEqualTo("gamma");
assertThat(result.get(1).isDocumented()).isFalse();
assertThat(result.get(1).isHidden()).isFalse();
assertThat(result.get(1).getUnconvertedValue()).isEqualTo("two");
assertThat(result.get(1).getSource()).isEqualTo("source");
- assertThat(result.get(1).getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(result.get(1).getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(result.get(2).getOptionDefinition().getOptionName()).isEqualTo("echo");
assertThat(result.get(2).isDocumented()).isFalse();
assertThat(result.get(2).isHidden()).isTrue();
assertThat(result.get(2).getUnconvertedValue()).isEqualTo("three");
assertThat(result.get(2).getSource()).isEqualTo("source");
- assertThat(result.get(2).getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(result.get(2).getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
}
@Test
public void asListOfExplicitOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(IntrospectionExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, "source",
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "source",
Arrays.asList("--alpha=one", "--gamma=two"));
List<ParsedOptionDescription> result = parser.asListOfExplicitOptions();
assertThat(result).isNotNull();
@@ -1596,13 +1626,15 @@
assertThat(result.get(0).isDocumented()).isTrue();
assertThat(result.get(0).getUnconvertedValue()).isEqualTo("one");
assertThat(result.get(0).getSource()).isEqualTo("source");
- assertThat(result.get(0).getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(result.get(0).getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(result.get(1).getOptionDefinition().getOptionName()).isEqualTo("gamma");
assertThat(result.get(1).isDocumented()).isFalse();
assertThat(result.get(1).getUnconvertedValue()).isEqualTo("two");
assertThat(result.get(1).getSource()).isEqualTo("source");
- assertThat(result.get(1).getPriority()).isEqualTo(OptionPriority.COMMAND_LINE);
+ assertThat(result.get(1).getPriority().getPriorityCategory())
+ .isEqualTo(OptionPriority.PriorityCategory.COMMAND_LINE);
}
private void assertOptionValue(
@@ -1615,18 +1647,21 @@
private void assertOptionValue(
String expectedName,
Object expectedValue,
- OptionPriority expectedPriority,
+ OptionPriority.PriorityCategory expectedPriority,
String expectedSource,
SingleOptionValueDescription actual) {
assertOptionValue(expectedName, expectedValue, actual);
assertThat(actual.getSourceString()).isEqualTo(expectedSource);
- assertThat(actual.getEffectiveOptionInstance().getPriority()).isEqualTo(expectedPriority);
+ assertThat(actual.getEffectiveOptionInstance().getPriority().getPriorityCategory())
+ .isEqualTo(expectedPriority);
}
@Test
public void asListOfEffectiveOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(IntrospectionExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, "command line source",
+ parser.parse(
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "command line source",
Arrays.asList("--alpha=alphaValueSetOnCommandLine", "--gamma=gammaValueSetOnCommandLine"));
List<OptionValueDescription> result = parser.asListOfEffectiveOptions();
assertThat(result).isNotNull();
@@ -1641,13 +1676,13 @@
assertOptionValue(
"alpha",
"alphaValueSetOnCommandLine",
- OptionPriority.COMMAND_LINE,
+ OptionPriority.PriorityCategory.COMMAND_LINE,
"command line source",
(SingleOptionValueDescription) map.get("alpha"));
assertOptionValue(
"gamma",
"gammaValueSetOnCommandLine",
- OptionPriority.COMMAND_LINE,
+ OptionPriority.PriorityCategory.COMMAND_LINE,
"command line source",
(SingleOptionValueDescription) map.get("gamma"));
assertOptionValue("beta", "betaDefaultValue", map.get("beta"));
@@ -1672,9 +1707,14 @@
@Test
public void overrideListOptions() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ListExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, "command line source", Arrays.asList("--alpha=cli"));
parser.parse(
- OptionPriority.RC_FILE, "rc file origin", Arrays.asList("--alpha=rc1", "--alpha=rc2"));
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "command line source",
+ Arrays.asList("--alpha=cli"));
+ parser.parse(
+ OptionPriority.PriorityCategory.RC_FILE,
+ "rc file origin",
+ Arrays.asList("--alpha=rc1", "--alpha=rc2"));
assertThat(parser.getOptions(ListExample.class).alpha)
.isEqualTo(Arrays.asList("rc1", "rc2", "cli"));
}
@@ -1682,18 +1722,33 @@
@Test
public void listOptionsHaveCorrectPriorities() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(ListExample.class);
- parser.parse(OptionPriority.COMMAND_LINE, "command line source", Arrays.asList("--alpha=cli"));
parser.parse(
- OptionPriority.RC_FILE, "rc file origin", Arrays.asList("--alpha=rc1", "--alpha=rc2"));
+ OptionPriority.PriorityCategory.COMMAND_LINE,
+ "command line source",
+ Arrays.asList("--alpha=cli"));
+ parser.parse(
+ OptionPriority.PriorityCategory.RC_FILE,
+ "rc file origin",
+ Arrays.asList("--alpha=rc1", "--alpha=rc2"));
OptionValueDescription alphaValue = parser.getOptionValueDescription("alpha");
assertThat(alphaValue).isInstanceOf(RepeatableOptionValueDescription.class);
+ // Rearrange the parsed options so we can group them by PriorityCategory.
RepeatableOptionValueDescription alpha = (RepeatableOptionValueDescription) alphaValue;
- assertThat(alpha.parsedOptions).containsKey(OptionPriority.RC_FILE);
- assertThat(alpha.parsedOptions).containsKey(OptionPriority.COMMAND_LINE);
- List<ParsedOptionDescription> rcOptions = alpha.parsedOptions.get(OptionPriority.RC_FILE);
- List<ParsedOptionDescription> cliOptions = alpha.parsedOptions.get(OptionPriority.COMMAND_LINE);
+
+ ListMultimap<PriorityCategory, ParsedOptionDescription> parsedOptions =
+ ArrayListMultimap.create();
+ for (Entry<OptionPriority, Collection<ParsedOptionDescription>> entry :
+ alpha.parsedOptions.asMap().entrySet()) {
+ parsedOptions.putAll(entry.getKey().getPriorityCategory(), entry.getValue());
+ }
+ assertThat(parsedOptions).containsKey(OptionPriority.PriorityCategory.RC_FILE);
+ assertThat(parsedOptions).containsKey(OptionPriority.PriorityCategory.COMMAND_LINE);
+ List<ParsedOptionDescription> rcOptions =
+ parsedOptions.get(OptionPriority.PriorityCategory.RC_FILE);
+ List<ParsedOptionDescription> cliOptions =
+ parsedOptions.get(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(rcOptions).hasSize(2);
assertThat(rcOptions.get(0).getSource()).matches("rc file origin");
@@ -1721,10 +1776,13 @@
public void commaSeparatedOptionsWithAllowMultiple() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(CommaSeparatedOptionsExample.class);
parser.parse(
- OptionPriority.COMMAND_LINE,
+ OptionPriority.PriorityCategory.COMMAND_LINE,
"command line source",
Arrays.asList("--alpha=one", "--alpha=two,three"));
- parser.parse(OptionPriority.RC_FILE, "rc file origin", Arrays.asList("--alpha=rc1,rc2"));
+ parser.parse(
+ OptionPriority.PriorityCategory.RC_FILE,
+ "rc file origin",
+ Arrays.asList("--alpha=rc1,rc2"));
assertThat(parser.getOptions(CommaSeparatedOptionsExample.class).alpha)
.isEqualTo(Arrays.asList("rc1", "rc2", "one", "two", "three"));
}
@@ -1733,19 +1791,32 @@
public void commaSeparatedListOptionsHaveCorrectPriorities() throws Exception {
OptionsParser parser = OptionsParser.newOptionsParser(CommaSeparatedOptionsExample.class);
parser.parse(
- OptionPriority.COMMAND_LINE,
+ OptionPriority.PriorityCategory.COMMAND_LINE,
"command line source",
Arrays.asList("--alpha=one", "--alpha=two,three"));
- parser.parse(OptionPriority.RC_FILE, "rc file origin", Arrays.asList("--alpha=rc1,rc2,rc3"));
+ parser.parse(
+ OptionPriority.PriorityCategory.RC_FILE,
+ "rc file origin",
+ Arrays.asList("--alpha=rc1,rc2,rc3"));
OptionValueDescription alphaValue = parser.getOptionValueDescription("alpha");
assertThat(alphaValue).isInstanceOf(RepeatableOptionValueDescription.class);
+ // Rearrange the parsed options so we can group them by PriorityCategory.
RepeatableOptionValueDescription alpha = (RepeatableOptionValueDescription) alphaValue;
- assertThat(alpha.parsedOptions).containsKey(OptionPriority.RC_FILE);
- assertThat(alpha.parsedOptions).containsKey(OptionPriority.COMMAND_LINE);
- List<ParsedOptionDescription> rcOptions = alpha.parsedOptions.get(OptionPriority.RC_FILE);
- List<ParsedOptionDescription> cliOptions = alpha.parsedOptions.get(OptionPriority.COMMAND_LINE);
+ ListMultimap<PriorityCategory, ParsedOptionDescription> parsedOptions =
+ ArrayListMultimap.create();
+ for (Entry<OptionPriority, Collection<ParsedOptionDescription>> entry :
+ alpha.parsedOptions.asMap().entrySet()) {
+ parsedOptions.putAll(entry.getKey().getPriorityCategory(), entry.getValue());
+ }
+
+ assertThat(parsedOptions).containsKey(OptionPriority.PriorityCategory.RC_FILE);
+ assertThat(parsedOptions).containsKey(OptionPriority.PriorityCategory.COMMAND_LINE);
+ List<ParsedOptionDescription> rcOptions =
+ parsedOptions.get(OptionPriority.PriorityCategory.RC_FILE);
+ List<ParsedOptionDescription> cliOptions =
+ parsedOptions.get(OptionPriority.PriorityCategory.COMMAND_LINE);
assertThat(rcOptions).hasSize(1);
assertThat(rcOptions.get(0).getSource()).matches("rc file origin");
diff --git a/src/test/shell/integration/incompatible_changes_conflict_test.sh b/src/test/shell/integration/incompatible_changes_conflict_test.sh
index 0b0ed50..af3bad3 100755
--- a/src/test/shell/integration/incompatible_changes_conflict_test.sh
+++ b/src/test/shell/integration/incompatible_changes_conflict_test.sh
@@ -26,9 +26,9 @@
# The clash canary flags are built into the canonicalize-flags command
# specifically for this test suite.
-canary_clash_error="The option 'flag_clash_canary' was expanded to from both "
-canary_clash_error+="options 'flag_clash_canary_expander1' and "
-canary_clash_error+="'flag_clash_canary_expander2'."
+canary_clash_error="option '--flag_clash_canary' was expanded to from both "
+canary_clash_error+="option '--flag_clash_canary_expander1' and "
+canary_clash_error+="option '--flag_clash_canary_expander2'."
# Ensures that we didn't change the formatting of the warning message or
# disable the warning.
@@ -53,8 +53,8 @@
function test_no_conflicts_among_incompatible_changes() {
bazel canonicalize-flags --show_warnings -- --all_incompatible_changes \
&>$TEST_log || fail "bazel canonicalize-flags failed";
- expected="The option '.*' was expanded to from both options "
- expected+="'.*' and '.*'."
+ expected="The option '.*' was expanded to from both option "
+ expected+="'.*' and option '.*'."
fail_msg="Options conflict in expansion of --all_incompatible_changes"
expect_not_log "$expected" "$fail_msg"
}
