Skip to content

Commit

Permalink
[7.2.0] Provide "did you mean ...?" suggestions for unknown command-l…
Browse files Browse the repository at this point in the history
…ine options (#22507)

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

Commit 0e9287f

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
iancha1992 and fmeum committed May 22, 2024
1 parent 4f60def commit e6479d5
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/common/options/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.common.options.OptionDefinition.NotAnOptionException;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.common.options.OptionsParser.ConstructionException;
import java.lang.reflect.Constructor;
import java.util.Arrays;
Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -790,9 +792,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) {
Expand Down Expand Up @@ -832,6 +841,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
Expand Down
28 changes: 26 additions & 2 deletions src/test/java/com/google/devtools/common/options/OptionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit e6479d5

Please sign in to comment.