[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
This commit is contained in:
Nathan James 2020-12-15 18:16:16 +00:00
parent 1183e55580
commit cfa1010c42
No known key found for this signature in database
GPG Key ID: CC007AFCDA90AA5F
4 changed files with 76 additions and 11 deletions

View File

@ -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 @@ private:
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 @@ private:
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 @@ private:
}
// 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());
}
};

View File

@ -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,

View File

@ -79,6 +79,12 @@ CompileFlags:
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();

View File

@ -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"))),