diff options
Diffstat (limited to 'src')
16 files changed, 123 insertions, 728 deletions
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 b6861545ec..87235dbc1b 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 @@ -16,8 +16,8 @@ package com.google.devtools.build.lib.runtime; import com.google.common.collect.ImmutableList; import com.google.devtools.common.options.Converter; -import com.google.devtools.common.options.ExpansionContext; import com.google.devtools.common.options.ExpansionFunction; +import com.google.devtools.common.options.IsolatedOptionsData; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDefinition; import com.google.devtools.common.options.OptionMetadataTag; @@ -153,12 +153,11 @@ public class AllIncompatibleChangesExpansion implements ExpansionFunction { } @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { // Grab all registered options that are identified as incompatible changes by either name or // by category. Ensure they satisfy our requirements. ArrayList<String> incompatibleChanges = new ArrayList<>(); - for (Map.Entry<String, OptionDefinition> entry : - context.getOptionsData().getAllOptionDefinitions()) { + for (Map.Entry<String, OptionDefinition> entry : optionsData.getAllOptionDefinitions()) { OptionDefinition optionDefinition = entry.getValue(); if (optionDefinition.getOptionName().startsWith(INCOMPATIBLE_NAME_PREFIX) || optionDefinition.getOptionCategory().equals(INCOMPATIBLE_CATEGORY)) { diff --git a/src/main/java/com/google/devtools/common/options/ExpansionContext.java b/src/main/java/com/google/devtools/common/options/ExpansionContext.java deleted file mode 100644 index c6aecc7a44..0000000000 --- a/src/main/java/com/google/devtools/common/options/ExpansionContext.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.common.options; - -import java.lang.reflect.Field; -import javax.annotation.Nullable; -import javax.annotation.concurrent.ThreadSafe; - -/** - * Encapsulates the data given to {@link ExpansionFunction} objects. This lets {@link - * ExpansionFunction} objects change how it expands flags based on the arguments given to the {@link - * OptionsParser}. - */ -@ThreadSafe -public final class ExpansionContext { - private final IsolatedOptionsData optionsData; - private final OptionDefinition optionDefinition; - @Nullable private final String unparsedValue; - - public ExpansionContext( - IsolatedOptionsData optionsData, - OptionDefinition optionDefinition, - @Nullable String unparsedValue) { - this.optionsData = optionsData; - this.optionDefinition = optionDefinition; - this.unparsedValue = unparsedValue; - } - - /** Metadata for the option that is being expanded. */ - public IsolatedOptionsData getOptionsData() { - return optionsData; - } - - /** {@link Field} object for option that is being expanded. */ - public OptionDefinition getOptionDefinition() { - return optionDefinition; - } - - /** Argument given to this flag during options parsing. Will be null if no argument was given. */ - @Nullable - public String getUnparsedValue() { - return unparsedValue; - } -} diff --git a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java index 09119b2dc4..d2c2693da2 100644 --- a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java +++ b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java @@ -30,5 +30,5 @@ public interface ExpansionFunction { * information is computed * @return An expansion to use on an empty list */ - ImmutableList<String> getExpansion(ExpansionContext context) throws OptionsParsingException; + ImmutableList<String> getExpansion(IsolatedOptionsData optionsData); } diff --git a/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java b/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java deleted file mode 100644 index d63b988570..0000000000 --- a/src/main/java/com/google/devtools/common/options/ExpansionNeedsValueException.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.common.options; - -/** - * Exception specific to evaluating {@link ExpansionFunction} objects. Used when expansion isn't - * possible because of a missing input. - */ -public final class ExpansionNeedsValueException extends OptionsParsingException { - - public ExpansionNeedsValueException(String message) { - super(message); - } -} 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 37af8b5fdb..a53ff5b7f6 100644 --- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java +++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java @@ -14,7 +14,6 @@ package com.google.devtools.common.options; import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; @@ -38,6 +37,7 @@ import java.util.Map; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; import javax.annotation.Nullable; /** @@ -259,8 +259,7 @@ public final class InvocationPolicyEnforcer { continue; } - OptionDescription optionDescription = - parser.getOptionDescription(policy.getFlagName(), origin); + OptionDescription optionDescription = parser.getOptionDescription(policy.getFlagName()); if (optionDescription == null) { // InvocationPolicy ignores policy on non-existing flags by design, for version // compatibility. @@ -273,8 +272,7 @@ public final class InvocationPolicyEnforcer { } FlagPolicyWithContext policyWithContext = new FlagPolicyWithContext(policy, optionDescription, origin); - List<FlagPolicyWithContext> policies = - expandPolicy(policyWithContext, currentPriority, parser, loglevel); + List<FlagPolicyWithContext> policies = expandPolicy(policyWithContext, parser, loglevel); expandedPolicies.addAll(policies); } @@ -300,75 +298,6 @@ public final class InvocationPolicyEnforcer { String.format("Disallow_Values on expansion flags like %s is not allowed.", flagName)); } - private static ImmutableList<ParsedOptionDescription> getExpansionsFromFlagPolicy( - FlagPolicyWithContext expansionPolicy, OptionPriority priority, OptionsParser parser) - throws OptionsParsingException { - if (!expansionPolicy.description.isExpansion()) { - return ImmutableList.of(); - } - String policyFlagName = expansionPolicy.policy.getFlagName(); - String optionName = expansionPolicy.description.getOptionDefinition().getOptionName(); - Preconditions.checkArgument( - policyFlagName.equals(optionName), - "The optionDescription provided (for flag %s) does not match the policy for flag %s.", - optionName, policyFlagName); - - ImmutableList.Builder<ParsedOptionDescription> resultsBuilder = ImmutableList.builder(); - switch (expansionPolicy.policy.getOperationCase()) { - case SET_VALUE: - { - SetValue setValue = expansionPolicy.policy.getSetValue(); - if (setValue.getFlagValueCount() > 0) { - for (String value : setValue.getFlagValueList()) { - resultsBuilder.addAll( - parser.getExpansionOptionValueDescriptions( - expansionPolicy.description.getOptionDefinition(), - value, - priority, - INVOCATION_POLICY_SOURCE)); - } - } else { - resultsBuilder.addAll( - parser.getExpansionOptionValueDescriptions( - expansionPolicy.description.getOptionDefinition(), - null, - priority, - INVOCATION_POLICY_SOURCE)); - } - } - break; - case USE_DEFAULT: - resultsBuilder.addAll( - parser.getExpansionOptionValueDescriptions( - expansionPolicy.description.getOptionDefinition(), - null, - priority, - INVOCATION_POLICY_SOURCE)); - break; - case ALLOW_VALUES: - // All expansions originally given to the parser have been expanded by now, so these two - // cases aren't necessary (the values given in the flag policy shouldn't need to be - // checked). If you care about blocking specific flag values you should block the behavior - // on the specific ones, not the expansion that contains them. - throwAllowValuesOnExpansionFlagException(optionName); - break; - case DISALLOW_VALUES: - throwDisallowValuesOnExpansionFlagException(optionName); - break; - case OPERATION_NOT_SET: - throw new PolicyOperationNotSetException(optionName); - default: - logger.warning( - String.format( - "Unknown operation '%s' from invocation policy for flag '%s'", - expansionPolicy.policy.getOperationCase(), - optionName)); - break; - } - - return resultsBuilder.build(); - } - /** * Expand a single policy. If the policy is not about an expansion flag, this will simply return a * list with a single element, oneself. If the policy is for an expansion flag, the policy will @@ -378,47 +307,20 @@ public final class InvocationPolicyEnforcer { */ private static List<FlagPolicyWithContext> expandPolicy( FlagPolicyWithContext originalPolicy, - OptionPriority priority, OptionsParser parser, Level loglevel) throws OptionsParsingException { List<FlagPolicyWithContext> expandedPolicies = new ArrayList<>(); - 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 { + boolean isExpansion = originalPolicy.description.isExpansion(); + ImmutableList<ParsedOptionDescription> subflags = + parser.getExpansionValueDescriptions( + originalPolicy.description.getOptionDefinition(), originalPolicy.origin); + + // If we have nothing to expand to, no need to do any further work. + if (subflags.isEmpty()) { 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 (logger.isLoggable(loglevel)) { // Log the expansion. This is only really useful for understanding the invocation policy @@ -450,8 +352,7 @@ public final class InvocationPolicyEnforcer { // UseDefault, when preventing it from being set. for (ParsedOptionDescription currentSubflag : subflags) { OptionDescription subflagOptionDescription = - parser.getOptionDescription( - currentSubflag.getOptionDefinition().getOptionName(), originOfSubflags); + parser.getOptionDescription(currentSubflag.getOptionDefinition().getOptionName()); if (currentSubflag.getOptionDefinition().allowsMultiple() && originalPolicy.policy.getOperationCase().equals(OperationCase.SET_VALUE)) { @@ -461,7 +362,7 @@ public final class InvocationPolicyEnforcer { getSingleValueSubflagAsPolicy( subflagOptionDescription, currentSubflag, originalPolicy, isExpansion); // In case any of the expanded flags are themselves expansions, recurse. - expandedPolicies.addAll(expandPolicy(subflagAsPolicy, priority, parser, loglevel)); + expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser, loglevel)); } } @@ -471,9 +372,26 @@ public final class InvocationPolicyEnforcer { for (OptionDescription repeatableFlag : repeatableSubflagsInSetValues.keySet()) { int numValues = repeatableSubflagsInSetValues.get(repeatableFlag).size(); ArrayList<String> newValues = new ArrayList<>(numValues); + ArrayList<OptionInstanceOrigin> origins = new ArrayList<>(numValues); for (ParsedOptionDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) { newValues.add(setValue.getUnconvertedValue()); + origins.add(setValue.getOrigin()); } + // These options come from expanding a single policy, so they have effectively the same + // priority. They could have come from different expansions or implicit requirements in the + // recursive resolving of the option list, so just pick the first one. Do collapse the source + // strings though, in case there are different sources. + OptionInstanceOrigin arbitraryFirstOptionOrigin = origins.get(0); + OptionInstanceOrigin originOfSubflags = + new OptionInstanceOrigin( + arbitraryFirstOptionOrigin.getPriority(), + origins + .stream() + .map(OptionInstanceOrigin::getSource) + .distinct() + .collect(Collectors.joining(", ")), + arbitraryFirstOptionOrigin.getImplicitDependent(), + arbitraryFirstOptionOrigin.getExpandedFrom()); expandedPolicies.add( getSetValueSubflagAsPolicy(repeatableFlag, newValues, originOfSubflags, originalPolicy)); } 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 69f360ad71..63cac249c2 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -14,14 +14,12 @@ package com.google.devtools.common.options; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; import java.util.Collection; import java.util.Map; -import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -32,86 +30,26 @@ import javax.annotation.concurrent.Immutable; @Immutable final class OptionsData extends IsolatedOptionsData { - /** - * Keeps track of all the information needed to calculate expansion flags, whether they come from - * a static list or a @{link ExpansionFunction} object. - */ - static class ExpansionData { - private final ImmutableList<String> staticExpansion; - @Nullable private final ExpansionFunction dynamicExpansions; - - ExpansionData(ImmutableList<String> staticExpansion) { - Preconditions.checkArgument(staticExpansion != null); - this.staticExpansion = staticExpansion; - this.dynamicExpansions = null; - } - - ExpansionData(ExpansionFunction dynamicExpansions) { - Preconditions.checkArgument(dynamicExpansions != null); - this.staticExpansion = EMPTY_EXPANSION; - this.dynamicExpansions = dynamicExpansions; - } - - ImmutableList<String> getExpansion(ExpansionContext context) throws OptionsParsingException { - Preconditions.checkArgument(context != null); - if (dynamicExpansions != null) { - ImmutableList<String> result = dynamicExpansions.getExpansion(context); - if (result == null) { - String valueString = - context.getUnparsedValue() != null ? context.getUnparsedValue() : "(null)"; - String name = context.getOptionDefinition().getOptionName(); - throw new OptionsParsingException( - String.format( - "Error expanding %s: no expansions defined for value: %s", - context.getOptionDefinition(), valueString), - name); - } - return result; - } else { - return staticExpansion; - } - } - - boolean isEmpty() { - return staticExpansion.isEmpty() && (dynamicExpansions == null); - } - } - - /** - * Mapping from each Option-annotated field with expansion information to the {@link - * ExpansionData} needed to caclulate it. - */ - private final ImmutableMap<OptionDefinition, ExpansionData> expansionDataForFields; + /** Mapping from each option to the (unparsed) options it expands to, if any. */ + private final ImmutableMap<OptionDefinition, ImmutableList<String>> evaluatedExpansions; /** Construct {@link OptionsData} by extending an {@link IsolatedOptionsData} with new info. */ private OptionsData( - IsolatedOptionsData base, Map<OptionDefinition, ExpansionData> expansionDataForFields) { + IsolatedOptionsData base, Map<OptionDefinition, ImmutableList<String>> evaluatedExpansions) { super(base); - this.expansionDataForFields = ImmutableMap.copyOf(expansionDataForFields); + this.evaluatedExpansions = ImmutableMap.copyOf(evaluatedExpansions); } private static final ImmutableList<String> EMPTY_EXPANSION = ImmutableList.<String>of(); - private static final ExpansionData EMPTY_EXPANSION_DATA = new ExpansionData(EMPTY_EXPANSION); /** * Returns the expansion of an options field, regardless of whether it was defined using {@link * Option#expansion} or {@link Option#expansionFunction}. If the field is not an expansion option, * returns an empty array. */ - public ImmutableList<String> getEvaluatedExpansion( - OptionDefinition optionDefinition, @Nullable String unparsedValue) - throws OptionsParsingException { - ExpansionData expansionData = expansionDataForFields.get(optionDefinition); - if (expansionData == null) { - return EMPTY_EXPANSION; - } - - return expansionData.getExpansion(new ExpansionContext(this, optionDefinition, unparsedValue)); - } - - ExpansionData getExpansionDataForField(OptionDefinition optionDefinition) { - ExpansionData result = expansionDataForFields.get(optionDefinition); - return result != null ? result : EMPTY_EXPANSION_DATA; + public ImmutableList<String> getEvaluatedExpansion(OptionDefinition optionDefinition) { + ImmutableList<String> result = evaluatedExpansions.get(optionDefinition); + return result != null ? result : EMPTY_EXPANSION; } /** @@ -125,8 +63,8 @@ final class OptionsData extends IsolatedOptionsData { IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes); // All that's left is to compute expansions. - ImmutableMap.Builder<OptionDefinition, ExpansionData> expansionDataBuilder = - ImmutableMap.<OptionDefinition, ExpansionData>builder(); + ImmutableMap.Builder<OptionDefinition, ImmutableList<String>> evaluatedExpansionsBuilder = + ImmutableMap.builder(); for (Map.Entry<String, OptionDefinition> entry : isolatedData.getAllOptionDefinitions()) { OptionDefinition optionDefinition = entry.getValue(); // Determine either the hard-coded expansion, or the ExpansionFunction class. The @@ -135,8 +73,7 @@ final class OptionsData extends IsolatedOptionsData { Class<? extends ExpansionFunction> expansionFunctionClass = optionDefinition.getExpansionFunction(); if (constExpansion.length > 0) { - expansionDataBuilder.put( - optionDefinition, new ExpansionData(ImmutableList.copyOf(constExpansion))); + evaluatedExpansionsBuilder.put(optionDefinition, ImmutableList.copyOf(constExpansion)); } else if (optionDefinition.usesExpansionFunction()) { if (Modifier.isAbstract(expansionFunctionClass.getModifiers())) { throw new AssertionError( @@ -152,23 +89,10 @@ final class OptionsData extends IsolatedOptionsData { // time it is used. throw new AssertionError(e); } - - ImmutableList<String> staticExpansion; - try { - staticExpansion = - instance.getExpansion(new ExpansionContext(isolatedData, optionDefinition, null)); - Preconditions.checkState( - 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 - // it later. - expansionDataBuilder.put(optionDefinition, new ExpansionData(instance)); - } catch (OptionsParsingException e) { - throw new IllegalStateException("Error expanding void expansion function: ", e); - } + ImmutableList<String> expansion = instance.getExpansion(isolatedData); + evaluatedExpansionsBuilder.put(optionDefinition, expansion); } } - return new OptionsData(isolatedData, expansionDataBuilder.build()); + return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build()); } } 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 ba3e57a237..f84ee474a2 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParser.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java @@ -39,7 +39,6 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; -import javax.annotation.Nullable; /** * A parser for options. Typical use case in a main method: @@ -153,11 +152,9 @@ public class OptionsParser implements OptionsProvider { return newOptionsParser(ImmutableList.<Class<? extends OptionsBase>>of(class1)); } - /** - * @see #newOptionsParser(Iterable) - */ - public static OptionsParser newOptionsParser(Class<? extends OptionsBase> class1, - Class<? extends OptionsBase> class2) + /** @see #newOptionsParser(Iterable) */ + public static OptionsParser newOptionsParser( + Class<? extends OptionsBase> class1, Class<? extends OptionsBase> class2) throws ConstructionException { return newOptionsParser(ImmutableList.of(class1, class2)); } @@ -240,52 +237,41 @@ public class OptionsParser implements OptionsProvider { /** The metadata about an option, in the context of this options parser. */ public static final class OptionDescription { - private final OptionDefinition optionDefinition; - private final OptionsData.ExpansionData expansionData; - private final ImmutableList<ParsedOptionDescription> implicitRequirements; + private final ImmutableList<String> evaluatedExpansion; - OptionDescription( - OptionDefinition definition, - OptionsData.ExpansionData expansionData, - ImmutableList<ParsedOptionDescription> implicitRequirements) { + OptionDescription(OptionDefinition definition, OptionsData optionsData) { this.optionDefinition = definition; - this.expansionData = expansionData; - this.implicitRequirements = implicitRequirements; + this.evaluatedExpansion = optionsData.getEvaluatedExpansion(optionDefinition); } public OptionDefinition getOptionDefinition() { return optionDefinition; } - public ImmutableList<ParsedOptionDescription> getImplicitRequirements() { - return implicitRequirements; - } - public boolean isExpansion() { - return !expansionData.isEmpty(); + return optionDefinition.isExpansionOption(); } /** Return a list of flags that this option expands to. */ - public ImmutableList<String> getExpansion(ExpansionContext context) - throws OptionsParsingException { - return expansionData.getExpansion(context); + public ImmutableList<String> getExpansion() throws OptionsParsingException { + return evaluatedExpansion; } @Override public boolean equals(Object obj) { if (obj instanceof OptionDescription) { OptionDescription other = (OptionDescription) obj; - // Check that the option is the same and that it is in the same context (expansionData) + // Check that the option is the same, with the same expansion. return other.optionDefinition.equals(optionDefinition) - && other.expansionData.equals(expansionData); + && other.evaluatedExpansion.equals(evaluatedExpansion); } return false; } @Override public int hashCode() { - return optionDefinition.hashCode() + expansionData.hashCode(); + return optionDefinition.hashCode() + evaluatedExpansion.hashCode(); } } @@ -534,22 +520,23 @@ public class OptionsParser implements OptionsProvider { * @return The {@link OptionDescription} for the option, or null if there is no option by the * given name. */ - OptionDescription getOptionDescription(String name, OptionInstanceOrigin origin) - throws OptionsParsingException { - return impl.getOptionDescription(name, origin); + OptionDescription getOptionDescription(String name) throws OptionsParsingException { + return impl.getOptionDescription(name); } /** - * Returns a description of the options values that get expanded from this option with the given - * value. + * Returns the parsed options that get expanded from this option, whether it expands due to an + * implicit requirement or expansion. * - * @return The {@link com.google.devtools.common.options.OptionValueDescription>} for the option, - * or null if there is no option by the given name. + * @param expansionOption the option that might need to be expanded. If this option does not + * expand to other options, the empty list will be returned. + * @param originOfExpansionOption the origin of the option that's being expanded. This function + * will take care of adjusting the source messages as necessary. */ - ImmutableList<ParsedOptionDescription> getExpansionOptionValueDescriptions( - OptionDefinition option, @Nullable String optionValue, OptionPriority priority, String source) + ImmutableList<ParsedOptionDescription> getExpansionValueDescriptions( + OptionDefinition expansionOption, OptionInstanceOrigin originOfExpansionOption) throws OptionsParsingException { - return impl.getExpansionOptionValueDescriptions(option, optionValue, priority, source); + return impl.getExpansionValueDescriptions(expansionOption, originOfExpansionOption); } /** @@ -845,4 +832,3 @@ public class OptionsParser implements OptionsProvider { + "}"); } } - 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 96f4c16649..78bfa07ac5 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -208,77 +208,58 @@ class OptionsParserImpl { return optionValues.get(optionDefinition); } - OptionDescription getOptionDescription(String name, OptionInstanceOrigin origin) - throws OptionsParsingException { + OptionDescription getOptionDescription(String name) throws OptionsParsingException { OptionDefinition optionDefinition = optionsData.getOptionDefinitionFromName(name); if (optionDefinition == null) { return null; } - - return new OptionDescription( - optionDefinition, - optionsData.getExpansionDataForField(optionDefinition), - getImplicitDependentDescriptions( - ImmutableList.copyOf(optionDefinition.getImplicitRequirements()), - optionDefinition, - origin)); - } - - /** @return A list of the descriptions corresponding to the implicit dependent flags passed in. */ - private ImmutableList<ParsedOptionDescription> getImplicitDependentDescriptions( - ImmutableList<String> options, - OptionDefinition implicitDependent, - OptionInstanceOrigin dependentsOrigin) - throws OptionsParsingException { - ImmutableList.Builder<ParsedOptionDescription> builder = ImmutableList.builder(); - Iterator<String> optionsIterator = options.iterator(); - - Function<OptionDefinition, String> sourceFunction = - o -> - String.format( - "implicitely required for %s (source: %s)", - implicitDependent, dependentsOrigin.getSource()); - while (optionsIterator.hasNext()) { - String unparsedFlagExpression = optionsIterator.next(); - ParsedOptionDescription parsedOption = - identifyOptionAndPossibleArgument( - unparsedFlagExpression, - optionsIterator, - dependentsOrigin.getPriority(), - sourceFunction, - implicitDependent, - null); - builder.add(parsedOption); - } - return builder.build(); + return new OptionDescription(optionDefinition, optionsData); } /** - * @return A list of the descriptions corresponding to options expanded from the flag for the - * given value. The value itself is a string, no conversion has taken place. + * Implementation of {@link OptionsParser#getExpansionValueDescriptions(OptionDefinition, + * OptionInstanceOrigin)} */ - ImmutableList<ParsedOptionDescription> getExpansionOptionValueDescriptions( - OptionDefinition expansionFlag, - @Nullable String flagValue, - OptionPriority priority, - String source) + ImmutableList<ParsedOptionDescription> getExpansionValueDescriptions( + OptionDefinition expansionFlag, OptionInstanceOrigin originOfExpansionFlag) throws OptionsParsingException { ImmutableList.Builder<ParsedOptionDescription> builder = ImmutableList.builder(); + OptionInstanceOrigin originOfSubflags; + ImmutableList<String> options; + if (expansionFlag.hasImplicitRequirements()) { + options = ImmutableList.copyOf(expansionFlag.getImplicitRequirements()); + originOfSubflags = + new OptionInstanceOrigin( + originOfExpansionFlag.getPriority(), + String.format( + "implicitly required by %s (source: %s)", + expansionFlag, originOfExpansionFlag.getSource()), + expansionFlag, + null); + } else if (expansionFlag.isExpansionOption()) { + options = optionsData.getEvaluatedExpansion(expansionFlag); + originOfSubflags = + new OptionInstanceOrigin( + originOfExpansionFlag.getPriority(), + String.format( + "expanded by %s (source: %s)", expansionFlag, originOfExpansionFlag.getSource()), + null, + expansionFlag); + } else { + return ImmutableList.of(); + } - 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, source); while (optionsIterator.hasNext()) { String unparsedFlagExpression = optionsIterator.next(); ParsedOptionDescription parsedOption = identifyOptionAndPossibleArgument( unparsedFlagExpression, optionsIterator, - priority, - sourceFunction, - null, - expansionFlag); + originOfSubflags.getPriority(), + o -> originOfSubflags.getSource(), + originOfSubflags.getImplicitDependent(), + originOfSubflags.getExpandedFrom()); builder.add(parsedOption); } return builder.build(); @@ -431,7 +412,7 @@ class OptionsParserImpl { StringBuilder sourceMessage = new StringBuilder(); ImmutableList<String> expansionArgs; if (optionDefinition.isExpansionOption()) { - expansionArgs = optionsData.getEvaluatedExpansion(optionDefinition, unconvertedValue); + expansionArgs = optionsData.getEvaluatedExpansion(optionDefinition); sourceMessage.append("expanded from option "); } else if (optionDefinition.hasImplicitRequirements()) { expansionArgs = ImmutableList.copyOf(optionDefinition.getImplicitRequirements()); diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java index ecc7a6b61b..6dee0eb7ac 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java +++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java @@ -89,14 +89,7 @@ class OptionsUsage { private static @Nullable ImmutableList<String> getExpansionIfKnown( OptionDefinition optionDefinition, OptionsData optionsData) { Preconditions.checkNotNull(optionDefinition); - try { - return optionsData.getEvaluatedExpansion(optionDefinition, null); - } catch (ExpansionNeedsValueException e) { - return null; - } catch (OptionsParsingException e) { - throw new IllegalStateException("Error expanding void expansion function: ", e); - } - + return optionsData.getEvaluatedExpansion(optionDefinition); } // Placeholder tag "UNKNOWN" is ignored. @@ -244,7 +237,7 @@ class OptionsUsage { usage.append('\n'); } - if (!optionsData.getExpansionDataForField(optionDefinition).isEmpty()) { + if (!optionsData.getEvaluatedExpansion(optionDefinition).isEmpty()) { // If this is an expansion option, list the expansion if known, or at least specify that we // don't know. usage.append("<br/>\n"); diff --git a/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java b/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java index 8b978d0e3d..1c34878715 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansionTest.java @@ -21,9 +21,9 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.UseDefault; import com.google.devtools.common.options.Converters; -import com.google.devtools.common.options.ExpansionContext; import com.google.devtools.common.options.ExpansionFunction; import com.google.devtools.common.options.InvocationPolicyEnforcer; +import com.google.devtools.common.options.IsolatedOptionsData; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; @@ -110,7 +110,7 @@ public class AllIncompatibleChangesExpansionTest { /** Dummy comment (linter suppression) */ public static class YExpansion implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { return ImmutableList.of("--noY"); } } diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java index 1bb13cfea4..c8d08cc110 100644 --- a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java +++ b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java @@ -262,80 +262,6 @@ public class InvocationPolicySetValueTest extends InvocationPolicyEnforcerTestBa } @Test - public void testSetValueWithExpansionFunctionFlags() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getSetValueBuilder() - .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - // Unrelated flag, but --test_expansion_function is not set - parser.parse("--test_string=throwaway value"); - - // The flags that --test_expansion_function expands into should still be their default values - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT); - - enforcer.enforce(parser, BUILD_COMMAND); - - // After policy enforcement, the flags should be the values from - // --test_expansion_function=valueA - testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_EXPANSION_FUNCTION_VALUE); - } - - @Test - public void testSetValueWithExpansionFunctionFlagsDefault() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getSetValueBuilder(); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - // Unrelated flag, but --test_expansion_function is not set - parser.parse("--test_string=throwaway value"); - - // The flags that --test_expansion_function expands into should still be their default values - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT); - - try { - enforcer.enforce(parser, BUILD_COMMAND); - fail(); - } catch (OptionsParsingException e) { - assertThat(e).hasMessage("Expansion value not set."); - } - } - - @Test - public void testSetValueWithExpansionFunctionFlagsWrongValue() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getSetValueBuilder() - .addFlagValue("unknown_value"); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - // Unrelated flag, but --test_expansion_function is not set - parser.parse("--test_string=throwaway value"); - - // The flags that --test_expansion_function expands into should still be their default values - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT); - - try { - enforcer.enforce(parser, BUILD_COMMAND); - fail(); - } catch (OptionsParsingException e) { - assertThat(e).hasMessage("Unrecognized expansion value: unknown_value"); - } - } - - @Test public void testOverridableSetValueWithExpansionFlags() throws Exception { InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); invocationPolicyBuilder @@ -369,33 +295,6 @@ public class InvocationPolicySetValueTest extends InvocationPolicyEnforcerTestBa } @Test - public void testOverridableSetValueWithExpansionFunction() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getSetValueBuilder() - .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE) - .setOverridable(true); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - // Unrelated flag, but --test_expansion_function is not set - parser.parse("--expanded_d=value that overrides"); - - // The flags that --test_expansion_function expands into should still be their default values - // except for the explicitly marked flag. - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo("value that overrides"); - - enforcer.enforce(parser, "build"); - - // After policy enforcement, the flags should be the values from --test_expansion_function, - // except for the user-set value, since the expansion flag was set to overridable. - testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo("value that overrides"); - } - - @Test public void testOverridableSetValueWithExpansionToRepeatingFlag() throws Exception { InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); invocationPolicyBuilder @@ -456,34 +355,6 @@ public class InvocationPolicySetValueTest extends InvocationPolicyEnforcerTestBa } @Test - public void testNonoverridableSetValueWithExpansionFlags() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getSetValueBuilder() - .addFlagValue(TestOptions.TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE) - .setOverridable(false); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - // Unrelated flag, but --test_expansion_function is not set - parser.parse("--expanded_d=value to override"); - - // The flags that --test_expansion_function expands into should still be their default values - // except for the explicitly marked flag. - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo("value to override"); - - enforcer.enforce(parser, "build"); - - // After policy enforcement, the flags should be the values from --test_expansion_function, - // including the value that the user tried to set, since the expansion flag was set - // non-overridably. - testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_EXPANSION_FUNCTION_VALUE); - } - - @Test public void testNonOverridableSetValueWithExpansionToRepeatingFlag() throws Exception { InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); invocationPolicyBuilder diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java index 3a41915a03..e66f7e02ce 100644 --- a/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java +++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java @@ -14,7 +14,6 @@ package com.google.devtools.common.options; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import org.junit.Test; @@ -124,28 +123,6 @@ public class InvocationPolicyUseDefaultTest extends InvocationPolicyEnforcerTest } @Test - public void testUseDefaultWithExpansionFunction() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_expansion_function") - .getUseDefaultBuilder(); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - parser.parse("--expanded_d=value to override"); - - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo("value to override"); - - try { - enforcer.enforce(parser, BUILD_COMMAND); - fail(); - } catch (OptionsParsingException e) { - assertThat(e).hasMessage("Expansion value not set."); - } - } - - @Test public void testUseDefaultWithExpansionFlagAndLaterOverride() throws Exception { InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); invocationPolicyBuilder 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 7d3c244003..79fd59880f 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -38,11 +38,11 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.ArrayList; 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; @@ -979,7 +979,7 @@ public class OptionsParserTest { /** ExpFunc */ public static class ExpFunc implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { return null; } } @@ -1002,41 +1002,8 @@ public class OptionsParserTest { newOptionsParser(NullExpansionsOptions.class); fail("Should have failed due to null expansion function result"); } catch (OptionsParser.ConstructionException e) { - assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class); - } - } - - /** NullExpansionOptions */ - public static class NullExpansionsWithArgumentOptions extends OptionsBase { - - /** ExpFunc */ - public static class ExpFunc implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { - return null; - } - } - - @Option( - name = "badness", - expansionFunction = ExpFunc.class, - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "null" - ) - public String badness; - } - - @Test - public void nullExpansionsWithArgument() throws Exception { - try { - // When an expansion takes a value, this exception should still happen at parse time. - newOptionsParser(NullExpansionsWithArgumentOptions.class); - fail("Should have failed due to null expansion function result"); - } catch (OptionsParser.ConstructionException e) { - assertThat(e) - .hasMessageThat() - .isEqualTo("Error calling expansion function for option '--badness'"); + assertThat(e).hasCauseThat().isInstanceOf(NullPointerException.class); + assertThat(e).hasCauseThat().hasMessageThat().contains("null value in entry"); } } @@ -1062,7 +1029,7 @@ public class OptionsParserTest { /** ExpFunc */ public static class ExpFunc implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { return ImmutableList.of("--expands"); } } @@ -1091,14 +1058,8 @@ public class OptionsParserTest { /** ExpFunc */ public static class ExpFunc implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext context) - throws OptionsParsingException { - String value = context.getUnparsedValue(); - if (value == null) { - throw new ExpansionNeedsValueException("No value given to 'expands_by_function'"); - } - - return ImmutableList.of("--underlying=pre_" + value, "--underlying=post_" + value); + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { + return ImmutableList.of("--underlying=pre_value", "--underlying=post_value"); } } @@ -1174,9 +1135,11 @@ public class OptionsParserTest { parser.parse( OptionPriority.PriorityCategory.COMMAND_LINE, null, - Arrays.asList("--expands_by_function=a", "--expands_by_function=b")); + Arrays.asList("--expands_by_function", "--expands_by_function")); ExpansionMultipleOptions options = parser.getOptions(ExpansionMultipleOptions.class); - assertThat(options.underlying).containsExactly("pre_a", "post_a", "pre_b", "post_b").inOrder(); + assertThat(options.underlying) + .containsExactly("pre_value", "post_value", "pre_value", "post_value") + .inOrder(); } @Test @@ -2395,7 +2358,7 @@ public class OptionsParserTest { } private List<String> visitOptionsToCollectTheirNames(Predicate<OptionDefinition> predicate) { - List<String> names = new LinkedList<>(); + List<String> names = new ArrayList<>(); Consumer<OptionDefinition> visitor = option -> names.add(option.getOptionName()); OptionsParser parser = OptionsParser.newOptionsParser(CompletionOptions.class); diff --git a/src/test/java/com/google/devtools/common/options/OptionsTest.java b/src/test/java/com/google/devtools/common/options/OptionsTest.java index 49c5767a76..5aec0b26b3 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsTest.java @@ -532,22 +532,4 @@ public class OptionsTest { Options.parse(TestOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"}); assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); } - @Test - public void dynamicExpansionFunctionWorks() throws Exception { - Options<TestOptions> options1 = - Options.parse(TestOptions.class, new String[] {"--dynamicexp=foo_bar"}); - Options<TestOptions> options2 = - Options.parse(TestOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"}); - assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); - } - - @Test - public void dynamicExpansionFunctionUnknowValue() throws Exception { - try { - Options.parse(TestOptions.class, new String[] {"--dynamicexp=foo"}); - fail("Unknown expansion argument should cause a failure."); - } catch (OptionsParsingException e) { - assertThat(e).hasMessage("Unexpected expansion argument: foo"); - } - } } diff --git a/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java b/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java index 3fa5a8602a..6f3550649b 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java @@ -565,61 +565,6 @@ public final class OptionsUsageTest { } @Test - public void expansionFunctionOptionThatReadsUserValue_shortTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_expansion_function", HelpVerbosity.SHORT)) - .isEqualTo(" --test_expansion_function\n"); - assertThat(getTerminalUsageWithoutTags("test_expansion_function", HelpVerbosity.SHORT)) - .isEqualTo(getTerminalUsageWithTags("test_expansion_function", HelpVerbosity.SHORT)); - } - - @Test - public void expansionFunctionOptionThatReadsUserValue_mediumTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_expansion_function", HelpVerbosity.MEDIUM)) - .isEqualTo(" --test_expansion_function\n"); - assertThat(getTerminalUsageWithoutTags("test_expansion_function", HelpVerbosity.MEDIUM)) - .isEqualTo(getTerminalUsageWithTags("test_expansion_function", HelpVerbosity.MEDIUM)); - } - - @Test - public void expansionFunctionOptionThatReadsUserValue_longTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_expansion_function", HelpVerbosity.LONG)) - .isEqualTo( - " --test_expansion_function\n" - + " this is for testing expansion-by-function functionality.\n" - + " Expands to unknown options.\n"); - assertThat(getTerminalUsageWithTags("test_expansion_function", HelpVerbosity.LONG)) - .isEqualTo( - " --test_expansion_function\n" - + " this is for testing expansion-by-function functionality.\n" - + " Expands to unknown options.\n" - + " Tags: no_op\n"); - } - - @Test - public void expansionFunctionOptionThatReadsUserValue_htmlOutput() { - assertThat(getHtmlUsageWithoutTags("test_expansion_function")) - .isEqualTo( - "<dt><code><a name=\"flag--test_expansion_function\"></a>" - + "--test_expansion_function</code></dt>\n" - + "<dd>\n" - + "this is for testing expansion-by-function functionality.\n" - + "<br/>\n" - + "Expands to unknown options.<br/>\n" - + "</dd>\n"); - assertThat(getHtmlUsageWithTags("test_expansion_function")) - .isEqualTo( - "<dt><code><a name=\"flag--test_expansion_function\"></a>" - + "--test_expansion_function</code></dt>\n" - + "<dd>\n" - + "this is for testing expansion-by-function functionality.\n" - + "<br/>\n" - + "Expands to unknown options.<br/>\n" - + "<br>Tags: \n" - + "<a href=\"#effect_tag_NO_OP\"><code>no_op</code></a>" - + "</dd>\n"); - } - - @Test public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_shortTerminalOutput() { assertThat(getTerminalUsageWithoutTags("prefix_expansion", HelpVerbosity.SHORT)) .isEqualTo(" --prefix_expansion\n"); diff --git a/src/test/java/com/google/devtools/common/options/TestOptions.java b/src/test/java/com/google/devtools/common/options/TestOptions.java index fe7b8e0729..2067b28672 100644 --- a/src/test/java/com/google/devtools/common/options/TestOptions.java +++ b/src/test/java/com/google/devtools/common/options/TestOptions.java @@ -237,45 +237,12 @@ public class TestOptions extends OptionsBase { ) public String testRecursiveImplicitRequirement; - public static final String TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE = "valueA"; - public static final String EXPANDED_D_EXPANSION_FUNCTION_VALUE = "expanded valueA"; - - /* - * Expansion function flags - */ - - /** Used for testing an expansion flag that requires a value. */ - public static class TestExpansionFunction implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(ExpansionContext expansionContext) - throws OptionsParsingException { - String value = expansionContext.getUnparsedValue(); - if (value == null) { - throw new ExpansionNeedsValueException("Expansion value not set."); - } else if (value.equals(TEST_EXPANSION_FUNCTION_ACCEPTED_VALUE)) { - return ImmutableList.of("--expanded_d", EXPANDED_D_EXPANSION_FUNCTION_VALUE); - } else { - throw new OptionsParsingException("Unrecognized expansion value: " + value); - } - } - } - - @Option( - name = "test_expansion_function", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - expansionFunction = TestExpansionFunction.class, - help = "this is for testing expansion-by-function functionality." - ) - public Void testExpansionFunction; - public static final String EXPANDED_D_VOID_EXPANSION_FUNCTION_VALUE = "void expanded"; /** Used for testing an expansion flag that doesn't requires a value. */ public static class TestVoidExpansionFunction implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext expansionContext) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { return ImmutableList.of("--expanded_d", EXPANDED_D_VOID_EXPANSION_FUNCTION_VALUE); } } @@ -303,9 +270,9 @@ public class TestOptions extends OptionsBase { */ public static class ExpansionDependsOnOtherOptionDefinitions implements ExpansionFunction { @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { + public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { TreeSet<String> flags = new TreeSet<>(); - for (Map.Entry<String, ?> entry : context.getOptionsData().getAllOptionDefinitions()) { + for (Map.Entry<String, ?> entry : optionsData.getAllOptionDefinitions()) { if (entry.getKey().startsWith("specialexp_")) { flags.add("--" + entry.getKey()); } @@ -324,36 +291,6 @@ public class TestOptions extends OptionsBase { ) public Void specialExp; - /** - * Defines an expansion function that adapts its expansion to the value assigned to the original - * expansion option. - */ - public static class ExpansionDependsOnFlagValue implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(ExpansionContext context) - throws OptionsParsingException { - String value = context.getUnparsedValue(); - if (value == null) { - throw new ExpansionNeedsValueException("Expansion value not set."); - } - if (value.equals("foo_bar")) { - return ImmutableList.<String>of("--specialexp_foo", "--specialexp_bar"); - } - - throw new OptionsParsingException("Unexpected expansion argument: " + value); - } - } - - @Option( - name = "dynamicexp", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "null", - expansionFunction = ExpansionDependsOnFlagValue.class, - help = "Expands depending on the value provided." - ) - public Void variableExpansion; - @Option( name = "specialexp_foo", documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, |