Dismantle Option.expansionFunction Formerly used by `--all_incompatible_changes`, which is going away. Options parsing is complex - remove some of the no-longer-necessary complexity. Partially to discourge new uses, partially to prevent us from piling on more. I don't think I fully dismantled all of the changes that expansion functions introduced, but I at least got enough to simplify things and prevent new uses. PiperOrigin-RevId: 395937981
diff --git a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java b/src/main/java/com/google/devtools/common/options/ExpansionFunction.java deleted file mode 100644 index d2c2693..0000000 --- a/src/main/java/com/google/devtools/common/options/ExpansionFunction.java +++ /dev/null
@@ -1,34 +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 com.google.common.collect.ImmutableList; - -/** - * A function from an option parser's static setup (what flags it knows about) to a list of - * expansion Strings to use for one of its options. - */ -@FunctionalInterface -public interface ExpansionFunction { - - /** - * Compute the expansion for an option. May be called at any time during or after the {@link - * OptionsParser}'s construction, or not at all. - * - * @param optionsData the parser's indexed information about its own options, before expansion - * information is computed - * @return An expansion to use on an empty list - */ - ImmutableList<String> getExpansion(IsolatedOptionsData optionsData); -}
diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java index 3ad32ab..925409d 100644 --- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java +++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -43,6 +43,7 @@ * <p>This class is immutable so long as the converters and default values associated with the * options are immutable. */ +// TODO(b/159980134): Can this be folded into OptionsData? @Immutable public class IsolatedOptionsData extends OpaqueOptionsData {
diff --git a/src/main/java/com/google/devtools/common/options/Option.java b/src/main/java/com/google/devtools/common/options/Option.java index f3baed2..5d24d73 100644 --- a/src/main/java/com/google/devtools/common/options/Option.java +++ b/src/main/java/com/google/devtools/common/options/Option.java
@@ -155,17 +155,6 @@ String[] expansion() default {}; /** - * A mechanism for specifying an expansion that is a function of the parser's {@link - * IsolatedOptionsData}. This can be used to create an option that expands to different strings - * depending on what other options the parser knows about. - * - * <p>If provided (i.e. not {@link ExpansionFunction}{@code .class}), the {@code expansion} field - * must not be set. The mechanism of expansion is as if the {@code expansion} field were set to - * whatever the return value of this function is. - */ - Class<? extends ExpansionFunction> expansionFunction() default ExpansionFunction.class; - - /** * Additional options that need to be implicitly added for this option. * * <p>Nothing guarantees that these options are not overridden by later or higher-priority values
diff --git a/src/main/java/com/google/devtools/common/options/OptionDefinition.java b/src/main/java/com/google/devtools/common/options/OptionDefinition.java index 6251f9f5..d4589ac 100644 --- a/src/main/java/com/google/devtools/common/options/OptionDefinition.java +++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -147,11 +147,6 @@ return optionAnnotation.expansion(); } - /** {@link Option#expansionFunction()} ()} */ - public Class<? extends ExpansionFunction> getExpansionFunction() { - return optionAnnotation.expansionFunction(); - } - /** {@link Option#implicitRequirements()} ()} */ public String[] getImplicitRequirements() { return optionAnnotation.implicitRequirements(); @@ -188,7 +183,7 @@ /** Returns whether the arg is an expansion option. */ public boolean isExpansionOption() { - return (getOptionExpansion().length > 0 || usesExpansionFunction()); + return getOptionExpansion().length > 0; } /** Returns whether the arg is an expansion option. */ @@ -197,14 +192,6 @@ } /** - * Returns whether the arg is an expansion option defined by an expansion function (and not a - * constant expansion value). - */ - public boolean usesExpansionFunction() { - return getExpansionFunction() != ExpansionFunction.class; - } - - /** * For an option that does not use {@link Option#allowMultiple}, returns its type. For an option * that does use it, asserts that the type is a {@code List<T>} and returns its element type * {@code T}.
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 ca8d0be..ffe2179 100644 --- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -329,8 +329,8 @@ /** * The form of a value for an expansion option, one that does not have its own value but expands - * 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()}. + * in place to other options. This should be used for flags with anN expansion defined in {@link + * Option#expansion()}. */ private static class ExpansionOptionValueDescription extends OptionValueDescription { private final List<String> expansion;
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 63cac24..2a3e485 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -16,8 +16,6 @@ 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.concurrent.Immutable; @@ -43,9 +41,8 @@ private static final ImmutableList<String> EMPTY_EXPANSION = ImmutableList.<String>of(); /** - * 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. + * Returns the expansion of an options field. If the field is not an expansion option returns an + * empty array. */ public ImmutableList<String> getEvaluatedExpansion(OptionDefinition optionDefinition) { ImmutableList<String> result = evaluatedExpansions.get(optionDefinition); @@ -55,9 +52,8 @@ /** * Constructs an {@link OptionsData} object for a parser that knows about the given {@link * OptionsBase} classes. In addition to the work done to construct the {@link - * IsolatedOptionsData}, this also computes expansion information. If an option has static - * expansions or uses an expansion function that takes a Void object, try to precalculate the - * expansion here. + * IsolatedOptionsData}, this also computes expansion information. If an option has an expansion, + * try to precalculate its here. */ static OptionsData from(Collection<Class<? extends OptionsBase>> classes) { IsolatedOptionsData isolatedData = IsolatedOptionsData.from(classes); @@ -67,30 +63,9 @@ 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 - // OptionProcessor checks at compile time that these aren't used together. String[] constExpansion = optionDefinition.getOptionExpansion(); - Class<? extends ExpansionFunction> expansionFunctionClass = - optionDefinition.getExpansionFunction(); if (constExpansion.length > 0) { evaluatedExpansionsBuilder.put(optionDefinition, ImmutableList.copyOf(constExpansion)); - } else if (optionDefinition.usesExpansionFunction()) { - if (Modifier.isAbstract(expansionFunctionClass.getModifiers())) { - throw new AssertionError( - "The expansionFunction type " + expansionFunctionClass + " must be a concrete type"); - } - // Evaluate the ExpansionFunction. - ExpansionFunction instance; - try { - Constructor<?> constructor = expansionFunctionClass.getConstructor(); - instance = (ExpansionFunction) constructor.newInstance(); - } catch (Exception e) { - // This indicates an error in the ExpansionFunction, and should be discovered the first - // time it is used. - throw new AssertionError(e); - } - ImmutableList<String> expansion = instance.getExpansion(isolatedData); - evaluatedExpansionsBuilder.put(optionDefinition, expansion); } } return new OptionsData(isolatedData, evaluatedExpansionsBuilder.build());
diff --git a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java index b06af6a..cf21e6a 100644 --- a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java +++ b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java
@@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.Converters; -import com.google.devtools.common.options.ExpansionFunction; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDefinition; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -454,26 +453,9 @@ */ private void checkExpansionOptions(VariableElement optionField) throws OptionProcessorException { Option annotation = optionField.getAnnotation(Option.class); - boolean isStaticExpansion = annotation.expansion().length > 0; + boolean isExpansion = annotation.expansion().length > 0; boolean hasImplicitRequirements = annotation.implicitRequirements().length > 0; - AnnotationMirror annotationMirror = - ProcessorUtils.getAnnotation(elementUtils, typeUtils, optionField, Option.class); - TypeElement expansionFunction = - ProcessorUtils.getClassTypeFromAnnotationField( - elementUtils, annotationMirror, "expansionFunction"); - TypeElement defaultExpansionFunction = - elementUtils.getTypeElement(ExpansionFunction.class.getCanonicalName()); - boolean isFunctionalExpansion = - !typeUtils.isSameType(expansionFunction.asType(), defaultExpansionFunction.asType()); - - if (isStaticExpansion && isFunctionalExpansion) { - throw new OptionProcessorException( - optionField, - "Options cannot expand using both a static expansion list and an expansion function."); - } - boolean isExpansion = isStaticExpansion || isFunctionalExpansion; - if (isExpansion && hasImplicitRequirements) { throw new OptionProcessorException( optionField,
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 e66f7e0..6e76c35 100644 --- a/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java +++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyUseDefaultTest.java
@@ -101,28 +101,6 @@ } @Test - public void testUseDefaultWithVoidExpansionFunction() throws Exception { - InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); - invocationPolicyBuilder - .addFlagPoliciesBuilder() - .setFlagName("test_void_expansion_function") - .getUseDefaultBuilder(); - - InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); - parser.parse("--expanded_d=value to override"); - - TestOptions testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo("value to override"); - - enforcer.enforce(parser, BUILD_COMMAND); - - // After policy enforcement, all the flags that --test_void_expansion_function expanded into - // should be back to their default values. - testOptions = getTestOptions(); - assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT); - } - - @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 5643fa3..848af47 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -20,7 +20,6 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter; import com.google.devtools.common.options.OptionPriority.PriorityCategory; @@ -642,40 +641,6 @@ fail(); } - /** NullExpansionOptions */ - public static class NullExpansionsOptions extends OptionsBase { - - /** ExpFunc */ - public static class ExpFunc implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { - return null; - } - } - - @Option( - name = "badness", - expansionFunction = ExpFunc.class, - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "null" - ) - public Void badness; - } - - @Test - public void nullExpansions() throws Exception { - // Ensure that we get the NPE at the time of parser construction, not later when actually - // parsing. - OptionsParser.ConstructionException e = - assertThrows( - "Should have failed due to null expansion function result", - OptionsParser.ConstructionException.class, - () -> OptionsParser.builder().optionsClasses(NullExpansionsOptions.class).build()); - assertThat(e).hasCauseThat().isInstanceOf(NullPointerException.class); - assertThat(e).hasCauseThat().hasMessageThat().contains("null value in entry"); - } - /** ExpansionOptions */ public static class ExpansionOptions extends OptionsBase { @Option( @@ -694,64 +659,15 @@ defaultValue = "null" ) public Void expands; - - /** ExpFunc */ - public static class ExpFunc implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { - return ImmutableList.of("--expands"); - } - } - - @Option( - name = "expands_by_function", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "null", - expansionFunction = ExpFunc.class - ) - public Void expandsByFunction; - } - - /** ExpansionMultipleOptions */ - public static class ExpansionMultipleOptions extends OptionsBase { - @Option( - name = "underlying", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - allowMultiple = true - ) - public List<String> underlying; - - /** ExpFunc */ - public static class ExpFunc implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { - return ImmutableList.of("--underlying=pre_value", "--underlying=post_value"); - } - } - - @Option( - name = "expands_by_function", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - expansionFunction = ExpFunc.class - ) - public Void expandsByFunction; } @Test public void describeOptionsWithExpansion() throws Exception { - // We have to test this here rather than in OptionsTest because expansion functions require - // that an options parser be constructed. OptionsParser parser = OptionsParser.builder().optionsClasses(ExpansionOptions.class).build(); String usage = parser.describeOptionsWithDeprecatedCategories( ImmutableMap.<String, String>of(), OptionsParser.HelpVerbosity.LONG); assertThat(usage).contains(" --expands\n Expands to: --underlying=from_expansion"); - assertThat(usage).contains(" --expands_by_function\n Expands to: --expands"); } @Test @@ -833,37 +749,53 @@ assertThat(parser.getWarnings()).isEmpty(); } - // Makes sure the expansion options are expanded in the right order if they affect flags that - // allow multiples. + /** ExpansionOptions to allow-multiple values. */ + public static class ExpansionOptionsToMultiple extends OptionsBase { + @Option( + name = "underlying", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "null", + allowMultiple = true) + public List<String> underlying; + + @Option( + name = "expands", + expansion = {"--underlying=from_expansion"}, + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "null") + public Void expands; + } + + /** + * Makes sure the expansion options are expanded in the right order if they affect flags that + * allow multiples. + */ @Test public void multipleExpansionOptionsWithValue() throws Exception { OptionsParser parser = - OptionsParser.builder().optionsClasses(ExpansionMultipleOptions.class).build(); + OptionsParser.builder().optionsClasses(ExpansionOptionsToMultiple.class).build(); parser.parse( OptionPriority.PriorityCategory.COMMAND_LINE, null, - Arrays.asList( - "--expands_by_function", "--underlying=direct_value", "--expands_by_function")); - ExpansionMultipleOptions options = parser.getOptions(ExpansionMultipleOptions.class); + Arrays.asList("--expands", "--underlying=direct_value", "--expands")); + ExpansionOptionsToMultiple options = parser.getOptions(ExpansionOptionsToMultiple.class); assertThat(options.underlying) - .containsExactly("pre_value", "post_value", "direct_value", "pre_value", "post_value") + .containsExactly("from_expansion", "direct_value", "from_expansion") .inOrder(); assertThat(parser.getWarnings()).isEmpty(); } @Test public void checkExpansionValueWarning() throws Exception { - OptionsParser parser = - OptionsParser.builder().optionsClasses(ExpansionMultipleOptions.class).build(); - parser.parse( - OptionPriority.PriorityCategory.COMMAND_LINE, - null, - Arrays.asList("--expands_by_function=no")); - ExpansionMultipleOptions options = parser.getOptions(ExpansionMultipleOptions.class); - assertThat(options.underlying).containsExactly("pre_value", "post_value").inOrder(); + OptionsParser parser = OptionsParser.builder().optionsClasses(ExpansionOptions.class).build(); + parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--expands=no")); + ExpansionOptions options = parser.getOptions(ExpansionOptions.class); + assertThat(options.underlying).isEqualTo("from_expansion"); assertThat(parser.getWarnings()) .containsExactly( - "option '--expands_by_function' is an expansion option. It does not accept values, " + "option '--expands' is an expansion option. It does not accept values, " + "and does not change its expansion based on the value provided. " + "Value 'no' will be ignored."); }
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 a169807..6a0e586 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsTest.java
@@ -497,25 +497,4 @@ Options.parse(HttpOptions.class, new String[] { "--special" }); assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); } - - @Test - public void usageForExpansionFunction() { - // Expect that the usage text contains the expansion appropriate to the options bases that were - // loaded into the options parser. - String usage = Options.getUsage(TestOptions.class); - assertThat(usage) - .contains( - " --prefix_expansion\n" - + " Expands to all options with a specific prefix.\n" - + " Expands to: --specialexp_bar --specialexp_foo"); - } - - @Test - public void expansionFunction() throws Exception { - Options<TestOptions> options1 = - Options.parse(TestOptions.class, new String[] {"--prefix_expansion"}); - Options<TestOptions> options2 = - Options.parse(TestOptions.class, new String[] {"--specialexp_foo", "--specialexp_bar"}); - assertThat(options1.getOptions()).isEqualTo(options2.getOptions()); - } }
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 01ffb55..d674e56 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java
@@ -581,132 +581,4 @@ + "<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"); - assertThat(getTerminalUsageWithoutTags("prefix_expansion", HelpVerbosity.SHORT)) - .isEqualTo(getTerminalUsageWithTags("prefix_expansion", HelpVerbosity.SHORT)); - } - - @Test - public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_mediumTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("prefix_expansion", HelpVerbosity.MEDIUM)) - .isEqualTo(" --prefix_expansion\n"); - assertThat(getTerminalUsageWithoutTags("prefix_expansion", HelpVerbosity.MEDIUM)) - .isEqualTo(getTerminalUsageWithTags("prefix_expansion", HelpVerbosity.MEDIUM)); - } - - @Test - public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_longTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("prefix_expansion", HelpVerbosity.LONG)) - .isEqualTo( - " --prefix_expansion\n" - + " Expands to all options with a specific prefix.\n" - + " Expands to: --specialexp_bar --specialexp_foo \n"); - assertThat(getTerminalUsageWithTags("prefix_expansion", HelpVerbosity.LONG)) - .isEqualTo( - " --prefix_expansion\n" - + " Expands to all options with a specific prefix.\n" - + " Expands to: --specialexp_bar --specialexp_foo \n" - + " Tags: no_op\n"); - } - - @Test - public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_htmlOutput() { - assertThat(getHtmlUsageWithoutTags("prefix_expansion")) - .isEqualTo( - "<dt id=\"flag--prefix_expansion\"><code>" - + "<a href=\"#flag--prefix_expansion\">--prefix_expansion</a>" - + "</code></dt>\n" - + "<dd>\n" - + "Expands to all options with a specific prefix.\n" - + "<br/>\n" - + "Expands to:<br/>\n" - + " <code>" - + "<a href=\"#flag--specialexp_bar\">--specialexp_bar</a></code><br/>\n" - + " <code>" - + "<a href=\"#flag--specialexp_foo\">--specialexp_foo</a></code><br/>\n" - + "</dd>\n"); - assertThat(getHtmlUsageWithTags("prefix_expansion")) - .isEqualTo( - "<dt id=\"flag--prefix_expansion\"><code>" - + "<a href=\"#flag--prefix_expansion\">--prefix_expansion</a>" - + "</code></dt>\n" - + "<dd>\n" - + "Expands to all options with a specific prefix.\n" - + "<br/>\n" - + "Expands to:<br/>\n" - + " <code>" - + "<a href=\"#flag--specialexp_bar\">--specialexp_bar</a></code><br/>\n" - + " <code>" - + "<a href=\"#flag--specialexp_foo\">--specialexp_foo</a></code><br/>\n" - + "<br>Tags: \n" - + "<a href=\"#effect_tag_NO_OP\"><code>no_op</code></a>" - + "</dd>\n"); - } - - @Test - public void tagHeavyExpansionOption_shortTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_void_expansion_function", HelpVerbosity.SHORT)) - .isEqualTo(" --test_void_expansion_function\n"); - assertThat(getTerminalUsageWithoutTags("test_void_expansion_function", HelpVerbosity.SHORT)) - .isEqualTo(getTerminalUsageWithTags("test_void_expansion_function", HelpVerbosity.SHORT)); - } - - @Test - public void tagHeavyExpansionOption_mediumTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_void_expansion_function", HelpVerbosity.MEDIUM)) - .isEqualTo(" --test_void_expansion_function\n"); - assertThat(getTerminalUsageWithoutTags("test_void_expansion_function", HelpVerbosity.MEDIUM)) - .isEqualTo(getTerminalUsageWithTags("test_void_expansion_function", HelpVerbosity.MEDIUM)); - } - - @Test - public void tagHeavyExpansionOption_longTerminalOutput() { - assertThat(getTerminalUsageWithoutTags("test_void_expansion_function", HelpVerbosity.LONG)) - .isEqualTo( - " --test_void_expansion_function\n" - + " Listing a ton of random tags to test the usage output.\n" - + " Expands to: --expanded_d void expanded \n"); - assertThat(getTerminalUsageWithTags("test_void_expansion_function", HelpVerbosity.LONG)) - .isEqualTo( - " --test_void_expansion_function\n" - + " Listing a ton of random tags to test the usage output.\n" - + " Expands to: --expanded_d void expanded \n" - + " Tags: action_command_lines, test_runner, terminal_output, experimental\n"); - } - - @Test - public void tagHeavyExpansionOption_htmlOutput() { - assertThat(getHtmlUsageWithoutTags("test_void_expansion_function")) - .isEqualTo( - "<dt id=\"flag--test_void_expansion_function\"><code><a" - + " href=\"#flag--test_void_expansion_function\">--test_void_expansion_function</a></code></dt>\n" - + "<dd>\n" - + "Listing a ton of random tags to test the usage output.\n" - + "<br/>\n" - + "Expands to:<br/>\n" - + " <code><a href=\"#flag--expanded_d\">--expanded_d</a></code><br/>\n" - + " <code><a href=\"#flagvoid expanded\">void" - + " expanded</a></code><br/>\n" - + "</dd>\n"); - assertThat(getHtmlUsageWithTags("test_void_expansion_function")) - .isEqualTo( - "<dt id=\"flag--test_void_expansion_function\"><code><a" - + " href=\"#flag--test_void_expansion_function\">--test_void_expansion_function</a></code></dt>\n" - + "<dd>\n" - + "Listing a ton of random tags to test the usage output.\n" - + "<br/>\n" - + "Expands to:<br/>\n" - + " <code><a href=\"#flag--expanded_d\">--expanded_d</a></code><br/>\n" - + " <code><a href=\"#flagvoid expanded\">void" - + " expanded</a></code><br/>\n" - + "<br>Tags: \n" - + "<a href=\"#effect_tag_ACTION_COMMAND_LINES\"><code>action_command_lines</code></a>," - + " <a href=\"#effect_tag_TEST_RUNNER\"><code>test_runner</code></a>, <a" - + " href=\"#effect_tag_TERMINAL_OUTPUT\"><code>terminal_output</code></a>, <a" - + " href=\"#metadata_tag_EXPERIMENTAL\"><code>experimental</code></a></dd>\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 3fbaa38..2e95799 100644 --- a/src/test/java/com/google/devtools/common/options/TestOptions.java +++ b/src/test/java/com/google/devtools/common/options/TestOptions.java
@@ -13,11 +13,8 @@ // limitations under the License. package com.google.devtools.common.options; -import com.google.common.collect.ImmutableList; import com.google.devtools.common.options.InvocationPolicyEnforcerTestBase.ToListConverter; import java.util.List; -import java.util.Map; -import java.util.TreeSet; /** Options for testing. */ public class TestOptions extends OptionsBase { @@ -244,76 +241,6 @@ ) public String testRecursiveImplicitRequirement; - 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(IsolatedOptionsData optionsData) { - return ImmutableList.of("--expanded_d", EXPANDED_D_VOID_EXPANSION_FUNCTION_VALUE); - } - } - - @Option( - name = "test_void_expansion_function", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = { - OptionEffectTag.ACTION_COMMAND_LINES, - OptionEffectTag.TEST_RUNNER, - OptionEffectTag.TERMINAL_OUTPUT - }, - metadataTags = {OptionMetadataTag.EXPERIMENTAL}, - expansionFunction = TestVoidExpansionFunction.class, - help = "Listing a ton of random tags to test the usage output." - ) - public Void testVoidExpansionFunction; - - // Interestingly, the class needs to be public, or else the default constructor ends up not - // being public and the expander can't be instantiated. - /** - * Defines an expansion function that looks at other options defined with it and expands to - * options that match a pattern. - */ - public static class ExpansionDependsOnOtherOptionDefinitions implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(IsolatedOptionsData optionsData) { - TreeSet<String> flags = new TreeSet<>(); - for (Map.Entry<String, ?> entry : optionsData.getAllOptionDefinitions()) { - if (entry.getKey().startsWith("specialexp_")) { - flags.add("--" + entry.getKey()); - } - } - return ImmutableList.copyOf(flags); - } - } - - @Option( - name = "prefix_expansion", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "null", - expansionFunction = ExpansionDependsOnOtherOptionDefinitions.class, - help = "Expands to all options with a specific prefix." - ) - public Void specialExp; - - @Option( - name = "specialexp_foo", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "false" - ) - public boolean specialExpFoo; - - @Option( - name = "specialexp_bar", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.NO_OP}, - defaultValue = "false" - ) - public boolean specialExpBar; - @Option( name = "test_deprecated", defaultValue = "default",
diff --git a/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java b/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java index aac02be..6d97553 100644 --- a/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java +++ b/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java
@@ -294,16 +294,6 @@ } @Test - public void functionalExpansionOptionThatAllowsMultipleIsRejected() { - assertAbout(javaSource()) - .that(getFile("FunctionalExpansionOptionWithAllowMultiple.java")) - .processedWith(new OptionProcessor()) - .failsToCompile() - .withErrorContaining( - "Can't set an option to accumulate multiple values and let it expand to other flags."); - } - - @Test public void expansionOptionWithImplicitRequirementIsRejected() { assertAbout(javaSource()) .that(getFile("ExpansionOptionWithImplicitRequirement.java")) @@ -312,14 +302,4 @@ .withErrorContaining( "Can't set an option to be both an expansion option and have implicit requirements."); } - - @Test - public void expansionOptionThatExpandsInTwoWaysIsRejected() { - assertAbout(javaSource()) - .that(getFile("DoubleExpansionOption.java")) - .processedWith(new OptionProcessor()) - .failsToCompile() - .withErrorContaining( - "Options cannot expand using both a static expansion list and an expansion function."); - } }
diff --git a/src/test/java/com/google/devtools/common/options/processor/optiontestsources/DoubleExpansionOption.java b/src/test/java/com/google/devtools/common/options/processor/optiontestsources/DoubleExpansionOption.java deleted file mode 100644 index 6b9b156..0000000 --- a/src/test/java/com/google/devtools/common/options/processor/optiontestsources/DoubleExpansionOption.java +++ /dev/null
@@ -1,43 +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.processor.optiontestsources; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.common.options.ExpansionContext; -import com.google.devtools.common.options.ExpansionFunction; -import com.google.devtools.common.options.Option; -import com.google.devtools.common.options.OptionDocumentationCategory; -import com.google.devtools.common.options.OptionEffectTag; -import com.google.devtools.common.options.OptionsBase; - -/** This example options class should fail to compile. */ -public class DoubleExpansionOption extends OptionsBase { - @Option( - name = "bad_option", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = OptionEffectTag.NO_OP, - expansion = "--foo=bar", - expansionFunction = FooBarExpansion.class - ) - public Void badOption; - - /** Dummy expansion class for the illegal option. */ - public static class FooBarExpansion implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { - return ImmutableList.of("--foo", "--bar"); - } - } -}
diff --git a/src/test/java/com/google/devtools/common/options/processor/optiontestsources/FunctionalExpansionOptionWithAllowMultiple.java b/src/test/java/com/google/devtools/common/options/processor/optiontestsources/FunctionalExpansionOptionWithAllowMultiple.java deleted file mode 100644 index 87489d2..0000000 --- a/src/test/java/com/google/devtools/common/options/processor/optiontestsources/FunctionalExpansionOptionWithAllowMultiple.java +++ /dev/null
@@ -1,43 +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.processor.optiontestsources; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.common.options.ExpansionContext; -import com.google.devtools.common.options.ExpansionFunction; -import com.google.devtools.common.options.Option; -import com.google.devtools.common.options.OptionDocumentationCategory; -import com.google.devtools.common.options.OptionEffectTag; -import com.google.devtools.common.options.OptionsBase; - -/** This example options class should fail to compile. */ -public class FunctionalExpansionOptionWithAllowMultiple extends OptionsBase { - @Option( - name = "bad_option", - defaultValue = "null", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = OptionEffectTag.NO_OP, - expansionFunction = FooBarExpansion.class, - allowMultiple = true - ) - public Void badOption; - - /** Dummy expansion class for the illegal option. */ - public static class FooBarExpansion implements ExpansionFunction { - @Override - public ImmutableList<String> getExpansion(ExpansionContext context) { - return ImmutableList.of("--foo", "--bar"); - } - } -}