[clangd][ObjC] Filter ObjC method completions on the remaining selector

[dgoldman]

Summary

Previously, clangd would filter completions only on the first part of
the selector (first typed chunk) instead of all remaining selector
fragments (all typed chunks).

[dgoldman]

Friendly ping

[sammccall]

Sorry about the delay :-(

clang-tools-extra/clangd/CodeComplete.cpp

1989

also here

clang/include/clang/Sema/CodeCompleteConsumer.h

612

I'm not sure we should add this new API.
Our only use case so far is an optimization (over getAllTypedText) and it's not a great fit for this (it requires us to do two traversals of the CCS in the common case).
On the other hand an interface that avoids this is specialized.

Instead maybe just place the loop at the callsite in getName()?

const Chunk *OnlyText = nullptr;
for (Chunk &C : CCS) {
  if (!C is TypedText) continue;
  if (OnlyText) return CCS.getAllTypedText()
  OnlyText = &C;
}
return C ? C->Text : nullptr;

[dgoldman]

Go through chunks in getName and add missed FilterText place

clang-tools-extra/clangd/CodeComplete.h

 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
 // We don't use the LSP structures here (unlike most features) as we want
 // to expose more data to allow for more precise testing and evaluation.
 struct CodeCompletion {
   // The unqualified name of the symbol or other completion item.
   std::string Name;
+  // The name of the symbol for filtering and sorting purposes. Typically the
+  // same as `Name`, but may be different e.g. for ObjC methods, `Name` is the
+  // first selector fragment but the `FilterText` is the entire selector.
+  std::string FilterText;
   // The scope qualifier for the symbol name. e.g. "ns1::ns2::"
   // Empty for non-symbol completions. Not inserted, but may be displayed.
   std::string Scope;
   // Text that must be inserted before the name, and displayed (e.g. base::).
   std::string RequiredQualifier;
   // Details to be displayed following the name. Not inserted.
   std::string Signature;
   // Text to be inserted following the name, in snippet format.

clang-tools-extra/clangd/CodeComplete.cpp

CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,
         EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
         IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
     Completion.Deprecated = true; // cleared by any non-deprecated overload.
     add(C, SemaCCS);
     if (C.SemaResult) {
       assert(ASTCtx);
       Completion.Origin |= SymbolOrigin::AST;
       Completion.Name = std::string(llvm::StringRef(SemaCCS->getTypedText()));
+      Completion.FilterText = SemaCCS->getAllTypedText();
       if (Completion.Scope.empty()) {
         if ((C.SemaResult->Kind == CodeCompletionResult::RK_Declaration) ||
             (C.SemaResult->Kind == CodeCompletionResult::RK_Pattern))
           if (const auto *D = C.SemaResult->getDeclaration())
             if (const auto *ND = dyn_cast<NamedDecl>(D))
               Completion.Scope = std::string(
                   splitQualifiedName(printQualifiedName(*ND)).first);
       }
CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,
     if (C.IndexResult) {
       Completion.Origin |= C.IndexResult->Origin;
       if (Completion.Scope.empty())
         Completion.Scope = std::string(C.IndexResult->Scope);
       if (Completion.Kind == CompletionItemKind::Missing)
         Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
       if (Completion.Name.empty())
         Completion.Name = std::string(C.IndexResult->Name);
+      if (Completion.FilterText.empty())
+        Completion.FilterText = Completion.Name;
       // If the completion was visible to Sema, no qualifier is needed. This
       // avoids unneeded qualifiers in cases like with `using ns::X`.
       if (Completion.RequiredQualifier.empty() && !C.SemaResult) {
         llvm::StringRef ShortestQualifier = C.IndexResult->Scope;
         for (llvm::StringRef Scope : QueryScopes) {
           llvm::StringRef Qualifier = C.IndexResult->Scope;
           if (Qualifier.consume_front(Scope) &&
               Qualifier.size() < ShortestQualifier.size())
             ShortestQualifier = Qualifier;
         }
         Completion.RequiredQualifier = std::string(ShortestQualifier);
       }
     }
     if (C.IdentifierResult) {
       Completion.Origin |= SymbolOrigin::Identifier;
       Completion.Kind = CompletionItemKind::Text;
       Completion.Name = std::string(C.IdentifierResult->Name);
+      Completion.FilterText = Completion.Name;
     }
 
     // Turn absolute path into a literal string that can be #included.
     auto Inserted = [&](llvm::StringRef Header)
         -> llvm::Expected<std::pair<std::string, bool>> {
       auto ResolvedDeclaring =
           URI::resolve(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
       if (!ResolvedDeclaring)
llvm::StringRef getName(const CodeCompletionResult &Result) {
     case CodeCompletionResult::RK_Keyword:
       return Result.Keyword;
     case CodeCompletionResult::RK_Macro:
       return Result.Macro->getName();
     case CodeCompletionResult::RK_Pattern:
       return Result.Pattern->getTypedText();
     }
     auto *CCS = codeCompletionString(Result);
+    if (!CCS->hasMultipleTypedTextChunks())
       return CCS->getTypedText();
+    std::string Name = CCS->getAllTypedText();
+    return CCAllocator->CopyString(Name);
   }
 
   // Build a CodeCompletion string for R, which must be from Results.
   // The CCS will be owned by this recorder.
   CodeCompletionString *codeCompletionString(const CodeCompletionResult &R) {
     // CodeCompletionResult doesn't seem to be const-correct. We own it, anyway.
     return const_cast<CodeCompletionResult &>(R).CreateCodeCompletionString(
         *CCSema, CCContext, *CCAllocator, CCTUInfo,
if (ParamNames.empty())
     return CodeCompleteResult();
 
   CodeCompleteResult Result;
   Result.Context = CodeCompletionContext::CCC_NaturalLanguage;
   for (llvm::StringRef Name : ParamNames) {
     if (!Name.startswith(Prefix))
       continue;
     CodeCompletion Item;
     Item.Name = Name.str() + "=";
     Item.Kind = CompletionItemKind::Text;
     Result.Completions.push_back(Item);
   }
 
   return Result;
 }
 
 // If Offset is inside what looks like argument comment (e.g.
CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   if (InsertInclude || Documentation) {
     markup::Document Doc;
     if (InsertInclude)
       Doc.addParagraph().appendText("From ").appendCode(InsertInclude->Header);
     if (Documentation)
       Doc.append(*Documentation);
     LSP.documentation = renderDoc(Doc, Opts.DocumentationFormat);
   }
-  LSP.sortText = sortText(Score.Total, Name);
-  LSP.filterText = Name;
+  LSP.sortText = sortText(Score.Total, FilterText);
+  LSP.filterText = FilterText;
   LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name};
   // Merge continuous additionalTextEdits into main edit. The main motivation
   // behind this is to help LSP clients, it seems most of them are confused when
   // they are provided with additionalTextEdits that are consecutive to main
   // edit.
   // Note that we store additional text edits from back to front in a line. That
   // is mainly to help LSP clients again, so that changes do not effect each
   // other.

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

 
 // GMock helpers for matching completion items.
 MATCHER_P(named, Name, "") { return arg.Name == Name; }
 MATCHER_P(mainFileRefs, Refs, "") { return arg.MainFileRefs == Refs; }
 MATCHER_P(scopeRefs, Refs, "") { return arg.ScopeRefsInFile == Refs; }
 MATCHER_P(nameStartsWith, Prefix, "") {
   return llvm::StringRef(arg.Name).startswith(Prefix);
 }
+MATCHER_P(filterText, F, "") { return arg.FilterText == F; }
 MATCHER_P(scope, S, "") { return arg.Scope == S; }
 MATCHER_P(qualifier, Q, "") { return arg.RequiredQualifier == Q; }
 MATCHER_P(labeled, Label, "") {
   return arg.RequiredQualifier + arg.Name + arg.Signature == Label;
 }
 MATCHER_P(sigHelpLabeled, Label, "") { return arg.label == Label; }
 MATCHER_P(kind, K, "") { return arg.Kind == K; }
 MATCHER_P(doc, D, "") {
TEST(CompletionTest, QualifiedNames) {
   EXPECT_THAT(
       Results.Completions,
       UnorderedElementsAre(scope("ns::"), scope("ns::"), scope("ns::")));
 }
 
 TEST(CompletionTest, Render) {
   CodeCompletion C;
   C.Name = "x";
+  C.FilterText = "x";
   C.Signature = "(bool) const";
   C.SnippetSuffix = "(${0:bool})";
   C.ReturnType = "int";
   C.RequiredQualifier = "Foo::";
   C.Scope = "ns::Foo::";
   C.Documentation.emplace();
   C.Documentation->addParagraph().appendText("This is ").appendCode("x()");
   C.Includes.emplace_back();
TEST(CompletionTest, Render) {
   EXPECT_EQ(R.filterText, "x");
   EXPECT_EQ(R.detail, "int");
   EXPECT_EQ(R.documentation->value, "From \"foo.h\"\nThis is x()");
   EXPECT_THAT(R.additionalTextEdits, IsEmpty());
   EXPECT_EQ(R.sortText, sortText(1.0, "x"));
   EXPECT_FALSE(R.deprecated);
   EXPECT_EQ(R.score, .5f);
 
+  C.FilterText = "xtra";
+  R = C.render(Opts);
+  EXPECT_EQ(R.filterText, "xtra");
+  EXPECT_EQ(R.sortText, sortText(1.0, "xtra"));
+
   Opts.EnableSnippets = true;
   R = C.render(Opts);
   EXPECT_EQ(R.insertText, "Foo::x(${0:bool})");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
 
   C.SnippetSuffix = "";
   R = C.render(Opts);
   EXPECT_EQ(R.insertText, "Foo::x");
TEST(CompletionTest, ObjectiveCMethodTwoArgumentsFromMiddle) {
   auto C = Results.Completions;
   EXPECT_THAT(C, ElementsAre(named("fooey:")));
   EXPECT_THAT(C, ElementsAre(kind(CompletionItemKind::Method)));
   EXPECT_THAT(C, ElementsAre(returnType("id")));
   EXPECT_THAT(C, ElementsAre(signature("(unsigned int)")));
   EXPECT_THAT(C, ElementsAre(snippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCMethodFilterOnEntireSelector) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      + (id)player:(id)player willRun:(id)run;
+      @end
+      id val = [Foo wi^]
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(named("player:")));
+  EXPECT_THAT(C, ElementsAre(filterText("player:willRun:")));
+  EXPECT_THAT(C, ElementsAre(kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(returnType("id")));
+  EXPECT_THAT(C, ElementsAre(signature("(id) willRun:(id)")));
+  EXPECT_THAT(C, ElementsAre(snippetSuffix("${1:(id)} willRun:${2:(id)}")));
+}
+
 TEST(CompletionTest, ObjectiveCSimpleMethodDeclaration) {
   auto Results = completions(R"objc(
       @interface Foo
       - (void)foo;
       @end
       @implementation Foo
       fo^
       @end

clang/include/clang/Sema/CodeCompleteConsumer.h

public:
   bool empty() const { return NumChunks == 0; }
   unsigned size() const { return NumChunks; }
 
   const Chunk &operator[](unsigned I) const {
     assert(I < size() && "Chunk index out-of-range");
     return begin()[I];
   }
 
-  /// Returns the text in the TypedText chunk.
+  /// Returns whether this completion string has multiple TypedText chunks.
+  /// Typically there is only, but some completions, like for Objective-C
+  /// methods, can have multiple.
+  bool hasMultipleTypedTextChunks() const;
+
+  /// Returns the text in the first TypedText chunk.
   const char *getTypedText() const;
 
+  /// Returns the combined text from all TypedText chunks.
+  std::string getAllTypedText() const;
+
   /// Retrieve the priority of this code completion result.
   unsigned getPriority() const { return Priority; }
 
   /// Retrieve the availability of this code completion result.
   unsigned getAvailability() const { return Availability; }
 
   /// Retrieve the number of annotations for this code completion result.
   unsigned getAnnotationCount() const;

clang/lib/Sema/CodeCompleteConsumer.cpp

for (const Chunk &C : *this) {
     default:
       OS << C.Text;
       break;
     }
   }
   return Result;
 }
 
+bool CodeCompletionString::hasMultipleTypedTextChunks() const {
+  bool SeenTypedChunk = false;
+  for (const Chunk &C : *this) {
+    if (C.Kind == CK_TypedText) {
+      if (SeenTypedChunk)
+        return true;
+      else
+        SeenTypedChunk = true;
+    }
+  }
+  return false;
+}
+
 const char *CodeCompletionString::getTypedText() const {
   for (const Chunk &C : *this)
     if (C.Kind == CK_TypedText)
       return C.Text;
 
   return nullptr;
 }
 
+std::string CodeCompletionString::getAllTypedText() const {
+  std::string Res;
+  for (const Chunk &C : *this)
+    if (C.Kind == CK_TypedText)
+      Res += C.Text;
+
+  return Res;
+}
+
 const char *CodeCompletionAllocator::CopyString(const Twine &String) {
   SmallString<128> Data;
   StringRef Ref = String.toStringRef(Data);
   // FIXME: It would be more efficient to teach Twine to tell us its size and
   // then add a routine there to fill in an allocated char* with the contents
   // of the string.
   char *Mem = (char *)Allocate(Ref.size() + 1, 1);
   std::copy(Ref.begin(), Ref.end(), Mem);