[clangd][ObjC] Fix ObjC method definition completion

[dgoldman]

Summary

D124637 improved filtering of method expressions, but not method
definitions. With this change, clangd will now filter ObjC method
definition completions based on their entire selector instead of
only the first selector fragment.

[sammccall]

Ah, this is subtle. Thanks!

clang-tools-extra/clangd/CodeComplete.cpp

866

weirdly, codeCompletionString(Result) looks like it would mutate Result and then return a pattern with the same chunks (but different extra fields). So not affect the result of this function, but possibly others as a side-effect.

(e.g. add BriefComment and ParentName in the case that there's a pattern + declaration, as there is here)

What you have is fine, but if you feel like digging deeper codeCompletionString(Result) may be either better or worse here!

[dgoldman]

clang-tools-extra/clangd/CodeComplete.cpp

866

Hmm from what I can tell, it appears those are unused by clangd, but for posterity I can swap it over

[dgoldman]

• Use codeCompletionString(Result) for RK_Pattern

clang-tools-extra/clangd/CodeComplete.cpp

struct CompletionRecorder : public CodeCompleteConsumer {
   }
 
   CodeCompletionAllocator &getAllocator() override { return *CCAllocator; }
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
 
   // Returns the filtering/sorting name for Result, which must be from Results.
   // Returned string is owned by this recorder (or the AST).
   llvm::StringRef getName(const CodeCompletionResult &Result) {
+    CodeCompletionString *CCS = nullptr;
     switch (Result.Kind) {
     case CodeCompletionResult::RK_Declaration:
       if (auto *ID = Result.Declaration->getIdentifier())
         return ID->getName();
+      CCS = codeCompletionString(Result);
       break;
     case CodeCompletionResult::RK_Keyword:
       return Result.Keyword;
     case CodeCompletionResult::RK_Macro:
       return Result.Macro->getName();
     case CodeCompletionResult::RK_Pattern:
-      return Result.Pattern->getTypedText();
+      CCS = Result.Pattern;
+      break;
     }
-    auto *CCS = codeCompletionString(Result);
     const CodeCompletionString::Chunk *OnlyText = nullptr;
     for (auto &C : *CCS) {
       if (C.Kind != CodeCompletionString::CK_TypedText)
         continue;
       if (OnlyText)
         return CCAllocator->CopyString(CCS->getAllTypedText());
       OnlyText = &C;
     }

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

TEST(CompletionTest, ObjectiveCMethodDeclaration) {
 
   auto C = Results.Completions;
   EXPECT_THAT(C, ElementsAre(named("valueForCharacter:")));
   EXPECT_THAT(C, ElementsAre(kind(CompletionItemKind::Method)));
   EXPECT_THAT(C, ElementsAre(qualifier("- (int)")));
   EXPECT_THAT(C, ElementsAre(signature("(char)c secondArgument:(id)object")));
 }
 
+TEST(CompletionTest, ObjectiveCMethodDeclarationFilterOnEntireSelector) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      - (int)valueForCharacter:(char)c secondArgument:(id)object;
+      @end
+      @implementation Foo
+      secondArg^
+      @end
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(filterText("valueForCharacter:secondArgument:")));
+  EXPECT_THAT(C, ElementsAre(kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(qualifier("- (int)")));
+  EXPECT_THAT(C, ElementsAre(signature("(char)c secondArgument:(id)object")));
+}
+
 TEST(CompletionTest, ObjectiveCMethodDeclarationPrefixTyped) {
   auto Results = completions(R"objc(
       @interface Foo
       - (int)valueForCharacter:(char)c;
       @end
       @implementation Foo
       - (int)valueFor^
       @end