diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/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. diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/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(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(int Size)"), + SnippetSuffix("<${1:typename T}>"), + Kind(CompletionItemKind::Constructor)))); + EXPECT_THAT( + completions(Context + "Container c = Cont^()", {}, Opts).Completions, Contains(AllOf(Labeled("Container(int Size)"), SnippetSuffix(""), Kind(CompletionItemKind::Constructor))));