[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

5 files changed, 210 insertions, 3 deletions

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index c2b09943184..08a0810d17d 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -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 InitializeParams &Params,
             {"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 CodeActionParams &Params,
   }
 }
 
-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::getFixes(StringRef File,
   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);
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 9063ea2188b..80052b4a291 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -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.
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 972ba25fbe1..fd94ca7d4bc 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -550,6 +550,29 @@ bool fromJSON(const json::Value &Params, TextDocumentPositionParams &R) {
          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:
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 94838a8575a..17b69bde562 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -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,
diff --git a/clang-tools-extra/test/clangd/completion-auto-trigger.test b/clang-tools-extra/test/clangd/completion-auto-trigger.test
new file mode 100644
index 00000000000..db3cc537ad2
--- /dev/null
+++ b/clang-tools-extra/test/clangd/completion-auto-trigger.test
@@ -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"}