Provide "did you mean ...?" suggestions for unknown command-line options

Examples: ``` $ bazel build //src:bazel-dev --experimental_remote_cache_evection_retries=5 ERROR: --experimental_remote_cache_evection_retries=5 :: Unrecognized option: --experimental_remote_cache_evection_retries=5 (did you mean '--experimental_remote_cache_eviction_retries'?) $ bazel build //src:bazel-dev --notest_kep_going ERROR: --notest_kep_going :: Unrecognized option: --notest_kep_going (did you mean '--notest_keep_going'?) Closes #22193. PiperOrigin-RevId: 636258381 Change-Id: Ie81344caa14054e1d328d5a6e9b94da506d6a577
bazelbuild · May 22, 2024 · 0e9287f · 0e9287f
1 parent cdace8f
commit 0e9287f
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 5 deletions.
diff --git a/src/main/java/com/google/devtools/common/options/BUILD b/src/main/java/com/google/devtools/common/options/BUILD
@@ -51,6 +51,7 @@ java_11_library(
     ),
     deps = [
         "//src/main/java/com/google/devtools/build/lib/util:pair",
+        "//src/main/java/net/starlark/java/spelling",
         "//third_party:auto_value",
         "//third_party:caffeine",
         "//third_party:guava",

diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.common.options.OptionsParser.ConstructionException;
 import java.lang.reflect.Constructor;
 import java.util.Arrays;
@@ -160,7 +161,7 @@ public OptionDefinition getOptionDefinitionFromName(String name) {
    * appear ordered first by their options class (the order in which they were passed to {@link
    * #from(Collection, boolean)}, and then in alphabetic order within each options class.
    */
-  public Iterable<Map.Entry<String, OptionDefinition>> getAllOptionDefinitions() {
+  public ImmutableSet<Map.Entry<String, OptionDefinition>> getAllOptionDefinitions() {
     return nameToField.entrySet();
   }
 

diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -44,7 +44,9 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import net.starlark.java.spelling.SpellChecker;
 
 /**
  * The implementation of the options parser. This is intentionally package private for full
@@ -791,9 +793,16 @@ private ParsedOptionDescriptionOrIgnoredArgs identifyOptionAndPossibleArgument(
       throw new OptionsParsingException("Invalid options syntax: " + arg, arg);
     }
 
+    // Do not recognize internal options, which are treated as if they did not exist.
     if (lookupResult == null || shouldIgnoreOption(lookupResult.definition)) {
-      // Do not recognize internal options, which are treated as if they did not exist.
-      throw new OptionsParsingException("Unrecognized option: " + arg, arg);
+      String suggestion;
+      // Do not offer suggestions for short-form options.
+      if (arg.startsWith("--")) {
+        suggestion = SpellChecker.didYouMean(arg, getAllValidArgs());
+      } else {
+        suggestion = "";
+      }
+      throw new OptionsParsingException("Unrecognized option: " + arg + suggestion, arg);
     }
 
     if (unconvertedValue == null) {
@@ -835,6 +844,22 @@ private ParsedOptionDescriptionOrIgnoredArgs identifyOptionAndPossibleArgument(
         Optional.empty());
   }
 
+  private Iterable<String> getAllValidArgs() {
+    return () ->
+        optionsData.getAllOptionDefinitions().stream()
+            .filter(entry -> !shouldIgnoreOption(entry.getValue()))
+            .flatMap(
+                definition -> {
+                  Stream.Builder<String> builder = Stream.builder();
+                  builder.add("--" + definition.getKey());
+                  if (definition.getValue().usesBooleanValueSyntax()) {
+                    builder.add("--no" + definition.getKey());
+                  }
+                  return builder.build();
+                })
+            .iterator();
+  }
+
   /**
    * Two option definitions are considered equivalent for parsing if they result in the same control
    * flow through {@link #identifyOptionAndPossibleArgument}. This is crucial to ensure that the

diff --git a/src/test/java/com/google/devtools/common/options/OptionsTest.java b/src/test/java/com/google/devtools/common/options/OptionsTest.java
@@ -435,12 +435,36 @@ public void usageWithCustomConverter() {
   }
 
   @Test
-  public void unknownBooleanOption() {
+  public void unknownBooleanOptionNegativeForm() {
     OptionsParsingException e =
         assertThrows(
             OptionsParsingException.class,
             () -> Options.parse(HttpOptions.class, new String[] {"--no-debug"}));
-    assertThat(e).hasMessageThat().isEqualTo("Unrecognized option: --no-debug");
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo("Unrecognized option: --no-debug (did you mean '--nodebug'?)");
+  }
+
+  @Test
+  public void unknownOption() {
+    OptionsParsingException e =
+        assertThrows(
+            OptionsParsingException.class,
+            () -> Options.parse(HttpOptions.class, new String[] {"--pert"}));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo("Unrecognized option: --pert (did you mean '--port'?)");
+  }
+
+  @Test
+  public void unknownBooleanOptionPositiveForm() {
+    OptionsParsingException e =
+        assertThrows(
+            OptionsParsingException.class,
+            () -> Options.parse(HttpOptions.class, new String[] {"--dbg"}));
+    assertThat(e)
+        .hasMessageThat()
+        .isEqualTo("Unrecognized option: --dbg (did you mean '--debug'?)");
   }
 
   public static class J extends OptionsBase {