diff options
Diffstat (limited to 'src/main/java/com')
3 files changed, 83 insertions, 37 deletions
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 96c471ed74..ec5d0d8f32 100644 --- a/src/main/java/com/google/devtools/common/options/OptionPriority.java +++ b/src/main/java/com/google/devtools/common/options/OptionPriority.java @@ -26,18 +26,42 @@ import java.util.Objects; public class OptionPriority implements Comparable<OptionPriority> { private final PriorityCategory priorityCategory; private final int index; + private final boolean locked; - private OptionPriority(PriorityCategory priorityCategory, int index) { + private OptionPriority(PriorityCategory priorityCategory, int index, boolean locked) { this.priorityCategory = priorityCategory; this.index = index; + this.locked = locked; } - public static OptionPriority lowestOptionPriorityAtCategory(PriorityCategory category) { - return new OptionPriority(category, 0); + /** Get the first OptionPriority for that category. */ + static OptionPriority lowestOptionPriorityAtCategory(PriorityCategory category) { + return new OptionPriority(category, 0, false); } - public static OptionPriority nextOptionPriority(OptionPriority priority) { - return new OptionPriority(priority.priorityCategory, priority.index + 1); + /** + * Get the priority for the option following this one. In normal, incremental option parsing, the + * returned priority would compareTo as after the current one. Does not increment locked + * priorities. + */ + static OptionPriority nextOptionPriority(OptionPriority priority) { + if (priority.locked) { + return priority; + } + return new OptionPriority(priority.priorityCategory, priority.index + 1, false); + } + + /** + * Return a priority for this option that will avoid priority increases by calls to + * nextOptionPriority. + * + * <p>Some options are expanded in-place, and need to be all parsed at the priority of the + * original option. In this case, parsing one of these after another should not cause the option + * to be considered as higher priority than the ones before it (this would cause overlap between + * the expansion of --expansion_flag and a option following it in the same list of options). + */ + public static OptionPriority getLockedPriority(OptionPriority priority) { + return new OptionPriority(priority.priorityCategory, priority.index, true); } public PriorityCategory getPriorityCategory() { @@ -66,6 +90,11 @@ public class OptionPriority implements Comparable<OptionPriority> { return Objects.hash(priorityCategory, index); } + @Override + public String toString() { + return String.format("OptionPriority(%s,%s)", priorityCategory, index); + } + /** * The priority of option values, in order of increasing priority. * 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 30c4377b00..9864a46945 100644 --- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java @@ -14,7 +14,6 @@ package com.google.devtools.common.options; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; @@ -114,7 +113,7 @@ public abstract class OptionValueDescription { return new DefaultOptionValueDescription(option); } - static class DefaultOptionValueDescription extends OptionValueDescription { + private static class DefaultOptionValueDescription extends OptionValueDescription { private DefaultOptionValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -147,7 +146,7 @@ public abstract class OptionValueDescription { * The form of a value for a default type of flag, one that does not accumulate multiple values * and has no expansion. */ - static class SingleOptionValueDescription extends OptionValueDescription { + private static class SingleOptionValueDescription extends OptionValueDescription { private ParsedOptionDescription effectiveOptionInstance; private Object effectiveValue; @@ -199,6 +198,11 @@ public abstract class OptionValueDescription { // Output warnings if there is conflicting options set different values in a way that might // not have been obvious to the user, such as through expansions and implicit requirements. if (!effectiveValue.equals(newValue)) { + boolean samePriorityCategory = + parsedOption + .getPriority() + .getPriorityCategory() + .equals(effectiveOptionInstance.getPriority().getPriorityCategory()); if ((implicitDependent != null) && (optionThatDependsOnEffectiveValue != null)) { if (!implicitDependent.equals(optionThatDependsOnEffectiveValue)) { warnings.add( @@ -206,8 +210,7 @@ public abstract class OptionValueDescription { "%s is implicitly defined by both %s and %s", optionDefinition, optionThatDependsOnEffectiveValue, implicitDependent)); } - } else if ((implicitDependent != null) - && parsedOption.getPriority().equals(effectiveOptionInstance.getPriority())) { + } else if ((implicitDependent != null) && samePriorityCategory) { warnings.add( String.format( "%s is implicitly defined by %s; the implicitly set value " @@ -219,7 +222,7 @@ public abstract class OptionValueDescription { "A new value for %s overrides a previous implicit setting of that " + "option by %s", optionDefinition, optionThatDependsOnEffectiveValue)); - } else if ((parsedOption.getPriority().equals(effectiveOptionInstance.getPriority())) + } else if (samePriorityCategory && ((optionThatExpandedToEffectiveValue == null) && (expandedFrom != null))) { // Create a warning if an expansion option overrides an explicit option: warnings.add( @@ -252,15 +255,10 @@ public abstract class OptionValueDescription { } return ImmutableList.of(); } - - @VisibleForTesting - ParsedOptionDescription getEffectiveOptionInstance() { - return effectiveOptionInstance; - } } /** The form of a value for an option that accumulates multiple values on the command line. */ - static class RepeatableOptionValueDescription extends OptionValueDescription { + private static class RepeatableOptionValueDescription extends OptionValueDescription { ListMultimap<OptionPriority, ParsedOptionDescription> parsedOptions; ListMultimap<OptionPriority, Object> optionValues; @@ -334,7 +332,7 @@ public abstract class OptionValueDescription { * in place to other options. This should be used for both flags with a static expansion defined * in {@link Option#expansion()} and flags with an {@link Option#expansionFunction()}. */ - static class ExpansionOptionValueDescription extends OptionValueDescription { + private static class ExpansionOptionValueDescription extends OptionValueDescription { private final List<String> expansion; private ExpansionOptionValueDescription( @@ -385,7 +383,8 @@ public abstract class OptionValueDescription { } /** The form of a value for a flag with implicit requirements. */ - static class OptionWithImplicitRequirementsValueDescription extends SingleOptionValueDescription { + private static class OptionWithImplicitRequirementsValueDescription + extends SingleOptionValueDescription { private OptionWithImplicitRequirementsValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -429,7 +428,7 @@ public abstract class OptionValueDescription { } /** Form for options that contain other options in the value text to which they expand. */ - static final class WrapperOptionValueDescription extends OptionValueDescription { + private static final class WrapperOptionValueDescription extends OptionValueDescription { WrapperOptionValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); 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 e8086c0680..96f70f3a2e 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -32,6 +32,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -70,15 +71,19 @@ class OptionsParserImpl { private final List<String> warnings = new ArrayList<>(); + /** + * Since parse() expects multiple calls to it with the same {@link PriorityCategory} to be treated + * as though the args in the later call have higher priority over the earlier calls, we need to + * track the high water mark of option priority at each category. Each call to parse will start at + * this level. + */ + private final Map<PriorityCategory, OptionPriority> nextPriorityPerPriorityCategory = + Stream.of(PriorityCategory.values()) + .collect(Collectors.toMap(p -> p, OptionPriority::lowestOptionPriorityAtCategory)); + private boolean allowSingleDashLongOptions = false; - private ArgsPreProcessor argsPreProcessor = - new ArgsPreProcessor() { - @Override - public List<String> preProcess(List<String> args) throws OptionsParsingException { - return args; - } - }; + private ArgsPreProcessor argsPreProcessor = args -> args; /** Create a new parser object. Do not accept a null OptionsData object. */ OptionsParserImpl(OptionsData optionsData) { @@ -261,12 +266,24 @@ class OptionsParserImpl { * order. */ List<String> parse( - OptionPriority.PriorityCategory priority, + PriorityCategory priorityCat, Function<OptionDefinition, String> sourceFunction, List<String> args) throws OptionsParsingException { - return parse( - OptionPriority.lowestOptionPriorityAtCategory(priority), sourceFunction, null, null, args); + ResidueAndPriority residueAndPriority = + parse(nextPriorityPerPriorityCategory.get(priorityCat), sourceFunction, null, null, args); + nextPriorityPerPriorityCategory.put(priorityCat, residueAndPriority.nextPriority); + return residueAndPriority.residue; + } + + private static final class ResidueAndPriority { + List<String> residue; + OptionPriority nextPriority; + + public ResidueAndPriority(List<String> residue, OptionPriority nextPriority) { + this.residue = residue; + this.nextPriority = nextPriority; + } } /** @@ -277,7 +294,7 @@ class OptionsParserImpl { * <p>The method treats options that have neither an implicitDependent nor an expandedFrom value * as explicitly set. */ - private List<String> parse( + private ResidueAndPriority parse( OptionPriority priority, Function<OptionDefinition, String> sourceFunction, OptionDefinition implicitDependent, @@ -304,6 +321,7 @@ class OptionsParserImpl { identifyOptionAndPossibleArgument( arg, argsIterator, priority, sourceFunction, implicitDependent, expandedFrom); handleNewParsedOption(parsedOption); + priority = OptionPriority.nextOptionPriority(priority); } // Go through the final values and make sure they are valid values for their option. Unlike any @@ -313,7 +331,7 @@ class OptionsParserImpl { valueDescription.getValue(); } - return unparsedArgs; + return new ResidueAndPriority(unparsedArgs, priority); } /** @@ -383,20 +401,20 @@ class OptionsParserImpl { } if (expansionBundle != null) { - List<String> unparsed = + ResidueAndPriority residueAndPriority = parse( - parsedOption.getPriority(), + OptionPriority.getLockedPriority(parsedOption.getPriority()), o -> expansionBundle.sourceOfExpansionArgs, optionDefinition.hasImplicitRequirements() ? optionDefinition : null, optionDefinition.isExpansionOption() ? optionDefinition : null, expansionBundle.expansionArgs); - if (!unparsed.isEmpty()) { + if (!residueAndPriority.residue.isEmpty()) { if (optionDefinition.isWrapperOption()) { throw new OptionsParsingException( "Unparsed options remain after unwrapping " + unconvertedValue + ": " - + Joiner.on(' ').join(unparsed)); + + Joiner.on(' ').join(residueAndPriority.residue)); } 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. @@ -404,7 +422,7 @@ class OptionsParserImpl { "Unparsed options remain after processing " + unconvertedValue + ": " - + Joiner.on(' ').join(unparsed)); + + Joiner.on(' ').join(residueAndPriority.residue)); } } } |