Index: clangd/Diagnostics.cpp =================================================================== --- clangd/Diagnostics.cpp +++ clangd/Diagnostics.cpp @@ -25,7 +25,9 @@ #include "llvm/Support/Path.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/Signals.h" +#include "llvm/Support/raw_ostream.h" #include +#include namespace clang { namespace clangd { @@ -437,6 +439,21 @@ LangOpts = None; } +/// Sanitizes a piece for presenting it in a synthesized fix message. Ensures +/// the result is not too large and does not contain newlines. +static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) { + constexpr unsigned MaxLen = 50; + + // Only show the first line if there are many. + llvm::StringRef R = Code.split('\n').first; + // Shorten the message if it's too long. + R = R.take_front(MaxLen); + + OS << R; + if (R.size() != Code.size()) + OS << "…"; +} + void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); @@ -494,12 +511,21 @@ llvm::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 << "'"; + if (!Remove.empty() && !Insert.empty()) { + M << "change '"; + writeCodeToFixMessage(M, Remove); + M << "' to '"; + writeCodeToFixMessage(M, Insert); + M << "'"; + } else if (!Remove.empty()) { + M << "remove '"; + writeCodeToFixMessage(M, Remove); + M << "'"; + } else if (!Insert.empty()) { + M << "insert '"; + writeCodeToFixMessage(M, Insert); + M << "'"; + } // Don't allow source code to inject newlines into diagnostics. std::replace(Message.begin(), Message.end(), '\n', ' '); } Index: clangd/unittests/DiagnosticsTests.cpp =================================================================== --- clangd/unittests/DiagnosticsTests.cpp +++ clangd/unittests/DiagnosticsTests.cpp @@ -120,7 +120,7 @@ "use of undeclared identifier 'goo'; did you mean 'foo'?"), DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"), WithFix( - Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")), + Fix(Test.range("typo"), "foo", "change 'go\\…' to 'foo'")), // This is a pretty normal range. WithNote(Diag(Test.range("decl"), "'foo' declared here"))), // This range is zero-width and insertion. Therefore make sure we are @@ -247,6 +247,36 @@ DiagSeverity(DiagnosticsEngine::Error)))); } +TEST(DiagnosticTest, LongFixMessages) { + // We limit the size of printed code. + Annotations Source(R"cpp( + int main() { + int somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier; + [[omereallyreallyreallyreallyreallyreallyreallyreallylongidentifier]]= 10; + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(WithFix(Fix( + Source.range(), + "somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier", + "change 'omereallyreallyreallyreallyreallyreallyreallyreall…' to " + "'somereallyreallyreallyreallyreallyreallyreallyreal…'")))); + // Only show changes up to a first newline. + Source = Annotations(R"cpp( + int main() { + int ident; + [[ide\ +n]] = 10; + } + )cpp"); + TU = TestTU::withCode(Source.code()); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(WithFix( + Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'")))); +} + TEST(DiagnosticTest, ClangTidyWarningAsErrorTrumpsSuppressionComment) { Annotations Main(R"cpp( int main() {