[clangd] Report an error on findDefinitions/signatureHelp on an unopened file instead of crashing.

Found by clangd-fuzzer.

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@316659 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 45 insertions, 29 deletions

diff --git a/clangd/ClangdLSPServer.cpp b/clangd/ClangdLSPServer.cpp
index 3b68f2a6..1689a5fd 100644
--- a/clangd/ClangdLSPServer.cpp
+++ b/clangd/ClangdLSPServer.cpp
@@ -160,24 +160,24 @@ void ClangdLSPServer::onCompletion(Ctx C, TextDocumentPositionParams &Params) {
 
 void ClangdLSPServer::onSignatureHelp(Ctx C,
                                       TextDocumentPositionParams &Params) {
-  C.reply(SignatureHelp::unparse(
-      Server
-          .signatureHelp(
-              Params.textDocument.uri.file,
-              Position{Params.position.line, Params.position.character})
-          .Value));
+  auto SignatureHelp = Server.signatureHelp(
+      Params.textDocument.uri.file,
+      Position{Params.position.line, Params.position.character});
+  if (!SignatureHelp)
+    return C.replyError(-32602, llvm::toString(SignatureHelp.takeError()));
+  C.reply(SignatureHelp::unparse(SignatureHelp->Value));
 }
 
 void ClangdLSPServer::onGoToDefinition(Ctx C,
                                        TextDocumentPositionParams &Params) {
-  auto Items = Server
-                   .findDefinitions(Params.textDocument.uri.file,
-                                    Position{Params.position.line,
-                                             Params.position.character})
-                   .Value;
+  auto Items = Server.findDefinitions(
+      Params.textDocument.uri.file,
+      Position{Params.position.line, Params.position.character});
+  if (!Items)
+    return C.replyError(-32602, llvm::toString(Items.takeError()));
 
   std::string Locations;
-  for (const auto &Item : Items) {
+  for (const auto &Item : Items->Value) {
     Locations += Location::unparse(Item);
     Locations += ",";
   }
diff --git a/clangd/ClangdServer.cpp b/clangd/ClangdServer.cpp
index 9338cb1b..528b6eba 100644
--- a/clangd/ClangdServer.cpp
+++ b/clangd/ClangdServer.cpp
@@ -13,6 +13,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
@@ -266,15 +267,17 @@ void ClangdServer::codeComplete(
   WorkScheduler.addToFront(std::move(Task), std::move(Callback));
 }
 
-Tagged<SignatureHelp>
+llvm::Expected<Tagged<SignatureHelp>>
 ClangdServer::signatureHelp(PathRef File, Position Pos,
                             llvm::Optional<StringRef> OverridenContents,
                             IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
   std::string DraftStorage;
   if (!OverridenContents) {
     auto FileContents = DraftMgr.getDraft(File);
-    assert(FileContents.Draft &&
-           "signatureHelp is called for non-added document");
+    if (!FileContents.Draft)
+      return llvm::make_error<llvm::StringError>(
+          "signatureHelp is called for non-added document",
+          llvm::errc::invalid_argument);
 
     DraftStorage = std::move(*FileContents.Draft);
     OverridenContents = DraftStorage;
@@ -285,7 +288,10 @@ ClangdServer::signatureHelp(PathRef File, Position Pos,
     *UsedFS = TaggedFS.Value;
 
   std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  assert(Resources && "Calling signatureHelp on non-added file");
+  if (!Resources)
+    return llvm::make_error<llvm::StringError>(
+        "signatureHelp is called for non-added document",
+        llvm::errc::invalid_argument);
 
   auto Preamble = Resources->getPossiblyStalePreamble();
   auto Result = clangd::signatureHelp(File, Resources->getCompileCommand(),
@@ -347,16 +353,15 @@ std::string ClangdServer::dumpAST(PathRef File) {
   return Result;
 }
 
-Tagged<std::vector<Location>> ClangdServer::findDefinitions(PathRef File,
-                                                            Position Pos) {
-  auto FileContents = DraftMgr.getDraft(File);
-  assert(FileContents.Draft &&
-         "findDefinitions is called for non-added document");
-
+llvm::Expected<Tagged<std::vector<Location>>>
+ClangdServer::findDefinitions(PathRef File, Position Pos) {
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
 
   std::shared_ptr<CppFile> Resources = Units.getFile(File);
-  assert(Resources && "Calling findDefinitions on non-added file");
+  if (!Resources)
+    return llvm::make_error<llvm::StringError>(
+        "findDefinitions called on non-added file",
+        llvm::errc::invalid_argument);
 
   std::vector<Location> Result;
   Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
diff --git a/clangd/ClangdServer.h b/clangd/ClangdServer.h
index f70fb5c8..1eddfd9e 100644
--- a/clangd/ClangdServer.h
+++ b/clangd/ClangdServer.h
@@ -271,13 +271,14 @@ public:
   /// will be used. If \p UsedFS is non-null, it will be overwritten by
   /// vfs::FileSystem used for signature help. This method should only be called
   /// for currently tracked files.
-  Tagged<SignatureHelp>
+  llvm::Expected<Tagged<SignatureHelp>>
   signatureHelp(PathRef File, Position Pos,
                 llvm::Optional<StringRef> OverridenContents = llvm::None,
                 IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
 
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
-  Tagged<std::vector<Location>> findDefinitions(PathRef File, Position Pos);
+  llvm::Expected<Tagged<std::vector<Location>>> findDefinitions(PathRef File,
+                                                                Position Pos);
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
diff --git a/test/clangd/definitions.test b/test/clangd/definitions.test
index 7d09c74b..c52e5dec 100644
--- a/test/clangd/definitions.test
+++ b/test/clangd/definitions.test
@@ -163,10 +163,15 @@ Content-Length: 148
 # Go to macro, being undefined

 # CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 2, "character": 8}, "end": {"line": 2, "character": 13}}}]}

 

-Content-Length: 44

+Content-Length: 156

 

-{"jsonrpc":"2.0","id":3,"method":"shutdown"}

-# CHECK: {"jsonrpc":"2.0","id":3,"result":null}

+{"jsonrpc":"2.0","id":2,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///doesnotexist.cpp"},"position":{"line":4,"character":7}}}

+# CHECK: {"jsonrpc":"2.0","id":2,"error":{"code":-32602,"message":"findDefinitions called on non-added file"}}

+

+Content-Length: 48

+

+{"jsonrpc":"2.0","id":10000,"method":"shutdown"}

+# CHECK: {"jsonrpc":"2.0","id":10000,"result":null}

 Content-Length: 33

 

 {"jsonrpc":"2.0":"method":"exit"}

diff --git a/test/clangd/signature-help.test b/test/clangd/signature-help.test
index fc3b070d..c28a309f 100644
--- a/test/clangd/signature-help.test
+++ b/test/clangd/signature-help.test
@@ -36,6 +36,11 @@ Content-Length: 151
 # CHECK-DAG: {"label":"bar(float x = 0, int y = 42) -> void","parameters":[{"label":"float x = 0"},{"label":"int y = 42"}]}

 # CHECK: ]}

 

+Content-Length: 159

+

+{"jsonrpc":"2.0","id":3,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"file:///doesnotexist.cpp"},"position":{"line":8,"character":9}}}

+# CHECK: {"jsonrpc":"2.0","id":3,"error":{"code":-32602,"message":"signatureHelp is called for non-added document"}}

+

 # Shutdown.

 Content-Length: 49

 

diff --git a/unittests/clangd/ClangdTests.cpp b/unittests/clangd/ClangdTests.cpp
index 5a182afe..06501553 100644
--- a/unittests/clangd/ClangdTests.cpp
+++ b/unittests/clangd/ClangdTests.cpp
@@ -1056,7 +1056,7 @@ int d;
         AddDocument(FileIndex);
 
       Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
-      Server.findDefinitions(FilePaths[FileIndex], Pos);
+      ASSERT_TRUE(!!Server.findDefinitions(FilePaths[FileIndex], Pos));
     };
 
     std::vector<std::function<void()>> AsyncRequests = {