ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric, hokein.
Herald added a subscriber: klimek.
We will return errors for non-added files for now.
Another alternative for clangd would be to read non-added files from
disk and provide useful features anyway.
There are still some cases that fail with assertion (e.g., code
complete). We should address those too, but they require more subtle
changes to the code and therefore out of scope of this patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42164
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
test/clangd/crash.test
Index: test/clangd/crash.test
===================================================================
--- /dev/null
+++ test/clangd/crash.test
@@ -0,0 +1,45 @@
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+#
+Content-Length: 746
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32602
+# CHECK-NEXT: "message": "onCodeAction called for non-added file"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 2,
+Content-Length: 233
+
+{"jsonrpc":"2.0","id":3,"method":"textDocument/rangeFormatting","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":1,"character":4},"end":{"line":1,"character":12}},"options":{"tabSize":4,"insertSpaces":true}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32602
+# CHECK-NEXT: "message": "onDocumentRangeFormatting called for non-added file"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 3,
+Content-Length: 153
+
+{"jsonrpc":"2.0","id":4,"method":"textDocument/formatting","params":{"textDocument":{"uri":"file:///foo.c"},"options":{"tabSize":4,"insertSpaces":true}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32602
+# CHECK-NEXT: "message": "onDocumentFormatting called for non-added file"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 4,
+Content-Length: 204
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/onTypeFormatting","params":{"textDocument":{"uri":"file:///foo.c"},"position":{"line":3,"character":1},"ch":"}","options":{"tabSize":4,"insertSpaces":true}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32602
+# CHECK-NEXT: "message": "onDocumentOnTypeFormatting called for non-added file"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 5,
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":6,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -308,11 +308,11 @@
PathRef File, Position Pos,
llvm::StringRef NewName);
- /// Gets current document contents for \p File. \p File must point to a
- /// currently tracked file.
+ /// Gets current document contents for \p File. Returns None if \p File is not
+ /// currently tracked.
/// FIXME(ibiryukov): This function is here to allow offset-to-Position
/// conversions in outside code, maybe there's a way to get rid of it.
- std::string getDocument(PathRef File);
+ llvm::Optional<std::string> getDocument(PathRef File);
/// Only for testing purposes.
/// Waits until all requests to worker thread are finished and dumps AST for
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -356,7 +356,6 @@
Expected<std::vector<tooling::Replacement>>
ClangdServer::rename(const Context &Ctx, PathRef File, Position Pos,
llvm::StringRef NewName) {
- std::string Code = getDocument(File);
std::shared_ptr<CppFile> Resources = Units.getFile(File);
RefactoringResultCollector ResultCollector;
Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) {
@@ -402,15 +401,17 @@
return Replacements;
}
-std::string ClangdServer::getDocument(PathRef File) {
- auto draft = DraftMgr.getDraft(File);
- assert(draft.Draft && "File is not tracked, cannot get contents");
- return *draft.Draft;
+llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
+ auto Latest = DraftMgr.getDraft(File);
+ if (!Latest.Draft)
+ return llvm::None;
+ return std::move(*Latest.Draft);
}
std::string ClangdServer::dumpAST(PathRef File) {
std::shared_ptr<CppFile> Resources = Units.getFile(File);
- assert(Resources && "dumpAST is called for non-added document");
+ if (!Resources)
+ return "<non-added file>";
std::string Result;
Resources->getAST().get()->runUnderLock([&Result](ParsedAST *AST) {
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -149,8 +149,13 @@
llvm::toString(Replacements.takeError()));
return;
}
- std::string Code = Server.getDocument(File);
- std::vector<TextEdit> Edits = replacementsToEdits(Code, *Replacements);
+
+ auto Code = Server.getDocument(File);
+ if (!Code)
+ return replyError(C, ErrorCode::InternalError,
+ "onRename called for non-added file");
+
+ std::vector<TextEdit> Edits = replacementsToEdits(*Code, *Replacements);
WorkspaceEdit WE;
WE.changes = {{Params.textDocument.uri.uri, Edits}};
reply(C, WE);
@@ -164,43 +169,59 @@
void ClangdLSPServer::onDocumentOnTypeFormatting(
Ctx C, DocumentOnTypeFormattingParams &Params) {
auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- auto ReplacementsOrError = Server.formatOnType(Code, File, Params.position);
+ auto Code = Server.getDocument(File);
+ if (!Code)
+ return replyError(C, ErrorCode::InvalidParams,
+ "onDocumentOnTypeFormatting called for non-added file");
+
+ auto ReplacementsOrError = Server.formatOnType(*Code, File, Params.position);
if (ReplacementsOrError)
- reply(C, json::ary(replacementsToEdits(Code, ReplacementsOrError.get())));
+ reply(C, json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
else
replyError(C, ErrorCode::UnknownErrorCode,
llvm::toString(ReplacementsOrError.takeError()));
}
void ClangdLSPServer::onDocumentRangeFormatting(
Ctx C, DocumentRangeFormattingParams &Params) {
auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- auto ReplacementsOrError = Server.formatRange(Code, File, Params.range);
+ auto Code = Server.getDocument(File);
+ if (!Code)
+ return replyError(C, ErrorCode::InvalidParams,
+ "onDocumentRangeFormatting called for non-added file");
+
+ auto ReplacementsOrError = Server.formatRange(*Code, File, Params.range);
if (ReplacementsOrError)
- reply(C, json::ary(replacementsToEdits(Code, ReplacementsOrError.get())));
+ reply(C, json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
else
replyError(C, ErrorCode::UnknownErrorCode,
llvm::toString(ReplacementsOrError.takeError()));
}
void ClangdLSPServer::onDocumentFormatting(Ctx C,
DocumentFormattingParams &Params) {
auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- auto ReplacementsOrError = Server.formatFile(Code, File);
+ auto Code = Server.getDocument(File);
+ if (!Code)
+ return replyError(C, ErrorCode::InvalidParams,
+ "onDocumentFormatting called for non-added file");
+
+ auto ReplacementsOrError = Server.formatFile(*Code, File);
if (ReplacementsOrError)
- reply(C, json::ary(replacementsToEdits(Code, ReplacementsOrError.get())));
+ reply(C, json::ary(replacementsToEdits(*Code, ReplacementsOrError.get())));
else
replyError(C, ErrorCode::UnknownErrorCode,
llvm::toString(ReplacementsOrError.takeError()));
}
void ClangdLSPServer::onCodeAction(Ctx C, CodeActionParams &Params) {
// We provide a code action for each diagnostic at the requested location
// which has FixIts available.
- std::string Code = Server.getDocument(Params.textDocument.uri.file);
+ auto Code = Server.getDocument(Params.textDocument.uri.file);
+ if (!Code)
+ return replyError(C, ErrorCode::InvalidParams,
+ "onCodeAction called for non-added file");
+
json::ary Commands;
for (Diagnostic &D : Params.context.diagnostics) {
auto Edits = getFixIts(Params.textDocument.uri.file, D);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
