Code completion should not ignore default parameters in functions.

[usaxena95]

Summary

Inorder to display the default arguments we must process the
CK_Optional chunks of CodeCompletionString while creating the Signature.

We do not create placeholders for default arguments.

[usaxena95]

Removed unused include of logger.

[sammccall]

clang-tools-extra/clangd/CodeCompletionStrings.cpp

35

maybe appendOptionalChunk? if in doubt, better to describe the semantics of the param than its type, since the type appears in the signature

156

please drop the extra message from the assert unless there's something additional to say (it just echoes the expression here)

(I don't have a strong opinion on this assert - I'm fine with keeping it but you could also drop it as we're enforcing someone else's clearly documented invariant)

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

943

for completeness, could you also add a unit test to CodeCompletionStringsTests.cpp?

[usaxena95]

Added tests in CodeCompletionStringsTests.cpp
Resolved comments.

[usaxena95]

Formatted new test.

clang-tools-extra/clangd/CodeCompletionStrings.cpp

 void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) {
   for (const auto Character : Text) {
     if (Character == '$' || Character == '}' || Character == '\\')
       Out->push_back('\\');
     Out->push_back(Character);
   }
 }
 
+void appendOptionalChunk(const CodeCompletionString &CCS, std::string *Out) {
+  for (const CodeCompletionString::Chunk &C : CCS) {
+    switch (C.Kind) {
+    case CodeCompletionString::CK_Optional:
+      assert(C.Optional &&
+             "Expected the optional code completion string to be non-null.");
+      appendOptionalChunk(*C.Optional, Out);
+      break;
+    default:
+      *Out += C.Text;
+      break;
+    }
+  }
+}
+
 bool looksLikeDocComment(llvm::StringRef CommentText) {
   // We don't report comments that only contain "special" chars.
   // This avoids reporting various delimiters, like:
   //   =================
   //   -----------------
   //   *****************
   return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos;
 }
case CodeCompletionString::CK_TypedText:
         }
       }
       break;
     case CodeCompletionString::CK_Text:
       *Signature += Chunk.Text;
       *Snippet += Chunk.Text;
       break;
     case CodeCompletionString::CK_Optional:
+      assert(Chunk.Optional);      
+      // No need to create placeholders for default arguments in Snippet.
+      appendOptionalChunk(*Chunk.Optional, Signature);
       break;
     case CodeCompletionString::CK_Placeholder:
       *Signature += Chunk.Text;
       ++SnippetArg;
       *Snippet +=
           "${" +
           std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':';
       appendEscapeSnippet(Chunk.Text, Snippet);

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

#if 0
   // really provide useful results in excluded branches.
   par^
 #endif
     }
 )cpp");
 
   EXPECT_TRUE(Results.Completions.empty());
 }
+
+TEST(CompletionTest, DefaultArgs) {
+  clangd::CodeCompleteOptions Opts;
+  std::string Context = R"cpp(
+    int X(int A = 0);
+    int Y(int A, int B = 0);
+    int Z(int A, int B = 0, int C = 0, int D = 0);
+  )cpp";
+  EXPECT_THAT(completions(Context + "int y = X^", {}, Opts).Completions,
+              UnorderedElementsAre(Labeled("X(int A = 0)")));
+  EXPECT_THAT(completions(Context + "int y = Y^", {}, Opts).Completions,
+              UnorderedElementsAre(AllOf(Labeled("Y(int A, int B = 0)"),
+                                         SnippetSuffix("(${1:int A})"))));
+  EXPECT_THAT(completions(Context + "int y = Z^", {}, Opts).Completions,
+              UnorderedElementsAre(
+                  AllOf(Labeled("Z(int A, int B = 0, int C = 0, int D = 0)"),
+                        SnippetSuffix("(${1:int A})"))));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
                          std::vector<Symbol> IndexSymbols = {}) {
   std::unique_ptr<SymbolIndex> Index;
   if (!IndexSymbols.empty())
     Index = memIndex(IndexSymbols);
 
   MockFSProvider FS;
   MockCompilationDatabase CDB;

clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp

TEST_F(CompletionStringTest, Function) {
 
   auto *CCS = Builder.TakeString();
   computeSignature(*CCS);
   EXPECT_EQ(Signature, "(p1, p2)");
   EXPECT_EQ(Snippet, "(${1:p1}, ${2:p2})");
   EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment");
 }
 
+TEST_F(CompletionStringTest, FunctionWithDefaultParams) {
+  // return_type foo(p1, p2 = 0, p3 = 0)
+  Builder.AddChunk(CodeCompletionString::CK_Comma);
+  Builder.AddTypedTextChunk("p3 = 0");
+  auto *DefaultParam2 = Builder.TakeString();
+
+  Builder.AddChunk(CodeCompletionString::CK_Comma);
+  Builder.AddTypedTextChunk("p2 = 0");
+  Builder.AddOptionalChunk(DefaultParam2);
+  auto *DefaultParam1 = Builder.TakeString();
+
+  Builder.AddResultTypeChunk("return_type");
+  Builder.AddTypedTextChunk("Foo");
+  Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+  Builder.AddPlaceholderChunk("p1");
+  Builder.AddOptionalChunk(DefaultParam1);
+  Builder.AddChunk(CodeCompletionString::CK_RightParen);
+
+  auto *CCS = Builder.TakeString();
+  computeSignature(*CCS);
+  EXPECT_EQ(Signature, "(p1, p2 = 0, p3 = 0)");
+  EXPECT_EQ(Snippet, "(${1:p1})");
+}
+
 TEST_F(CompletionStringTest, EscapeSnippet) {
   Builder.AddTypedTextChunk("Foo");
   Builder.AddChunk(CodeCompletionString::CK_LeftParen);
   Builder.AddPlaceholderChunk("$p}1\\");
   Builder.AddChunk(CodeCompletionString::CK_RightParen);
 
   computeSignature(*Builder.TakeString());
   EXPECT_EQ(Signature, "($p}1\\)");