From f3d00dea235c3c32a3ab3b3bb5d01fd5083126f4 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Sat, 20 May 2017 12:31:07 -0700 Subject: [PATCH] Implement code actions using clang FixIts - Also make server check client version and show an error message if they do not match. --- README.md | 1 + src/clang_utils.cc | 32 ++++++++++++++- src/command_line.cc | 82 ++++++++++++++++++++++++++++++++++++++- src/ipc.cc | 2 + src/ipc.h | 1 + src/language_server_api.h | 59 +++++++++++++++++++++++++--- src/serializer.h | 10 +++++ src/working_files.h | 2 + 8 files changed, 180 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 1cb434cf..d5641a18 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,7 @@ be productive with cquery. Here's a list of implemented features: * global symbol search * hover * diagnostics + * code actions (clang FixIts) # Setup - build cquery, install extension, setup project diff --git a/src/clang_utils.cc b/src/clang_utils.cc index 61e54cb1..bc731ba3 100644 --- a/src/clang_utils.cc +++ b/src/clang_utils.cc @@ -2,6 +2,24 @@ #include "libclangmm/Utility.h" +namespace { + +lsRange GetLsRangeForFixIt(const CXSourceRange& range) { + CXSourceLocation start = clang_getRangeStart(range); + CXSourceLocation end = clang_getRangeEnd(range); + + unsigned int start_line, start_column; + clang_getSpellingLocation(start, nullptr, &start_line, &start_column, nullptr); + unsigned int end_line, end_column; + clang_getSpellingLocation(end, nullptr, &end_line, &end_column, nullptr); + + return lsRange( + lsPosition(start_line - 1, start_column - 1) /*start*/, + lsPosition(end_line - 1, end_column) /*end*/); +} + +} // namespace + optional BuildDiagnostic(CXDiagnostic diagnostic) { // Skip diagnostics in system headers. CXSourceLocation diag_loc = clang_getDiagnosticLocation(diagnostic); @@ -45,7 +63,19 @@ optional BuildDiagnostic(CXDiagnostic diagnostic) { break; } - // TODO: integrate FixIts (ie, textDocument/codeAction) + // Report fixits + unsigned num_fixits = clang_getDiagnosticNumFixIts(diagnostic); + if (num_fixits > 0) + std::cerr << "!!!! Got " << num_fixits << " fixits" << std::endl; + for (unsigned i = 0; i < num_fixits; ++i) { + CXSourceRange replacement_range; + CXString text = clang_getDiagnosticFixIt(diagnostic, i, &replacement_range); + + lsTextEdit edit; + edit.newText = clang::ToString(text); + edit.range = GetLsRangeForFixIt(replacement_range); + ls_diagnostic.fixits_.push_back(edit); + } clang_disposeDiagnostic(diagnostic); diff --git a/src/command_line.cc b/src/command_line.cc index 3d52b2a2..f0638fd3 100644 --- a/src/command_line.cc +++ b/src/command_line.cc @@ -31,11 +31,15 @@ // ie, a fully linear view of a function with inline function calls expanded. // We can probably use vscode decorators to achieve it. +// TODO: implement ThreadPool type which monitors CPU usage / number of work items +// per second completed and scales up/down number of running threads. + namespace { std::vector kEmptyArgs; - +// Expected client version. We show an error if this doesn't match. +const int kExpectedClientVersion = 1; @@ -898,6 +902,7 @@ void RegisterMessageTypes() { MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); + MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); @@ -1103,6 +1108,11 @@ void ParseFile(IndexerConfig* config, diag.params.uri = lsDocumentUri::FromPath(new_index->path); diag.params.diagnostics = new_index->diagnostics; IpcManager::instance()->SendOutMessageToClient(IpcId::TextDocumentPublishDiagnostics, diag); + + // Cache diagnostics so we can show fixits. + WorkingFile* working_file = working_files->GetFileByFilename(new_index->path); + if (working_file) + working_file->diagnostics = new_index->diagnostics; } @@ -1418,6 +1428,19 @@ bool QueryDbMainLoop( *config = *request->params.initializationOptions; + // Check client version. + if (config->clientVersion != kExpectedClientVersion) { + Out_ShowLogMessage out; + out.display_type = Out_ShowLogMessage::DisplayType::Show; + out.params.type = lsMessageType::Error; + out.params.message = "cquery client (v" + std::to_string(config->clientVersion) + ") and server (v" + std::to_string(kExpectedClientVersion) + ") version mismatch. Please update "; + if (config->clientVersion > kExpectedClientVersion) + out.params.message += "the cquery binary."; + else + out.params.message += "your extension client (VSIX file)."; + out.Write(std::cout); + } + // Make sure cache directory is valid. if (config->cacheDirectory.empty()) { std::cerr << "No cache directory" << std::endl; @@ -1486,6 +1509,8 @@ bool QueryDbMainLoop( response.result.capabilities.hoverProvider = true; response.result.capabilities.referencesProvider = true; + response.result.capabilities.codeActionProvider = true; + response.result.capabilities.documentSymbolProvider = true; response.result.capabilities.workspaceSymbolProvider = true; @@ -1716,7 +1741,7 @@ bool QueryDbMainLoop( if (file) params.position = file->FindStableCompletionSource(params.position); - CompletionManager::OnComplete callback = std::bind([code_complete_cache](BaseIpcMessage* message, NonElidedVector results, NonElidedVector diagnostics) { + CompletionManager::OnComplete callback = std::bind([working_files, code_complete_cache](BaseIpcMessage* message, NonElidedVector results, NonElidedVector diagnostics) { auto msg = static_cast(message); auto ipc = IpcManager::instance(); @@ -1725,13 +1750,21 @@ bool QueryDbMainLoop( complete_response.result.isIncomplete = false; complete_response.result.items = results; + // Emit completion results. ipc->SendOutMessageToClient(IpcId::TextDocumentCompletion, complete_response); + // Emit diagnostics. Out_TextDocumentPublishDiagnostics diagnostic_response; diagnostic_response.params.uri = msg->params.textDocument.uri; diagnostic_response.params.diagnostics = diagnostics; ipc->SendOutMessageToClient(IpcId::TextDocumentPublishDiagnostics, diagnostic_response); + // Cache diagnostics so we can show fixits. + WorkingFile* working_file = working_files->GetFileByFilename(msg->params.textDocument.uri.GetPath()); + if (working_file) + working_file->diagnostics = diagnostics; + + // Cache completion results so if the user types backspace we can respond faster. code_complete_cache->cached_path = msg->params.textDocument.uri.GetPath(); code_complete_cache->cached_completion_position = msg->params.position; code_complete_cache->cached_results = results; @@ -2021,6 +2054,50 @@ bool QueryDbMainLoop( break; } + case IpcId::TextDocumentCodeAction: { + // NOTE: This code snippet will generate some FixIts for testing: + // + // struct origin { int x, int y }; + // void foo() { + // point origin = { + // x: 0.0, + // y: 0.0 + // }; + // } + // + auto msg = static_cast(message.get()); + + WorkingFile* working_file = working_files->GetFileByFilename(msg->params.textDocument.uri.GetPath()); + if (!working_file) { + // TODO: send error response. + std::cerr << "[error] textDocument/codeAction could not find working file" << std::endl; + break; + } + + int target_line = msg->params.range.start.line; + + Out_TextDocumentCodeAction response; + response.id = msg->id; + + for (lsDiagnostic& diag : working_file->diagnostics) { + // clang does not provide accurate ennough column reporting for + // diagnostics to do good column filtering, so report all + // diagnostics on the line. + if (!diag.fixits_.empty() && diag.range.start.line == target_line) { + Out_TextDocumentCodeAction::Command command; + command.title = "FixIt: " + diag.message; + command.command = "cquery._applyFixIt"; + command.arguments.textDocumentUri = msg->params.textDocument.uri; + command.arguments.edits = diag.fixits_; + response.result.push_back(command); + } + } + + response.Write(std::cerr); + ipc->SendOutMessageToClient(IpcId::TextDocumentCodeAction, response); + break; + } + case IpcId::TextDocumentCodeLens: { auto msg = static_cast(message.get()); @@ -2374,6 +2451,7 @@ void LanguageServerStdinLoop(IndexerConfig* config, std::unordered_map @@ -591,6 +597,9 @@ struct lsDiagnostic { // The diagnostic's message. std::string message; + + // Non-serialized set of fixits. + NonElidedVector fixits_; }; MAKE_REFLECT_STRUCT(lsDiagnostic, range, severity, source, message); @@ -1325,17 +1334,17 @@ struct lsSignatureHelp { // The active signature. If omitted or the value lies outside the // range of `signatures` the value defaults to zero or is ignored if - // `signatures.length === 0`. Whenever possible implementors should - // make an active decision about the active signature and shouldn't + // `signatures.length === 0`. Whenever possible implementors should + // make an active decision about the active signature and shouldn't // rely on a default value. // In future version of the protocol this property might become // mandantory to better express this. optional activeSignature; // The active parameter of the active signature. If omitted or the value - // lies outside the range of `signatures[activeSignature].parameters` - // defaults to 0 if the active signature has parameters. If - // the active signature has no parameters it is ignored. + // lies outside the range of `signatures[activeSignature].parameters` + // defaults to 0 if the active signature has parameters. If + // the active signature has no parameters it is ignored. // In future version of the protocol this property might become // mandantory to better express the active parameter if the // active signature does have any. @@ -1422,6 +1431,44 @@ struct Out_TextDocumentReferences : public lsOutMessage { + const static IpcId kIpcId = IpcId::TextDocumentCodeAction; + // Contains additional diagnostic information about the context in which + // a code action is run. + struct lsCodeActionContext { + // An array of diagnostics. + NonElidedVector diagnostics; + }; + // Params for the CodeActionRequest + struct lsCodeActionParams { + // The document in which the command was invoked. + lsTextDocumentIdentifier textDocument; + // The range for which the command was invoked. + lsRange range; + // Context carrying additional information. + lsCodeActionContext context; + }; + + lsRequestId id; + lsCodeActionParams params; +}; +MAKE_REFLECT_STRUCT(Ipc_TextDocumentCodeAction::lsCodeActionContext, diagnostics); +MAKE_REFLECT_STRUCT(Ipc_TextDocumentCodeAction::lsCodeActionParams, textDocument, range, context); +MAKE_REFLECT_STRUCT(Ipc_TextDocumentCodeAction, id, params); +struct Out_TextDocumentCodeAction : public lsOutMessage { + struct CommandArgs { + lsDocumentUri textDocumentUri; + NonElidedVector edits; + }; + using Command = lsCommand; + + lsRequestId id; + NonElidedVector result; +}; +MAKE_REFLECT_STRUCT_WRITER_AS_ARRAY(Out_TextDocumentCodeAction::CommandArgs, textDocumentUri, edits); +MAKE_REFLECT_STRUCT(Out_TextDocumentCodeAction, jsonrpc, id, result); + // List symbols in a document. struct lsDocumentSymbolParams { lsTextDocumentIdentifier textDocument; diff --git a/src/serializer.h b/src/serializer.h index 513d937c..d989472c 100644 --- a/src/serializer.h +++ b/src/serializer.h @@ -56,7 +56,17 @@ struct IndexFile; REFLECT_MEMBER_END(); \ } +#define _MAPPABLE_REFLECT_ARRAY(name) \ + Reflect(visitor, value.name); +// Reflects the struct so it is serialized as an array instead of an object. +// This currently only supports writers. +#define MAKE_REFLECT_STRUCT_WRITER_AS_ARRAY(type, ...) \ + inline void Reflect(Writer& visitor, type& value) { \ + visitor.StartArray(); \ + MACRO_MAP(_MAPPABLE_REFLECT_ARRAY, __VA_ARGS__) \ + visitor.EndArray(); \ + } diff --git a/src/working_files.h b/src/working_files.h index 0b755042..26f321b0 100644 --- a/src/working_files.h +++ b/src/working_files.h @@ -27,6 +27,8 @@ struct WorkingFile { // This map goes from buffer-line -> indices+1 in all_buffer_lines. // Note: The items in the value entry are 1-based liness. std::unordered_map> all_buffer_lines_lookup; + // A set of diagnostics that have been reported for this file. + std::vector diagnostics; WorkingFile(const std::string& filename, const std::string& buffer_content);