Fix expansion option output formatting in html output

Like the terminal output, there was some discrepancy between the expansion function and static expansion behavior. Also adds html usage output tests.

PiperOrigin-RevId: 165584454

2 files changed, 255 insertions, 73 deletions

diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
index 61829e2d99..8e36c4a84d 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -201,22 +201,29 @@ class OptionsUsage {
       usage.append(paragraphFill(escaper.escape(annotation.help()), /*indent=*/ 0, /*width=*/ 80));
       usage.append('\n');
     }
-    ImmutableList<String> expansion = getExpansionIfKnown(optionField, optionsData);
-    if (expansion == null) {
-      usage.append("    Expands to unknown options.<br>\n");
-    } else if (!expansion.isEmpty()) {
+
+    if (!optionsData.getExpansionDataForField(optionField).isEmpty()) {
+      // If this is an expansion option, list the expansion if known, or at least specify that we
+      // don't know.
       usage.append("<br/>\n");
-      StringBuilder expandsMsg = new StringBuilder("Expands to:<br/>\n");
-      for (String exp : expansion) {
-        // TODO(ulfjack): Can we link to the expanded flags here?
-        expandsMsg
-            .append("&nbsp;&nbsp;<code>")
-            .append(escaper.escape(exp))
-            .append("</code><br/>\n");
+      ImmutableList<String> expansion = getExpansionIfKnown(optionField, optionsData);
+      StringBuilder expandsMsg;
+      if (expansion == null) {
+        expandsMsg = new StringBuilder("Expands to unknown options.<br/>\n");
+      } else {
+        Preconditions.checkArgument(!expansion.isEmpty());
+        expandsMsg = new StringBuilder("Expands to:<br/>\n");
+        for (String exp : expansion) {
+          // TODO(ulfjack): Can we link to the expanded flags here?
+          expandsMsg
+              .append("&nbsp;&nbsp;<code>")
+              .append(escaper.escape(exp))
+              .append("</code><br/>\n");
+        }
       }
       usage.append(expandsMsg.toString());
-      usage.append('\n');
     }
+
     usage.append("</dd>\n");
   }
 
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 c720163e21..c81af7d2dc 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsUsageTest.java
@@ -15,6 +15,8 @@ package com.google.devtools.common.options;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.escape.Escaper;
+import com.google.common.html.HtmlEscapers;
 import com.google.devtools.common.options.OptionsParser.HelpVerbosity;
 import org.junit.Before;
 import org.junit.Test;
@@ -26,91 +28,160 @@ import org.junit.runners.JUnit4;
 public final class OptionsUsageTest {
 
   private OptionsData data;
+  private static final Escaper HTML_ESCAPER = HtmlEscapers.htmlEscaper();
 
   @Before
   public void setUp() {
     data = OptionsParser.getOptionsDataInternal(TestOptions.class);
   }
 
-  private String getUsage(String fieldName, HelpVerbosity verbosity) {
+  private String getHtmlUsage(String fieldName) {
+    StringBuilder builder = new StringBuilder();
+    OptionsUsage.getUsageHtml(data.getFieldFromName(fieldName), builder, HTML_ESCAPER, data);
+    return builder.toString();
+  }
+
+  private String getTerminalUsage(String fieldName, HelpVerbosity verbosity) {
     StringBuilder builder = new StringBuilder();
     OptionsUsage.getUsage(data.getFieldFromName(fieldName), builder, verbosity, data);
     return builder.toString();
   }
 
   @Test
-  public void stringValue_short() {
-    assertThat(getUsage("test_string", HelpVerbosity.SHORT)).isEqualTo("  --test_string\n");
+  public void stringValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_string", HelpVerbosity.SHORT)).isEqualTo("  --test_string\n");
   }
 
   @Test
-  public void stringValue_medium() {
-    assertThat(getUsage("test_string", HelpVerbosity.MEDIUM))
+  public void stringValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_string", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_string (a string; default: \"test string default\")\n");
   }
 
   @Test
-  public void stringValue_long() {
-    assertThat(getUsage("test_string", HelpVerbosity.LONG))
+  public void stringValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_string", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_string (a string; default: \"test string default\")\n"
                 + "    a string-valued option to test simple option operations\n");
   }
 
   @Test
-  public void intValue_short() {
-    assertThat(getUsage("expanded_c", HelpVerbosity.SHORT)).isEqualTo("  --expanded_c\n");
+  public void stringValue_htmlOutput() {
+    assertThat(getHtmlUsage("test_string"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_string\"></a>"
+                + "--test_string=&lt;a string&gt</code> default: \"test string default\"</dt>\n"
+                + "<dd>\n"
+                + "a string-valued option to test simple option operations\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void intValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_c", HelpVerbosity.SHORT)).isEqualTo("  --expanded_c\n");
   }
 
   @Test
-  public void intValue_medium() {
-    assertThat(getUsage("expanded_c", HelpVerbosity.MEDIUM))
+  public void intValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_c", HelpVerbosity.MEDIUM))
         .isEqualTo("  --expanded_c (an integer; default: \"12\")\n");
   }
 
   @Test
-  public void intValue_long() {
-    assertThat(getUsage("expanded_c", HelpVerbosity.LONG))
+  public void intValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_c", HelpVerbosity.LONG))
         .isEqualTo(
             "  --expanded_c (an integer; default: \"12\")\n"
                 + "    an int-value'd flag used to test expansion logic\n");
   }
 
   @Test
-  public void multipleValue_short() {
-    assertThat(getUsage("test_multiple_string", HelpVerbosity.SHORT))
+  public void intValue_htmlOutput() {
+    assertThat(getHtmlUsage("expanded_c"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--expanded_c\"></a>"
+                + "--expanded_c=&lt;an integer&gt</code> default: \"12\"</dt>\n"
+                + "<dd>\n"
+                + "an int-value&#39;d flag used to test expansion logic\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void booleanValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_a", HelpVerbosity.SHORT))
+        .isEqualTo("  --[no]expanded_a\n");
+  }
+
+  @Test
+  public void booleanValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_a", HelpVerbosity.MEDIUM))
+        .isEqualTo("  --[no]expanded_a (a boolean; default: \"true\")\n");
+  }
+
+  @Test
+  public void booleanValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("expanded_a", HelpVerbosity.LONG))
+        .isEqualTo("  --[no]expanded_a (a boolean; default: \"true\")\n");
+  }
+
+  @Test
+  public void booleanValue_htmlOutput() {
+    assertThat(getHtmlUsage("expanded_a"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--expanded_a\"></a>"
+                + "--[no]expanded_a</code> default: \"true\"</dt>\n"
+                + "<dd>\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void multipleValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_multiple_string", HelpVerbosity.SHORT))
         .isEqualTo("  --test_multiple_string\n");
   }
 
   @Test
-  public void multipleValue_medium() {
-    assertThat(getUsage("test_multiple_string", HelpVerbosity.MEDIUM))
+  public void multipleValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_multiple_string", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_multiple_string (a string; may be used multiple times)\n");
   }
 
   @Test
-  public void multipleValue_long() {
-    assertThat(getUsage("test_multiple_string", HelpVerbosity.LONG))
+  public void multipleValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_multiple_string", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_multiple_string (a string; may be used multiple times)\n"
                 + "    a repeatable string-valued flag with its own unhelpful help text\n");
   }
 
   @Test
-  public void customConverterValue_short() {
-    assertThat(getUsage("test_list_converters", HelpVerbosity.SHORT))
+  public void multipleValue_htmlOutput() {
+    assertThat(getHtmlUsage("test_multiple_string"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_multiple_string\"></a>"
+                + "--test_multiple_string=&lt;a string&gt</code> "
+                + "multiple uses are accumulated</dt>\n"
+                + "<dd>\n"
+                + "a repeatable string-valued flag with its own unhelpful help text\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void customConverterValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_list_converters", HelpVerbosity.SHORT))
         .isEqualTo("  --test_list_converters\n");
   }
 
   @Test
-  public void customConverterValue_medium() {
-    assertThat(getUsage("test_list_converters", HelpVerbosity.MEDIUM))
+  public void customConverterValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_list_converters", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_list_converters (a list of strings; may be used multiple times)\n");
   }
 
   @Test
-  public void customConverterValue_long() {
-    assertThat(getUsage("test_list_converters", HelpVerbosity.LONG))
+  public void customConverterValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_list_converters", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_list_converters (a list of strings; may be used multiple times)\n"
                 + "    a repeatable flag that accepts lists, but doesn't want to have lists of \n"
@@ -118,18 +189,33 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void staticExpansionOption_short() {
-    assertThat(getUsage("test_expansion", HelpVerbosity.SHORT)).isEqualTo("  --test_expansion\n");
+  public void customConverterValue_htmlOutput() {
+    assertThat(getHtmlUsage("test_list_converters"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_list_converters\"></a>"
+                + "--test_list_converters=&lt;a list of strings&gt</code> "
+                + "multiple uses are accumulated</dt>\n"
+                + "<dd>\n"
+                + "a repeatable flag that accepts lists, but doesn&#39;t want to have lists of \n"
+                + "lists as a final type\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void staticExpansionOption_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion", HelpVerbosity.SHORT))
+        .isEqualTo("  --test_expansion\n");
   }
 
   @Test
-  public void staticExpansionOption_medium() {
-    assertThat(getUsage("test_expansion", HelpVerbosity.MEDIUM)).isEqualTo("  --test_expansion\n");
+  public void staticExpansionOption_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion", HelpVerbosity.MEDIUM))
+        .isEqualTo("  --test_expansion\n");
   }
 
   @Test
-  public void staticExpansionOption_long() {
-    assertThat(getUsage("test_expansion", HelpVerbosity.LONG))
+  public void staticExpansionOption_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_expansion\n"
                 + "    this expands to an alphabet soup.\n"
@@ -138,20 +224,39 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void recursiveExpansionOption_short() {
-    assertThat(getUsage("test_recursive_expansion_top_level", HelpVerbosity.SHORT))
+  public void staticExpansionOption_htmlOutput() {
+    assertThat(getHtmlUsage("test_expansion"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_expansion\"></a>"
+                + "--test_expansion</code></dt>\n"
+                + "<dd>\n"
+                + "this expands to an alphabet soup.\n"
+                + "<br/>\n"
+                + "Expands to:<br/>\n"
+                + "&nbsp;&nbsp;<code>--noexpanded_a</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--expanded_b=false</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--expanded_c</code><br/>\n"
+                + "&nbsp;&nbsp;<code>42</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--expanded_d</code><br/>\n"
+                + "&nbsp;&nbsp;<code>bar</code><br/>\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void recursiveExpansionOption_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_recursive_expansion_top_level", HelpVerbosity.SHORT))
         .isEqualTo("  --test_recursive_expansion_top_level\n");
   }
 
   @Test
-  public void recursiveExpansionOption_medium() {
-    assertThat(getUsage("test_recursive_expansion_top_level", HelpVerbosity.MEDIUM))
+  public void recursiveExpansionOption_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_recursive_expansion_top_level", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_recursive_expansion_top_level\n");
   }
 
   @Test
-  public void recursiveExpansionOption_long() {
-    assertThat(getUsage("test_recursive_expansion_top_level", HelpVerbosity.LONG))
+  public void recursiveExpansionOption_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_recursive_expansion_top_level", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_recursive_expansion_top_level\n"
                 + "    Lets the children do all the work.\n"
@@ -160,20 +265,35 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void expansionToMultipleValue_short() {
-    assertThat(getUsage("test_expansion_to_repeatable", HelpVerbosity.SHORT))
+  public void recursiveExpansionOption_htmlOutput() {
+    assertThat(getHtmlUsage("test_recursive_expansion_top_level"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_recursive_expansion_top_level\"></a>"
+                + "--test_recursive_expansion_top_level</code></dt>\n"
+                + "<dd>\n"
+                + "Lets the children do all the work.\n"
+                + "<br/>\n"
+                + "Expands to:<br/>\n"
+                + "&nbsp;&nbsp;<code>--test_recursive_expansion_middle1</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--test_recursive_expansion_middle2</code><br/>\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void expansionToMultipleValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_to_repeatable", HelpVerbosity.SHORT))
         .isEqualTo("  --test_expansion_to_repeatable\n");
   }
 
   @Test
-  public void expansionToMultipleValue_medium() {
-    assertThat(getUsage("test_expansion_to_repeatable", HelpVerbosity.MEDIUM))
+  public void expansionToMultipleValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_to_repeatable", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_expansion_to_repeatable\n");
   }
 
   @Test
-  public void expansionToMultipleValue_long() {
-    assertThat(getUsage("test_expansion_to_repeatable", HelpVerbosity.LONG))
+  public void expansionToMultipleValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_to_repeatable", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_expansion_to_repeatable\n"
                 + "    Go forth and multiply, they said.\n"
@@ -182,20 +302,35 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void implicitRequirementOption_short() {
-    assertThat(getUsage("test_implicit_requirement", HelpVerbosity.SHORT))
+  public void expansionToMultipleValue_htmlOutput() {
+    assertThat(getHtmlUsage("test_expansion_to_repeatable"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_expansion_to_repeatable\"></a>"
+                + "--test_expansion_to_repeatable</code></dt>\n"
+                + "<dd>\n"
+                + "Go forth and multiply, they said.\n"
+                + "<br/>\n"
+                + "Expands to:<br/>\n"
+                + "&nbsp;&nbsp;<code>--test_multiple_string=expandedFirstValue</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--test_multiple_string=expandedSecondValue</code><br/>\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void implicitRequirementOption_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_implicit_requirement", HelpVerbosity.SHORT))
         .isEqualTo("  --test_implicit_requirement\n");
   }
 
   @Test
-  public void implicitRequirementOption_medium() {
-    assertThat(getUsage("test_implicit_requirement", HelpVerbosity.MEDIUM))
+  public void implicitRequirementOption_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_implicit_requirement", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_implicit_requirement (a string; default: \"direct implicit\")\n");
   }
 
   @Test
-  public void implicitRequirementOption_long() {
-    assertThat(getUsage("test_implicit_requirement", HelpVerbosity.LONG))
+  public void implicitRequirementOption_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_implicit_requirement", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_implicit_requirement (a string; default: \"direct implicit\")\n"
                 + "    this option really needs that other one, isolation of purpose has failed.\n"
@@ -204,20 +339,32 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void expansionFunctionOptionThatReadsUserValue_short() {
-    assertThat(getUsage("test_expansion_function", HelpVerbosity.SHORT))
+  public void implicitRequirementOption_htmlOutput() {
+    assertThat(getHtmlUsage("test_implicit_requirement"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_implicit_requirement\"></a>"
+                + "--test_implicit_requirement=&lt;a string&gt</code> "
+                + "default: \"direct implicit\"</dt>\n"
+                + "<dd>\n"
+                + "this option really needs that other one, isolation of purpose has failed.\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void expansionFunctionOptionThatReadsUserValue_shortTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_function", HelpVerbosity.SHORT))
         .isEqualTo("  --test_expansion_function\n");
   }
 
   @Test
-  public void expansionFunctionOptionThatReadsUserValue_medium() {
-    assertThat(getUsage("test_expansion_function", HelpVerbosity.MEDIUM))
+  public void expansionFunctionOptionThatReadsUserValue_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_function", HelpVerbosity.MEDIUM))
         .isEqualTo("  --test_expansion_function\n");
   }
 
   @Test
-  public void expansionFunctionOptionThatReadsUserValue_long() {
-    assertThat(getUsage("test_expansion_function", HelpVerbosity.LONG))
+  public void expansionFunctionOptionThatReadsUserValue_longTerminalOutput() {
+    assertThat(getTerminalUsage("test_expansion_function", HelpVerbosity.LONG))
         .isEqualTo(
             "  --test_expansion_function\n"
                 + "    this is for testing expansion-by-function functionality.\n"
@@ -225,23 +372,51 @@ public final class OptionsUsageTest {
   }
 
   @Test
-  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_short() {
-    assertThat(getUsage("prefix_expansion", HelpVerbosity.SHORT))
+  public void expansionFunctionOptionThatReadsUserValue_htmlOutput() {
+    assertThat(getHtmlUsage("test_expansion_function"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--test_expansion_function\"></a>"
+                + "--test_expansion_function</code></dt>\n"
+                + "<dd>\n"
+                + "this is for testing expansion-by-function functionality.\n"
+                + "<br/>\n"
+                + "Expands to unknown options.<br/>\n"
+                + "</dd>\n");
+  }
+
+  @Test
+  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_shortTerminalOutput() {
+    assertThat(getTerminalUsage("prefix_expansion", HelpVerbosity.SHORT))
         .isEqualTo("  --prefix_expansion\n");
   }
 
   @Test
-  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_medium() {
-    assertThat(getUsage("prefix_expansion", HelpVerbosity.MEDIUM))
+  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_mediumTerminalOutput() {
+    assertThat(getTerminalUsage("prefix_expansion", HelpVerbosity.MEDIUM))
         .isEqualTo("  --prefix_expansion\n");
   }
 
   @Test
-  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_long() {
-    assertThat(getUsage("prefix_expansion", HelpVerbosity.LONG))
+  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_longTerminalOutput() {
+    assertThat(getTerminalUsage("prefix_expansion", HelpVerbosity.LONG))
         .isEqualTo(
             "  --prefix_expansion\n"
                 + "    Expands to all options with a specific prefix.\n"
                 + "      Expands to: --specialexp_bar --specialexp_foo \n");
   }
+
+  @Test
+  public void expansionFunctionOptionThatExpandsBasedOnOtherLoadedOptions_htmlOutput() {
+    assertThat(getHtmlUsage("prefix_expansion"))
+        .isEqualTo(
+            "<dt><code><a name=\"flag--prefix_expansion\"></a>"
+                + "--prefix_expansion</code></dt>\n"
+                + "<dd>\n"
+                + "Expands to all options with a specific prefix.\n"
+                + "<br/>\n"
+                + "Expands to:<br/>\n"
+                + "&nbsp;&nbsp;<code>--specialexp_bar</code><br/>\n"
+                + "&nbsp;&nbsp;<code>--specialexp_foo</code><br/>\n"
+                + "</dd>\n");
+  }
 }