From 0e9287f41ceeea6c980045cbbc126a885a47f42c Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 22 May 2024 12:37:30 -0700 Subject: [PATCH] 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 --- .../com/google/devtools/common/options/BUILD | 1 + .../common/options/IsolatedOptionsData.java | 3 +- .../common/options/OptionsParserImpl.java | 29 +++++++++++++++++-- .../devtools/common/options/OptionsTest.java | 28 ++++++++++++++++-- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/common/options/BUILD b/src/main/java/com/google/devtools/common/options/BUILD index 6090c3af29ca8d..935c0eba6d8bd5 100644 --- 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 index 1f3ecf1685a8b9..61fe7fb4eaed97 100644 --- 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> getAllOptionDefinitions() { + public ImmutableSet> 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 index 14d1dfb2864904..09d74a25cc1d5f 100644 --- 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 getAllValidArgs() { + return () -> + optionsData.getAllOptionDefinitions().stream() + .filter(entry -> !shouldIgnoreOption(entry.getValue())) + .flatMap( + definition -> { + Stream.Builder 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 index b037507966b435..9528c714d99bc5 100644 --- 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 {