[clangd] Autocomplete fixes for methods

clang-tools-extra/clangd/CodeCompletionStrings.cpp

@@ -39,16 +39,29 @@ void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) {
   }
 }
 
-void appendOptionalChunk(const CodeCompletionString &CCS, std::string *Out) {
+/// Removes the value for defaults arguments.
+static void addWithoutValue(std::string *Out, const std::string &ToAdd) {
+  size_t Val = ToAdd.find('=');
+  if (Val != ToAdd.npos)
+    *Out += ToAdd.substr(0, Val - 1); // removing value in definition
+  else
+    *Out += ToAdd;
+}
+
+void appendOptionalChunk(const CodeCompletionString &CCS, std::string *Out,
+                         bool RemoveValues = false) {
   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);
+      appendOptionalChunk(*C.Optional, Out, RemoveValues);
       break;
     default:
-      *Out += C.Text;
+      if (RemoveValues)
+        addWithoutValue(Out, C.Text);
+      else
+        *Out += C.Text;
       break;
     }
   }
@@ -184,6 +197,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
   bool HadInformativeChunks = false;
+  int IsTemplateArgument = 0;
 
   std::optional<unsigned> TruncateSnippetAt;
   for (const auto &Chunk : CCS) {
@@ -252,33 +266,50 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
         }
       }
       break;
-    case CodeCompletionString::CK_Text:
+    case CodeCompletionString::CK_FunctionQualifier:
+      if (!IncludeFunctionArguments) // if not a call
+        *Snippet += Chunk.Text;
       *Signature += Chunk.Text;
+      break;
+    case CodeCompletionString::CK_Text:
       *Snippet += Chunk.Text;
+      *Signature += Chunk.Text;
       break;
     case CodeCompletionString::CK_Optional:
       assert(Chunk.Optional);
       // No need to create placeholders for default arguments in Snippet.
       appendOptionalChunk(*Chunk.Optional, Signature);
+      if (!IncludeFunctionArguments) { // complete args with default value in
+                                       // definition
+        appendOptionalChunk(*Chunk.Optional, Snippet, /*RemoveValues=*/true);
+      }
       break;
     case CodeCompletionString::CK_Placeholder:
       *Signature += Chunk.Text;
-      ++SnippetArg;
-      if (SnippetArg == CursorSnippetArg) {
-        // We'd like to make $0 a placeholder too, but vscode does not support
-        // this (https://github.com/microsoft/vscode/issues/152837).
-        *Snippet += "$0";
+      if (IncludeFunctionArguments || IsTemplateArgument) {
+        ++SnippetArg;
+        if (SnippetArg == CursorSnippetArg) {
+          // We'd like to make $0 a placeholder too, but vscode does not support
+          // this (https://github.com/microsoft/vscode/issues/152837).
+          *Snippet += "$0";
+        } else {
+          *Snippet += "${" + std::to_string(SnippetArg) + ':';
+          appendEscapeSnippet(Chunk.Text, Snippet);
+          *Snippet += '}';
+        }
       } else {
-        *Snippet += "${" + std::to_string(SnippetArg) + ':';
-        appendEscapeSnippet(Chunk.Text, Snippet);
-        *Snippet += '}';
+        *Snippet += Chunk.Text;
       }
       break;
     case CodeCompletionString::CK_Informative:
       HadInformativeChunks = true;
       // For example, the word "const" for a const method, or the name of
       // the base class for methods that are part of the base class.
       *Signature += Chunk.Text;
+      if (strcmp(Chunk.Text, " const") == 0 ||
+          strcmp(Chunk.Text, " volatile") == 0 ||
+          strcmp(Chunk.Text, " restrict") == 0)
+        *Snippet += Chunk.Text;
       // Don't put the informative chunks in the snippet.
       break;
     case CodeCompletionString::CK_ResultType:
@@ -290,21 +321,22 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
       llvm_unreachable("Unexpected CK_CurrentParameter while collecting "
                        "CompletionItems");
       break;
+    case CodeCompletionString::CK_LeftAngle:
+      IsTemplateArgument++;
+      *Signature += Chunk.Text;
+      *Snippet += Chunk.Text;
+      break;
+    case CodeCompletionString::CK_RightAngle:
+      IsTemplateArgument--;
+      *Signature += Chunk.Text;
+      *Snippet += Chunk.Text;
+      break;
     case CodeCompletionString::CK_LeftParen:
-      // We're assuming that a LeftParen in a declaration starts a function
-      // call, and arguments following the parenthesis could be discarded if
-      // IncludeFunctionArguments is false.
-      if (!IncludeFunctionArguments &&
-          ResultKind == CodeCompletionResult::RK_Declaration)
-        TruncateSnippetAt.emplace(Snippet->size());
-      [[fallthrough]];
     case CodeCompletionString::CK_RightParen:
     case CodeCompletionString::CK_LeftBracket:
     case CodeCompletionString::CK_RightBracket:
     case CodeCompletionString::CK_LeftBrace:
     case CodeCompletionString::CK_RightBrace:
-    case CodeCompletionString::CK_LeftAngle:
-    case CodeCompletionString::CK_RightAngle:
     case CodeCompletionString::CK_Comma:
     case CodeCompletionString::CK_Colon:
     case CodeCompletionString::CK_SemiColon:
@@ -319,8 +351,6 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
       break;
     }
   }
-  if (TruncateSnippetAt)
-    *Snippet = Snippet->substr(0, *TruncateSnippetAt);
 }
 
 std::string formatDocumentation(const CodeCompletionString &CCS,

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

@@ -586,14 +586,14 @@ TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) {
     auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts);
     EXPECT_THAT(Results.Completions,
                 Contains(AllOf(named("method"), signature("(int) const"),
-                               snippetSuffix(""))));
+                               snippetSuffix("(int) const"))));
     // We don't have any arguments to deduce against if this isn't a call.
     // Thus, we should emit these deducible template arguments explicitly.
     EXPECT_THAT(
         Results.Completions,
-        Contains(AllOf(named("generic"),
-                       signature("<typename T, typename U>(U, V)"),
-                       snippetSuffix("<${1:typename T}, ${2:typename U}>"))));
+        Contains(
+            AllOf(named("generic"), signature("<typename T, typename U>(U, V)"),
+                  snippetSuffix("<${1:typename T}, ${2:typename U}>(U, V)"))));
   }
 
   for (const auto &P : Code.points("canBeCall")) {
@@ -620,6 +620,27 @@ TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) {
   }
 }
 
+TEST(CompletionTest, DefaultArgsWithValues) {
+  clangd::CodeCompleteOptions Opts;
+  Opts.EnableSnippets = true;
+  auto Results = completions(
+      R"cpp(
+    struct Arg {
+        Arg(int a, int b);
+    };
+    struct Foo {
+      void foo(int x = 42, int y = 0, Arg arg = Arg(42,  0));
+    };
+    void Foo::foo^
+  )cpp",
+      /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(Results.Completions,
+              Contains(AllOf(
+                  named("foo"),
+                  signature("(int x = 42, int y = 0, Arg arg = Arg(42,  0))"),
+                  snippetSuffix("(int x, int y, Arg arg)"))));
+}
+
 TEST(CompletionTest, NoSnippetsInUsings) {
   clangd::CodeCompleteOptions Opts;
   Opts.EnableSnippets = true;
@@ -4490,14 +4511,14 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
     EXPECT_THAT(
         Result.Completions,
         ElementsAre(AllOf(named("foo"), signature("<class self:auto>(int arg)"),
-                          snippetSuffix("<${1:class self:auto}>"))));
+                          snippetSuffix("<${1:class self:auto}>(int arg)"))));
   }
   {
     auto Result = codeComplete(testPath(TU.Filename), Code.point("c3"),
                                Preamble.get(), Inputs, Opts);
     EXPECT_THAT(Result.Completions,
                 ElementsAre(AllOf(named("bar"), signature("(int arg)"),
-                                  snippetSuffix(""))));
+                                  snippetSuffix("(int arg)"))));
   }
 }
 

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

@@ -177,7 +177,7 @@ TEST_F(CompletionStringTest, DropFunctionArguments) {
       /*IncludeFunctionArguments=*/false);
   // Arguments dropped from snippet, kept in signature.
   EXPECT_EQ(Signature, "<typename T, int U>(arg1, arg2)");
-  EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>");
+  EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>(arg1, arg2)");
 }
 
 TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {

clang/include/clang/Sema/CodeCompleteConsumer.h

@@ -473,6 +473,12 @@ class CodeCompletionString {
     /// A piece of text that describes something about the result but
     /// should not be inserted into the buffer.
     CK_Informative,
+
+    /// A piece of text that holds function qualifiers. Should be inserted
+    /// into the buffer only in definition, not on call.
+    /// e.g. const for a function or a method.
+    CK_FunctionQualifier,
+
     /// A piece of text that describes the type of an entity or, for
     /// functions and methods, the return type.
     CK_ResultType,
@@ -534,7 +540,7 @@ class CodeCompletionString {
 
     union {
       /// The text string associated with a CK_Text, CK_Placeholder,
-      /// CK_Informative, or CK_Comma chunk.
+      /// CK_Informative, CK_FunctionQualifier, or CK_Comma chunk.
       /// The string is owned by the chunk and will be deallocated
       /// (with delete[]) when the chunk is destroyed.
       const char *Text;
@@ -561,6 +567,9 @@ class CodeCompletionString {
     /// Create a new informative chunk.
     static Chunk CreateInformative(const char *Informative);
 
+    /// Create a new declaration informative chunk.
+    static Chunk CreateFunctionQualifier(const char *FunctionQualifier);
+
     /// Create a new result type chunk.
     static Chunk CreateResultType(const char *ResultType);
 
@@ -737,6 +746,9 @@ class CodeCompletionBuilder {
   /// Add a new informative chunk.
   void AddInformativeChunk(const char *Text);
 
+  /// Add a new function qualifier chunk.
+  void AddFunctionQualifierChunk(const char *Text);
+
   /// Add a new result-type chunk.
   void AddResultTypeChunk(const char *ResultType);
 

clang/lib/Sema/CodeCompleteConsumer.cpp

@@ -183,6 +183,7 @@ CodeCompletionString::Chunk::Chunk(ChunkKind Kind, const char *Text)
   case CK_Text:
   case CK_Placeholder:
   case CK_Informative:
+  case CK_FunctionQualifier:
   case CK_ResultType:
   case CK_CurrentParameter:
     this->Text = Text;
@@ -272,6 +273,12 @@ CodeCompletionString::Chunk::CreateInformative(const char *Informative) {
   return Chunk(CK_Informative, Informative);
 }
 
+CodeCompletionString::Chunk
+CodeCompletionString::Chunk::CreateFunctionQualifier(
+    const char *FunctionQualifier) {
+  return Chunk(CK_FunctionQualifier, FunctionQualifier);
+}
+
 CodeCompletionString::Chunk
 CodeCompletionString::Chunk::CreateResultType(const char *ResultType) {
   return Chunk(CK_ResultType, ResultType);
@@ -326,6 +333,7 @@ std::string CodeCompletionString::getAsString() const {
       OS << "<#" << C.Text << "#>";
       break;
     case CK_Informative:
+    case CK_FunctionQualifier:
     case CK_ResultType:
       OS << "[#" << C.Text << "#]";
       break;
@@ -461,6 +469,10 @@ void CodeCompletionBuilder::AddInformativeChunk(const char *Text) {
   Chunks.push_back(Chunk::CreateInformative(Text));
 }
 
+void CodeCompletionBuilder::AddFunctionQualifierChunk(const char *Text) {
+  Chunks.push_back(Chunk::CreateFunctionQualifier(Text));
+}
+
 void CodeCompletionBuilder::AddResultTypeChunk(const char *ResultType) {
   Chunks.push_back(Chunk::CreateResultType(ResultType));
 }
@@ -728,6 +740,7 @@ static std::string getOverloadAsString(const CodeCompletionString &CCS) {
   for (auto &C : CCS) {
     switch (C.Kind) {
     case CodeCompletionString::CK_Informative:
+    case CodeCompletionString::CK_FunctionQualifier:
     case CodeCompletionString::CK_ResultType:
       OS << "[#" << C.Text << "#]";
       break;

clang/lib/Sema/SemaCodeComplete.cpp

@@ -3440,17 +3440,17 @@ static void AddFunctionTypeQuals(CodeCompletionBuilder &Result,
 
   // Handle single qualifiers without copying
   if (Quals.hasOnlyConst()) {
-    Result.AddInformativeChunk(" const");
+    Result.AddFunctionQualifierChunk(" const");
     return;
   }
 
   if (Quals.hasOnlyVolatile()) {
-    Result.AddInformativeChunk(" volatile");
+    Result.AddFunctionQualifierChunk(" volatile");
     return;
   }
 
   if (Quals.hasOnlyRestrict()) {
-    Result.AddInformativeChunk(" restrict");
+    Result.AddFunctionQualifierChunk(" restrict");
     return;
   }
 
@@ -3462,7 +3462,7 @@ static void AddFunctionTypeQuals(CodeCompletionBuilder &Result,
     QualsStr += " volatile";
   if (Quals.hasRestrict())
     QualsStr += " restrict";
-  Result.AddInformativeChunk(Result.getAllocator().CopyString(QualsStr));
+  Result.AddFunctionQualifierChunk(Result.getAllocator().CopyString(QualsStr));
 }
 
 static void
@@ -6859,12 +6859,26 @@ void SemaCodeCompletion::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
   // resolves to a dependent record.
   DeclContext *Ctx = SemaRef.computeDeclContext(SS, /*EnteringContext=*/true);
 
+  // This is used to change context to access private methods for completion
+  DeclContext *SavedContext = nullptr;
+  if (!SemaRef.CurContext->isFunctionOrMethod() &&
+      !SemaRef.CurContext->isRecord()) {
+    // We are in global scope or namespace
+    // -> likely a method definition
+    SavedContext = SemaRef.CurContext;
+    SemaRef.CurContext = Ctx; // Simulate that we are in class scope
+                              // to access private methods
+  }
+
   // Try to instantiate any non-dependent declaration contexts before
   // we look in them. Bail out if we fail.
   NestedNameSpecifier NNS = SS.getScopeRep();
   if (NNS && !NNS.isDependent()) {
-    if (Ctx == nullptr || SemaRef.RequireCompleteDeclContext(SS, Ctx))
+    if (Ctx == nullptr || SemaRef.RequireCompleteDeclContext(SS, Ctx)) {
+      if (SavedContext)
+        SemaRef.CurContext = SavedContext;
       return;
+    }
   }
 
   ResultBuilder Results(SemaRef, CodeCompleter->getAllocator(),
@@ -6910,6 +6924,8 @@ void SemaCodeCompletion::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
                                /*IncludeDependentBases=*/true,
                                CodeCompleter->loadExternal());
   }
+  if (SavedContext)
+    SemaRef.CurContext = SavedContext;
 
   HandleCodeCompleteResults(&SemaRef, CodeCompleter,
                             Results.getCompletionContext(), Results.data(),

clang/tools/libclang/CIndexCodeCompletion.cpp

@@ -70,6 +70,8 @@ clang_getCompletionChunkKind(CXCompletionString completion_string,
   case CodeCompletionString::CK_Placeholder:
     return CXCompletionChunk_Placeholder;
   case CodeCompletionString::CK_Informative:
+  case CodeCompletionString::CK_FunctionQualifier:
+    // as FunctionQualifier are informative, except for completion.
     return CXCompletionChunk_Informative;
   case CodeCompletionString::CK_ResultType:
     return CXCompletionChunk_ResultType;
@@ -120,6 +122,7 @@ CXString clang_getCompletionChunkText(CXCompletionString completion_string,
   case CodeCompletionString::CK_Placeholder:
   case CodeCompletionString::CK_CurrentParameter:
   case CodeCompletionString::CK_Informative:
+  case CodeCompletionString::CK_FunctionQualifier:
   case CodeCompletionString::CK_LeftParen:
   case CodeCompletionString::CK_RightParen:
   case CodeCompletionString::CK_LeftBracket:
@@ -159,6 +162,7 @@ clang_getCompletionChunkCompletionString(CXCompletionString completion_string,
   case CodeCompletionString::CK_Placeholder:
   case CodeCompletionString::CK_CurrentParameter:
   case CodeCompletionString::CK_Informative:
+  case CodeCompletionString::CK_FunctionQualifier:
   case CodeCompletionString::CK_LeftParen:
   case CodeCompletionString::CK_RightParen:
   case CodeCompletionString::CK_LeftBracket: