[Option] Add 'findNearest' method to catch typos

Summary:
Add a method `OptTable::findNearest`, which allows users of OptTable to
check user input for misspelled options. In addition, have llvm-mt
check for misspelled options. For example, if a user invokes
`llvm-mt /oyt:foo`, the error message will indicate that while an
option named `/oyt:` does not exist, `/out:` does.

The method ports the functionality of the `LookupNearestOption` method
from LLVM CommandLine to libLLVMOption. This allows tools like Clang
and Swift, which do not use CommandLine, to use this functionality to
suggest similarly spelled options.

As room for future improvement, the new method as-is cannot yet properly suggest
nearby "joined" options -- that is, for an option string "-FozBar", where
"-Foo" is the correct option name and "Bar" is the value being passed along
with the misspelled option, this method will calculate an edit distance of 4,
by deleting "Bar" and changing "z" to "o". It should instead calculate an edit
distance of just 1, by changing "z" to "o" and recognizing "Bar" as a
value. This commit includes a disabled test that expresses this limitation.

Test Plan: `check-llvm`

Reviewers: yamaguchi, v.g.vassilev, teemperor, ruiu, jroelofs

Reviewed By: jroelofs

Subscribers: jroelofs, llvm-commits

Differential Revision: https://reviews.llvm.org/D41732

llvm-svn: 321877

include/llvm/Option/OptTable.h

@@ -143,6 +143,26 @@ public:
   std::vector<std::string> findByPrefix(StringRef Cur,
                                         unsigned short DisableFlags) const;
 
+  /// Find the OptTable option that most closely matches the given string.
+  ///
+  /// \param [in] Option - A string, such as "-stdlibs=l", that represents user
+  /// input of an option that may not exist in the OptTable. Note that the
+  /// string includes prefix dashes "-" as well as values "=l".
+  /// \param [out] NearestString - The nearest option string found in the
+  /// OptTable.
+  /// \param [in] FlagsToInclude - Only find options with any of these flags.
+  /// Zero is the default, which includes all flags.
+  /// \param [in] FlagsToExclude - Don't find options with this flag. Zero
+  /// is the default, and means exclude nothing.
+  /// \param [in] MinimumLength - Don't find options shorter than this length.
+  /// For example, a minimum length of 3 prevents "-x" from being considered
+  /// near to "-S".
+  ///
+  /// \return The edit distance of the nearest string found.
+  unsigned findNearest(StringRef Option, std::string &NearestString,
+                       unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 0,
+                       unsigned MinimumLength = 4) const;
+
   /// Add Values to Option's Values class
   ///
   /// \param [in] Option - Prefix + Name of the flag which Values will be

lib/Option/OptTable.cpp

@@ -247,6 +247,69 @@ OptTable::findByPrefix(StringRef Cur, unsigned short DisableFlags) const {
   return Ret;
 }
 
+unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
+                               unsigned FlagsToInclude, unsigned FlagsToExclude,
+                               unsigned MinimumLength) const {
+  assert(!Option.empty());
+
+  // Consider each option as a candidate, finding the closest match.
+  unsigned BestDistance = UINT_MAX;
+  for (const Info &CandidateInfo :
+       ArrayRef<Info>(OptionInfos).drop_front(FirstSearchableIndex)) {
+    StringRef CandidateName = CandidateInfo.Name;
+
+    // Ignore option candidates with empty names, such as "--", or names
+    // that do not meet the minimum length.
+    if (CandidateName.empty() || CandidateName.size() < MinimumLength)
+      continue;
+
+    // If FlagsToInclude were specified, ignore options that don't include
+    // those flags.
+    if (FlagsToInclude && !(CandidateInfo.Flags & FlagsToInclude))
+      continue;
+    // Ignore options that contain the FlagsToExclude.
+    if (CandidateInfo.Flags & FlagsToExclude)
+      continue;
+
+    // Ignore positional argument option candidates (which do not
+    // have prefixes).
+    if (!CandidateInfo.Prefixes)
+      continue;
+    // Find the most appropriate prefix. For example, if a user asks for
+    // "--helm", suggest "--help" over "-help".
+    StringRef Prefix;
+    for (int P = 0; CandidateInfo.Prefixes[P]; P++) {
+      if (Option.startswith(CandidateInfo.Prefixes[P]))
+        Prefix = CandidateInfo.Prefixes[P];
+    }
+
+    // Check if the candidate ends with a character commonly used when
+    // delimiting an option from its value, such as '=' or ':'. If it does,
+    // attempt to split the given option based on that delimiter.
+    std::string Delimiter = "";
+    char Last = CandidateName.back();
+    if (Last == '=' || Last == ':')
+      Delimiter = std::string(1, Last);
+
+    StringRef LHS, RHS;
+    if (Delimiter.empty())
+      LHS = Option;
+    else
+      std::tie(LHS, RHS) = Option.split(Last);
+
+    std::string NormalizedName =
+        (LHS.drop_front(Prefix.size()) + Delimiter).str();
+    unsigned Distance =
+        CandidateName.edit_distance(NormalizedName, /*AllowReplacements=*/true,
+                                    /*MaxEditDistance=*/BestDistance);
+    if (Distance < BestDistance) {
+      BestDistance = Distance;
+      NearestString = (Prefix + CandidateName + RHS).str();
+    }
+  }
+  return BestDistance;
+}
+
 bool OptTable::addValues(const char *Option, const char *Values) {
   for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
     Info &In = OptionInfos[I];

test/tools/llvm-mt/help.test

@@ -3,5 +3,8 @@ RUN: llvm-mt /h | FileCheck %s -check-prefix=HELP
 HELP:      OVERVIEW: Manifest Tool
 
 RUN: not llvm-mt /foo 2>&1 >/dev/null | FileCheck %s -check-prefix=INVALID
+INVALID: llvm-mt error: invalid option '/foo'
+
+RUN: not llvm-mt /oyt:%t 2>&1 | FileCheck %s -check-prefix=INVALID-BUT-CLOSE
+INVALID-BUT-CLOSE: llvm-mt error: invalid option '/oyt:{{.*}}/help.test.tmp', did you mean '/out:{{.*}}/help.test.tmp'?
 
-INVALID: llvm-mt error: invalid option /foo

tools/llvm-mt/llvm-mt.cpp

@@ -103,8 +103,18 @@ int main(int argc, const char **argv) {
   ArrayRef<const char *> ArgsArr = makeArrayRef(argv + 1, argc);
   opt::InputArgList InputArgs = T.ParseArgs(ArgsArr, MAI, MAC);
 
-  for (auto *Arg : InputArgs.filtered(OPT_INPUT))
+  for (auto *Arg : InputArgs.filtered(OPT_INPUT)) {
-    reportError(Twine("invalid option ") + Arg->getSpelling());
+    auto ArgString = Arg->getAsString(InputArgs);
+    std::string Diag;
+    raw_string_ostream OS(Diag);
+    OS << "invalid option '" << ArgString << "'";
+
+    std::string Nearest;
+    if (T.findNearest(ArgString, Nearest) < 2)
+      OS << ", did you mean '" << Nearest << "'?";
+
+    reportError(OS.str());
+  }
 
   for (auto &Arg : InputArgs) {
     if (Arg->getOption().matches(OPT_unsupported)) {

unittests/Option/OptionParsingTest.cpp

@@ -266,3 +266,46 @@ TEST(Option, FlagAliasToJoined) {
   EXPECT_EQ(1U, AL.getAllArgValues(OPT_B).size());
   EXPECT_EQ("", AL.getAllArgValues(OPT_B)[0]);
 }
+
+TEST(Option, FindNearest) {
+  TestOptTable T;
+  std::string Nearest;
+
+  // Options that are too short should not be considered
+  // "near" other short options.
+  EXPECT_GT(T.findNearest("-A", Nearest), 4U);
+  EXPECT_GT(T.findNearest("/C", Nearest), 4U);
+  EXPECT_GT(T.findNearest("--C=foo", Nearest), 4U);
+
+  // The nearest candidate should mirror the amount of prefix
+  // characters used in the original string.
+  EXPECT_EQ(1U, T.findNearest("-blorb", Nearest));
+  EXPECT_EQ(Nearest, "-blorp");
+  EXPECT_EQ(1U, T.findNearest("--blorm", Nearest));
+  EXPECT_EQ(Nearest, "--blorp");
+
+  // The nearest candidate respects the prefix and value delimiter
+  // of the original string.
+  EXPECT_EQ(1U, T.findNearest("/framb:foo", Nearest));
+  EXPECT_EQ(Nearest, "/cramb:foo");
+
+  // Flags should be included and excluded as specified.
+  EXPECT_EQ(1U, T.findNearest("-doopf", Nearest, /*FlagsToInclude=*/OptFlag2));
+  EXPECT_EQ(Nearest, "-doopf2");
+  EXPECT_EQ(1U, T.findNearest("-doopf", Nearest,
+                              /*FlagsToInclude=*/0,
+                              /*FlagsToExclude=*/OptFlag2));
+  EXPECT_EQ(Nearest, "-doopf1");
+}
+
+TEST(DISABLED_Option, FindNearestFIXME) {
+  TestOptTable T;
+  std::string Nearest;
+
+  // FIXME: Options with joined values should not have those values considered
+  // when calculating distance. The test below would fail if run, but it should
+  // succeed.
+  EXPECT_EQ(1U, T.findNearest("--erbghFoo", Nearest));
+  EXPECT_EQ(Nearest, "--ermghFoo");
+
+}

unittests/Option/Opts.td

@@ -28,3 +28,10 @@ def K : Flag<["-"], "K">, Alias<B>;
 def Slurp : Option<["-"], "slurp", KIND_REMAINING_ARGS>;
 
 def SlurpJoined : Option<["-"], "slurpjoined", KIND_REMAINING_ARGS_JOINED>;
+
+def Blorp : Flag<["-", "--"], "blorp">, HelpText<"The blorp option">, Flags<[OptFlag1]>;
+def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
+def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
+def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
+def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
+def DashDash : Option<["--"], "", KIND_REMAINING_ARGS>;