njames93 updated this revision to Diff 310814. njames93 added a comment. Fix lit test failure
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92990/new/ https://reviews.llvm.org/D92990 Files: clang-tools-extra/clangd/ConfigYAML.cpp clang-tools-extra/clangd/test/config.test clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ 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"))), Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp +++ 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(); Index: clang-tools-extra/clangd/test/config.test =================================================================== --- clang-tools-extra/clangd/test/config.test +++ 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, Index: clang-tools-extra/clangd/ConfigYAML.cpp =================================================================== --- clang-tools-extra/clangd/ConfigYAML.cpp +++ clang-tools-extra/clangd/ConfigYAML.cpp @@ -167,6 +167,7 @@ return; } llvm::SmallSet<std::string, 8> Seen; + llvm::SmallVector<Located<std::string>, 0> UnknownKeys; // We *must* consume all items, even on error, or the parser will assert. for (auto &KV : llvm::cast<MappingNode>(N)) { auto *K = KV.getKey(); @@ -198,9 +199,62 @@ Warn = UnknownHandler( Located<std::string>(**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<llvm::StringRef, 4> + getUnseenKeys(const llvm::SmallSet<std::string, 8> &SeenKeys) const { + llvm::SmallVector<llvm::StringRef, 4> Result; + for (const auto &KeyAndHandler : Keys) + if (!SeenKeys.count(KeyAndHandler.first.str())) + Result.push_back(KeyAndHandler.first); + return Result; + } + + static llvm::Optional<llvm::StringRef> + getBestGuess(llvm::StringRef Search, llvm::ArrayRef<llvm::StringRef> 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<llvm::StringRef> 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<Located<std::string>> UnknownKeys, + llvm::ArrayRef<llvm::StringRef> UnseenKeys) const { + for (const Located<std::string> &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<llvm::SMFixIt> 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<llvm::SMFixIt> 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()); } };
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits