diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -24,6 +24,29 @@ using llvm::yaml::ScalarNode; using llvm::yaml::SequenceNode; +static llvm::Optional +bestGuess(llvm::StringRef Search, + llvm::ArrayRef AllowedValues) { + unsigned MaxEdit = (Search.size() + 1) / 3; + if (!MaxEdit) + return llvm::None; + llvm::Optional Result; + for (const auto &AllowedValue : AllowedValues) { + unsigned EditDistance = Search.edit_distance(AllowedValue, true, MaxEdit); + // We can't do better than an edit distance of 1, so just return this and + // save computing other values. + if (EditDistance == 1U) + return AllowedValue; + if (EditDistance == MaxEdit && !Result) { + Result = AllowedValue; + } else if (EditDistance < MaxEdit) { + Result = AllowedValue; + MaxEdit = EditDistance; + } + } + return Result; +} + class Parser { llvm::SourceMgr &SM; bool HadError = false; @@ -167,6 +190,7 @@ return; } llvm::SmallSet Seen; + llvm::SmallVector, 0> UnknownKeys; // We *must* consume all items, even on error, or the parser will assert. for (auto &KV : llvm::cast(N)) { auto *K = KV.getKey(); @@ -198,9 +222,30 @@ Warn = UnknownHandler( Located(**Key, K->getSourceRange()), *Value); if (Warn) - Outer->warning("Unknown " + Description + " key " + **Key, *K); + UnknownKeys.push_back(std::move(*Key)); } } + if (!UnknownKeys.empty()) + warnUnknownKeys(UnknownKeys, Seen); + } + + private: + void warnUnknownKeys(llvm::ArrayRef> UnknownKeys, + const llvm::SmallSet &SeenKeys) const { + llvm::SmallVector UnseenKeys; + for (const auto &KeyAndHandler : Keys) + if (!SeenKeys.count(KeyAndHandler.first.str())) + UnseenKeys.push_back(KeyAndHandler.first); + + for (const Located &UnknownKey : UnknownKeys) + if (auto BestGuess = bestGuess(*UnknownKey, UnseenKeys)) + Outer->warning("Unknown " + Description + " key '" + *UnknownKey + + "'; did you mean '" + *BestGuess + "'?", + UnknownKey.Range); + else + Outer->warning("Unknown " + Description + " key '" + *UnknownKey + + "'", + UnknownKey.Range); } }; @@ -238,16 +283,23 @@ } // Report a "hard" error, reflecting a config file that can never be valid. - void error(const llvm::Twine &Msg, const Node &N) { + void error(const llvm::Twine &Msg, llvm::SMRange Range) { HadError = true; - SM.PrintMessage(N.getSourceRange().Start, llvm::SourceMgr::DK_Error, Msg, - N.getSourceRange()); + SM.PrintMessage(Range.Start, llvm::SourceMgr::DK_Error, Msg, Range); + } + + // Report a "hard" error, reflecting a config file that can never be valid. + void error(const llvm::Twine &Msg, const Node &N) { + return error(Msg, N.getSourceRange()); + } + // Report a "soft" error that could be caused by e.g. version skew. + void warning(const llvm::Twine &Msg, llvm::SMRange Range) { + SM.PrintMessage(Range.Start, llvm::SourceMgr::DK_Warning, Msg, Range); } // Report a "soft" error that could be caused by e.g. version skew. void warning(const llvm::Twine &Msg, const Node &N) { - SM.PrintMessage(N.getSourceRange().Start, llvm::SourceMgr::DK_Warning, Msg, - N.getSourceRange()); + return warning(Msg, N.getSourceRange()); } }; diff --git a/clang-tools-extra/clangd/test/config.test b/clang-tools-extra/clangd/test/config.test --- a/clang-tools-extra/clangd/test/config.test +++ b/clang-tools-extra/clangd/test/config.test @@ -19,7 +19,7 @@ # CHECK-NEXT: "params": { # CHECK-NEXT: "diagnostics": [ # CHECK-NEXT: { -# CHECK-NEXT: "message": "Unknown Config key Foo", +# CHECK-NEXT: "message": "Unknown Config key 'Foo'", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { # CHECK-NEXT: "character": 3, diff --git a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp @@ -79,6 +79,12 @@ Unknown: 42 )yaml"; +const char *AddFooWithTypoErr = R"yaml( +CompileFlags: + Add: foo + Removr: 42 +)yaml"; + const char *AddBarBaz = R"yaml( CompileFlags: Add: bar @@ -95,7 +101,7 @@ auto P = Provider::fromYAMLFile(testPath("foo.yaml"), /*Directory=*/"", FS); auto Cfg = P->getConfig(Params(), Diags.callback()); EXPECT_THAT(Diags.Diagnostics, - ElementsAre(DiagMessage("Unknown CompileFlags key Unknown"))); + ElementsAre(DiagMessage("Unknown CompileFlags key 'Unknown'"))); EXPECT_THAT(Diags.Files, ElementsAre(testPath("foo.yaml"))); EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo")); Diags.clear(); @@ -105,6 +111,16 @@ EXPECT_THAT(Diags.Files, IsEmpty()); EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo")); + FS.Files["foo.yaml"] = AddFooWithTypoErr; + Cfg = P->getConfig(Params(), Diags.callback()); + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre(DiagMessage( + "Unknown CompileFlags key 'Removr'; did you mean 'Remove'?"))); + EXPECT_THAT(Diags.Files, ElementsAre(testPath("foo.yaml"))); + EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo")); + Diags.clear(); + FS.Files["foo.yaml"] = AddBarBaz; Cfg = P->getConfig(Params(), Diags.callback()); EXPECT_THAT(Diags.Diagnostics, IsEmpty()) << "New config, no errors"; @@ -143,7 +159,7 @@ Cfg = P->getConfig(ABCParams, Diags.callback()); EXPECT_THAT(Diags.Diagnostics, - ElementsAre(DiagMessage("Unknown CompileFlags key Unknown"))); + ElementsAre(DiagMessage("Unknown CompileFlags key 'Unknown'"))); // FIXME: fails on windows: paths have mixed slashes like C:\a/b\c.yaml EXPECT_THAT(Diags.Files, ElementsAre(testPath("a/foo.yaml"), testPath("a/b/c/foo.yaml"))); @@ -178,7 +194,7 @@ FS.Files["foo.yaml"] = AddFooWithErr; auto Cfg = P->getConfig(StaleOK, Diags.callback()); EXPECT_THAT(Diags.Diagnostics, - ElementsAre(DiagMessage("Unknown CompileFlags key Unknown"))); + ElementsAre(DiagMessage("Unknown CompileFlags key 'Unknown'"))); EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo")); Diags.clear(); diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -113,7 +113,7 @@ ASSERT_THAT( Diags.Diagnostics, - ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"), + ElementsAre(AllOf(DiagMessage("Unknown If key 'UnknownCondition'"), DiagKind(llvm::SourceMgr::DK_Warning), DiagPos(YAML.range("unknown").start), DiagRange(YAML.range("unknown"))),