diff options
4 files changed, 89 insertions, 90 deletions
diff --git a/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java index 8b5ba08..59d1a41 100644 --- a/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java +++ b/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java @@ -232,7 +232,7 @@ public final class InvocationPolicyEnforcer { String.format("Disallow_Values on expansion flags like %s is not allowed.", flagName)); } - private static ImmutableList<OptionValueDescription> getExpansionsFromFlagPolicy( + private static ImmutableList<UnparsedOptionValueDescription> getExpansionsFromFlagPolicy( FlagPolicy expansionPolicy, OptionDescription optionDescription, OptionsParser parser) throws OptionsParsingException { if (!optionDescription.isExpansion()) { @@ -248,7 +248,7 @@ public final class InvocationPolicyEnforcer { optionDescription.getOptionDefinition().getOptionName(), expansionPolicy.getFlagName())); - ImmutableList.Builder<OptionValueDescription> resultsBuilder = ImmutableList.builder(); + ImmutableList.Builder<UnparsedOptionValueDescription> resultsBuilder = ImmutableList.builder(); switch (expansionPolicy.getOperationCase()) { case SET_VALUE: { @@ -327,10 +327,10 @@ public final class InvocationPolicyEnforcer { return expandedPolicies; } - ImmutableList<OptionValueDescription> expansions = + ImmutableList<UnparsedOptionValueDescription> expansions = getExpansionsFromFlagPolicy(originalPolicy, originalOptionDescription, parser); - ImmutableList.Builder<OptionValueDescription> subflagBuilder = ImmutableList.builder(); - ImmutableList<OptionValueDescription> subflags = + ImmutableList.Builder<UnparsedOptionValueDescription> subflagBuilder = ImmutableList.builder(); + ImmutableList<UnparsedOptionValueDescription> subflags = subflagBuilder .addAll(originalOptionDescription.getImplicitRequirements()) .addAll(expansions) @@ -342,7 +342,7 @@ public final class InvocationPolicyEnforcer { // only really useful for understanding the invocation policy itself. Most of the time, // invocation policy does not change, so this can be a log level fine. List<String> subflagNames = new ArrayList<>(subflags.size()); - for (OptionValueDescription subflag : subflags) { + for (UnparsedOptionValueDescription subflag : subflags) { subflagNames.add("--" + subflag.getOptionDefinition().getOptionName()); } @@ -360,13 +360,13 @@ public final class InvocationPolicyEnforcer { // Repeated flags are special, and could set multiple times in an expansion, with the user // expecting both values to be valid. Collect these separately. - Multimap<OptionDefinition, OptionValueDescription> repeatableSubflagsInSetValues = + Multimap<OptionDefinition, UnparsedOptionValueDescription> repeatableSubflagsInSetValues = ArrayListMultimap.create(); // Create a flag policy for the child that looks like the parent's policy "transferred" to its // child. Note that this only makes sense for SetValue, when setting an expansion flag, or // UseDefault, when preventing it from being set. - for (OptionValueDescription currentSubflag : subflags) { + for (UnparsedOptionValueDescription currentSubflag : subflags) { if (currentSubflag.getOptionDefinition().allowsMultiple() && originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE)) { repeatableSubflagsInSetValues.put(currentSubflag.getOptionDefinition(), currentSubflag); @@ -384,8 +384,9 @@ public final class InvocationPolicyEnforcer { for (OptionDefinition repeatableFlag : repeatableSubflagsInSetValues.keySet()) { int numValues = repeatableSubflagsInSetValues.get(repeatableFlag).size(); ArrayList<String> newValues = new ArrayList<>(numValues); - for (OptionValueDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) { - newValues.add(setValue.getOriginalValueString()); + for (UnparsedOptionValueDescription setValue : + repeatableSubflagsInSetValues.get(repeatableFlag)) { + newValues.add(setValue.getUnconvertedValue()); } expandedPolicies.add(getSetValueSubflagAsPolicy(repeatableFlag, newValues, originalPolicy)); } @@ -443,7 +444,7 @@ public final class InvocationPolicyEnforcer { * corresponding policy. */ private static FlagPolicy getSingleValueSubflagAsPolicy( - OptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion) + UnparsedOptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion) throws OptionsParsingException { FlagPolicy subflagAsPolicy = null; switch (originalPolicy.getOperationCase()) { @@ -456,10 +457,10 @@ public final class InvocationPolicyEnforcer { // Accept null originalValueStrings, they are expected when the subflag is also an expansion // flag. List<String> subflagValue; - if (currentSubflag.getOriginalValueString() == null) { + if (currentSubflag.getUnconvertedValue() == null) { subflagValue = ImmutableList.of(); } else { - subflagValue = ImmutableList.of(currentSubflag.getOriginalValueString()); + subflagValue = ImmutableList.of(currentSubflag.getUnconvertedValue()); } subflagAsPolicy = getSetValueSubflagAsPolicy( diff --git a/java/com/google/devtools/common/options/OptionsParser.java b/java/com/google/devtools/common/options/OptionsParser.java index 68a9f02..881fb38 100644 --- a/java/com/google/devtools/common/options/OptionsParser.java +++ b/java/com/google/devtools/common/options/OptionsParser.java @@ -235,12 +235,12 @@ public class OptionsParser implements OptionsProvider { private final OptionDefinition optionDefinition; private final OptionsData.ExpansionData expansionData; - private final ImmutableList<OptionValueDescription> implicitRequirements; + private final ImmutableList<UnparsedOptionValueDescription> implicitRequirements; OptionDescription( OptionDefinition definition, OptionsData.ExpansionData expansionData, - ImmutableList<OptionValueDescription> implicitRequirements) { + ImmutableList<UnparsedOptionValueDescription> implicitRequirements) { this.optionDefinition = definition; this.expansionData = expansionData; this.implicitRequirements = implicitRequirements; @@ -250,7 +250,7 @@ public class OptionsParser implements OptionsProvider { return optionDefinition; } - public ImmutableList<OptionValueDescription> getImplicitRequirements() { + public ImmutableList<UnparsedOptionValueDescription> getImplicitRequirements() { return implicitRequirements; } @@ -417,7 +417,7 @@ public class OptionsParser implements OptionsProvider { * @return The {@link com.google.devtools.common.options.OptionValueDescription>} for the option, * or null if there is no option by the given name. */ - ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions( + ImmutableList<UnparsedOptionValueDescription> getExpansionOptionValueDescriptions( OptionDefinition option, @Nullable String optionValue, OptionPriority priority, String source) throws OptionsParsingException { return impl.getExpansionOptionValueDescriptions(option, optionValue, priority, source); diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java index 5e58435..b79e72d 100644 --- a/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -200,16 +200,17 @@ class OptionsParserImpl { // Warnings should not end with a '.' because the internal reporter adds one automatically. private void setValue( - OptionDefinition optionDefinition, - Object value, - OptionPriority priority, - String source, + UnparsedOptionValueDescription optionValue, OptionDefinition implicitDependant, - OptionDefinition expandedFrom) { - OptionValueDescription entry = parsedValues.get(optionDefinition); + OptionDefinition expandedFrom) + throws OptionsParsingException { + OptionDefinition optionDefinition = optionValue.getOptionDefinition(); + Preconditions.checkArgument(!optionDefinition.allowsMultiple()); + Object convertedValue = optionValue.getConvertedValue(); + OptionValueDescription entry = parsedValues.get(optionValue.getOptionDefinition()); if (entry != null) { // Override existing option if the new value has higher or equal priority. - if (priority.compareTo(entry.getPriority()) >= 0) { + if (optionValue.getPriority().compareTo(entry.getPriority()) >= 0) { // Output warnings: if ((implicitDependant != null) && (entry.getImplicitDependant() != null)) { if (!implicitDependant.equals(entry.getImplicitDependant())) { @@ -222,7 +223,8 @@ class OptionsParserImpl { + implicitDependant.getOptionName() + "'"); } - } else if ((implicitDependant != null) && priority.equals(entry.getPriority())) { + } else if ((implicitDependant != null) + && optionValue.getPriority().equals(entry.getPriority())) { warnings.add( "Option '" + optionDefinition.getOptionName() @@ -236,7 +238,7 @@ class OptionsParserImpl { + "' overrides a previous implicit setting of that option by option '" + entry.getImplicitDependant().getOptionName() + "'"); - } else if ((priority == entry.getPriority()) + } else if ((optionValue.getPriority() == entry.getPriority()) && ((entry.getExpansionParent() == null) && (expandedFrom != null))) { // Create a warning if an expansion option overrides an explicit option: warnings.add( @@ -261,24 +263,37 @@ class OptionsParserImpl { parsedValues.put( optionDefinition, OptionValueDescription.newOptionValue( - optionDefinition, null, value, priority, source, implicitDependant, expandedFrom)); + optionDefinition, + null, + convertedValue, + optionValue.getPriority(), + optionValue.getSource(), + implicitDependant, + expandedFrom)); } } else { parsedValues.put( optionDefinition, OptionValueDescription.newOptionValue( - optionDefinition, null, value, priority, source, implicitDependant, expandedFrom)); + optionDefinition, + null, + convertedValue, + optionValue.getPriority(), + optionValue.getSource(), + implicitDependant, + expandedFrom)); maybeAddDeprecationWarning(optionDefinition); } } private void addListValue( - OptionDefinition optionDefinition, - Object value, - OptionPriority priority, - String source, + UnparsedOptionValueDescription optionValue, OptionDefinition implicitDependant, - OptionDefinition expandedFrom) { + OptionDefinition expandedFrom) + throws OptionsParsingException { + OptionDefinition optionDefinition = optionValue.getOptionDefinition(); + Preconditions.checkArgument(optionDefinition.allowsMultiple()); + OptionValueDescription entry = parsedValues.get(optionDefinition); if (entry == null) { entry = @@ -286,14 +301,15 @@ class OptionsParserImpl { optionDefinition, /* originalValueString */ null, ArrayListMultimap.create(), - priority, - source, + optionValue.getPriority(), + optionValue.getSource(), implicitDependant, expandedFrom); parsedValues.put(optionDefinition, entry); maybeAddDeprecationWarning(optionDefinition); } - entry.addValue(priority, value); + Object convertedValue = optionValue.getConvertedValue(); + entry.addValue(optionValue.getPriority(), convertedValue); } OptionValueDescription clearValue(OptionDefinition optionDefinition) @@ -329,13 +345,13 @@ class OptionsParserImpl { } /** @return A list of the descriptions corresponding to the implicit dependant flags passed in. */ - private ImmutableList<OptionValueDescription> getImplicitDependantDescriptions( + private ImmutableList<UnparsedOptionValueDescription> getImplicitDependantDescriptions( ImmutableList<String> options, OptionDefinition implicitDependant, OptionPriority priority, String source) throws OptionsParsingException { - ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder(); + ImmutableList.Builder<UnparsedOptionValueDescription> builder = ImmutableList.builder(); Iterator<String> optionsIterator = options.iterator(); Function<OptionDefinition, String> sourceFunction = @@ -348,15 +364,7 @@ class OptionsParserImpl { UnparsedOptionValueDescription unparsedOption = identifyOptionAndPossibleArgument( unparsedFlagExpression, optionsIterator, priority, sourceFunction, false); - builder.add( - OptionValueDescription.newOptionValue( - unparsedOption.getOptionDefinition(), - unparsedOption.getUnconvertedValue(), - /* value */ null, - unparsedOption.getPriority(), - unparsedOption.getSource(), - implicitDependant, - /* expendedFrom */ null)); + builder.add(unparsedOption); } return builder.build(); } @@ -367,13 +375,13 @@ class OptionsParserImpl { * correct priority or source for these, as they are not actually set values. The value itself * is also a string, no conversion has taken place. */ - ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions( + ImmutableList<UnparsedOptionValueDescription> getExpansionOptionValueDescriptions( OptionDefinition expansionFlag, @Nullable String flagValue, OptionPriority priority, String source) throws OptionsParsingException { - ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder(); + ImmutableList.Builder<UnparsedOptionValueDescription> builder = ImmutableList.builder(); ImmutableList<String> options = optionsData.getEvaluatedExpansion(expansionFlag, flagValue); Iterator<String> optionsIterator = options.iterator(); @@ -384,15 +392,7 @@ class OptionsParserImpl { UnparsedOptionValueDescription unparsedOption = identifyOptionAndPossibleArgument( unparsedFlagExpression, optionsIterator, priority, sourceFunction, false); - builder.add( - OptionValueDescription.newOptionValue( - unparsedOption.getOptionDefinition(), - unparsedOption.getUnconvertedValue(), - /* value */ null, - unparsedOption.getPriority(), - unparsedOption.getSource(), - /* implicitDependant */ null, - expansionFlag)); + builder.add(unparsedOption); } return builder.build(); } @@ -528,40 +528,14 @@ class OptionsParserImpl { + Joiner.on(' ').join(unparsed)); } } else { - Converter<?> converter = optionDefinition.getConverter(); - Object convertedValue; - try { - convertedValue = converter.convert(unconvertedValue); - } catch (OptionsParsingException e) { - // The converter doesn't know the option name, so we supply it here by - // re-throwing: - throw new OptionsParsingException("While parsing option " + arg - + ": " + e.getMessage(), e); - } - // ...but allow duplicates of single-use options across separate calls to // parse(); latest wins: if (!optionDefinition.allowsMultiple()) { - setValue( - optionDefinition, - convertedValue, - priority, - sourceFunction.apply(optionDefinition), - implicitDependent, - expandedFrom); + setValue(unparsedOption, implicitDependent, expandedFrom); } else { - // But if it's a multiple-use option, then just accumulate the - // values, in the order in which they were seen. - // Note: The type of the list member is not known; Java introspection - // only makes it available in String form via the signature string - // for the field declaration. - addListValue( - optionDefinition, - convertedValue, - priority, - sourceFunction.apply(optionDefinition), - implicitDependent, - expandedFrom); + // But if it's a multiple-use option, then accumulate the values, in the order in which + // they were seen. + addListValue(unparsedOption, implicitDependent, expandedFrom); } } @@ -605,6 +579,9 @@ class OptionsParserImpl { boolean explicit) throws OptionsParsingException { + // Store the way this option was parsed on the command line. + StringBuilder commandLineForm = new StringBuilder(); + commandLineForm.append(arg); String unparsedValue = null; OptionDefinition optionDefinition; boolean booleanValue = true; @@ -668,7 +645,9 @@ class OptionsParserImpl { && !optionDefinition.isWrapperOption()) { // This is expected, Void type options have no args (unless they're wrapper options). } else if (nextArgs.hasNext()) { - unparsedValue = nextArgs.next(); // "--flag value" form + // "--flag value" form + unparsedValue = nextArgs.next(); + commandLineForm.append(" ").append(unparsedValue); } else { throw new OptionsParsingException("Expected value after " + arg); } @@ -676,6 +655,7 @@ class OptionsParserImpl { return new UnparsedOptionValueDescription( optionDefinition, + commandLineForm.toString(), unparsedValue, priority, sourceFunction.apply(optionDefinition), diff --git a/java/com/google/devtools/common/options/UnparsedOptionValueDescription.java b/java/com/google/devtools/common/options/UnparsedOptionValueDescription.java index 8cd858a..c6fbbf7 100644 --- a/java/com/google/devtools/common/options/UnparsedOptionValueDescription.java +++ b/java/com/google/devtools/common/options/UnparsedOptionValueDescription.java @@ -23,11 +23,12 @@ import javax.annotation.Nullable; * <p>This class represents an option as the parser received it, which is distinct from the final * value of an option, as these values may be overridden or combined in some way. * - * <p>The origin includes the value it was set to, its priority, a message about where it came + * <p>The origin includes the form it had when parsed, its priority, a message about where it came * from, and whether it was set explicitly or expanded/implied by other flags. */ public final class UnparsedOptionValueDescription { private final OptionDefinition optionDefinition; + private final String commandLineForm; @Nullable private final String unconvertedValue; private final OptionPriority priority; @Nullable private final String source; @@ -39,11 +40,13 @@ public final class UnparsedOptionValueDescription { public UnparsedOptionValueDescription( OptionDefinition optionDefinition, + String commandLineForm, @Nullable String unconvertedValue, OptionPriority priority, @Nullable String source, boolean explicit) { this.optionDefinition = optionDefinition; + this.commandLineForm = commandLineForm; this.unconvertedValue = unconvertedValue; this.priority = priority; this.source = source; @@ -54,6 +57,10 @@ public final class UnparsedOptionValueDescription { return optionDefinition; } + public String getCommandLineForm() { + return commandLineForm; + } + public boolean isBooleanOption() { return optionDefinition.getType().equals(boolean.class); } @@ -99,6 +106,17 @@ public final class UnparsedOptionValueDescription { return explicit; } + public Object getConvertedValue() throws OptionsParsingException { + Converter<?> converter = optionDefinition.getConverter(); + try { + return converter.convert(unconvertedValue); + } catch (OptionsParsingException e) { + // The converter doesn't know the option name, so we supply it here by re-throwing: + throw new OptionsParsingException( + String.format("While parsing option %s: %s", commandLineForm, e.getMessage()), e); + } + } + @Override public String toString() { StringBuilder result = new StringBuilder(); |