[clang-tools-extra] r329090 - [clangd] synthesize fix message when the diagnostic doesn't provide one.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 3 10:35:58 PDT 2018
Author: sammccall
Date: Tue Apr 3 10:35:57 2018
New Revision: 329090
URL: http://llvm.org/viewvc/llvm-project?rev=329090&view=rev
Log:
[clangd] synthesize fix message when the diagnostic doesn't provide one.
Summary:
Currently if a fix is attached directly to a diagnostic, we repeat the
diagnostic message as the fix message. From eyeballing the top diagnostics,
it seems describing the textual replacement would be much clearer.
e.g.
error: use of undeclared identifier 'goo'; did you mean 'foo'?
action before: use of undeclared identifier 'goo'; did you mean 'foo'?
action after: change 'goo' to 'foo'
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45069
Modified:
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=329090&r1=329089&r2=329090&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Tue Apr 3 10:35:57 2018
@@ -297,7 +297,7 @@ void StoreDiags::HandleDiagnostic(Diagno
return D;
};
- auto AddFix = [&]() -> bool {
+ auto AddFix = [&](bool SyntheticMessage) -> bool {
assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
if (!InsideMainFile)
@@ -312,7 +312,27 @@ void StoreDiags::HandleDiagnostic(Diagno
}
llvm::SmallString<64> Message;
- Info.FormatDiagnostic(Message);
+ // If requested and possible, create a message like "change 'foo' to 'bar'".
+ if (SyntheticMessage && Info.getNumFixItHints() == 1) {
+ const auto &FixIt = Info.getFixItHint(0);
+ bool Invalid = false;
+ StringRef Remove = Lexer::getSourceText(
+ FixIt.RemoveRange, Info.getSourceManager(), *LangOpts, &Invalid);
+ StringRef Insert = FixIt.CodeToInsert;
+ if (!Invalid) {
+ llvm::raw_svector_ostream M(Message);
+ if (!Remove.empty() && !Insert.empty())
+ M << "change '" << Remove << "' to '" << Insert << "'";
+ else if (!Remove.empty())
+ M << "remove '" << Remove << "'";
+ else if (!Insert.empty())
+ M << "insert '" << Insert << "'";
+ // Don't allow source code to inject newlines into diagnostics.
+ std::replace(Message.begin(), Message.end(), '\n', ' ');
+ }
+ }
+ if (Message.empty()) // either !SytheticMessage, or we failed to make one.
+ Info.FormatDiagnostic(Message);
LastDiag->Fixes.push_back(Fix{Message.str(), std::move(Edits)});
return true;
};
@@ -325,7 +345,7 @@ void StoreDiags::HandleDiagnostic(Diagno
FillDiagBase(*LastDiag);
if (!Info.getFixItHints().empty())
- AddFix();
+ AddFix(true /* try to invent a message instead of repeating the diag */);
} else {
// Handle a note to an existing diagnostic.
if (!LastDiag) {
@@ -337,7 +357,7 @@ void StoreDiags::HandleDiagnostic(Diagno
if (!Info.getFixItHints().empty()) {
// A clang note with fix-it is not a separate diagnostic in clangd. We
// attach it as a Fix to the main diagnostic instead.
- if (!AddFix())
+ if (!AddFix(false /* use the note as the message */))
IgnoreDiagnostics::log(DiagLevel, Info);
} else {
// A clang note without fix-its corresponds to clangd::Note.
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=329090&r1=329089&r2=329090&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Tue Apr 3 10:35:57 2018
@@ -92,14 +92,6 @@ Position pos(int line, int character) {
return Res;
}
-/// Matches diagnostic that has exactly one fix with the same range and message
-/// as the diagnostic itself.
-testing::Matcher<const clangd::Diag &> DiagWithEqualFix(clangd::Range Range,
- std::string Replacement,
- std::string Message) {
- return AllOf(Diag(Range, Message), WithFix(Fix(Range, Replacement, Message)));
-}
-
TEST(DiagnosticsTest, DiagnosticRanges) {
// Check we report correct ranges, including various edge-cases.
Annotations Test(R"cpp(
@@ -111,19 +103,19 @@ o]]();
$unk[[unknown]]();
}
)cpp");
- llvm::errs() << Test.code();
EXPECT_THAT(
buildDiags(Test.code()),
ElementsAre(
// This range spans lines.
- AllOf(DiagWithEqualFix(
- Test.range("typo"), "foo",
- "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+ AllOf(Diag(Test.range("typo"),
+ "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+ WithFix(
+ Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
// This is a pretty normal range.
WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
// This range is zero-width, and at the end of a line.
- DiagWithEqualFix(Test.range("semicolon"), ";",
- "expected ';' after expression"),
+ AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
+ WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))),
// This range isn't provided by clang, we expand to the token.
Diag(Test.range("unk"), "use of undeclared identifier 'unknown'")));
}
@@ -131,8 +123,9 @@ o]]();
TEST(DiagnosticsTest, FlagsMatter) {
Annotations Test("[[void]] main() {}");
EXPECT_THAT(buildDiags(Test.code()),
- ElementsAre(DiagWithEqualFix(Test.range(), "int",
- "'main' must return 'int'")));
+ ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
+ WithFix(Fix(Test.range(), "int",
+ "change 'void' to 'int'")))));
// Same code built as C gets different diagnostics.
EXPECT_THAT(
buildDiags(Test.code(), {"-x", "c"}),
More information about the cfe-commits
mailing list