njames93 updated this revision to Diff 311067.
njames93 marked 3 inline comments as done.
njames93 added a comment.
Address some comments
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,57 @@
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, unseenKeys(Seen));
+ }
+
+ private:
+ llvm::SmallVector<llvm::StringRef>
+ unseenKeys(const llvm::SmallSet<std::string, 8> &SeenKeys) const {
+ llvm::SmallVector<llvm::StringRef> Result;
+ for (const auto &KeyAndHandler : Keys)
+ if (!SeenKeys.count(KeyAndHandler.first.str()))
+ Result.push_back(KeyAndHandler.first);
+ return Result;
+ }
+
+ static llvm::Optional<llvm::StringRef>
+ bestGuess(llvm::StringRef Search,
+ llvm::ArrayRef<llvm::StringRef> AllowedValues) {
+ unsigned MaxEdit = (Search.size() + 1) / 3;
+ if (!MaxEdit)
+ return llvm::None;
+ llvm::Optional<llvm::StringRef> Result;
+ for (const auto &AllowedValue : AllowedValues) {
+ unsigned EditDistance =
+ Search.edit_distance(AllowedValue, true, MaxEdit);
+ if (EditDistance == MaxEdit) {
+ Result = Result ? llvm::StringRef() : AllowedValue;
+ } else if (EditDistance < MaxEdit) {
+ Result = AllowedValue;
+ 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 = bestGuess(*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 +287,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
