[PATCH] D89870: [clangd] Drop template argument lists from completions followed by <

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Now, given `template <typename T> foo() {}` when user types `fo^<int>()` the
completion snippet will not contain `<int>()`.

Also, when the next token is opening parenthesis (`(`) and completion snippet
contains template argument list, it is still emitted.

This patch complements D81380 <https://reviews.llvm.org/D81380>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89870

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2979,6 +2979,8 @@
       Container(int Size) {}
     };
   )cpp";
+  // FIXME(kirillbobyrev): Also strip prefix of identifier token before the
+  // cursor from completion item ("fo" in this case).
   EXPECT_THAT(completions(Context + "int y = fo^", {}, Opts).Completions,
               UnorderedElementsAre(
                   AllOf(Labeled("foo(int A)"), SnippetSuffix("(${1:int A})"))));
@@ -3006,10 +3008,13 @@
       Contains(AllOf(Labeled("Container<typename T>(int Size)"),
                      SnippetSuffix("<${1:typename T}>(${2:int Size})"),
                      Kind(CompletionItemKind::Constructor))));
-  // FIXME(kirillbobyrev): It would be nice to still produce the template
-  // snippet part: in this case it should be "<${1:typename T}>".
   EXPECT_THAT(
       completions(Context + "Container c = Cont^()", {}, Opts).Completions,
+      Contains(AllOf(Labeled("Container<typename T>(int Size)"),
+                     SnippetSuffix("<${1:typename T}>"),
+                     Kind(CompletionItemKind::Constructor))));
+  EXPECT_THAT(
+      completions(Context + "Container c = Cont^<int>()", {}, Opts).Completions,
       Contains(AllOf(Labeled("Container<typename T>(int Size)"),
                      SnippetSuffix(""),
                      Kind(CompletionItemKind::Constructor))));
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -433,13 +433,29 @@
   std::string summarizeSnippet() const {
     if (IsUsingDeclaration)
       return "";
-    // Suppress function argument snippets if args are already present.
-    if ((Completion.Kind == CompletionItemKind::Function ||
-         Completion.Kind == CompletionItemKind::Method ||
-         Completion.Kind == CompletionItemKind::Constructor) &&
-        NextTokenKind == tok::l_paren)
-      return "";
     auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
+    // Suppress function argument snippets cursor is followed by left
+    // parenthesis (and potentially arguments) or if there are potentially
+    // template arguments. There are cases where it would be wrong (e.g. next
+    // '<' token is a comparison rather than template argument list start) but
+    // it is less common and suppressing snippet provides better UX.
+    if (Snippet && (Completion.Kind == CompletionItemKind::Function ||
+                    Completion.Kind == CompletionItemKind::Method ||
+                    Completion.Kind == CompletionItemKind::Constructor)) {
+      // Potentially followed by argument list.
+      if (NextTokenKind == tok::l_paren) {
+        // If snippet contains template arguments we will emit them and drop
+        // function arguments.
+        if (Snippet->front() == '<')
+          return Snippet->substr(0, Snippet->find('('));
+        return "";
+      }
+      // If there is a potential template argument list, drop snippet. Ideally,
+      // this could generate an edit that would paste function arguments after
+      // template argument list but it would be complicated.
+      if (NextTokenKind == tok::less && Snippet->front() == '<')
+        return "";
+    }
     if (!Snippet)
       // All bundles are function calls.
       // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89870.299638.patch
Type: text/x-patch
Size: 3648 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201021/932c9ff5/attachment.bin>