[llvm-branch-commits] [clang-tools-extra] cfa1010 - [clangd] Provide suggestions with invalid config keys
Nathan James via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Dec 15 10:21:37 PST 2020
Author: Nathan James
Date: 2020-12-15T18:16:17Z
New Revision: cfa1010c42429f689186125270dfbad7d6a1c01d
URL: https://github.com/llvm/llvm-project/commit/cfa1010c42429f689186125270dfbad7d6a1c01d
DIFF: https://github.com/llvm/llvm-project/commit/cfa1010c42429f689186125270dfbad7d6a1c01d.diff
LOG: [clangd] Provide suggestions with invalid config keys
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.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92990
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 022d890a9687..20cdc0b2c001 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -24,6 +24,29 @@ using llvm::yaml::Node;
using llvm::yaml::ScalarNode;
using llvm::yaml::SequenceNode;
+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);
+ // 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 @@ class Parser {
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 +222,30 @@ class Parser {
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, Seen);
+ }
+
+ private:
+ void warnUnknownKeys(llvm::ArrayRef<Located<std::string>> UnknownKeys,
+ const llvm::SmallSet<std::string, 8> &SeenKeys) const {
+ llvm::SmallVector<llvm::StringRef> UnseenKeys;
+ for (const auto &KeyAndHandler : Keys)
+ if (!SeenKeys.count(KeyAndHandler.first.str()))
+ UnseenKeys.push_back(KeyAndHandler.first);
+
+ 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);
+ else
+ Outer->warning("Unknown " + Description + " key '" + *UnknownKey +
+ "'",
+ UnknownKey.Range);
}
};
@@ -238,16 +283,20 @@ class Parser {
}
// 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);
+ }
+ 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);
+ }
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
index eb556feefd95..3d5e2bbfc0c8 100644
--- 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
index 9c45266d1a83..899896dd2fb4 100644
--- a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -79,6 +79,12 @@ const char *AddFooWithErr = R"yaml(
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 @@ TEST(ProviderTest, FromYAMLFile) {
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 @@ TEST(ProviderTest, FromYAMLFile) {
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 @@ TEST(ProviderTest, FromAncestorRelativeYAMLFiles) {
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 @@ TEST(ProviderTest, Staleness) {
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
index 54ef7d1d85cd..4039a9be71a0 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -113,7 +113,7 @@ CompileFlags: {$unexpected^
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"))),
More information about the llvm-branch-commits
mailing list