njames93 created this revision.
njames93 added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
Update the config file warning when an unknown key is detected which is likely
a typo by suggesting the likely key.
This won't suggest a key that has already been seen in the block.
Appends the fix to the diag, however right now there is no support for
presenting that fix to the user.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92990
Files:
clang-tools-extra/clangd/ConfigYAML.cpp
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/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,61 @@
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 (!MaxEdit)
+ MaxEdit = (Search.size() + 1) / 3;
+ // If Search is sufficiently short (size() < 2), just return nothing.
+ if (!MaxEdit)
+ return llvm::None;
+ 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 +291,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
