Author: ibiryukov Date: Fri May 24 03:26:23 2019 New Revision: 361625 URL: http://llvm.org/viewvc/llvm-project?rev=361625&view=rev Log: [clangd] Limit the size of synthesized fix message
Summary: A temporary workaround until we figure out a better way to present fixes. Reviewers: kadircet Reviewed By: kadircet Subscribers: MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D62372 Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=361625&r1=361624&r2=361625&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original) +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Fri May 24 03:26:23 2019 @@ -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 <algorithm> +#include <cstddef> namespace clang { namespace clangd { @@ -437,6 +439,21 @@ void StoreDiags::EndSourceFile() { 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 @@ void StoreDiags::HandleDiagnostic(Diagno 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', ' '); } Modified: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp?rev=361625&r1=361624&r2=361625&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Fri May 24 03:26:23 2019 @@ -120,7 +120,7 @@ o]](); "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 @@ TEST(DiagnosticTest, ClangTidyWarningAsE 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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
