[clangd] Handle template args for disabled function arg snippets

[ilya-biryukov]

clangd/CodeComplete.cpp

1699

Sorry, leftover from debug printing. Will remove

[kadircet]

LGTM

clangd/CodeComplete.cpp

507

maybe have something like <$1>()$0 when arguments are empty so that user can jump to end.

[ilya-biryukov]

• Remove debug formatting
• Update snippet for no-arg case with the suggested changes

[ilya-biryukov]

clangd/CodeComplete.cpp

507

Thanks, this looks consistent with other cases

clangd/CodeComplete.cpp

 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/Sema.h"
 #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"
 #include "llvm/Support/ScopedPrinter.h"
 #include <algorithm>
 #include <iterator>
 #include <queue>
 
if (auto *RT = onlyValue<&BundledEntry::ReturnType>())
       return *RT;
     return "";
   }
 
   std::string summarizeSnippet() const {
     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;
   }
 
   std::string summarizeSignature() const {
     if (auto *Signature = onlyValue<&BundledEntry::Signature>())
       return *Signature;
     // All bundles are function calls.
     return "(…)";
CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   for (const auto &FixIt : FixIts) {
     if (IsRangeConsecutive(FixIt.range, LSP.textEdit->range)) {
       LSP.textEdit->newText = FixIt.newText + LSP.textEdit->newText;
       LSP.textEdit->range.start = FixIt.range.start;
     } else {
       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.
   LSP.insertText = LSP.textEdit->newText;
   LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
                                              : InsertTextFormat::PlainText;
   if (InsertInclude && InsertInclude->Insertion)
     LSP.additionalTextEdits.push_back(*InsertInclude->Insertion);

unittests/clangd/CodeCompleteTests.cpp

EXPECT_THAT(
                   AllOf(Sig("foo(int, int) -> int", {"int", "int"}),
                         SigDoc("Doc from sema"))));
 }
 
 TEST(CompletionTest, CompletionFunctionArgsDisabled) {
   CodeCompleteOptions Opts;
   Opts.EnableSnippets = true;
   Opts.EnableFunctionArgSnippets = false;
-  const std::string Header =
+
+  {
+    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) {
   constexpr const char *const Text(R"cpp(
   class A {
    public:
     virtual void vfunc(bool param);
     virtual void vfunc(bool param, int p);