[clang-tools-extra] r350304 - [clangd] Check preceding char when completion triggers on ':' or '>'
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 3 05:37:12 PST 2019
Author: ibiryukov
Date: Thu Jan 3 05:37:12 2019
New Revision: 350304
URL: http://llvm.org/viewvc/llvm-project?rev=350304&view=rev
Log:
[clangd] Check preceding char when completion triggers on ':' or '>'
Summary:
Only run completion when we were trigerred on '->' and '::', otherwise
send an error code in return.
To avoid automatically invoking completions in cases like 'a >^' or
'a ? b :^'.
Reviewers: hokein
Reviewed By: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D55994
Added:
clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=350304&r1=350303&r2=350304&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Jan 3 05:37:12 2019
@@ -22,6 +22,15 @@ using namespace llvm;
namespace clang {
namespace clangd {
namespace {
+class IgnoreCompletionError : public llvm::ErrorInfo<CancelledError> {
+public:
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "ignored auto-triggered completion, preceding char did not match";
+ }
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::operation_canceled);
+ }
+};
void adjustSymbolKinds(llvm::MutableArrayRef<DocumentSymbol> Syms,
SymbolKindBitset Kinds) {
@@ -310,6 +319,8 @@ void ClangdLSPServer::onInitialize(const
{"completionProvider",
json::Object{
{"resolveProvider", false},
+ // We do extra checks for '>' and ':' in completion to only
+ // trigger on '->' and '::'.
{"triggerCharacters", {".", ">", ":"}},
}},
{"signatureHelpProvider",
@@ -612,8 +623,10 @@ void ClangdLSPServer::onCodeAction(const
}
}
-void ClangdLSPServer::onCompletion(const TextDocumentPositionParams &Params,
+void ClangdLSPServer::onCompletion(const CompletionParams &Params,
Callback<CompletionList> Reply) {
+ if (!shouldRunCompletion(Params))
+ return Reply(llvm::make_error<IgnoreCompletionError>());
Server->codeComplete(
Params.textDocument.uri.file(), Params.position, CCOpts,
Bind(
@@ -773,6 +786,41 @@ std::vector<Fix> ClangdLSPServer::getFix
return FixItsIter->second;
}
+bool ClangdLSPServer::shouldRunCompletion(
+ const CompletionParams &Params) const {
+ StringRef Trigger = Params.context.triggerCharacter;
+ if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter ||
+ (Trigger != ">" && Trigger != ":"))
+ return true;
+
+ auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
+ if (!Code)
+ return true; // completion code will log the error for untracked doc.
+
+ // A completion request is sent when the user types '>' or ':', but we only
+ // want to trigger on '->' and '::'. We check the preceeding character to make
+ // sure it matches what we expected.
+ // Running the lexer here would be more robust (e.g. we can detect comments
+ // and avoid triggering completion there), but we choose to err on the side
+ // of simplicity here.
+ auto Offset = positionToOffset(*Code, Params.position,
+ /*AllowColumnsBeyondLineLength=*/false);
+ if (!Offset) {
+ vlog("could not convert position '{0}' to offset for file '{1}'",
+ Params.position, Params.textDocument.uri.file());
+ return true;
+ }
+ if (*Offset < 2)
+ return false;
+
+ if (Trigger == ">")
+ return (*Code)[*Offset - 2] == '-'; // trigger only on '->'.
+ if (Trigger == ":")
+ return (*Code)[*Offset - 2] == ':'; // trigger only on '::'.
+ assert(false && "unhandled trigger character");
+ return true;
+}
+
void ClangdLSPServer::onDiagnosticsReady(PathRef File,
std::vector<Diag> Diagnostics) {
auto URI = URIForFile::canonicalize(File, /*TUPath=*/File);
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=350304&r1=350303&r2=350304&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Thu Jan 3 05:37:12 2019
@@ -74,8 +74,7 @@ private:
void onDocumentSymbol(const DocumentSymbolParams &,
Callback<llvm::json::Value>);
void onCodeAction(const CodeActionParams &, Callback<llvm::json::Value>);
- void onCompletion(const TextDocumentPositionParams &,
- Callback<CompletionList>);
+ void onCompletion(const CompletionParams &, Callback<CompletionList>);
void onSignatureHelp(const TextDocumentPositionParams &,
Callback<SignatureHelp>);
void onGoToDefinition(const TextDocumentPositionParams &,
@@ -98,6 +97,12 @@ private:
std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
+ /// Checks if completion request should be ignored. We need this due to the
+ /// limitation of the LSP. Per LSP, a client sends requests for all "trigger
+ /// character" we specify, but for '>' and ':' we need to check they actually
+ /// produce '->' and '::', respectively.
+ bool shouldRunCompletion(const CompletionParams &Params) const;
+
/// Forces a reparse of all currently opened files. As a result, this method
/// may be very expensive. This method is normally called when the
/// compilation database is changed.
Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=350304&r1=350303&r2=350304&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu Jan 3 05:37:12 2019
@@ -550,6 +550,29 @@ bool fromJSON(const json::Value &Params,
O.map("position", R.position);
}
+bool fromJSON(const llvm::json::Value &Params, CompletionContext &R) {
+ json::ObjectMapper O(Params);
+ if (!O)
+ return false;
+
+ int triggerKind;
+ if (!O.map("triggerKind", triggerKind))
+ return false;
+ R.triggerKind = static_cast<CompletionTriggerKind>(triggerKind);
+
+ if (auto *TC = Params.getAsObject()->get("triggerCharacter"))
+ return fromJSON(*TC, R.triggerCharacter);
+ return true;
+}
+
+bool fromJSON(const llvm::json::Value &Params, CompletionParams &R) {
+ if (!fromJSON(Params, static_cast<TextDocumentPositionParams &>(R)))
+ return false;
+ if (auto *Context = Params.getAsObject()->get("context"))
+ return fromJSON(*Context, R.context);
+ return true;
+}
+
static StringRef toTextKind(MarkupKind Kind) {
switch (Kind) {
case MarkupKind::PlainText:
Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=350304&r1=350303&r2=350304&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Thu Jan 3 05:37:12 2019
@@ -775,6 +775,31 @@ struct TextDocumentPositionParams {
};
bool fromJSON(const llvm::json::Value &, TextDocumentPositionParams &);
+enum class CompletionTriggerKind {
+ /// Completion was triggered by typing an identifier (24x7 code
+ /// complete), manual invocation (e.g Ctrl+Space) or via API.
+ Invoked = 1,
+ /// Completion was triggered by a trigger character specified by
+ /// the `triggerCharacters` properties of the `CompletionRegistrationOptions`.
+ TriggerCharacter = 2,
+ /// Completion was re-triggered as the current completion list is incomplete.
+ TriggerTriggerForIncompleteCompletions = 3
+};
+
+struct CompletionContext {
+ /// How the completion was triggered.
+ CompletionTriggerKind triggerKind = CompletionTriggerKind::Invoked;
+ /// The trigger character (a single character) that has trigger code complete.
+ /// Is undefined if `triggerKind !== CompletionTriggerKind.TriggerCharacter`
+ std::string triggerCharacter;
+};
+bool fromJSON(const llvm::json::Value &, CompletionContext &);
+
+struct CompletionParams : TextDocumentPositionParams {
+ CompletionContext context;
+};
+bool fromJSON(const llvm::json::Value &, CompletionParams &);
+
enum class MarkupKind {
PlainText,
Markdown,
Added: clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test?rev=350304&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test (added)
+++ clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test Thu Jan 3 05:37:12 2019
@@ -0,0 +1,106 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"namespace ns { int ns_member; } struct vector { int size; static int default_capacity; };\nvoid test(vector *a, vector *b) {\n if (a > b) {} \n a->size = 10;\n\n a ? a : b;\n ns::ns_member = 10;\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":9},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32001,
+# CHECK-NEXT: "message": "ignored auto-triggered completion, preceding char did not match"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 1,
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":3,"character":5},"context":{"triggerKind":2,"triggerCharacter":">"}}}
+# CHECK: "id": 2,
+# CHECK-NEXT: "jsonrpc": "2.0"
+# CHECK-NEXT: "result": {
+# CHECK-NEXT: "isIncomplete": false,
+# CHECK-NEXT: "items": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "size",
+# CHECK-NEXT: "insertText": "size",
+# CHECK-NEXT: "insertTextFormat": 1,
+# CHECK-NEXT: "kind": 5,
+# CHECK-NEXT: "label": " size",
+# CHECK-NEXT: "sortText": "3eacccccsize",
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT: "newText": "size",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "default_capacity",
+# CHECK-NEXT: "insertText": "default_capacity",
+# CHECK-NEXT: "insertTextFormat": 1,
+# CHECK-NEXT: "kind": 10,
+# CHECK-NEXT: "label": " default_capacity",
+# CHECK-NEXT: "sortText": "3fd70a3ddefault_capacity",
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT: "newText": "default_capacity",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 3
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":5,"character":9},"context":{"triggerKind":2,"triggerCharacter":":"}}}
+# CHECK: "error": {
+# CHECK-NEXT: "code": -32001,
+# CHECK-NEXT: "message": "ignored auto-triggered completion, preceding char did not match"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 3,
+---
+{"jsonrpc":"2.0","id":4,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":6,"character":6},"context":{"triggerKind":2,"triggerCharacter":":"}}}
+---
+# CHECK: "id": 4,
+# CHECK-NEXT: "jsonrpc": "2.0"
+# CHECK-NEXT: "result": {
+# CHECK-NEXT: "isIncomplete": false,
+# CHECK-NEXT: "items": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "ns_member",
+# CHECK-NEXT: "insertText": "ns_member",
+# CHECK-NEXT: "insertTextFormat": 1,
+# CHECK-NEXT: "kind": 6,
+# CHECK-NEXT: "label": " ns_member",
+# CHECK-NEXT: "sortText": "3f2cccccns_member",
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT: "newText": "ns_member",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 6,
+# CHECK-NEXT: "line": 6
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 6,
+# CHECK-NEXT: "line": 6
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
More information about the cfe-commits
mailing list