[clang-tools-extra] r361625 - [clangd] Limit the size of synthesized fix message

[Ilya Biryukov via cfe-commits]

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() {