diff options
Diffstat (limited to 'src/main/java/com/google/devtools/common/options/OptionValueDescription.java')
| -rw-r--r-- | src/main/java/com/google/devtools/common/options/OptionValueDescription.java | 54 |
1 files changed, 19 insertions, 35 deletions
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 8222aee12b..0d81d49a84 100644 --- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java @@ -26,9 +26,7 @@ import java.util.stream.Collectors; /** * The value of an option. * - * <p>This takes care of tracking the final value as multiple instances of an option are parsed. It - * also tracks additional metadata describing its priority, source, whether it was set via an - * implicit dependency, and if so, by which other option. + * <p>This takes care of tracking the final value as multiple instances of an option are parsed. */ public abstract class OptionValueDescription { @@ -48,12 +46,8 @@ public abstract class OptionValueDescription { /** Returns the source(s) of this option, if there were multiple, duplicates are removed. */ public abstract String getSourceString(); - // TODO(b/65540004) implicitDependant and expandedFrom are artifacts of an option instance, and - // should be in ParsedOptionDescription. abstract void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) throws OptionsParsingException; @@ -105,8 +99,6 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) { throw new IllegalStateException( "Cannot add values to the default option value. Create a modifiable " @@ -121,8 +113,6 @@ public abstract class OptionValueDescription { static class SingleOptionValueDescription extends OptionValueDescription { private ParsedOptionDescription effectiveOptionInstance; private Object effectiveValue; - private OptionDefinition optionThatDependsOnEffectiveValue; - private OptionDefinition optionThatExpandedToEffectiveValue; private SingleOptionValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -134,8 +124,6 @@ public abstract class OptionValueDescription { } effectiveOptionInstance = null; effectiveValue = null; - optionThatDependsOnEffectiveValue = null; - optionThatExpandedToEffectiveValue = null; } @Override @@ -152,15 +140,11 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) throws OptionsParsingException { // This might be the first value, in that case, just store it! if (effectiveOptionInstance == null) { effectiveOptionInstance = parsedOption; - optionThatDependsOnEffectiveValue = implicitDependant; - optionThatExpandedToEffectiveValue = expandedFrom; effectiveValue = effectiveOptionInstance.getConvertedValue(); return; } @@ -168,23 +152,31 @@ public abstract class OptionValueDescription { // If there was another value, check whether the new one will override it, and if so, // log warnings describing the change. if (parsedOption.getPriority().compareTo(effectiveOptionInstance.getPriority()) >= 0) { + // Identify the option that might have led to the current and new value of this option. + OptionDefinition implicitDependent = parsedOption.getImplicitDependent(); + OptionDefinition expandedFrom = parsedOption.getExpandedFrom(); + OptionDefinition optionThatDependsOnEffectiveValue = + effectiveOptionInstance.getImplicitDependent(); + OptionDefinition optionThatExpandedToEffectiveValue = + effectiveOptionInstance.getExpandedFrom(); + // Output warnings: - if ((implicitDependant != null) && (optionThatDependsOnEffectiveValue != null)) { - if (!implicitDependant.equals(optionThatDependsOnEffectiveValue)) { + if ((implicitDependent != null) && (optionThatDependsOnEffectiveValue != null)) { + if (!implicitDependent.equals(optionThatDependsOnEffectiveValue)) { warnings.add( String.format( "Option '%s' is implicitly defined by both option '%s' and option '%s'", optionDefinition.getOptionName(), optionThatDependsOnEffectiveValue.getOptionName(), - implicitDependant.getOptionName())); + implicitDependent.getOptionName())); } - } else if ((implicitDependant != null) + } 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 " + "overrides the previous one", - optionDefinition.getOptionName(), implicitDependant.getOptionName())); + optionDefinition.getOptionName(), implicitDependent.getOptionName())); } else if (optionThatDependsOnEffectiveValue != null) { warnings.add( String.format( @@ -211,8 +203,6 @@ public abstract class OptionValueDescription { // Record the new value: effectiveOptionInstance = parsedOption; - optionThatDependsOnEffectiveValue = implicitDependant; - optionThatExpandedToEffectiveValue = expandedFrom; effectiveValue = parsedOption.getConvertedValue(); } else { // The new value does not override the old value, as it has lower priority. @@ -275,17 +265,17 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) throws OptionsParsingException { // For repeatable options, we allow flags that take both single values and multiple values, // potentially collapsing them down. Object convertedValue = parsedOption.getConvertedValue(); + OptionPriority priority = parsedOption.getPriority(); + parsedOptions.put(priority, parsedOption); if (convertedValue instanceof List<?>) { - optionValues.putAll(parsedOption.getPriority(), (List<?>) convertedValue); + optionValues.putAll(priority, (List<?>) convertedValue); } else { - optionValues.put(parsedOption.getPriority(), convertedValue); + optionValues.put(priority, convertedValue); } } } @@ -318,8 +308,6 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) { // TODO(b/65540004) Deal with expansion options here instead of in parse(), and track their // link to the options they expanded to to. @@ -341,13 +329,11 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) throws OptionsParsingException { // This is a valued flag, its value is handled the same way as a normal // SingleOptionValueDescription. - super.addOptionInstance(parsedOption, implicitDependant, expandedFrom, warnings); + super.addOptionInstance(parsedOption, warnings); // Now deal with the implicit requirements. // TODO(b/65540004) Deal with options with implicit requirements here instead of in parse(), @@ -375,8 +361,6 @@ public abstract class OptionValueDescription { @Override void addOptionInstance( ParsedOptionDescription parsedOption, - OptionDefinition implicitDependant, - OptionDefinition expandedFrom, List<String> warnings) throws OptionsParsingException { // TODO(b/65540004) Deal with options with implicit requirements here instead of in parse(), |