diff options
| -rw-r--r-- | src/main/java/com/google/devtools/common/options/OptionValueDescription.java | 6 | ||||
| -rw-r--r-- | src/test/java/com/google/devtools/common/options/OptionsParserTest.java | 92 |
2 files changed, 94 insertions, 4 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 bb6f24266d..616b3b58c8 100644 --- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java @@ -276,8 +276,10 @@ public abstract class OptionValueDescription { public String getSourceString() { return parsedOptions .asMap() - .values() + .entrySet() .stream() + .sorted(Comparator.comparing(Entry::getKey)) + .map(Entry::getValue) .flatMap(Collection::stream) .map(ParsedOptionDescription::getSource) .distinct() @@ -323,6 +325,8 @@ public abstract class OptionValueDescription { .sorted(Comparator.comparing(Entry::getKey)) .map(Entry::getValue) .flatMap(Collection::stream) + // Only provide the options that aren't implied elsewhere. + .filter(optionDesc -> optionDesc.getImplicitDependent() == null) .collect(ImmutableList.toImmutableList()); } } 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 25ce6dd0cd..8368e7821d 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -40,9 +40,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** - * Tests {@link OptionsParser}. - */ +/** Tests {@link OptionsParser}. */ @RunWith(JUnit4.class) public class OptionsParserTest { @@ -1041,6 +1039,94 @@ public class OptionsParserTest { assertThat(parser.getWarnings()).isEmpty(); } + /** + * Options for testing the behavior of canonicalization when an option implicitly requires a + * repeatable option. + */ + public static class ImplicitDependencyOnAllowMultipleOptions extends OptionsBase { + @Option( + name = "first", + implicitRequirements = "--second=requiredByFirst", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "false" + ) + public boolean first; + + @Option( + name = "second", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "null", + allowMultiple = true + ) + public List<String> second; + + @Option( + name = "third", + implicitRequirements = "--second=requiredByThird", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.NO_OP}, + defaultValue = "null" + ) + public String third; + } + + @Test + public void testCanonicalizeExcludesImplicitDependencyOnRepeatableOption() + throws OptionsParsingException { + OptionsParser parser = + OptionsParser.newOptionsParser(ImplicitDependencyOnAllowMultipleOptions.class); + parser.parse( + OptionPriority.PriorityCategory.COMMAND_LINE, + null, + Arrays.asList("--first", "--second=explicitValue")); + OptionValueDescription first = parser.getOptionValueDescription("first"); + assertThat(first).isNotNull(); + assertThat(first.getCanonicalInstances()).hasSize(1); + + OptionValueDescription second = parser.getOptionValueDescription("second"); + assertThat(second).isNotNull(); + assertThat(second.getSourceString()).matches("implicit requirement of option '--first', null"); + // Implicit requirements don't get listed as canonical. Check that this excludes the implicit + // value, but still tracks the explicit one. + assertThat(second.getCanonicalInstances()).isNotNull(); + assertThat(second.getCanonicalInstances()).hasSize(1); + assertThat(parser.canonicalize()).containsExactly("--first=1", "--second=explicitValue"); + + ImplicitDependencyOnAllowMultipleOptions options = + parser.getOptions(ImplicitDependencyOnAllowMultipleOptions.class); + assertThat(options.first).isTrue(); + assertThat(options.second).containsExactly("explicitValue", "requiredByFirst"); + assertThat(parser.getWarnings()).isEmpty(); + } + + @Test + public void testCanonicalizeExcludesImplicitDependencyForOtherwiseUnmentionedRepeatableOption() + throws OptionsParsingException { + OptionsParser parser = + OptionsParser.newOptionsParser(ImplicitDependencyOnAllowMultipleOptions.class); + parser.parse(OptionPriority.PriorityCategory.COMMAND_LINE, null, Arrays.asList("--first")); + OptionValueDescription first = parser.getOptionValueDescription("first"); + assertThat(first).isNotNull(); + assertThat(first.getCanonicalInstances()).hasSize(1); + + OptionValueDescription second = parser.getOptionValueDescription("second"); + assertThat(second).isNotNull(); + assertThat(second.getSourceString()).matches("implicit requirement of option '--first'"); + // Implicit requirements don't get listed as canonical. Check that this excludes the implicit + // value, leaving behind no mention of second. + assertThat(second.getCanonicalInstances()).isNotNull(); + assertThat(second.getCanonicalInstances()).isEmpty(); + assertThat(parser.canonicalize()).containsExactly("--first=1"); + + ImplicitDependencyOnAllowMultipleOptions options = + parser.getOptions(ImplicitDependencyOnAllowMultipleOptions.class); + assertThat(options.first).isTrue(); + assertThat(options.second).containsExactly("requiredByFirst"); + assertThat(parser.getWarnings()).isEmpty(); + } + public static class WarningOptions extends OptionsBase { @Deprecated @Option( |