Index: clangd/ASTManager.h =================================================================== --- clangd/ASTManager.h +++ clangd/ASTManager.h @@ -12,6 +12,8 @@ #include "DocumentStore.h" #include "JSONRPCDispatcher.h" +#include "Protocol.h" +#include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include #include @@ -29,17 +31,27 @@ class ASTManager : public DocumentStoreListener { public: - ASTManager(JSONOutput &Output, DocumentStore &Store); + ASTManager(JSONOutput &Output, DocumentStore &Store, bool RunSynchronously); ~ASTManager() override; void onDocumentAdd(StringRef Uri) override; // FIXME: Implement onDocumentRemove // FIXME: Implement codeComplete + /// Get the fixes associated with a certain diagnostic as replacements. + llvm::ArrayRef + getFixIts(const clangd::Diagnostic &D); + + DocumentStore &getStore() const { return Store; } + private: JSONOutput &Output; DocumentStore &Store; + // Set to true if requests should block instead of being processed + // asynchronously. + bool RunSynchronously; + /// Loads a compilation database for URI. May return nullptr if it fails. The /// database is cached for subsequent accesses. clang::tooling::CompilationDatabase * @@ -50,6 +62,7 @@ createASTUnitForFile(StringRef Uri, const DocumentStore &Docs); void runWorker(); + void parseFileAndPublishDiagnostics(StringRef File); /// Clang objects. llvm::StringMap> ASTs; @@ -57,6 +70,11 @@ CompilationDatabases; std::shared_ptr PCHs; + typedef std::map> + DiagnosticToReplacementMap; + DiagnosticToReplacementMap FixIts; + std::mutex FixItLock; + /// Queue of requests. std::deque RequestQueue; /// Setting Done to true will make the worker thread terminate. Index: clangd/ASTManager.cpp =================================================================== --- clangd/ASTManager.cpp +++ clangd/ASTManager.cpp @@ -54,8 +54,9 @@ llvm_unreachable("Unknown diagnostic level!"); } -ASTManager::ASTManager(JSONOutput &Output, DocumentStore &Store) - : Output(Output), Store(Store), +ASTManager::ASTManager(JSONOutput &Output, DocumentStore &Store, + bool RunSynchronously) + : Output(Output), Store(Store), RunSynchronously(RunSynchronously), PCHs(std::make_shared()), ClangWorker([this]() { runWorker(); }) {} @@ -67,9 +68,8 @@ std::unique_lock Lock(RequestLock); // Check if there's another request pending. We keep parsing until // our one-element queue is empty. - ClangRequestCV.wait(Lock, [this] { - return !RequestQueue.empty() || Done; - }); + ClangRequestCV.wait(Lock, + [this] { return !RequestQueue.empty() || Done; }); if (RequestQueue.empty() && Done) return; @@ -78,49 +78,67 @@ RequestQueue.pop_back(); } // unlock. - auto &Unit = ASTs[File]; // Only one thread can access this at a time. + parseFileAndPublishDiagnostics(File); + } +} - if (!Unit) { - Unit = createASTUnitForFile(File, this->Store); - } else { - // Do a reparse if this wasn't the first parse. - // FIXME: This might have the wrong working directory if it changed in the - // meantime. - Unit->Reparse(PCHs, getRemappedFiles(this->Store)); - } +void ASTManager::parseFileAndPublishDiagnostics(StringRef File) { + auto &Unit = ASTs[File]; // Only one thread can access this at a time. - if (!Unit) - continue; + if (!Unit) { + Unit = createASTUnitForFile(File, this->Store); + } else { + // Do a reparse if this wasn't the first parse. + // FIXME: This might have the wrong working directory if it changed in the + // meantime. + Unit->Reparse(PCHs, getRemappedFiles(this->Store)); + } - // Send the diagnotics to the editor. - // FIXME: If the diagnostic comes from a different file, do we want to - // show them all? Right now we drop everything not coming from the - // main file. - // FIXME: Send FixIts to the editor. - std::string Diagnostics; - for (ASTUnit::stored_diag_iterator D = Unit->stored_diag_begin(), - DEnd = Unit->stored_diag_end(); - D != DEnd; ++D) { - if (!D->getLocation().isValid() || - !D->getLocation().getManager().isInMainFile(D->getLocation())) - continue; - Position P; - P.line = D->getLocation().getSpellingLineNumber() - 1; - P.character = D->getLocation().getSpellingColumnNumber(); - Range R = {P, P}; - Diagnostics += - R"({"range":)" + Range::unparse(R) + - R"(,"severity":)" + std::to_string(getSeverity(D->getLevel())) + - R"(,"message":")" + llvm::yaml::escape(D->getMessage()) + - R"("},)"; + if (!Unit) + return; + + // Send the diagnotics to the editor. + // FIXME: If the diagnostic comes from a different file, do we want to + // show them all? Right now we drop everything not coming from the + // main file. + std::string Diagnostics; + DiagnosticToReplacementMap LocalFixIts; // Temporary storage + for (ASTUnit::stored_diag_iterator D = Unit->stored_diag_begin(), + DEnd = Unit->stored_diag_end(); + D != DEnd; ++D) { + if (!D->getLocation().isValid() || + !D->getLocation().getManager().isInMainFile(D->getLocation())) + continue; + Position P; + P.line = D->getLocation().getSpellingLineNumber() - 1; + P.character = D->getLocation().getSpellingColumnNumber(); + Range R = {P, P}; + Diagnostics += + R"({"range":)" + Range::unparse(R) + + R"(,"severity":)" + std::to_string(getSeverity(D->getLevel())) + + R"(,"message":")" + llvm::yaml::escape(D->getMessage()) + + R"("},)"; + + // We convert to Replacements to become independent of the SourceManager. + clangd::Diagnostic Diag = {R, getSeverity(D->getLevel()), D->getMessage()}; + auto &FixItsForDiagnostic = LocalFixIts[Diag]; + for (const FixItHint &Fix : D->getFixIts()) { + FixItsForDiagnostic.push_back(clang::tooling::Replacement( + Unit->getSourceManager(), Fix.RemoveRange, Fix.CodeToInsert)); } + } - if (!Diagnostics.empty()) - Diagnostics.pop_back(); // Drop trailing comma. - Output.writeMessage( - R"({"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":")" + - File + R"(","diagnostics":[)" + Diagnostics + R"(]}})"); + // Put FixIts into place. + { + std::lock_guard Guard(FixItLock); + FixIts = std::move(LocalFixIts); } + + if (!Diagnostics.empty()) + Diagnostics.pop_back(); // Drop trailing comma. + Output.writeMessage( + R"({"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":")" + + File + R"(","diagnostics":[)" + Diagnostics + R"(]}})"); } ASTManager::~ASTManager() { @@ -134,6 +152,10 @@ } void ASTManager::onDocumentAdd(StringRef Uri) { + if (RunSynchronously) { + parseFileAndPublishDiagnostics(Uri); + return; + } std::lock_guard Guard(RequestLock); // Currently we discard all pending requests and just enqueue the latest one. RequestQueue.clear(); @@ -201,3 +223,12 @@ /*IncludeBriefCommentsInCodeCompletion=*/true, /*AllowPCHWithCompilerErrors=*/true)); } + +llvm::ArrayRef +ASTManager::getFixIts(const clangd::Diagnostic &D) { + std::lock_guard Guard(FixItLock); + auto I = FixIts.find(D); + if (I != FixIts.end()) + return I->second; + return llvm::None; +} Index: clangd/ClangDMain.cpp =================================================================== --- clangd/ClangDMain.cpp +++ clangd/ClangDMain.cpp @@ -11,13 +11,20 @@ #include "DocumentStore.h" #include "JSONRPCDispatcher.h" #include "ProtocolHandlers.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Program.h" #include #include using namespace clang::clangd; +static llvm::cl::opt + RunSynchronously("run-synchronously", + llvm::cl::desc("parse on main thread"), + llvm::cl::init(false), llvm::cl::Hidden); + int main(int argc, char *argv[]) { + llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); llvm::raw_ostream &Outs = llvm::outs(); llvm::raw_ostream &Logs = llvm::errs(); JSONOutput Out(Outs, Logs); @@ -28,14 +35,14 @@ // Set up a document store and intialize all the method handlers for JSONRPC // dispatching. DocumentStore Store; - ASTManager AST(Out, Store); + ASTManager AST(Out, Store, RunSynchronously); Store.addListener(&AST); JSONRPCDispatcher Dispatcher(llvm::make_unique(Out)); Dispatcher.registerHandler("initialize", llvm::make_unique(Out)); auto ShutdownPtr = llvm::make_unique(Out); auto *ShutdownHandler = ShutdownPtr.get(); - Dispatcher.registerHandler("shutdown",std::move(ShutdownPtr)); + Dispatcher.registerHandler("shutdown", std::move(ShutdownPtr)); Dispatcher.registerHandler( "textDocument/didOpen", llvm::make_unique(Out, Store)); @@ -52,6 +59,8 @@ Dispatcher.registerHandler( "textDocument/formatting", llvm::make_unique(Out, Store)); + Dispatcher.registerHandler("textDocument/codeAction", + llvm::make_unique(Out, AST)); while (std::cin.good()) { // A Language Server Protocol message starts with a HTTP header, delimited Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -44,6 +44,15 @@ /// Character offset on a line in a document (zero-based). int character; + friend bool operator==(const Position &LHS, const Position &RHS) { + return std::tie(LHS.line, LHS.character) == + std::tie(RHS.line, RHS.character); + } + friend bool operator<(const Position &LHS, const Position &RHS) { + return std::tie(LHS.line, LHS.character) < + std::tie(RHS.line, RHS.character); + } + static llvm::Optional parse(llvm::yaml::MappingNode *Params); static std::string unparse(const Position &P); }; @@ -55,6 +64,13 @@ /// The range's end position. Position end; + friend bool operator==(const Range &LHS, const Range &RHS) { + return std::tie(LHS.start, LHS.end) == std::tie(RHS.start, RHS.end); + } + friend bool operator<(const Range &LHS, const Range &RHS) { + return std::tie(LHS.start, LHS.end) < std::tie(RHS.start, RHS.end); + } + static llvm::Optional parse(llvm::yaml::MappingNode *Params); static std::string unparse(const Range &P); }; @@ -172,6 +188,51 @@ parse(llvm::yaml::MappingNode *Params); }; +struct Diagnostic { + /// The range at which the message applies. + Range range; + + /// The diagnostic's severity. Can be omitted. If omitted it is up to the + /// client to interpret diagnostics as error, warning, info or hint. + int severity; + + /// The diagnostic's message. + std::string message; + + friend bool operator==(const Diagnostic &LHS, const Diagnostic &RHS) { + return std::tie(LHS.range, LHS.severity, LHS.message) == + std::tie(RHS.range, RHS.severity, RHS.message); + } + friend bool operator<(const Diagnostic &LHS, const Diagnostic &RHS) { + return std::tie(LHS.range, LHS.severity, LHS.message) < + std::tie(RHS.range, RHS.severity, RHS.message); + } + + static llvm::Optional parse(llvm::yaml::MappingNode *Params); +}; + +struct CodeActionContext { + /// An array of diagnostics. + std::vector diagnostics; + + static llvm::Optional + parse(llvm::yaml::MappingNode *Params); +}; + +struct CodeActionParams { + /// The document in which the command was invoked. + TextDocumentIdentifier textDocument; + + /// The range for which the command was invoked. + Range range; + + /// Context carrying additional information. + CodeActionContext context; + + static llvm::Optional + parse(llvm::yaml::MappingNode *Params); +}; + } // namespace clangd } // namespace clang Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -457,3 +457,116 @@ } return Result; } + +llvm::Optional Diagnostic::parse(llvm::yaml::MappingNode *Params) { + Diagnostic Result; + for (auto &NextKeyValue : *Params) { + auto *KeyString = dyn_cast(NextKeyValue.getKey()); + if (!KeyString) + return llvm::None; + + llvm::SmallString<10> KeyStorage; + StringRef KeyValue = KeyString->getValue(KeyStorage); + + llvm::SmallString<10> Storage; + if (KeyValue == "range") { + auto *Value = + dyn_cast_or_null(NextKeyValue.getValue()); + if (!Value) + return llvm::None; + auto Parsed = Range::parse(Value); + if (!Parsed) + return llvm::None; + Result.range = std::move(*Parsed); + } else if (KeyValue == "severity") { + auto *Value = + dyn_cast_or_null(NextKeyValue.getValue()); + if (!Value) + return llvm::None; + long long Val; + if (llvm::getAsSignedInteger(Value->getValue(Storage), 0, Val)) + return llvm::None; + Result.severity = Val; + } else if (KeyValue == "message") { + auto *Value = + dyn_cast_or_null(NextKeyValue.getValue()); + if (!Value) + return llvm::None; + Result.message = Value->getValue(Storage); + } else { + return llvm::None; + } + } + return Result; +} + +llvm::Optional +CodeActionContext::parse(llvm::yaml::MappingNode *Params) { + CodeActionContext Result; + for (auto &NextKeyValue : *Params) { + auto *KeyString = dyn_cast(NextKeyValue.getKey()); + if (!KeyString) + return llvm::None; + + llvm::SmallString<10> KeyStorage; + StringRef KeyValue = KeyString->getValue(KeyStorage); + auto *Value = NextKeyValue.getValue(); + + llvm::SmallString<10> Storage; + if (KeyValue == "diagnostics") { + auto *Seq = dyn_cast(Value); + if (!Seq) + return llvm::None; + for (auto &Item : *Seq) { + auto *I = dyn_cast(&Item); + if (!I) + return llvm::None; + auto Parsed = Diagnostic::parse(I); + if (!Parsed) + return llvm::None; + Result.diagnostics.push_back(std::move(*Parsed)); + } + } else { + return llvm::None; + } + } + return Result; +} + +llvm::Optional +CodeActionParams::parse(llvm::yaml::MappingNode *Params) { + CodeActionParams Result; + for (auto &NextKeyValue : *Params) { + auto *KeyString = dyn_cast(NextKeyValue.getKey()); + if (!KeyString) + return llvm::None; + + llvm::SmallString<10> KeyStorage; + StringRef KeyValue = KeyString->getValue(KeyStorage); + auto *Value = + dyn_cast_or_null(NextKeyValue.getValue()); + if (!Value) + return llvm::None; + + llvm::SmallString<10> Storage; + if (KeyValue == "textDocument") { + auto Parsed = TextDocumentIdentifier::parse(Value); + if (!Parsed) + return llvm::None; + Result.textDocument = std::move(*Parsed); + } else if (KeyValue == "range") { + auto Parsed = Range::parse(Value); + if (!Parsed) + return llvm::None; + Result.range = std::move(*Parsed); + } else if (KeyValue == "context") { + auto Parsed = CodeActionContext::parse(Value); + if (!Parsed) + return llvm::None; + Result.context = std::move(*Parsed); + } else { + return llvm::None; + } + } + return Result; +} Index: clangd/ProtocolHandlers.h =================================================================== --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -22,6 +22,7 @@ namespace clang { namespace clangd { +class ASTManager; class DocumentStore; struct InitializeHandler : Handler { @@ -34,7 +35,8 @@ "textDocumentSync": 1, "documentFormattingProvider": true, "documentRangeFormattingProvider": true, - "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]} + "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}, + "codeActionProvider": true }}})"); } }; @@ -102,6 +104,16 @@ DocumentStore &Store; }; +struct CodeActionHandler : Handler { + CodeActionHandler(JSONOutput &Output, ASTManager &AST) + : Handler(Output), AST(AST) {} + + void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override; + +private: + ASTManager &AST; +}; + } // namespace clangd } // namespace clang Index: clangd/ProtocolHandlers.cpp =================================================================== --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "ProtocolHandlers.h" +#include "ASTManager.h" #include "DocumentStore.h" #include "clang/Format/Format.h" using namespace clang; @@ -58,18 +59,9 @@ return {Lines, Cols}; } -static std::string formatCode(StringRef Code, StringRef Filename, - ArrayRef Ranges, StringRef ID) { - // Call clang-format. - // FIXME: Don't ignore style. - format::FormatStyle Style = format::getLLVMStyle(); - // On windows FileManager doesn't like file://. Just strip it, clang-format - // doesn't need it. - Filename.consume_front("file://"); - tooling::Replacements Replacements = - format::reformat(Style, Code, Ranges, Filename); - - // Now turn the replacements into the format specified by the Language Server +template +static std::string replacementsToEdits(StringRef Code, const T &Replacements) { + // Turn the replacements into the format specified by the Language Server // Protocol. Fuse them into one big JSON array. std::string Edits; for (auto &R : Replacements) { @@ -83,6 +75,21 @@ if (!Edits.empty()) Edits.pop_back(); + return Edits; +} + +static std::string formatCode(StringRef Code, StringRef Filename, + ArrayRef Ranges, StringRef ID) { + // Call clang-format. + // FIXME: Don't ignore style. + format::FormatStyle Style = format::getLLVMStyle(); + // On windows FileManager doesn't like file://. Just strip it, clang-format + // doesn't need it. + Filename.consume_front("file://"); + tooling::Replacements Replacements = + format::reformat(Style, Code, Ranges, Filename); + + std::string Edits = replacementsToEdits(Code, Replacements); return R"({"jsonrpc":"2.0","id":)" + ID.str() + R"(,"result":[)" + Edits + R"(]})"; } @@ -138,3 +145,36 @@ writeMessage(formatCode(Code, DFP->textDocument.uri, {clang::tooling::Range(0, Code.size())}, ID)); } + +void CodeActionHandler::handleMethod(llvm::yaml::MappingNode *Params, + StringRef ID) { + auto CAP = CodeActionParams::parse(Params); + if (!CAP) { + Output.log("Failed to decode CodeActionParams!\n"); + return; + } + + // We provide a code action for each diagnostic at the requested location + // which has FixIts available. + std::string Code = AST.getStore().getDocument(CAP->textDocument.uri); + std::string Commands; + for (Diagnostic &D : CAP->context.diagnostics) { + std::vector Fixes = AST.getFixIts(D); + std::string Edits = replacementsToEdits(Code, Fixes); + + if (!Edits.empty()) + Commands += + R"({"title":"Apply FixIt ')" + llvm::yaml::escape(D.message) + + R"('", "command": "clangd.applyFix", "arguments": [")" + + llvm::yaml::escape(CAP->textDocument.uri) + + R"(", [)" + Edits + + R"(]]},)"; + } + if (!Commands.empty()) + Commands.pop_back(); + + writeMessage( + R"({"jsonrpc":"2.0","id":)" + ID.str() + + R"(, "result": [)" + Commands + + R"(]})"); +} Index: clangd/clients/clangd-vscode/src/extension.ts =================================================================== --- clangd/clients/clangd-vscode/src/extension.ts +++ clangd/clients/clangd-vscode/src/extension.ts @@ -18,9 +18,25 @@ const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions); + function applyTextEdits(uri: string, edits: vscodelc.TextEdit[]) { + let textEditor = vscode.window.activeTextEditor; + + if (textEditor && textEditor.document.uri.toString() === uri) { + textEditor.edit(mutator => { + for (const edit of edits) { + mutator.replace(vscodelc.Protocol2Code.asRange(edit.range), edit.newText); + } + }).then((success) => { + if (!success) { + vscode.window.showErrorMessage('Failed to apply fixes to the document.'); + } + }); + } + } + console.log('Clang Language Server is now active!'); const disposable = clangdClient.start(); - context.subscriptions.push(disposable); + context.subscriptions.push(disposable, vscode.commands.registerCommand('clangd.applyFix', applyTextEdits)); } \ No newline at end of file Index: test/clangd/fixits.test =================================================================== --- /dev/null +++ test/clangd/fixits.test @@ -0,0 +1,22 @@ +# RUN: clangd -run-synchronously < %s | FileCheck %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: 180 + +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}} +# +# CHECK: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.c","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"}]}} +# +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"}]}}} +# +# CHECK: {"jsonrpc":"2.0","id":2, "result": [{"title":"Apply FixIt 'place parentheses around the assignment to silence this warning'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 32}}, "newText": "("},{"range": {"start": {"line": 0, "character": 37}, "end": {"line": 0, "character": 37}}, "newText": ")"}]]},{"title":"Apply FixIt 'use '==' to turn this assignment into an equality comparison'", "command": "clangd.applyFix", "arguments": ["file:///foo.c", [{"range": {"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}}, "newText": "=="}]]}] +# +Content-Length: 44 + +{"jsonrpc":"2.0","id":3,"method":"shutdown"} Index: test/clangd/formatting.test =================================================================== --- test/clangd/formatting.test +++ test/clangd/formatting.test @@ -4,12 +4,13 @@ Content-Length: 125 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} -# CHECK: Content-Length: 294 +# CHECK: Content-Length: 332 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{ # CHECK: "textDocumentSync": 1, # CHECK: "documentFormattingProvider": true, # CHECK: "documentRangeFormattingProvider": true, -# CHECK: "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]} +# CHECK: "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}, +# CHECK: "codeActionProvider": true # CHECK: }}} # Content-Length: 193