ilya-biryukov updated this revision to Diff 130127.
ilya-biryukov added a comment.
- Do the check in rename before calling into ClangdServer
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42164
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
test/clangd/crash-non-added-files.test
Index: test/clangd/crash-non-added-files.test
===================================================================
--- /dev/null
+++ test/clangd/crash-non-added-files.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
@@ -143,14 +143,19 @@
void ClangdLSPServer::onRename(Ctx C, RenameParams &Params) {
auto File = Params.textDocument.uri.file;
+ auto Code = Server.getDocument(File);
+ if (!Code)
+ return replyError(C, ErrorCode::InvalidParams,
+ "onRename called for non-added file");
+
auto Replacements = Server.rename(C, File, Params.position, Params.newName);
if (!Replacements) {
replyError(C, ErrorCode::InternalError,
llvm::toString(Replacements.takeError()));
return;
}
- std::string Code = Server.getDocument(File);
- std::vector<TextEdit> Edits = replacementsToEdits(Code, *Replacements);
+
+ 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
