[PATCH] D52422: [clangd] Handle template args for disabled function arg snippets

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: kadircet, ioeric, sammccall.
Herald added subscribers: arphaman, jkorous, MaskRay.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52422

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1741,32 +1741,65 @@
   CodeCompleteOptions Opts;
   Opts.EnableSnippets = true;
   Opts.EnableFunctionArgSnippets = false;
-  const std::string Header =
-      R"cpp(
+
+  {
+    auto Results = completions(
+        R"cpp(
       void xfoo();
       void xfoo(int x, int y);
-      void xbar();
-      void f() {
-    )cpp";
-  {
-    auto Results = completions(Header + "\nxfo^", {}, Opts);
+      void f() { xfo^ })cpp",
+        {}, Opts);
     EXPECT_THAT(
         Results.Completions,
         UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("()")),
                              AllOf(Named("xfoo"), SnippetSuffix("($0)"))));
   }
   {
-    auto Results = completions(Header + "\nxba^", {}, Opts);
+    auto Results = completions(
+        R"cpp(
+      void xbar();
+      void f() { xba^ })cpp",
+        {}, Opts);
     EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
                                          Named("xbar"), SnippetSuffix("()"))));
   }
   {
     Opts.BundleOverloads = true;
-    auto Results = completions(Header + "\nxfo^", {}, Opts);
+    auto Results = completions(
+        R"cpp(
+      void xfoo();
+      void xfoo(int x, int y);
+      void f() { xfo^ })cpp",
+        {}, Opts);
     EXPECT_THAT(
         Results.Completions,
         UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("($0)"))));
   }
+  {
+    auto Results = completions(
+        R"cpp(
+      template <class T, class U>
+      void xfoo(int a, U b);
+      void f() { xfo^ })cpp",
+        {}, Opts);
+    EXPECT_THAT(
+        Results.Completions,
+        UnorderedElementsAre(AllOf(Named("xfoo"), SnippetSuffix("<$1>($0)"))));
+  }
+  {
+    auto Results = completions(
+        R"cpp(
+      template <class T>
+      class foo_class{};
+      template <class T>
+      using foo_alias = T**;
+      void f() { foo_^ })cpp",
+        {}, Opts);
+    EXPECT_THAT(
+        Results.Completions,
+        UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")),
+                             AllOf(Named("foo_alias"), SnippetSuffix("<$0>"))));
+  }
 }
 
 TEST(CompletionTest, SuggestOverrides) {
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -46,6 +46,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -482,13 +483,43 @@
     auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
     if (!Snippet)
       // All bundles are function calls.
+      // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
+      // we need to complete 'forward<$1>($0)'.
       return "($0)";
-    if (!Snippet->empty() && !EnableFunctionArgSnippets &&
-        ((Completion.Kind == CompletionItemKind::Function) ||
-         (Completion.Kind == CompletionItemKind::Method)) &&
-        (Snippet->front() == '(') && (Snippet->back() == ')'))
-      // Check whether function has any parameters or not.
-      return Snippet->size() > 2 ? "($0)" : "()";
+    if (EnableFunctionArgSnippets)
+      return *Snippet;
+
+    // Replace argument snippets with a simplified pattern.
+    if (Snippet->empty())
+      return "";
+    if (Completion.Kind == CompletionItemKind::Function ||
+        Completion.Kind == CompletionItemKind::Method) {
+      // Functions snippets can be of 2 types:
+      // - containing only function arguments, e.g.
+      //   foo(${1:int p1}, ${2:int p2});
+      //   We transform this pattern to '($0)' or '()'.
+      // - template arguments and function arguments, e.g.
+      //   foo<${1:class}>(${2:int p1}).
+      //   We transform this pattern to '<$1>($0)' or '<$0>()'.
+
+      bool EmptyArgs = llvm::StringRef(*Snippet).endswith("()");
+      if (Snippet->front() == '<')
+        return EmptyArgs ? "<$0>()" : "<$1>($0)";
+      if (Snippet->front() == '(')
+        return EmptyArgs ? "()" : "($0)";
+      return *Snippet; // Not an arg snippet?
+    }
+    if (Completion.Kind == CompletionItemKind::Reference ||
+        Completion.Kind == CompletionItemKind::Class) {
+      if (Snippet->front() != '<')
+        return *Snippet; // Not an arg snippet?
+
+      // Classes and template using aliases can only have template arguments,
+      // e.g. Foo<${1:class}>.
+      if (llvm::StringRef(*Snippet).endswith("<>"))
+        return "<>"; // can happen with defaulted template arguments.
+      return "<$0>";
+    }
     return *Snippet;
   }
 
@@ -1664,8 +1695,10 @@
       LSP.additionalTextEdits.push_back(FixIt);
     }
   }
-  if (Opts.EnableSnippets)
+  if (Opts.EnableSnippets) {
+    log("Suffix: {0}", SnippetSuffix);
     LSP.textEdit->newText += SnippetSuffix;
+  }
 
   // FIXME(kadircet): Do not even fill insertText after making sure textEdit is
   // compatible with most of the editors.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits