Move final static option checks to compile time.

Check that the option has a non-empty name and that it does not use deprecated categories. While we're at it, check that the names for options that are flags (all but INTERNAL flags, which are not meant to be used on the command line) are sensible.

PiperOrigin-RevId: 167182172

3 files changed, 13 insertions, 57 deletions

diff --git a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
index 2f442fd924..648b271a0f 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
@@ -19,7 +19,6 @@ import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.truth.Correspondence;
-import com.google.devtools.common.options.OptionsParser.ConstructionException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -251,32 +250,6 @@ public class OptionsDataTest {
     }
   }
 
-  /** Dummy options class using deprecated category. */
-  public static class InvalidUndocumentedCategory extends OptionsBase {
-    @Option(
-      name = "experimental_foo",
-      category = "undocumented",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.NO_OP},
-      defaultValue = "true"
-    )
-    public boolean experimentalFoo;
-  }
-
-  @Test
-  public void invalidUndocumentedCategoryFails() {
-    try {
-      construct(InvalidUndocumentedCategory.class);
-      fail();
-    } catch (ConstructionException e) {
-      // Expected exception
-      assertThat(e).hasMessageThat().contains(
-          "Documentation level is no longer read from the option category.");
-      assertThat(e).hasMessageThat().contains("undocumented");
-      assertThat(e).hasMessageThat().contains("experimental_foo");
-    }
-  }
-
   /**
    * Dummy options class.
    *
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 8b8754bf77..bf8387ffc7 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -193,14 +193,6 @@ public class OptionsParserTest {
       defaultValue = "super secret"
     )
     public String privateString;
-
-    @Option(
-      name = "public string",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {OptionEffectTag.NO_OP},
-      defaultValue = "not a secret"
-    )
-    public String publicString;
   }
 
   public static class StringConverter implements Converter<String> {
@@ -631,15 +623,6 @@ public class OptionsParserTest {
   }
 
   @Test
-  public void parsingSucceedsWithSpacesInFlagName() throws OptionsParsingException {
-    OptionsParser parser = newOptionsParser(ExampleInternalOptions.class);
-    List<String> spacedOpts = asList("--public string=value with spaces");
-    parser.parse(spacedOpts);
-    assertThat(parser.getOptions(ExampleInternalOptions.class).publicString)
-        .isEqualTo("value with spaces");
-  }
-
-  @Test
   public void parsingFailsForInternalOptionWithValueInSameArgAsIfUnknown() {
     OptionsParser parser = newOptionsParser(ExampleInternalOptions.class);
     List<String> internalOpts = asList("--internal_string=any_value");
diff --git a/src/test/java/com/google/devtools/common/options/testing/OptionsTesterTest.java b/src/test/java/com/google/devtools/common/options/testing/OptionsTesterTest.java
index a791d149f3..626d08786d 100644
--- a/src/test/java/com/google/devtools/common/options/testing/OptionsTesterTest.java
+++ b/src/test/java/com/google/devtools/common/options/testing/OptionsTesterTest.java
@@ -40,7 +40,7 @@ public final class OptionsTesterTest {
   /** Test options class for optionAnnotationCheck_PassesWhenAllFieldsAnnotated. */
   public static class BaseAllFieldsAnnotated extends OptionsBase {
     @Option(
-      name = "public inherited field with annotation",
+      name = "public_inherited_field_with_annotation",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       defaultValue = "defaultFoo"
@@ -51,7 +51,7 @@ public final class OptionsTesterTest {
   /** Test options class for optionAnnotationCheck_PassesWhenAllFieldsAnnotated. */
   public static final class OptionAnnotationCheckAllFieldsAnnotated extends BaseAllFieldsAnnotated {
     @Option(
-        name = "public declared field with annotation",
+        name = "public_declared_field_with_annotation",
         documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
         effectTags = {OptionEffectTag.NO_OP},
         defaultValue = "defaultFoo"
@@ -59,7 +59,7 @@ public final class OptionsTesterTest {
     public String publicField;
 
     @Option(
-        name = "other public declared field with annotation",
+        name = "other_public_declared_field_with_annotation",
         documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
         effectTags = {OptionEffectTag.NO_OP},
         defaultValue = "defaultFoo"
@@ -139,7 +139,7 @@ public final class OptionsTesterTest {
   /** Test options class for optionAccessCheck_PassesWhenAllFieldsPublicNotStaticNotFinal. */
   public static class BaseAllFieldsPublicNotStaticNotFinal extends OptionsBase {
     @Option(
-      name = "public inherited field with annotation",
+      name = "public_inherited_field_with_annotation",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       defaultValue = "defaultFoo"
@@ -151,7 +151,7 @@ public final class OptionsTesterTest {
   public static final class OptionAccessCheckAllFieldsPublicNotStaticNotFinal
       extends BaseAllFieldsPublicNotStaticNotFinal {
     @Option(
-      name = "public declared field with annotation",
+      name = "public_declared_field_with_annotation",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       defaultValue = "defaultFoo"
@@ -201,7 +201,7 @@ public final class OptionsTesterTest {
   /** Test options class for defaultTestCheck_PassesIfAllDefaultsTested. */
   public static final class DefaultTestCheck extends OptionsBase {
     @Option(
-      name = "tested field",
+      name = "tested_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -210,7 +210,7 @@ public final class OptionsTesterTest {
     public String testedField;
 
     @Option(
-      name = "field implicitly using default converter",
+      name = "field_implicitly_using_default_converter",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       defaultValue = "implicitConverterDefault"
@@ -218,7 +218,7 @@ public final class OptionsTesterTest {
     public String implicitConverterField;
 
     @Option(
-      name = "other tested field",
+      name = "other_tested_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -227,7 +227,7 @@ public final class OptionsTesterTest {
     public String otherTestedField;
 
     @Option(
-      name = "field with null default",
+      name = "field_with_null_default",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -236,7 +236,7 @@ public final class OptionsTesterTest {
     public String nullDefaultField;
 
     @Option(
-      name = "allowMultiple field",
+      name = "allowMultiple_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -280,7 +280,7 @@ public final class OptionsTesterTest {
    */
   public static final class DefaultTestCheckUntestedField extends OptionsBase {
     @Option(
-      name = "untested field",
+      name = "untested_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -304,7 +304,7 @@ public final class OptionsTesterTest {
   /** Test options class for defaultTestCheck_FailsIfTesterIsAbsentEvenForNullDefault. */
   public static final class DefaultTestCheckUntestedNullField extends OptionsBase {
     @Option(
-      name = "untested field",
+      name = "untested_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,
@@ -328,7 +328,7 @@ public final class OptionsTesterTest {
   /** Test options class for defaultTestCheck_FailsIfTesterIsAbsentEvenForAllowMultiple. */
   public static final class DefaultTestCheckUntestedMultipleField extends OptionsBase {
     @Option(
-      name = "untested field",
+      name = "untested_field",
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.NO_OP},
       converter = TestConverter.class,