dgoldman updated this revision to Diff 340619.
dgoldman marked 4 inline comments as done.
dgoldman added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100798/new/
https://reviews.llvm.org/D100798
Files:
clang-tools-extra/clangd/CodeCompletionStrings.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2783,6 +2783,61 @@
EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
}
+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;
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@
}
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 @@
// 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 @@
// 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 differs 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 @@
*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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
