Remove wrapper options support.
RELNOTES: None.
PiperOrigin-RevId: 179588174
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 87235db..9309855 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
@@ -45,8 +45,7 @@
* <li>the {@link Option#help} field must be set, and must refer the user to information about
* what the change does and how to migrate their code
* <li>the following fields may not be used: {@link Option#abbrev}, {@link Option#valueHelp},
- * {@link Option#converter}, {@link Option#allowMultiple}, {@link Option#oldName}, and {@link
- * Option#wrapperOption}
+ * {@link Option#converter}, {@link Option#allowMultiple}, and {@link Option#oldName}
* </ul>
*
* Example:
@@ -122,9 +121,6 @@
if (!optionDefinition.getOldOptionName().equals(defaultString)) {
throw new IllegalArgumentException(prefix + "must not use the oldName field");
}
- if (optionDefinition.isWrapperOption()) {
- throw new IllegalArgumentException(prefix + "must not use the wrapperOption field");
- }
// Validate the fields that are actually allowed.
if (!optionDefinition.getOptionName().startsWith(INCOMPATIBLE_NAME_PREFIX)) {
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 45f320a..f26a136 100644
--- a/src/main/java/com/google/devtools/common/options/Option.java
+++ b/src/main/java/com/google/devtools/common/options/Option.java
@@ -186,22 +186,4 @@
* that the old name is deprecated and the new name should be used.
*/
String oldName() default "";
-
- /**
- * Indicates that this option is a wrapper for other options, and will be unwrapped when parsed.
- * For example, if foo is a wrapper option, then "--foo=--bar=baz" will be parsed as the flag
- * "--bar=baz" (rather than --foo taking the value "--bar=baz"). A wrapper option should have the
- * type {@link Void} (if it is something other than Void, the parser will not assign a value to
- * it). The {@link Option#implicitRequirements()}, {@link Option#expansion()}, {@link
- * Option#converter()} attributes will not be processed. Wrapper options are implicitly repeatable
- * (i.e., as though {@link Option#allowMultiple()} is true regardless of its value in the
- * annotation).
- *
- * <p>Wrapper options are provided only for transitioning flags which appear as values to other
- * flags, to top-level flags. Wrapper options should not be used in Invocation Policy, as
- * expansion flags to other flags, or as implicit requirements to other flags. Use the inner flags
- * instead.
- */
- @Deprecated
- boolean wrapperOption() default false;
}
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 84a9d2d..7b87744 100644
--- a/src/main/java/com/google/devtools/common/options/OptionDefinition.java
+++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -156,11 +156,6 @@
return optionAnnotation.oldName();
}
- /** {@link Option#wrapperOption()} ()} ()} */
- public boolean isWrapperOption() {
- return optionAnnotation.wrapperOption();
- }
-
/** Returns whether an option --foo has a negative equivalent --nofoo. */
public boolean hasNegativeOption() {
return getType().equals(boolean.class) || getType().equals(TriState.class);
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 616b3b5..3fde138 100644
--- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
+++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -98,8 +98,6 @@
return new RepeatableOptionValueDescription(option);
} else if (option.hasImplicitRequirements()) {
return new OptionWithImplicitRequirementsValueDescription(option);
- } else if (option.isWrapperOption()) {
- return new WrapperOptionValueDescription(option);
} else {
return new SingleOptionValueDescription(option);
}
@@ -430,50 +428,6 @@
optionDefinition, parsedOption.getSource()));
}
}
-
- /** Form for options that contain other options in the value text to which they expand. */
- private static final class WrapperOptionValueDescription extends OptionValueDescription {
-
- WrapperOptionValueDescription(OptionDefinition optionDefinition) {
- super(optionDefinition);
- }
-
- @Override
- public Object getValue() {
- return null;
- }
-
- @Override
- public String getSourceString() {
- return null;
- }
-
- @Override
- ExpansionBundle addOptionInstance(ParsedOptionDescription parsedOption, List<String> warnings)
- throws OptionsParsingException {
- if (!parsedOption.getUnconvertedValue().startsWith("-")) {
- throw new OptionsParsingException(
- String.format(
- "Invalid value format for %s. You may have meant --%s=--%s",
- optionDefinition,
- optionDefinition.getOptionName(),
- parsedOption.getUnconvertedValue()));
- }
- return new ExpansionBundle(
- ImmutableList.of(parsedOption.getUnconvertedValue()),
- (parsedOption.getSource() == null)
- ? String.format("unwrapped from %s", optionDefinition)
- : String.format(
- "unwrapped from %s (source %s)", optionDefinition, parsedOption.getSource()));
- }
-
- @Override
- public ImmutableList<ParsedOptionDescription> getCanonicalInstances() {
- // No wrapper options get listed in the canonical form - the options they are wrapping will
- // be in the right place.
- return ImmutableList.of();
- }
- }
}
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 496927b..5ce35da 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -392,16 +392,15 @@
@Nullable String unconvertedValue = parsedOption.getUnconvertedValue();
// There are 3 types of flags that expand to other flag values. Expansion flags are the
- // accepted way to do this, but two legacy features remain: implicit requirements and wrapper
- // options. We rely on the OptionProcessor compile-time check's guarantee that no option sets
- // multiple of these behaviors. (In Bazel, --config is another such flag, but that expansion
+ // accepted way to do this, but implicit requirements also do this. We rely on the
+ // OptionProcessor compile-time check's guarantee that no option sets
+ // both expansion behaviors. (In Bazel, --config is another such flag, but that expansion
// is not controlled within the options parser, so we ignore it here)
// As much as possible, we want the behaviors of these different types of flags to be
// identical, as this minimizes the number of edge cases, but we do not yet track these values
- // in the same way. Wrapper options are replaced by their value and implicit requirements are
- // hidden from the reported lists of parsed options.
- if (parsedOption.getImplicitDependent() == null && !optionDefinition.isWrapperOption()) {
+ // in the same way.
+ if (parsedOption.getImplicitDependent() == null) {
// Log explicit options and expanded options in the order they are parsed (can be sorted
// later). This information is needed to correctly canonicalize flags.
parsedOptions.add(parsedOption);
@@ -416,13 +415,7 @@
optionDefinition.isExpansionOption() ? optionDefinition : null,
expansionBundle.expansionArgs);
if (!residueAndPriority.residue.isEmpty()) {
- if (optionDefinition.isWrapperOption()) {
- throw new OptionsParsingException(
- "Unparsed options remain after unwrapping "
- + unconvertedValue
- + ": "
- + Joiner.on(' ').join(residueAndPriority.residue));
- } else {
+
// Throw an assertion here, because this indicates an error in the definition of this
// option's expansion or requirements, not with the input as provided by the user.
throw new AssertionError(
@@ -430,7 +423,7 @@
+ unconvertedValue
+ ": "
+ Joiner.on(' ').join(residueAndPriority.residue));
- }
+
}
}
}
@@ -506,9 +499,8 @@
// Special-case boolean to supply value based on presence of "no" prefix.
if (optionDefinition.usesBooleanValueSyntax()) {
unconvertedValue = booleanValue ? "1" : "0";
- } else if (optionDefinition.getType().equals(Void.class)
- && !optionDefinition.isWrapperOption()) {
- // This is expected, Void type options have no args (unless they're wrapper options).
+ } else if (optionDefinition.getType().equals(Void.class)) {
+ // This is expected, Void type options have no args.
} else if (nextArgs.hasNext()) {
// "--flag value" form
unconvertedValue = nextArgs.next();
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 fd7c023..76b9640 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
@@ -476,10 +476,6 @@
}
if (isExpansion || hasImplicitRequirements) {
- if (annotation.wrapperOption()) {
- throw new OptionProcessorException(
- optionField, "Wrapper options cannot have expansions or implicit requirements.");
- }
if (annotation.allowMultiple()) {
throw new OptionProcessorException(
optionField,
@@ -488,30 +484,6 @@
}
}
- /**
- * Some flags wrap other flags. They are objectively useless, as there is no difference between
- * passing --wrapper=--foo and --foo other than the "source" information tracked. This
- * functionality comes from requiring compatibility at some past point in time, but is actively
- * being deprecated. No non-deprecated flag can use this feature.
- */
- private void checkWrapperOptions(VariableElement optionField) throws OptionProcessorException {
- Option annotation = optionField.getAnnotation(Option.class);
- if (annotation.wrapperOption()) {
- if (annotation.deprecationWarning().isEmpty()) {
- throw new OptionProcessorException(
- optionField,
- "Can't have non deprecated wrapper options, this feature is deprecated. "
- + "Please add a deprecationWarning.");
- }
- if (!ImmutableList.copyOf(annotation.metadataTags()).contains(OptionMetadataTag.DEPRECATED)) {
- throw new OptionProcessorException(
- optionField,
- "Can't have non deprecated wrapper options, this feature is deprecated. "
- + "Please add the metadata tag DEPRECATED.");
- }
- }
- }
-
@Override
public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
for (Element annotatedElement : roundEnv.getElementsAnnotatedWith(Option.class)) {
@@ -528,7 +500,6 @@
checkConverter(optionField);
checkEffectTagRationality(optionField);
checkMetadataTagAndCategoryRationality(optionField);
- checkWrapperOptions(optionField);
} catch (OptionProcessorException e) {
error(e.getElementInError(), e.getMessage());
}
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 1c34878..60baf97 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
@@ -438,25 +438,4 @@
public void badOldName() {
assertBadness(BadOldNameOptions.class, "must not use the oldName field");
}
-
- /** Dummy comment (linter suppression) */
- public static class BadWrapperOptionOptions extends OptionsBase {
- @Option(
- name = "incompatible_bad",
- category = "incompatible changes",
- metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE, OptionMetadataTag.DEPRECATED},
- documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "false",
- help = "nohelp",
- deprecationWarning = "wrapper options are deprecated, including this one.",
- wrapperOption = true
- )
- public boolean bad;
- }
-
- @Test
- public void badWrapperOption() {
- assertBadness(BadWrapperOptionOptions.class, "must not use the wrapperOption field");
- }
}
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 8368e78..581b098 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -1873,72 +1873,6 @@
}
}
- public static class WrapperOptionExample extends OptionsBase {
- @Option(
- name = "wrapper",
- documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
- metadataTags = {OptionMetadataTag.DEPRECATED},
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "null",
- deprecationWarning = "wrapper options are deprecated, including this one.",
- wrapperOption = true
- )
- public Void wrapperOption;
-
- @Option(
- name = "flag1",
- documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "false"
- )
- public boolean flag1;
-
- @Option(
- name = "flag2",
- documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "42"
- )
- public int flag2;
-
- @Option(
- name = "flag3",
- documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
- effectTags = {OptionEffectTag.NO_OP},
- defaultValue = "foo"
- )
- public String flag3;
- }
-
- @Test
- public void testWrapperOption() throws OptionsParsingException {
- OptionsParser parser = newOptionsParser(WrapperOptionExample.class);
- parser.parse("--wrapper=--flag1=true", "--wrapper=--flag2=87", "--wrapper=--flag3=bar");
- WrapperOptionExample result = parser.getOptions(WrapperOptionExample.class);
- assertThat(result.flag1).isTrue();
- assertThat(result.flag2).isEqualTo(87);
- assertThat(result.flag3).isEqualTo("bar");
- }
-
- @Test
- public void testInvalidWrapperOptionFormat() {
- OptionsParser parser = newOptionsParser(WrapperOptionExample.class);
- try {
- parser.parse("--wrapper=foo");
- fail();
- } catch (OptionsParsingException e) {
- // Check that the message looks like it's suggesting the correct format.
- assertThat(e).hasMessageThat().contains("--foo");
- }
- }
-
- @Test
- public void testWrapperCanonicalization() throws OptionsParsingException {
- List<String> canonicalized = canonicalize(WrapperOptionExample.class,
- "--wrapper=--flag1=true", "--wrapper=--flag2=87", "--wrapper=--flag3=bar");
- assertThat(canonicalized).isEqualTo(Arrays.asList("--flag1=1", "--flag2=87", "--flag3=bar"));
- }
-
/** Dummy options that declares it uses only core types. */
@UsesOnlyCoreTypes
public static class CoreTypesOptions extends OptionsBase implements Serializable {
