[PATCH] D60821: [clangd] Emit better error messages when rename fails.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE358658: [clangd] Emit better error messages when rename 
fails. (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60821?vs=195715&id=195717#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60821/new/

https://reviews.llvm.org/D60821

Files:
  clangd/ClangdServer.cpp
  test/clangd/rename.test


Index: test/clangd/rename.test
===================================================================
--- test/clangd/rename.test
+++ test/clangd/rename.test
@@ -29,7 +29,7 @@
 
{"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
 #      CHECK:  "error": {
 # CHECK-NEXT:    "code": -32001,
-# CHECK-NEXT:    "message": "clang diagnostic"
+# CHECK-NEXT:    "message": "there is no symbol at the given location"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0"
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -44,14 +44,24 @@
 namespace clangd {
 namespace {
 
+// Expand a DiagnosticError to make it print-friendly (print the detailed
+// message, rather than "clang diagnostic").
+llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
+  if (auto Diag = DiagnosticError::take(Err)) {
+    llvm::cantFail(std::move(Err));
+    SmallVector<char, 128> DiagMessage;
+    Diag->second.EmitToString(DE, DiagMessage);
+    return llvm::make_error<llvm::StringError>(DiagMessage,
+                                               llvm::inconvertibleErrorCode());
+  }
+  return Err;
+}
+
 class RefactoringResultCollector final
     : public tooling::RefactoringResultConsumer {
 public:
   void handleError(llvm::Error Err) override {
     assert(!Result.hasValue());
-    // FIXME: figure out a way to return better message for DiagnosticError.
-    // clangd uses llvm::toString to convert the Err to string, however, for
-    // DiagnosticError, only "clang diagnostic" will be generated.
     Result = std::move(Err);
   }
 
@@ -301,13 +311,15 @@
     auto Rename = clang::tooling::RenameOccurrences::initiate(
         Context, SourceRange(SourceLocationBeg), NewName);
     if (!Rename)
-      return CB(Rename.takeError());
+      return CB(expandDiagnostics(Rename.takeError(),
+                                  AST.getASTContext().getDiagnostics()));
 
     Rename->invoke(ResultCollector, Context);
 
     assert(ResultCollector.Result.hasValue());
     if (!ResultCollector.Result.getValue())
-      return CB(ResultCollector.Result->takeError());
+      return CB(expandDiagnostics(ResultCollector.Result->takeError(),
+                                  AST.getASTContext().getDiagnostics()));
 
     std::vector<TextEdit> Replacements;
     for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {



Index: test/clangd/rename.test
===================================================================
--- test/clangd/rename.test
+++ test/clangd/rename.test
@@ -29,7 +29,7 @@
 {"jsonrpc":"2.0","id":2,"method":"textDocument/rename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2},"newName":"bar"}}
 #      CHECK:  "error": {
 # CHECK-NEXT:    "code": -32001,
-# CHECK-NEXT:    "message": "clang diagnostic"
+# CHECK-NEXT:    "message": "there is no symbol at the given location"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0"
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -44,14 +44,24 @@
 namespace clangd {
 namespace {
 
+// Expand a DiagnosticError to make it print-friendly (print the detailed
+// message, rather than "clang diagnostic").
+llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
+  if (auto Diag = DiagnosticError::take(Err)) {
+    llvm::cantFail(std::move(Err));
+    SmallVector<char, 128> DiagMessage;
+    Diag->second.EmitToString(DE, DiagMessage);
+    return llvm::make_error<llvm::StringError>(DiagMessage,
+                                               llvm::inconvertibleErrorCode());
+  }
+  return Err;
+}
+
 class RefactoringResultCollector final
     : public tooling::RefactoringResultConsumer {
 public:
   void handleError(llvm::Error Err) override {
     assert(!Result.hasValue());
-    // FIXME: figure out a way to return better message for DiagnosticError.
-    // clangd uses llvm::toString to convert the Err to string, however, for
-    // DiagnosticError, only "clang diagnostic" will be generated.
     Result = std::move(Err);
   }
 
@@ -301,13 +311,15 @@
     auto Rename = clang::tooling::RenameOccurrences::initiate(
         Context, SourceRange(SourceLocationBeg), NewName);
     if (!Rename)
-      return CB(Rename.takeError());
+      return CB(expandDiagnostics(Rename.takeError(),
+                                  AST.getASTContext().getDiagnostics()));
 
     Rename->invoke(ResultCollector, Context);
 
     assert(ResultCollector.Result.hasValue());
     if (!ResultCollector.Result.getValue())
-      return CB(ResultCollector.Result->takeError());
+      return CB(expandDiagnostics(ResultCollector.Result->takeError(),
+                                  AST.getASTContext().getDiagnostics()));
 
     std::vector<TextEdit> Replacements;
     for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits