Improve completion

blacklist some undesired candidates
additionalTextEdits if clang>=7
Use CodePatterns for preprocessor directive completion if there is a #
Prefer textEdit over insertText

src/clang_complete.cc

@@ -71,6 +71,18 @@ struct ProxyFileSystem : FileSystem {
 #endif
 
 namespace ccls {
+
+lsTextEdit ToTextEdit(const clang::SourceManager &SM,
+                      const clang::LangOptions &L,
+                      const clang::FixItHint &FixIt) {
+  lsTextEdit edit;
+  edit.newText = FixIt.CodeToInsert;
+  auto r = FromCharSourceRange(SM, L, FixIt.RemoveRange);
+  edit.range =
+      lsRange{{r.start.line, r.start.column}, {r.end.line, r.end.column}};
+  return edit;
+}
+
 struct PreambleStatCache {
   llvm::StringMap<ErrorOr<vfs::Status>> Cache;
 
@@ -235,12 +247,7 @@ public:
       for (const FixItHint &FixIt : Info.getFixItHints()) {
         if (!IsConcerned(SM, FixIt.RemoveRange.getBegin()))
           return false;
-        lsTextEdit edit;
+        last->edits.push_back(ToTextEdit(SM, *LangOpts, FixIt));
-        edit.newText = FixIt.CodeToInsert;
-        auto r = FromCharSourceRange(SM, *LangOpts, FixIt.RemoveRange);
-        edit.range =
-            lsRange{{r.start.line, r.start.column}, {r.end.line, r.end.column}};
-        last->edits.push_back(std::move(edit));
       }
       return true;
     };

src/clang_complete.hh

@@ -49,6 +49,10 @@ struct Diag : DiagBase {
   std::vector<lsTextEdit> edits;
 };
 
+lsTextEdit ToTextEdit(const clang::SourceManager &SM,
+                      const clang::LangOptions &L,
+                      const clang::FixItHint &FixIt);
+
 struct CompletionSession
     : public std::enable_shared_from_this<CompletionSession> {
   std::mutex mutex;
@@ -184,9 +188,8 @@ struct CompleteConsumerCache {
     std::lock_guard lock(mutex);
     action();
   }
-  bool IsCacheValid(const lsTextDocumentPositionParams &params) {
+  bool IsCacheValid(const std::string path, lsPosition position) {
     std::lock_guard lock(mutex);
-    return path == params.textDocument.uri.GetPath() &&
+    return this->path == path && this->position == position;
-           position == params.position;
   }
 };

src/include_complete.cc

@@ -84,8 +84,7 @@ lsCompletionItem BuildCompletionItem(const std::string &path,
   lsCompletionItem item;
   item.label = ElideLongPath(path);
   item.detail = path; // the include path, used in de-duplicating
-  item.textEdit = lsTextEdit();
+  item.textEdit.newText = path;
-  item.textEdit->newText = path;
   item.insertTextFormat = lsInsertTextFormat::PlainText;
   item.use_angle_brackets_ = use_angle_brackets;
   if (is_stl) {

src/lsp_completion.h

@@ -114,12 +114,12 @@ struct lsCompletionItem {
   //
   // *Note:* The range of the edit must be a single line range and it must
   // contain the position at which completion has been requested.
-  std::optional<lsTextEdit> textEdit;
+  lsTextEdit textEdit;
 
   // An std::optional array of additional text edits that are applied when
   // selecting this completion. Edits must not overlap with the main edit
   // nor with themselves.
-  // std::vector<TextEdit> additionalTextEdits;
+  std::vector<lsTextEdit> additionalTextEdits;
 
   // An std::optional command that is executed *after* inserting this
   // completion. *Note* that additional modifications to the current document
@@ -128,18 +128,7 @@ struct lsCompletionItem {
   // An data entry field that is preserved on a completion item between
   // a completion and a completion resolve request.
   // data ? : any
-
-  // Use this helper to figure out what content the completion item will insert
-  // into the document, as it could live in either |textEdit|, |insertText|, or
-  // |label|.
-  const std::string &InsertedContent() const {
-    if (textEdit)
-      return textEdit->newText;
-    if (!insertText.empty())
-      return insertText;
-    return label;
-  }
 };
 MAKE_REFLECT_STRUCT(lsCompletionItem, label, kind, detail, documentation,
-                    sortText, insertText, filterText, insertTextFormat,
+                    sortText, filterText, insertText, insertTextFormat,
-                    textEdit);
+                    textEdit, additionalTextEdits);

src/messages/textDocument_completion.cc

@@ -107,8 +107,8 @@ void DecorateIncludePaths(const std::smatch &match,
     else
       quote0 = quote1 = '"';
 
-    item.textEdit->newText =
+    item.textEdit.newText =
-        prefix + quote0 + item.textEdit->newText + quote1 + suffix;
+        prefix + quote0 + item.textEdit.newText + quote1 + suffix;
     item.label = prefix + quote0 + item.label + quote1 + suffix;
     item.filterText = std::nullopt;
   }
@@ -137,27 +137,6 @@ ParseIncludeLineResult ParseIncludeLine(const std::string &line) {
   return {ok, match[3], match[5], match[6], match};
 }
 
-static const std::vector<std::string> preprocessorKeywords = {
-    "define", "undef", "include", "if",   "ifdef", "ifndef",
-    "else",   "elif",  "endif",   "line", "error", "pragma"};
-
-std::vector<lsCompletionItem>
-PreprocessorKeywordCompletionItems(const std::smatch &match) {
-  std::vector<lsCompletionItem> items;
-  for (auto &keyword : preprocessorKeywords) {
-    lsCompletionItem item;
-    item.label = keyword;
-    item.priority_ = (keyword == "include" ? 2 : 1);
-    item.textEdit = lsTextEdit();
-    std::string space = (keyword == "else" || keyword == "endif") ? "" : " ";
-    item.textEdit->newText = match[1].str() + "#" + match[2].str() + keyword +
-                             space + match[6].str();
-    item.insertTextFormat = lsInsertTextFormat::PlainText;
-    items.push_back(item);
-  }
-  return items;
-}
-
 template <typename T> char *tofixedbase64(T input, char *out) {
   const char *digits = "./0123456789"
                        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -174,11 +153,9 @@ template <typename T> char *tofixedbase64(T input, char *out) {
 // Pre-filters completion responses before sending to vscode. This results in a
 // significantly snappier completion experience as vscode is easily overloaded
 // when given 1000+ completion items.
-void FilterAndSortCompletionResponse(
+void FilterCandidates(Out_TextDocumentComplete *complete_response,
-    Out_TextDocumentComplete *complete_response,
+                      const std::string &complete_text, lsPosition begin_pos,
-    const std::string &complete_text, bool has_open_paren) {
+                      lsPosition end_pos, bool has_open_paren) {
-  if (!g_config->completion.filterAndSort)
-    return;
   auto &items = complete_response->result.items;
 
   auto finalize = [&]() {
@@ -188,9 +165,21 @@ void FilterAndSortCompletionResponse(
       complete_response->result.isIncomplete = true;
     }
 
-    if (has_open_paren)
+    for (auto &item : items) {
-      for (auto &item : items)
+      item.textEdit.range = lsRange{begin_pos, end_pos};
-        item.insertText = item.label;
+      if (has_open_paren)
+        item.textEdit.newText = item.label;
+      // https://github.com/Microsoft/language-server-protocol/issues/543
+      // Order of textEdit and additionalTextEdits is unspecified.
+      auto &edits = item.additionalTextEdits;
+      if (edits.size() && edits[0].range.end == begin_pos) {
+        item.textEdit.range.start = edits[0].range.start;
+        item.textEdit.newText = edits[0].newText + item.textEdit.newText;
+        edits.erase(edits.begin());
+      }
+      // Compatibility
+      item.insertText = item.textEdit.newText;
+    }
 
     // Set sortText. Note that this happens after resizing - we could do it
     // before, but then we should also sort by priority.
@@ -200,7 +189,7 @@ void FilterAndSortCompletionResponse(
   };
 
   // No complete text; don't run any filtering logic except to trim the items.
-  if (complete_text.empty()) {
+  if (!g_config->completion.filterAndSort || complete_text.empty()) {
     finalize();
     return;
   }
@@ -261,19 +250,6 @@ bool IsOpenParenOrAngle(const std::vector<std::string> &lines,
   return false;
 }
 
-unsigned GetCompletionPriority(const CodeCompletionString &CCS,
-                               CXCursorKind result_kind,
-                               const std::optional<std::string> &typedText) {
-  unsigned priority = CCS.getPriority();
-  if (CCS.getAvailability() != CXAvailability_Available ||
-      result_kind == CXCursor_Destructor ||
-      result_kind == CXCursor_ConversionFunction ||
-      (result_kind == CXCursor_CXXMethod && typedText &&
-       StartsWith(*typedText, "operator")))
-    priority *= 100;
-  return priority;
-}
-
 lsCompletionItemKind GetCompletionKind(CXCursorKind cursor_kind) {
   switch (cursor_kind) {
   case CXCursor_UnexposedDecl:
@@ -366,11 +342,11 @@ lsCompletionItemKind GetCompletionKind(CXCursorKind cursor_kind) {
   }
 }
 
-void BuildItem(std::vector<lsCompletionItem> &out,
+void BuildItem(const CodeCompletionResult &R, const CodeCompletionString &CCS,
-               const CodeCompletionString &CCS) {
+               std::vector<lsCompletionItem> &out) {
   assert(!out.empty());
   auto first = out.size() - 1;
-
+  bool ignore = false;
   std::string result_type;
 
   for (const auto &Chunk : CCS) {
@@ -404,7 +380,7 @@ void BuildItem(std::vector<lsCompletionItem> &out,
       // Duplicate last element, the recursive call will complete it.
       if (g_config->completion.duplicateOptional) {
         out.push_back(out.back());
-        BuildItem(out, *Chunk.Optional);
+        BuildItem(R, *Chunk.Optional, out);
       }
       continue;
     }
@@ -415,15 +391,20 @@ void BuildItem(std::vector<lsCompletionItem> &out,
 
     for (auto i = first; i < out.size(); ++i) {
       out[i].label += text;
-      if (!g_config->client.snippetSupport && !out[i].parameters_.empty())
+      if (ignore ||
+          (!g_config->client.snippetSupport && out[i].parameters_.size()))
         continue;
 
       if (Kind == CodeCompletionString::CK_Placeholder) {
-        out[i].insertText +=
+        if (R.Kind == CodeCompletionResult::RK_Pattern) {
+          ignore = true;
+          continue;
+        }
+        out[i].textEdit.newText +=
             "${" + std::to_string(out[i].parameters_.size()) + ":" + text + "}";
         out[i].insertTextFormat = lsInsertTextFormat::Snippet;
       } else if (Kind != CodeCompletionString::CK_Informative) {
-        out[i].insertText += text;
+        out[i].textEdit.newText += text;
       }
     }
   }
@@ -453,12 +434,25 @@ public:
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *Results,
                                   unsigned NumResults) override {
+    if (Context.getKind() == CodeCompletionContext::CCC_Recovery)
+      return;
     ls_items.reserve(NumResults);
     for (unsigned i = 0; i != NumResults; i++) {
       auto &R = Results[i];
       if (R.Availability == CXAvailability_NotAccessible ||
           R.Availability == CXAvailability_NotAvailable)
         continue;
+      if (R.Declaration) {
+        if (R.Declaration->getKind() == Decl::CXXDestructor)
+          continue;
+        if (auto *RD = dyn_cast<RecordDecl>(R.Declaration))
+          if (RD->isInjectedClassName())
+            continue;
+        auto NK = R.Declaration->getDeclName().getNameKind();
+        if (NK == DeclarationName::CXXOperatorName ||
+            NK == DeclarationName::CXXLiteralOperatorName)
+          continue;
+      }
       CodeCompletionString *CCS = R.CreateCodeCompletionString(
           S, Context, getAllocator(), getCodeCompletionTUInfo(),
           includeBriefComments());
@@ -470,19 +464,27 @@ public:
 
       size_t first_idx = ls_items.size();
       ls_items.push_back(ls_item);
-      BuildItem(ls_items, *CCS);
+      BuildItem(R, *CCS, ls_items);
 
       for (size_t j = first_idx; j < ls_items.size(); j++) {
         if (g_config->client.snippetSupport &&
             ls_items[j].insertTextFormat == lsInsertTextFormat::Snippet)
-          ls_items[j].insertText += "$0";
+          ls_items[j].textEdit.newText += "$0";
-        ls_items[j].priority_ = GetCompletionPriority(
+        ls_items[j].priority_ = CCS->getPriority();
-            *CCS, Results[i].CursorKind, ls_items[j].filterText);
         if (!g_config->completion.detailedLabel) {
           ls_items[j].detail = ls_items[j].label;
           ls_items[j].label = ls_items[j].filterText.value_or("");
         }
       }
+#if LLVM_VERSION_MAJOR >= 7
+      for (const FixItHint &FixIt : R.FixIts) {
+        auto &AST = S.getASTContext();
+        lsTextEdit ls_edit =
+            ccls::ToTextEdit(AST.getSourceManager(), AST.getLangOpts(), FixIt);
+        for (size_t j = first_idx; j < ls_items.size(); j++)
+          ls_items[j].additionalTextEdits.push_back(ls_edit);
+      }
+#endif
     }
   }
 
@@ -497,7 +499,7 @@ struct Handler_TextDocumentCompletion
   void Run(In_TextDocumentComplete *request) override {
     static CompleteConsumerCache<std::vector<lsCompletionItem>> cache;
 
-    auto &params = request->params;
+    const auto &params = request->params;
     Out_TextDocumentComplete out;
     out.id = request->id;
 
@@ -556,55 +558,36 @@ struct Handler_TextDocumentCompletion
 
     std::string completion_text;
     lsPosition end_pos = params.position;
-    params.position = file->FindStableCompletionSource(
+    lsPosition begin_pos = file->FindStableCompletionSource(
-      request->params.position, &completion_text, &end_pos);
+        params.position, &completion_text, &end_pos);
 
-    ParseIncludeLineResult result = ParseIncludeLine(buffer_line);
+    ParseIncludeLineResult preprocess = ParseIncludeLine(buffer_line);
     bool has_open_paren = IsOpenParenOrAngle(file->buffer_lines, end_pos);
 
-    if (result.ok) {
+    if (preprocess.ok && preprocess.keyword.compare("include") == 0) {
       Out_TextDocumentComplete out;
       out.id = request->id;
-
+      {
-      if (result.quote.empty() && result.pattern.empty()) {
+        std::unique_lock<std::mutex> lock(
-        // no quote or path of file, do preprocessor keyword completion
+            include_complete->completion_items_mutex, std::defer_lock);
-        if (!std::any_of(preprocessorKeywords.begin(),
+        if (include_complete->is_scanning)
-                         preprocessorKeywords.end(),
+          lock.lock();
-                         [&result](std::string_view k) {
+        std::string quote = preprocess.match[5];
-                           return k == result.keyword;
+        for (auto &item : include_complete->completion_items)
-                         })) {
+          if (quote.empty() || quote == (item.use_angle_brackets_ ? "<" : "\""))
-          out.result.items = PreprocessorKeywordCompletionItems(result.match);
+            out.result.items.push_back(item);
-          FilterAndSortCompletionResponse(&out, result.keyword, has_open_paren);
-        }
-      } else if (result.keyword.compare("include") == 0) {
-        {
-          // do include completion
-          std::unique_lock<std::mutex> lock(
-              include_complete->completion_items_mutex, std::defer_lock);
-          if (include_complete->is_scanning)
-            lock.lock();
-          std::string quote = result.match[5];
-          for (auto &item : include_complete->completion_items)
-            if (quote.empty() ||
-                quote == (item.use_angle_brackets_ ? "<" : "\""))
-              out.result.items.push_back(item);
-        }
-        FilterAndSortCompletionResponse(&out, result.pattern, has_open_paren);
-        DecorateIncludePaths(result.match, &out.result.items);
       }
-
+      begin_pos.character = 0;
-      for (lsCompletionItem &item : out.result.items) {
+      end_pos.character = (int)buffer_line.size();
-        item.textEdit->range.start.line = params.position.line;
+      FilterCandidates(&out, preprocess.pattern, begin_pos, end_pos,
-        item.textEdit->range.start.character = 0;
+                       has_open_paren);
-        item.textEdit->range.end.line = params.position.line;
+      DecorateIncludePaths(preprocess.match, &out.result.items);
-        item.textEdit->range.end.character = (int)buffer_line.size();
-      }
-
       pipeline::WriteStdout(kMethodType, out);
     } else {
+      std::string path = params.textDocument.uri.GetPath();
       CompletionManager::OnComplete callback =
-          [completion_text, has_open_paren, id = request->id,
+          [completion_text, path, begin_pos, end_pos, has_open_paren,
-           params = request->params](CodeCompleteConsumer *OptConsumer) {
+           id = request->id](CodeCompleteConsumer *OptConsumer) {
             if (!OptConsumer)
               return;
             auto *Consumer = static_cast<CompletionConsumer *>(OptConsumer);
@@ -612,14 +595,13 @@ struct Handler_TextDocumentCompletion
             out.id = id;
             out.result.items = Consumer->ls_items;
 
-            FilterAndSortCompletionResponse(&out, completion_text,
+            FilterCandidates(&out, completion_text, begin_pos, end_pos,
-                                            has_open_paren);
+                             has_open_paren);
             pipeline::WriteStdout(kMethodType, out);
             if (!Consumer->from_cache) {
-              std::string path = params.textDocument.uri.GetPath();
               cache.WithLock([&]() {
                 cache.path = path;
-                cache.position = params.position;
+                cache.position = begin_pos;
                 cache.result = Consumer->ls_items;
               });
             }
@@ -627,18 +609,19 @@ struct Handler_TextDocumentCompletion
 
       clang::CodeCompleteOptions CCOpts;
       CCOpts.IncludeBriefComments = true;
+      CCOpts.IncludeCodePatterns = preprocess.ok; // if there is a #
 #if LLVM_VERSION_MAJOR >= 7
       CCOpts.IncludeFixIts = true;
 #endif
       CCOpts.IncludeMacros = true;
-      if (cache.IsCacheValid(params)) {
+      if (cache.IsCacheValid(path, begin_pos)) {
         CompletionConsumer Consumer(CCOpts, true);
         cache.WithLock([&]() { Consumer.ls_items = cache.result; });
         callback(&Consumer);
       } else {
         clang_complete->completion_request_.PushBack(
           std::make_unique<CompletionManager::CompletionRequest>(
-            request->id, params.textDocument, params.position,
+            request->id, params.textDocument, begin_pos,
             std::make_unique<CompletionConsumer>(CCOpts, false), CCOpts,
             callback));
       }

src/messages/textDocument_signatureHelp.cc

@@ -184,18 +184,18 @@ struct Handler_TextDocumentSignatureHelp
   void Run(In_TextDocumentSignatureHelp *request) override {
     static CompleteConsumerCache<lsSignatureHelp> cache;
 
-    auto &params = request->params;
+    const auto &params = request->params;
     std::string path = params.textDocument.uri.GetPath();
+    lsPosition begin_pos = params.position;
     if (WorkingFile *file = working_files->GetFileByFilename(path)) {
       std::string completion_text;
       lsPosition end_pos = params.position;
-      params.position = file->FindStableCompletionSource(
+      begin_pos = file->FindStableCompletionSource(
         request->params.position, &completion_text, &end_pos);
     }
 
     CompletionManager::OnComplete callback =
-        [id = request->id,
+        [id = request->id, path, begin_pos](CodeCompleteConsumer *OptConsumer) {
-         params = request->params](CodeCompleteConsumer *OptConsumer) {
           if (!OptConsumer)
             return;
           auto *Consumer = static_cast<SignatureHelpConsumer *>(OptConsumer);
@@ -204,10 +204,9 @@ struct Handler_TextDocumentSignatureHelp
           out.result = Consumer->ls_sighelp;
           pipeline::WriteStdout(kMethodType, out);
           if (!Consumer->from_cache) {
-            std::string path = params.textDocument.uri.GetPath();
             cache.WithLock([&]() {
               cache.path = path;
-              cache.position = params.position;
+              cache.position = begin_pos;
               cache.result = Consumer->ls_sighelp;
             });
           }
@@ -217,7 +216,7 @@ struct Handler_TextDocumentSignatureHelp
     CCOpts.IncludeGlobals = false;
     CCOpts.IncludeMacros = false;
     CCOpts.IncludeBriefComments = false;
-    if (cache.IsCacheValid(params)) {
+    if (cache.IsCacheValid(path, begin_pos)) {
       SignatureHelpConsumer Consumer(CCOpts, true);
       cache.WithLock([&]() { Consumer.ls_sighelp = cache.result; });
       callback(&Consumer);