Treat parsed option values differently by option type.
There is a vexingly large world of possible option types, each with its own quirks of how it interfaces with new inputs as they come in: values can be
- overridden (default)
- concatenated (allowMultiple)
- flattened (allowMultiple that accepts list inputs)
- disappear into additional flag inputs (expansion flags)
Or some combination of the above, in the case of flags with implicit dependencies and wrapper options.
Begin removing the error-prone treatment of all option types with conditional branches. This model of the different options will make it much easier to isolate the option-type specific logic with the command-line parsing logic. Flags that affect other flags (implicit requirements, expansions, and wrappers) will be migrated in a later change.
This CL does not change flag parsing semantics, just migrates the current parsing logic to the new class structure.
RELNOTES: None.
PiperOrigin-RevId: 169239182
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 4285f33..742acb6 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -547,7 +547,7 @@
"Keeping value '%s' from source '%s' for flag '%s' "
+ "because the invocation policy specifying the value(s) '%s' is overridable",
valueDescription.getValue(),
- valueDescription.getSource(),
+ valueDescription.getSourceString(),
optionDefinition.getOptionName(),
setValue.getFlagValueList());
} else {
@@ -561,17 +561,17 @@
for (String flagValue : setValue.getFlagValueList()) {
if (valueDescription == null) {
logInApplySetValueOperation(
- "Setting value for flag '%s' from invocation "
- + "policy to '%s', overriding the default value '%s'",
+ "Setting value for flag '%s' from invocation policy to '%s', overriding the "
+ + "default value '%s'",
optionDefinition.getOptionName(), flagValue, optionDefinition.getDefaultValue());
} else {
logInApplySetValueOperation(
- "Setting value for flag '%s' from invocation "
- + "policy to '%s', overriding value '%s' from '%s'",
+ "Setting value for flag '%s' from invocation policy to '%s', overriding "
+ + "value '%s' from '%s'",
optionDefinition.getOptionName(),
flagValue,
valueDescription.getValue(),
- valueDescription.getSource());
+ valueDescription.getSourceString());
}
setFlagValue(parser, optionDefinition, flagValue);
}
@@ -585,8 +585,6 @@
if (clearedValueDescription != null) {
// Log the removed value.
String clearedFlagName = clearedValueDescription.getOptionDefinition().getOptionName();
- String originalValue = clearedValueDescription.getValue().toString();
- String source = clearedValueDescription.getSource();
OptionDescription desc =
parser.getOptionDescription(
@@ -597,9 +595,13 @@
}
logger.info(
String.format(
- "Using default value '%s' for flag '%s' as "
- + "specified by %s invocation policy, overriding original value '%s' from '%s'",
- clearedFlagDefaultValue, clearedFlagName, policyType, originalValue, source));
+ "Using default value '%s' for flag '%s' as specified by %s invocation policy, "
+ + "overriding original value '%s' from '%s'",
+ clearedFlagDefaultValue,
+ clearedFlagName,
+ policyType,
+ clearedValueDescription.getValue(),
+ clearedValueDescription.getSourceString()));
}
}
@@ -724,8 +726,8 @@
// Use the default value from the policy.
logger.info(
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 flag '%s' with value '%s' specified by "
+ + "invocation policy. %sed values are: %s",
optionDefinition.getDefaultValue(),
optionDefinition.getOptionName(),
newValue,
@@ -738,8 +740,7 @@
throw new OptionsParsingException(
String.format(
"Default flag value '%s' for flag '%s' is not allowed by invocation policy, but "
- + "the policy does not provide a new value. "
- + "%sed values are: %s",
+ + "the policy does not provide a new value. %sed values are: %s",
optionDescription.getOptionDefinition().getDefaultValue(),
optionDefinition.getOptionName(),
policyType,
