rwols updated this revision to Diff 124307.
rwols added a comment.
- Rebase to latest upstream revision.
- Go all-in with TextEdit, even down to ClangdUnit.cpp.
- Move FixItsMap to ClangdServer. ClangdLSPServer is much cleaner now.
- Remove the cached FixIts when the client closes the document.
- Fix (or introduce bug?) wrong conversion between clang line/columns and LSP
line/columns. From what I understand, clang line/cols are 1-based, and LSP
line/cols are 0-based. The line numbers were already converted correctly
(subtracting 1 when going from clang line numbers to LSP line numbers), but the
column numbers were not converted correctly. Consequently, a bunch of tests had
to be adjusted for the correct (or wrong?) column numbers.
https://reviews.llvm.org/D39430
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
test/clangd/diagnostics.test
test/clangd/execute-command.test
test/clangd/extra-flags.test
test/clangd/fixits.test
Index: test/clangd/fixits.test
===================================================================
--- test/clangd/fixits.test
+++ test/clangd/fixits.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "using the result of an assignment as a condition without parentheses",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "place parentheses around the assignment to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -43,11 +43,11 @@
# CHECK-NEXT: "message": "use '==' to turn this assignment into an equality comparison",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -58,7 +58,7 @@
# CHECK-NEXT: }
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": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
# CHECK: "id": 2,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": [
@@ -109,7 +109,7 @@
# CHECK-NEXT: "newText": "==",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
@@ -128,7 +128,7 @@
# CHECK-NEXT: ]
Content-Length: 771
-{"jsonrpc":"2.0","id":3,"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": 35}, "end": {"line": 0, "character": 35}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
# Make sure unused "code" and "source" fields ignored gracefully
# CHECK: "id": 3,
# CHECK-NEXT: "jsonrpc": "2.0",
@@ -180,7 +180,7 @@
# CHECK-NEXT: "newText": "==",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
Index: test/clangd/extra-flags.test
===================================================================
--- test/clangd/extra-flags.test
+++ test/clangd/extra-flags.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "variable 'i' is uninitialized when used here",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "initialize the variable 'i' to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -52,11 +52,11 @@
# CHECK-NEXT: "message": "variable 'i' is uninitialized when used here",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -66,11 +66,11 @@
# CHECK-NEXT: "message": "initialize the variable 'i' to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: test/clangd/execute-command.test
===================================================================
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "using the result of an assignment as a condition without parentheses",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "place parentheses around the assignment to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -43,11 +43,11 @@
# CHECK-NEXT: "message": "use '==' to turn this assignment into an equality comparison",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: test/clangd/diagnostics.test
===================================================================
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "return type of 'main' is not 'int'",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "change return type to 'int'",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -17,6 +17,7 @@
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/YAMLParser.h"
#include <iosfwd>
#include <mutex>
@@ -44,6 +45,9 @@
/// Unlike other methods of JSONOutput, mirrorInput is not thread-safe.
void mirrorInput(const Twine &Message);
+ /// Send a notification to the client.
+ void notify(llvm::StringRef Method, json::Expr &&Params);
+
private:
llvm::raw_ostream &Outs;
llvm::raw_ostream &Logs;
@@ -64,11 +68,13 @@
SPAN_ATTACH(tracer(), "ID", *this->ID);
}
- /// Sends a successful reply.
+ /// Send a response to a request from the client.
void reply(json::Expr &&Result);
- /// Sends an error response to the client, and logs it.
+ /// Send an error response to the client, and logs it.
void replyError(ErrorCode code, const llvm::StringRef &Message);
- /// Sends a request to the client.
+ /// Send all error messages contained in Err to the client, and log them.
+ void replyError(llvm::Error Err);
+ /// Send a request to the client.
void call(llvm::StringRef Method, json::Expr &&Params);
trace::Span &tracer() { return *Tracer; }
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -39,6 +39,14 @@
Outs.flush();
}
+void JSONOutput::notify(StringRef Method, json::Expr &&Params) {
+ writeMessage(json::obj{
+ {"jsonrpc", "2.0"},
+ {"method", Method},
+ {"params", std::move(Params)},
+ });
+}
+
void JSONOutput::log(const Twine &Message) {
trace::log(Message);
std::lock_guard<std::mutex> Guard(StreamMutex);
@@ -83,6 +91,11 @@
}
}
+void RequestContext::replyError(llvm::Error Err) {
+ const auto Message = llvm::toString(std::move(Err));
+ replyError(ErrorCode::UnknownErrorCode, Message);
+}
+
void RequestContext::call(StringRef Method, json::Expr &&Params) {
// FIXME: Generate/Increment IDs for every request so that we can get proper
// replies once we need to.
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -45,7 +45,7 @@
/// A diagnostic with its FixIts.
struct DiagWithFixIts {
clangd::Diagnostic Diag;
- llvm::SmallVector<tooling::Replacement, 1> FixIts;
+ llvm::SmallVector<TextEdit, 1> FixIts;
};
// Stores Preamble and associated data.
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -158,21 +158,41 @@
return Result;
}
+/// Convert a SourceLocation to a Position
+Position SourceLocToPosition(const SourceManager &SrcMgr,
+ SourceLocation Location) {
+ Position P;
+ // LSP rows and columns are 0-based, clang rows and columns are 1-based.
+ P.line = static_cast<int>(SrcMgr.getSpellingLineNumber(Location)) - 1;
+ P.character = static_cast<int>(SrcMgr.getSpellingColumnNumber(Location)) - 1;
+ assert(P.line >= 0 && "Unexpected negative value for P.line");
+ assert(P.character >= 0 && "Unepxected negative value for P.character");
+ return P;
+}
+
+/// Convert a FullSourceLoc to a Position
+Position SourceLocToPosition(const FullSourceLoc &Location) {
+ return SourceLocToPosition(Location.getManager(), Location);
+}
+
llvm::Optional<DiagWithFixIts> toClangdDiag(const StoredDiagnostic &D) {
- auto Location = D.getLocation();
- if (!Location.isValid() || !Location.getManager().isInMainFile(Location))
+ const auto &Location = D.getLocation();
+ const auto &SrcMgr = Location.getManager();
+ if (!Location.isValid() || !SrcMgr.isInMainFile(Location))
return llvm::None;
- Position P;
- P.line = Location.getSpellingLineNumber() - 1;
- P.character = Location.getSpellingColumnNumber();
+ const auto P = SourceLocToPosition(Location);
Range R = {P, P};
clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()};
- llvm::SmallVector<tooling::Replacement, 1> FixItsForDiagnostic;
+ llvm::SmallVector<TextEdit, 1> FixItsForDiagnostic;
for (const FixItHint &Fix : D.getFixIts()) {
- FixItsForDiagnostic.push_back(clang::tooling::Replacement(
- Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert));
+ Range ReplacementRange;
+ ReplacementRange.start =
+ SourceLocToPosition(SrcMgr, Fix.RemoveRange.getBegin());
+ ReplacementRange.end =
+ SourceLocToPosition(SrcMgr, Fix.RemoveRange.getEnd());
+ FixItsForDiagnostic.push_back({ReplacementRange, Fix.CodeToInsert});
}
return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)};
}
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -284,16 +284,24 @@
/// given a header file and vice versa.
llvm::Optional<Path> switchSourceHeader(PathRef Path);
- /// Run formatting for \p Rng inside \p File.
- std::vector<tooling::Replacement> formatRange(PathRef File, Range Rng);
- /// Run formatting for the whole \p File.
- std::vector<tooling::Replacement> formatFile(PathRef File);
- /// Run formatting after a character was typed at \p Pos in \p File.
- std::vector<tooling::Replacement> formatOnType(PathRef File, Position Pos);
+ /// Get "FixIt" replacements for the given diagnostic.
+ llvm::SmallVector<TextEdit, 1> getFixIts(StringRef File,
+ const clangd::Diagnostic &D);
+
+ /// Run formatting for \p Rng inside \p File with content \p Code.
+ llvm::Expected<std::vector<TextEdit>> formatRange(PathRef File, Range Rng);
/// Rename all occurrences of the symbol at the \p Pos in \p File to
/// \p NewName.
- Expected<std::vector<tooling::Replacement>> rename(PathRef File, Position Pos,
- llvm::StringRef NewName);
+ Expected<std::vector<TextEdit>> rename(PathRef File, Position Pos,
+ llvm::StringRef NewName);
+
+ /// Run formatting for the whole \p File with content \p Code.
+ llvm::Expected<std::vector<TextEdit>> formatFile(PathRef File);
+
+ /// Run formatting after a character was typed at \p Pos in \p File with
+ /// content \p Code.
+ llvm::Expected<std::vector<TextEdit>> formatOnType(PathRef File,
+ Position Pos);
/// Gets current document contents for \p File. \p File must point to a
/// currently tracked file.
@@ -309,6 +317,12 @@
void onFileEvent(const DidChangeWatchedFilesParams &Params);
private:
+ /// FIXME: This stats several files to find a .clang-format file. I/O can be
+ /// slow. Think of a way to cache this.
+ llvm::Expected<std::vector<TextEdit>>
+ formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef<tooling::Range> Ranges);
+
std::future<void>
scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
std::shared_ptr<CppFile> Resources,
@@ -334,6 +348,11 @@
std::mutex DiagnosticsMutex;
/// Maps from a filename to the latest version of reported diagnostics.
llvm::StringMap<DocVersion> ReportedDiagnosticVersions;
+ std::mutex FixItsMutex;
+ typedef std::map<clangd::Diagnostic, llvm::SmallVector<TextEdit, 1>>
+ DiagnosticToTextEditMap;
+ /// Caches FixIts per file and diagnostics
+ llvm::StringMap<DiagnosticToTextEditMap> FixItsMap;
// WorkScheduler has to be the last member, because its destructor has to be
// called before all other members to stop the worker thread that references
// ClangdServer
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -28,6 +28,35 @@
namespace {
+TextEdit replacementToEdit(StringRef Code,
+ const tooling::Replacement &Replacement) {
+ TextEdit Edit;
+ Edit.range = {offsetToPosition(Code, Replacement.getOffset()),
+ offsetToPosition(Code, Replacement.getOffset() +
+ Replacement.getLength())};
+ Edit.newText = Replacement.getReplacementText();
+ return Edit;
+}
+
+template <class ForwardIterator>
+std::vector<TextEdit> replacementsToEdits(StringRef Code, ForwardIterator Begin,
+ ForwardIterator End) {
+ // Turn the replacements into the format specified by the Language Server
+ // Protocol. Fuse them into one big JSON array.
+ std::vector<TextEdit> Edits;
+ Edits.reserve(std::distance(Begin, End));
+ while (Begin != End) {
+ Edits.emplace_back(replacementToEdit(Code, *Begin));
+ ++Begin;
+ }
+ return Edits;
+}
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+ tooling::Replacements Replacements) {
+ return replacementsToEdits(Code, Replacements.begin(), Replacements.end());
+}
+
class FulfillPromiseGuard {
public:
FulfillPromiseGuard(std::promise<void> &Promise) : Promise(Promise) {}
@@ -38,16 +67,6 @@
std::promise<void> &Promise;
};
-std::vector<tooling::Replacement> formatCode(StringRef Code, StringRef Filename,
- ArrayRef<tooling::Range> Ranges) {
- // Call clang-format.
- // FIXME: Don't ignore style.
- format::FormatStyle Style = format::getLLVMStyle();
- auto Result = format::reformat(Style, Code, Ranges, Filename);
-
- return std::vector<tooling::Replacement>(Result.begin(), Result.end());
-}
-
std::string getStandardResourceDir() {
static int Dummy; // Just an address in this process.
return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
@@ -204,6 +223,7 @@
std::future<void> ClangdServer::removeDocument(PathRef File) {
DraftMgr.removeDraft(File);
std::shared_ptr<CppFile> Resources = Units.removeIfPresent(File);
+ FixItsMap.erase(File);
return scheduleCancelRebuild(std::move(Resources));
}
@@ -329,37 +349,36 @@
return make_tagged(std::move(Result), TaggedFS.Tag);
}
-std::vector<tooling::Replacement> ClangdServer::formatRange(PathRef File,
- Range Rng) {
- std::string Code = getDocument(File);
-
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatRange(PathRef File,
+ Range Rng) {
+ const auto Code = getDocument(File);
size_t Begin = positionToOffset(Code, Rng.start);
size_t Len = positionToOffset(Code, Rng.end) - Begin;
return formatCode(Code, File, {tooling::Range(Begin, Len)});
}
-std::vector<tooling::Replacement> ClangdServer::formatFile(PathRef File) {
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatFile(PathRef File) {
// Format everything.
- std::string Code = getDocument(File);
+ const auto Code = getDocument(File);
return formatCode(Code, File, {tooling::Range(0, Code.size())});
}
-std::vector<tooling::Replacement> ClangdServer::formatOnType(PathRef File,
- Position Pos) {
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatOnType(PathRef File,
+ Position Pos) {
+ const auto Code = getDocument(File);
// Look for the previous opening brace from the character position and
// format starting from there.
- std::string Code = getDocument(File);
size_t CursorPos = positionToOffset(Code, Pos);
- size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
+ size_t PreviousLBracePos = Code.find_last_of('{', CursorPos);
if (PreviousLBracePos == StringRef::npos)
PreviousLBracePos = CursorPos;
size_t Len = 1 + CursorPos - PreviousLBracePos;
return formatCode(Code, File, {tooling::Range(PreviousLBracePos, Len)});
}
-Expected<std::vector<tooling::Replacement>>
-ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName) {
+Expected<std::vector<TextEdit>> ClangdServer::rename(PathRef File, Position Pos,
+ llvm::StringRef NewName) {
std::string Code = getDocument(File);
std::shared_ptr<CppFile> Resources = Units.getFile(File);
RefactoringResultCollector ResultCollector;
@@ -386,7 +405,7 @@
if (!ResultCollector.Result.getValue())
return ResultCollector.Result->takeError();
- std::vector<tooling::Replacement> Replacements;
+ std::vector<TextEdit> Edits;
for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
tooling::Replacements ChangeReps = Change.getReplacements();
for (const auto &Rep : ChangeReps) {
@@ -400,10 +419,10 @@
// * rename globally in project
// * rename in open files
if (Rep.getFilePath() == File)
- Replacements.push_back(Rep);
+ Edits.push_back(replacementToEdit(Code, Rep));
}
}
- return Replacements;
+ return Edits;
}
std::string ClangdServer::getDocument(PathRef File) {
@@ -507,6 +526,36 @@
return llvm::None;
}
+llvm::SmallVector<TextEdit, 1>
+ClangdServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
+ std::lock_guard<std::mutex> Lock(FixItsMutex);
+ auto DiagToFixItsIter = FixItsMap.find(File);
+ if (DiagToFixItsIter == FixItsMap.end())
+ return {};
+
+ const auto &DiagToFixItsMap = DiagToFixItsIter->second;
+ auto FixItsIter = DiagToFixItsMap.find(D);
+ if (FixItsIter == DiagToFixItsMap.end())
+ return {};
+
+ return FixItsIter->second;
+}
+
+llvm::Expected<std::vector<TextEdit>>
+ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef<tooling::Range> Ranges) {
+ // Call clang-format.
+ auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+ auto StyleOrError =
+ format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
+ if (!StyleOrError) {
+ return StyleOrError.takeError();
+ } else {
+ return replacementsToEdits(
+ Code, format::reformat(StyleOrError.get(), Code, Ranges, File));
+ }
+}
+
std::future<void> ClangdServer::scheduleReparseAndDiags(
PathRef File, VersionedDraft Contents, std::shared_ptr<CppFile> Resources,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
@@ -548,6 +597,14 @@
return;
LastReportedDiagsVersion = Version;
+ // Cache FixIts for when the client requests textDocument/codeAction.
+ DiagnosticToTextEditMap M;
+ for (const auto& Diag : *Diags) M.emplace(Diag.Diag, Diag.FixIts);
+ {
+ std::unique_lock<std::mutex> Guard(FixItsMutex);
+ FixItsMap[FileStr] = M;
+ }
+
DiagConsumer.onDiagnosticsReady(FileStr,
make_tagged(std::move(*Diags), Tag));
};
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -86,12 +86,6 @@
/// It's used to break out of the LSP parsing loop.
bool IsDone = false;
- std::mutex FixItsMutex;
- typedef std::map<clangd::Diagnostic, std::vector<clang::tooling::Replacement>>
- DiagnosticToReplacementMap;
- /// Caches FixIts per file and diagnostics
- llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
-
// Various ClangdServer parameters go here. It's important they're created
// before ClangdServer.
DirectoryBasedGlobalCompilationDatabase CDB;
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -15,25 +15,6 @@
using namespace clang::clangd;
using namespace clang;
-namespace {
-
-std::vector<TextEdit>
-replacementsToEdits(StringRef Code,
- const std::vector<tooling::Replacement> &Replacements) {
- // Turn the replacements into the format specified by the Language Server
- // Protocol. Fuse them into one big JSON array.
- std::vector<TextEdit> Edits;
- for (auto &R : Replacements) {
- Range ReplacementRange = {
- offsetToPosition(Code, R.getOffset()),
- offsetToPosition(Code, R.getOffset() + R.getLength())};
- Edits.push_back({ReplacementRange, R.getReplacementText()});
- }
- return Edits;
-}
-
-} // namespace
-
void ClangdLSPServer::onInitialize(Ctx C, InitializeParams &Params) {
C.reply(json::obj{
{{"capabilities",
@@ -130,16 +111,13 @@
void ClangdLSPServer::onRename(Ctx C, RenameParams &Params) {
auto File = Params.textDocument.uri.file;
- auto Replacements = Server.rename(File, Params.position, Params.newName);
- if (!Replacements) {
- C.replyError(ErrorCode::InternalError,
- llvm::toString(Replacements.takeError()));
+ auto Edits = Server.rename(File, Params.position, Params.newName);
+ if (!Edits) {
+ C.replyError(ErrorCode::InternalError, llvm::toString(Edits.takeError()));
return;
}
- std::string Code = Server.getDocument(File);
- std::vector<TextEdit> Edits = replacementsToEdits(Code, *Replacements);
WorkspaceEdit WE;
- WE.changes = {{llvm::yaml::escape(Params.textDocument.uri.uri), Edits}};
+ WE.changes = {{llvm::yaml::escape(Params.textDocument.uri.uri), *Edits}};
C.reply(WorkspaceEdit::unparse(WE));
}
@@ -150,41 +128,48 @@
void ClangdLSPServer::onDocumentOnTypeFormatting(
Ctx C, DocumentOnTypeFormattingParams &Params) {
- auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- C.reply(json::ary(
- replacementsToEdits(Code, Server.formatOnType(File, Params.position))));
+ const PathRef File = Params.textDocument.uri.file;
+ auto EditsOrError = Server.formatOnType(File, Params.position);
+ if (EditsOrError)
+ C.reply(json::ary{EditsOrError.get()});
+ else
+ C.replyError(EditsOrError.takeError());
}
void ClangdLSPServer::onDocumentRangeFormatting(
Ctx C, DocumentRangeFormattingParams &Params) {
- auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- C.reply(json::ary(
- replacementsToEdits(Code, Server.formatRange(File, Params.range))));
+ const PathRef File = Params.textDocument.uri.file;
+ auto EditsOrError = Server.formatRange(File, Params.range);
+ if (EditsOrError)
+ C.reply(json::ary{EditsOrError.get()});
+ else
+ C.replyError(EditsOrError.takeError());
}
void ClangdLSPServer::onDocumentFormatting(Ctx C,
DocumentFormattingParams &Params) {
- auto File = Params.textDocument.uri.file;
- std::string Code = Server.getDocument(File);
- C.reply(json::ary(replacementsToEdits(Code, Server.formatFile(File))));
+ const PathRef File = Params.textDocument.uri.file;
+ auto EditsOrError = Server.formatFile(File);
+ if (EditsOrError)
+ C.reply(json::ary{EditsOrError.get()});
+ else
+ C.replyError(EditsOrError.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);
json::ary Commands;
- for (Diagnostic &D : Params.context.diagnostics) {
- std::vector<clang::tooling::Replacement> Fixes =
- getFixIts(Params.textDocument.uri.file, D);
- auto Edits = replacementsToEdits(Code, Fixes);
- if (!Edits.empty()) {
+ for (const auto &Diag : Params.context.diagnostics) {
+ const auto ServerEdits =
+ Server.getFixIts(Params.textDocument.uri.file, Diag);
+ if (!ServerEdits.empty()) {
WorkspaceEdit WE;
- WE.changes = {{Params.textDocument.uri.uri, std::move(Edits)}};
+ WE.changes = {
+ {Params.textDocument.uri.uri,
+ std::vector<TextEdit>{ServerEdits.begin(), ServerEdits.end()}}};
Commands.push_back(json::obj{
- {"title", llvm::formatv("Apply FixIt {0}", D.message)},
+ {"title", llvm::formatv("Apply FixIt {0}", Diag.message)},
{"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
{"arguments", {WE}},
});
@@ -264,54 +249,21 @@
return ShutdownRequestReceived;
}
-std::vector<clang::tooling::Replacement>
-ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
- std::lock_guard<std::mutex> Lock(FixItsMutex);
- auto DiagToFixItsIter = FixItsMap.find(File);
- if (DiagToFixItsIter == FixItsMap.end())
- return {};
-
- const auto &DiagToFixItsMap = DiagToFixItsIter->second;
- auto FixItsIter = DiagToFixItsMap.find(D);
- if (FixItsIter == DiagToFixItsMap.end())
- return {};
-
- return FixItsIter->second;
-}
-
void ClangdLSPServer::onDiagnosticsReady(
PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) {
json::ary DiagnosticsJSON;
- DiagnosticToReplacementMap LocalFixIts; // Temporary storage
for (auto &DiagWithFixes : Diagnostics.Value) {
auto Diag = DiagWithFixes.Diag;
DiagnosticsJSON.push_back(json::obj{
{"range", Diag.range},
{"severity", Diag.severity},
{"message", Diag.message},
});
- // We convert to Replacements to become independent of the SourceManager.
- auto &FixItsForDiagnostic = LocalFixIts[Diag];
- std::copy(DiagWithFixes.FixIts.begin(), DiagWithFixes.FixIts.end(),
- std::back_inserter(FixItsForDiagnostic));
- }
-
- // Cache FixIts
- {
- // FIXME(ibiryukov): should be deleted when documents are removed
- std::lock_guard<std::mutex> Lock(FixItsMutex);
- FixItsMap[File] = LocalFixIts;
}
- // Publish diagnostics.
- Out.writeMessage(json::obj{
- {"jsonrpc", "2.0"},
- {"method", "textDocument/publishDiagnostics"},
- {"params",
- json::obj{
- {"uri", URI::fromFile(File)},
- {"diagnostics", std::move(DiagnosticsJSON)},
- }},
- });
+ // Publish diagnostics by sending an LSP notification to the client.
+ Out.notify("textDocument/publishDiagnostics",
+ json::obj{{"uri", URI::fromFile(File)},
+ {"diagnostics", std::move(DiagnosticsJSON)}});
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
