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 @@ -167,6 +167,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 +199,62 @@ 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, getUnseenKeys(Seen)); + } + + private: + llvm::SmallVector + getUnseenKeys(const llvm::SmallSet &SeenKeys) const { + llvm::SmallVector Result; + for (const auto &KeyAndHandler : Keys) + if (!SeenKeys.count(KeyAndHandler.first.str())) + Result.push_back(KeyAndHandler.first); + return Result; + } + + static llvm::Optional + getBestGuess(llvm::StringRef Search, llvm::ArrayRef Items, + unsigned MaxEdit = 0) { + + // If Search is sufficiently short (size() < 2), just return nothing. + if (Search.size() < 2) + return llvm::None; + if (!MaxEdit) + MaxEdit = (Search.size() + 1) / 3; + llvm::Optional Result; + for (const auto &Item : Items) { + unsigned EditDistance = Search.edit_distance(Item, true, MaxEdit); + if (EditDistance == MaxEdit) { + if (!Result) + Result = Item; + else + Result = llvm::StringRef(); + } else if (EditDistance < MaxEdit) { + Result = Item; + MaxEdit = EditDistance; + } + } + if (Result && !Result->empty()) + return Result; + return llvm::None; + } + + void warnUnknownKeys(llvm::ArrayRef> UnknownKeys, + llvm::ArrayRef UnseenKeys) const { + for (const Located &UnknownKey : UnknownKeys) + if (auto BestGuess = getBestGuess(*UnknownKey, UnseenKeys)) + Outer->warning("Unknown " + Description + " key '" + *UnknownKey + + "'; did you mean '" + *BestGuess + "'?", + UnknownKey.Range, + llvm::SMFixIt(UnknownKey.Range, *BestGuess)); + else + Outer->warning("Unknown " + Description + " key '" + *UnknownKey + + "'", + UnknownKey.Range); } }; @@ -238,16 +292,26 @@ } // 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, + llvm::ArrayRef Fixes = {}) { HadError = true; - SM.PrintMessage(N.getSourceRange().Start, llvm::SourceMgr::DK_Error, Msg, - N.getSourceRange()); + SM.PrintMessage(Range.Start, llvm::SourceMgr::DK_Error, Msg, Range, Fixes); + } + + // 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, + llvm::ArrayRef Fixes = {}) { + SM.PrintMessage(Range.Start, llvm::SourceMgr::DK_Warning, Msg, Range, + Fixes); } // 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"))),