From 3001faf9a8660b4aa5474da54f25b864639a8ef7 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Mon, 15 May 2017 00:28:53 -0700 Subject: [PATCH] Signature help and snippets for code completion --- src/code_completion.cc | 176 +++++++++++++++++++++++-------------- src/command_line.cc | 77 ++++++++++++++++ src/ipc.cc | 2 + src/ipc.h | 1 + src/language_server_api.cc | 4 + src/language_server_api.h | 83 ++++++++++++++++- src/working_files.cc | 136 ++++++++++++++++++++++++++++ src/working_files.h | 7 ++ 8 files changed, 416 insertions(+), 70 deletions(-) diff --git a/src/code_completion.cc b/src/code_completion.cc index 556295bc..e5e09713 100644 --- a/src/code_completion.cc +++ b/src/code_completion.cc @@ -42,71 +42,86 @@ int GetCompletionPriority(const CXCompletionString& str, CXCursorKind result_kin return priority; } +bool IsCallKind(CXCursorKind kind) { + switch (kind) { + case CXCursor_ObjCInstanceMethodDecl: + case CXCursor_CXXMethod: + case CXCursor_FunctionTemplate: + case CXCursor_FunctionDecl: + case CXCursor_Constructor: + case CXCursor_Destructor: + case CXCursor_ConversionFunction: + return true; + default: + return false; + } +} + lsCompletionItemKind GetCompletionKind(CXCursorKind cursor_kind) { switch (cursor_kind) { - case CXCursor_ObjCInstanceMethodDecl: - case CXCursor_CXXMethod: - return lsCompletionItemKind::Method; + case CXCursor_ObjCInstanceMethodDecl: + case CXCursor_CXXMethod: + return lsCompletionItemKind::Method; - case CXCursor_FunctionTemplate: - case CXCursor_FunctionDecl: - return lsCompletionItemKind::Function; + case CXCursor_FunctionTemplate: + case CXCursor_FunctionDecl: + return lsCompletionItemKind::Function; - case CXCursor_Constructor: - case CXCursor_Destructor: - case CXCursor_ConversionFunction: - return lsCompletionItemKind::Constructor; + case CXCursor_Constructor: + case CXCursor_Destructor: + case CXCursor_ConversionFunction: + return lsCompletionItemKind::Constructor; - case CXCursor_FieldDecl: - return lsCompletionItemKind::Field; + case CXCursor_FieldDecl: + return lsCompletionItemKind::Field; - case CXCursor_VarDecl: - case CXCursor_ParmDecl: - return lsCompletionItemKind::Variable; + case CXCursor_VarDecl: + case CXCursor_ParmDecl: + return lsCompletionItemKind::Variable; - case CXCursor_UnionDecl: - case CXCursor_ClassTemplate: - case CXCursor_ClassTemplatePartialSpecialization: - case CXCursor_ClassDecl: - case CXCursor_StructDecl: - case CXCursor_UsingDeclaration: - case CXCursor_TypedefDecl: - case CXCursor_TypeAliasDecl: - case CXCursor_TypeAliasTemplateDecl: - return lsCompletionItemKind::Class; + case CXCursor_UnionDecl: + case CXCursor_ClassTemplate: + case CXCursor_ClassTemplatePartialSpecialization: + case CXCursor_ClassDecl: + case CXCursor_StructDecl: + case CXCursor_UsingDeclaration: + case CXCursor_TypedefDecl: + case CXCursor_TypeAliasDecl: + case CXCursor_TypeAliasTemplateDecl: + return lsCompletionItemKind::Class; - case CXCursor_EnumConstantDecl: - case CXCursor_EnumDecl: - return lsCompletionItemKind::Enum; + case CXCursor_EnumConstantDecl: + case CXCursor_EnumDecl: + return lsCompletionItemKind::Enum; - case CXCursor_MacroInstantiation: - case CXCursor_MacroDefinition: - return lsCompletionItemKind::Interface; + case CXCursor_MacroInstantiation: + case CXCursor_MacroDefinition: + return lsCompletionItemKind::Interface; - case CXCursor_Namespace: - case CXCursor_NamespaceAlias: - case CXCursor_NamespaceRef: - return lsCompletionItemKind::Module; + case CXCursor_Namespace: + case CXCursor_NamespaceAlias: + case CXCursor_NamespaceRef: + return lsCompletionItemKind::Module; - case CXCursor_MemberRef: - case CXCursor_TypeRef: - return lsCompletionItemKind::Reference; + case CXCursor_MemberRef: + case CXCursor_TypeRef: + return lsCompletionItemKind::Reference; - //return lsCompletionItemKind::Property; - //return lsCompletionItemKind::Unit; - //return lsCompletionItemKind::Value; - //return lsCompletionItemKind::Keyword; - //return lsCompletionItemKind::Snippet; - //return lsCompletionItemKind::Color; - //return lsCompletionItemKind::File; + //return lsCompletionItemKind::Property; + //return lsCompletionItemKind::Unit; + //return lsCompletionItemKind::Value; + //return lsCompletionItemKind::Keyword; + //return lsCompletionItemKind::Snippet; + //return lsCompletionItemKind::Color; + //return lsCompletionItemKind::File; - case CXCursor_NotImplemented: - return lsCompletionItemKind::Text; + case CXCursor_NotImplemented: + return lsCompletionItemKind::Text; - default: - std::cerr << "[complete] Unhandled completion kind " << cursor_kind << std::endl; - return lsCompletionItemKind::Text; + default: + std::cerr << "[complete] Unhandled completion kind " << cursor_kind << std::endl; + return lsCompletionItemKind::Text; } } @@ -125,9 +140,7 @@ std::string BuildLabelString(CXCompletionString completion_string) { return label; } -std::string BuildDetailString(CXCompletionString completion_string) { - std::string detail; - +void BuildDetailString(CXCompletionString completion_string, std::string& detail, std::string& insert, std::vector* parameters) { int num_chunks = clang_getNumCompletionChunks(completion_string); for (int i = 0; i < num_chunks; ++i) { CXCompletionChunkKind kind = clang_getCompletionChunkKind(completion_string, i); @@ -135,23 +148,29 @@ std::string BuildDetailString(CXCompletionString completion_string) { switch (kind) { case CXCompletionChunk_Optional: { CXCompletionString nested = clang_getCompletionChunkCompletionString(completion_string, i); - detail += BuildDetailString(nested); + BuildDetailString(nested, detail, insert, parameters); break; } case CXCompletionChunk_Placeholder: { - // TODO: send this info to vscode. - CXString text = clang_getCompletionChunkText(completion_string, i); - detail += clang::ToString(text); + std::string text = clang::ToString(clang_getCompletionChunkText(completion_string, i)); + parameters->push_back(text); + detail += text; + insert += "${" + std::to_string(parameters->size()) + ":" + text + "}"; break; } + case CXCompletionChunk_CurrentParameter: + // We have our own parsing logic for active parameter. This doesn't seem + // to be very reliable. + break; + case CXCompletionChunk_TypedText: case CXCompletionChunk_Text: - case CXCompletionChunk_Informative: - case CXCompletionChunk_CurrentParameter: { - CXString text = clang_getCompletionChunkText(completion_string, i); - detail += clang::ToString(text); + case CXCompletionChunk_Informative: { + std::string text = clang::ToString(clang_getCompletionChunkText(completion_string, i)); + detail += text; + insert += text; break; } @@ -164,48 +183,59 @@ std::string BuildDetailString(CXCompletionString completion_string) { case CXCompletionChunk_LeftParen: detail += "("; + insert += "("; break; case CXCompletionChunk_RightParen: detail += ")"; + insert += ")"; break; case CXCompletionChunk_LeftBracket: - detail += "]"; + detail += "["; + insert += "["; break; case CXCompletionChunk_RightBracket: - detail += "["; + detail += "]"; + insert += "]"; break; case CXCompletionChunk_LeftBrace: detail += "{"; + insert += "{"; break; case CXCompletionChunk_RightBrace: detail += "}"; + insert += "}"; break; case CXCompletionChunk_LeftAngle: detail += "<"; + insert += "<"; break; case CXCompletionChunk_RightAngle: detail += ">"; + insert += ">"; break; case CXCompletionChunk_Comma: detail += ", "; + insert += ", "; break; case CXCompletionChunk_Colon: detail += ":"; + insert += ":"; break; case CXCompletionChunk_SemiColon: detail += ";"; + insert += ";"; break; case CXCompletionChunk_Equal: detail += "="; + insert += "="; break; case CXCompletionChunk_HorizontalSpace: case CXCompletionChunk_VerticalSpace: detail += " "; + insert += " "; break; } } - - return detail; } void EnsureDocumentParsed(CompletionSession* session, @@ -288,6 +318,16 @@ void CompletionQueryMain(CompletionManager* completion_manager) { timer.Reset(); for (unsigned i = 0; i < cx_results->NumResults; ++i) { CXCompletionResult& result = cx_results->Results[i]; + + // TODO: Try to figure out how we can hide base method calls without also + // hiding method implementation assistance, ie, + // + // void Foo::* { + // } + // + + if (clang_getCompletionAvailability(result.CompletionString) == CXAvailability_NotAvailable) + continue; // TODO: fill in more data lsCompletionItem ls_completion_item; @@ -295,9 +335,13 @@ void CompletionQueryMain(CompletionManager* completion_manager) { // kind/label/detail/docs/sortText ls_completion_item.kind = GetCompletionKind(result.CursorKind); ls_completion_item.label = BuildLabelString(result.CompletionString); - ls_completion_item.detail = BuildDetailString(result.CompletionString); + BuildDetailString(result.CompletionString, ls_completion_item.detail, ls_completion_item.insertText, &ls_completion_item.parameters_); ls_completion_item.documentation = clang::ToString(clang_getCompletionBriefComment(result.CompletionString)); ls_completion_item.sortText = uint64_t(GetCompletionPriority(result.CompletionString, result.CursorKind, ls_completion_item.label)); + + // If this function is slow we can skip building insertText at the cost of some code duplication. + if (!IsCallKind(result.CursorKind)) + ls_completion_item.insertText = ""; ls_result.push_back(ls_completion_item); } diff --git a/src/command_line.cc b/src/command_line.cc index e2560072..100bddb0 100644 --- a/src/command_line.cc +++ b/src/command_line.cc @@ -812,6 +812,7 @@ void RegisterMessageTypes() { MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); + MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); MessageRegistry::instance()->Register(); @@ -1342,6 +1343,11 @@ bool QueryDbMainLoop( // See https://github.com/Microsoft/language-server-protocol/issues/138. response.result.capabilities.completionProvider->triggerCharacters = { ".", ":", ">" }; + response.result.capabilities.signatureHelpProvider = lsSignatureHelpOptions(); + // NOTE: If updating signature help tokens make sure to also update + // WorkingFile::FindClosestCallNameInBuffer. + response.result.capabilities.signatureHelpProvider->triggerCharacters = { "(", "," }; + response.result.capabilities.codeLensProvider = lsCodeLensOptions(); response.result.capabilities.codeLensProvider->resolveProvider = false; @@ -1599,6 +1605,76 @@ bool QueryDbMainLoop( break; } + case IpcId::TextDocumentSignatureHelp: { + auto msg = static_cast(message.get()); + lsTextDocumentPositionParams params = msg->params; + //std::cerr << "!! SignatureHelp @ " << msg->params.position.line << ":" << msg->params.position.character << std::endl; + + WorkingFile* file = working_files->GetFileByFilename(params.textDocument.uri.GetPath()); + std::string search; + int active_param = 0; + if (file) { + lsPosition completion_position; + search = file->FindClosestCallNameInBuffer(params.position, &active_param, &completion_position); + // Move completion position back by one; completer will automatically increment it by one to deal with -> and . accesses. + if (completion_position.character > 0) + completion_position.character -= 1; + std::cerr << "[completion] Changing completion position from " << params.position.ToString() << " to " << completion_position.ToString() << std::endl; + params.position = completion_position; + } + std::cerr << "[completion] Returning signatures for " << search << std::endl; + if (search.empty()) + break; + + CompletionManager::OnComplete callback = std::bind([](BaseIpcMessage* message, std::string search, int active_param, const NonElidedVector& results) { + auto msg = static_cast(message); + auto ipc = IpcManager::instance(); + + Out_TextDocumentSignatureHelp response; + response.id = msg->id; + + for (auto& result : results) { + if (result.label != search) + continue; + + lsSignatureInformation signature; + signature.label = result.detail; + for (auto& parameter : result.parameters_) { + lsParameterInformation ls_param; + ls_param.label = parameter; + signature.parameters.push_back(ls_param); + } + response.result.signatures.push_back(signature); + } + + // Guess the signature the user wants based on available parameter + // count. + response.result.activeSignature = 0; + for (size_t i = 0; i < response.result.signatures.size(); ++i) { + if (active_param < response.result.signatures.size()) { + response.result.activeSignature = i; + break; + } + } + + // Set signature to what we parsed from the working file. + response.result.activeParameter = active_param; + + response.Write(std::cerr); + std::cerr << std::endl; + + Timer timer; + ipc->SendOutMessageToClient(IpcId::TextDocumentSignatureHelp, response); + timer.ResetAndPrint("[complete] Writing signature help results"); + + delete message; + }, message.release(), search, active_param, std::placeholders::_1); + + completion_manager->CodeComplete(params, std::move(callback)); + + break; + } + case IpcId::TextDocumentDefinition: { auto msg = static_cast(message.get()); @@ -2144,6 +2220,7 @@ void LanguageServerStdinLoop(IndexerConfig* config, std::unordered_map parameters_; + // The label of this completion item. By default // also the text that is inserted when selecting // this completion. @@ -447,11 +452,11 @@ struct lsCompletionItem { // A string that should be inserted a document when selecting // this completion. When `falsy` the label is used. - //std::string insertText; + std::string insertText; // The format of the insert text. The format applies to both the `insertText` property // and the `newText` property of a provided `textEdit`. - //lsInsertTextFormat insertTextFormat; + lsInsertTextFormat insertTextFormat = lsInsertTextFormat::Snippet; // An edit which is applied to a document when selecting this completion. When an edit is provided the value of // `insertText` is ignored. @@ -479,7 +484,9 @@ MAKE_REFLECT_STRUCT(lsCompletionItem, kind, detail, documentation, - sortText); + sortText, + insertText, + insertTextFormat); struct lsTextDocumentItem { @@ -937,7 +944,7 @@ MAKE_REFLECT_STRUCT(lsCompletionOptions, resolveProvider, triggerCharacters); // Signature help options. struct lsSignatureHelpOptions { // The characters that trigger signature help automatically. - NonElidedVector triggerCharacters; + std::vector triggerCharacters; }; MAKE_REFLECT_STRUCT(lsSignatureHelpOptions, triggerCharacters); @@ -1273,6 +1280,74 @@ struct Out_TextDocumentComplete : public lsOutMessage }; MAKE_REFLECT_STRUCT(Out_TextDocumentComplete, jsonrpc, id, result); +// Signature help. +struct Ipc_TextDocumentSignatureHelp : public IpcMessage { + const static IpcId kIpcId = IpcId::TextDocumentSignatureHelp; + + lsRequestId id; + lsTextDocumentPositionParams params; +}; +MAKE_REFLECT_STRUCT(Ipc_TextDocumentSignatureHelp, id, params); +// Represents a parameter of a callable-signature. A parameter can +// have a label and a doc-comment. +struct lsParameterInformation { + // The label of this parameter. Will be shown in + // the UI. + std::string label; + + // The human-readable doc-comment of this parameter. Will be shown + // in the UI but can be omitted. + optional documentation; +}; +MAKE_REFLECT_STRUCT(lsParameterInformation, label, documentation); +// Represents the signature of something callable. A signature +// can have a label, like a function-name, a doc-comment, and +// a set of parameters. +struct lsSignatureInformation { + // The label of this signature. Will be shown in + // the UI. + std::string label; + + // The human-readable doc-comment of this signature. Will be shown + // in the UI but can be omitted. + optional documentation; + + // The parameters of this signature. + std::vector parameters; +}; +MAKE_REFLECT_STRUCT(lsSignatureInformation, label, documentation, parameters); +// Signature help represents the signature of something +// callable. There can be multiple signature but only one +// active and only one active parameter. +struct lsSignatureHelp { + // One or more signatures. + NonElidedVector signatures; + + // 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 + // 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. + // In future version of the protocol this property might become + // mandantory to better express the active parameter if the + // active signature does have any. + optional activeParameter; +}; +MAKE_REFLECT_STRUCT(lsSignatureHelp, signatures, activeSignature, activeParameter); +struct Out_TextDocumentSignatureHelp : public lsOutMessage { + lsRequestId id; + lsSignatureHelp result; +}; +MAKE_REFLECT_STRUCT(Out_TextDocumentSignatureHelp, jsonrpc, id, result); + // Goto definition struct Ipc_TextDocumentDefinition : public IpcMessage { const static IpcId kIpcId = IpcId::TextDocumentDefinition; diff --git a/src/working_files.cc b/src/working_files.cc index 3d30f15f..4162aed3 100644 --- a/src/working_files.cc +++ b/src/working_files.cc @@ -2,10 +2,32 @@ #include "position.h" +#include + #include namespace { +lsPosition GetPositionForOffset(const std::string& content, int offset) { + if (offset >= content.size()) + offset = content.size() - 1; + + lsPosition result; + int i = 0; + while (i < offset) { + if (content[i] == '\n') { + result.line += 1; + result.character = 0; + } + else { + result.character += 1; + } + ++i; + } + + return result; +} + int GetOffsetForPosition(lsPosition position, const std::string& content) { int offset = 0; @@ -142,6 +164,50 @@ optional WorkingFile::GetIndexLineFromBufferLine(int buffer_line) const { return closest_index_line; } +std::string WorkingFile::FindClosestCallNameInBuffer(lsPosition position, int* active_parameter, lsPosition* completion_position) const { + *active_parameter = 0; + + int offset = GetOffsetForPosition(position, buffer_content); + + // If vscode auto-inserts closing ')' we will begin on ')' token in foo() + // which will make the below algorithm think it's a nested call. + if (offset > 0 && buffer_content[offset] == ')') + --offset; + + // Scan back out of call context. + int balance = 0; + while (offset > 0) { + char c = buffer_content[offset]; + if (c == ')') ++balance; + else if (c == '(') --balance; + + if (balance == 0 && c == ',') + *active_parameter += 1; + + --offset; + + if (balance == -1) + break; + } + + if (offset < 0) + return ""; + + // Scan back entire identifier. + int start_offset = offset; + while (offset > 0) { + char c = buffer_content[offset - 1]; + if (isalnum(c) == false && c != '_') + break; + --offset; + } + + if (completion_position) + *completion_position = GetPositionForOffset(buffer_content, offset); + + return buffer_content.substr(offset, start_offset - offset + 1); +} + CXUnsavedFile WorkingFile::AsUnsavedFile() const { CXUnsavedFile result; result.Filename = filename.c_str(); @@ -237,3 +303,73 @@ std::vector WorkingFiles::AsUnsavedFiles() { result.push_back(file->AsUnsavedFile()); return result; } + +TEST_SUITE("WorkingFile"); + +lsPosition CharPos(const WorkingFile& file, char character, int character_offset = 0) { + const std::string& search = file.buffer_content; + + lsPosition result; + int index = 0; + while (index < search.size()) { + char c = search[index]; + if (c == character) + break; + if (c == '\n') { + result.line += 1; + result.character = 0; + } + else { + result.character += 1; + } + ++index; + } + REQUIRE(index < search.size()); + result.character += character_offset; + return result; +} + +TEST_CASE("simple call") { + WorkingFile f("foo.cc", "abcd(1, 2"); + int active_param = 0; + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, '('), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, '1'), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, ','), &active_param) == "abcd"); + REQUIRE(active_param == 1); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, ' '), &active_param) == "abcd"); + REQUIRE(active_param == 1); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, '2'), &active_param) == "abcd"); + REQUIRE(active_param == 1); +} + +TEST_CASE("nested call") { + WorkingFile f("foo.cc", "abcd(efg(), 2"); + int active_param = 0; + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, '('), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, 'e'), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, 'f'), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, 'g'), &active_param) == "abcd"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, 'g', 1), &active_param) == "efg"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, 'g', 2), &active_param) == "efg"); + REQUIRE(active_param == 0); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, ','), &active_param) == "abcd"); + REQUIRE(active_param == 1); + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, ' '), &active_param) == "abcd"); + REQUIRE(active_param == 1); +} + +TEST_CASE("auto-insert )") { + WorkingFile f("foo.cc", "abc()"); + int active_param = 0; + REQUIRE(f.FindClosestCallNameInBuffer(CharPos(f, ')'), &active_param) == "abc"); + REQUIRE(active_param == 0); +} + +TEST_SUITE_END(); \ No newline at end of file diff --git a/src/working_files.h b/src/working_files.h index 7547d67b..79e9230f 100644 --- a/src/working_files.h +++ b/src/working_files.h @@ -49,6 +49,13 @@ struct WorkingFile { // accepts and returns 1-based lines. optional GetIndexLineFromBufferLine(int buffer_line) const; + // Finds the closest 'callable' name prior to position. This is used for + // signature help to filter code completion results. + // + // |completion_position| will be point to a good code completion location to + // for fetching signatures. + std::string FindClosestCallNameInBuffer(lsPosition position, int* active_parameter, lsPosition* completion_position = nullptr) const; + CXUnsavedFile AsUnsavedFile() const; };