Don't hard remove --no_, give a warning first.
PiperOrigin-RevId: 153610163
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 9c4c3a6..1352a33 100644
--- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -341,9 +341,11 @@
Map<String, String> booleanAliasMap,
String optionName) {
// Check that the negating alias does not conflict with existing flags.
+ checkForCollisions(nameToFieldMap, "no_" + optionName, "boolean option alias");
checkForCollisions(nameToFieldMap, "no" + optionName, "boolean option alias");
// Record that the boolean option takes up additional namespace for its negating alias.
+ booleanAliasMap.put("no_" + optionName, optionName);
booleanAliasMap.put("no" + optionName, optionName);
}
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 c5f31c4..a4d905a 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -620,18 +620,14 @@
// Look for a "no"-prefixed option name: "no<optionName>".
if (field == null && name.startsWith("no")) {
// Give a nice error if someone is using the deprecated --no_ prefix.
- // Note: With this check in place, is impossible to specify "--no_foo" for a flag named
- // "--_foo", if a --foo flag also exists, since that'll be interpreted as the "no_"
- // negating prefix for "--foo". Let that be a warning to anyone wanting to make flags that
- // start with underscores.
// TODO(Bazel-team): Remove the --no_ check when sufficient time has passed for users of
// that feature to have stopped using it.
- if (name.startsWith("no_") && optionsData.getFieldFromName(name.substring(3)) != null) {
- throw new OptionsParsingException(
- "'no_' prefixes are no longer accepted, --no<flag> is an accepted alternative.",
- name.substring(3));
- }
name = name.substring(2);
+ if (name.startsWith("_") && optionsData.getFieldFromName(name.substring(1)) != null) {
+ name = name.substring(1);
+ warnings.add("Option '" + name + "' is specified using the deprecated --no_ prefix. "
+ + "Use --no without the underscore instead.");
+ }
field = optionsData.getFieldFromName(name);
booleanValue = false;
if (field != null) {
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 3d85315..5b79dab 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -1583,16 +1583,16 @@
}
@Test
- public void testBooleanUnderscorePrefixError() {
- try {
- OptionsParser parser = newOptionsParser(ExampleBooleanFooOptions.class);
- parser.parse("--no_foo");
+ public void testBooleanUnderscorePrefixError() throws OptionsParsingException {
+ OptionsParser parser = newOptionsParser(ExampleBooleanFooOptions.class);
+ parser.parse("--no_foo");
+ ExampleBooleanFooOptions result = parser.getOptions(ExampleBooleanFooOptions.class);
+ assertThat(result.foo).isFalse();
+ List<String> warning = parser.getWarnings();
+ assertThat(warning).hasSize(1);
+ assertThat(warning.get(0)).contains("Option 'foo' is specified using the deprecated "
+ + "--no_ prefix. Use --no without the underscore instead");
- fail("--no_foo should fail to parse and provide a nice error message.");
- } catch (OptionsParsingException e) {
- assertThat(e.getMessage()).contains(
- "'no_' prefixes are no longer accepted, --no<flag> is an accepted alternative.");
- }
}
public static class WrapperOptionExample extends OptionsBase {
