[clang-tools-extra] 2a030e6 - [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2021-06-01T13:35:05-04:00
New Revision: 2a030e680e0812c652ed4ae2b012e285a5514ffa

URL: https://github.com/llvm/llvm-project/commit/2a030e680e0812c652ed4ae2b012e285a5514ffa
DIFF: https://github.com/llvm/llvm-project/commit/2a030e680e0812c652ed4ae2b012e285a5514ffa.diff

LOG: [clangd][ObjC] Fix issue completing a method decl by name

When completing an Objective-C method declaration by name, we need to
preserve the leading text as a `qualifier` so we insert it properly
before the first typed text chunk.

Differential Revision: https://reviews.llvm.org/D100798

Added: 
    

Modified: 
    clang-tools-extra/clangd/CodeCompletionStrings.cpp
    clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index d4a3bdafcae0..8205c88a5b66 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
   }
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
+  bool HadInformativeChunks = false;
   for (const auto &Chunk : CCS) {
     // Informative qualifier chunks only clutter completion results, skip
     // them.
@@ -129,10 +130,14 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
       //   reclassified as qualifiers.
       //
       // Objective-C:
-      //   Objective-C methods may have multiple typed-text chunks, so we must
-      //   treat them carefully. For Objective-C methods, all typed-text chunks
-      //   will end in ':' (unless there are no arguments, in which case we
-      //   can safely treat them as C++).
+      //   Objective-C methods expressions may have multiple typed-text chunks,
+      //   so we must treat them carefully. For Objective-C methods, all
+      //   typed-text and informative chunks will end in ':' (unless there are
+      //   no arguments, in which case we can safely treat them as C++).
+      //
+      //   Completing a method declaration itself (not a method expression) is
+      //   similar except that we use the `RequiredQualifiers` to store the
+      //   text before the selector, e.g. `- (void)`.
       if (!llvm::StringRef(Chunk.Text).endswith(":")) { // Treat as C++.
         if (RequiredQualifiers)
           *RequiredQualifiers = std::move(*Signature);
@@ -147,6 +152,28 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
         // methods.
         if (!HadObjCArguments) {
           HadObjCArguments = true;
+          // If we have no previous informative chunks (informative selector
+          // fragments in practice), we treat any previous chunks as
+          // `RequiredQualifiers` so they will be added as a prefix during the
+          // completion.
+          //
+          // e.g. to complete `- (void)doSomething:(id)argument`:
+          // - Completion name: `doSomething:`
+          // - RequiredQualifiers: `- (void)`
+          // - Snippet/Signature suffix: `(id)argument`
+          //
+          // This 
diff ers from the case when we're completing a method
+          // expression with a previous informative selector fragment.
+          //
+          // e.g. to complete `[self doSomething:nil ^somethingElse:(id)]`:
+          // - Previous Informative Chunk: `doSomething:`
+          // - Completion name: `somethingElse:`
+          // - Snippet/Signature suffix: `(id)`
+          if (!HadInformativeChunks) {
+            if (RequiredQualifiers)
+              *RequiredQualifiers = std::move(*Signature);
+            Snippet->clear();
+          }
           Signature->clear();
         } else { // Subsequent argument, considered part of snippet/signature.
           *Signature += Chunk.Text;
@@ -173,6 +200,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
       *Snippet += '}';
       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;

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e4e56f2b5e38..78f6e7c4aa9f 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2796,6 +2796,79 @@ TEST(CompletionTest, ObjectiveCMethodTwoArgumentsFromMiddle) {
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCSimpleMethodDeclaration) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      - (void)foo;
+      @end
+      @implementation Foo
+      fo^
+      @end
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("foo")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (void)")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclaration) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      - (int)valueForCharacter:(char)c secondArgument:(id)object;
+      @end
+      @implementation Foo
+      valueFor^
+      @end
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  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, ObjectiveCMethodDeclarationPrefixTyped) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      - (int)valueForCharacter:(char)c;
+      @end
+      @implementation Foo
+      - (int)valueFor^
+      @end
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationFromMiddle) {
+  auto Results = completions(R"objc(
+      @interface Foo
+      - (int)valueForCharacter:(char)c secondArgument:(id)object;
+      @end
+      @implementation Foo
+      - (int)valueForCharacter:(char)c second^
+      @end
+    )objc",
+                             /*IndexSymbols=*/{},
+                             /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("secondArgument:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(id)object")));
+}
+
 TEST(CompletionTest, CursorInSnippets) {
   clangd::CodeCompleteOptions Options;
   Options.EnableSnippets = true;