diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -94,10 +94,14 @@ case SK::Struct: return CompletionItemKind::Struct; case SK::Class: - case SK::Protocol: case SK::Extension: case SK::Union: return CompletionItemKind::Class; + case SK::Protocol: + // Use interface instead of class for differentiation of classes and + // protocols with the same name (e.g. @interface NSObject vs. @protocol + // NSObject). + return CompletionItemKind::Interface; case SK::TypeAlias: // We use the same kind as the VSCode C++ extension. // FIXME: pick a better option when we have one. @@ -712,13 +716,13 @@ case CodeCompletionContext::CCC_Type: case CodeCompletionContext::CCC_ParenthesizedExpression: case CodeCompletionContext::CCC_ObjCInterfaceName: - case CodeCompletionContext::CCC_ObjCCategoryName: case CodeCompletionContext::CCC_Symbol: case CodeCompletionContext::CCC_SymbolOrNewName: return true; case CodeCompletionContext::CCC_OtherWithMacros: case CodeCompletionContext::CCC_DotMemberAccess: case CodeCompletionContext::CCC_ArrowMemberAccess: + case CodeCompletionContext::CCC_ObjCCategoryName: case CodeCompletionContext::CCC_ObjCPropertyAccess: case CodeCompletionContext::CCC_MacroName: case CodeCompletionContext::CCC_MacroNameUse: @@ -1343,6 +1347,22 @@ llvm_unreachable("invalid NestedNameSpecifier kind"); } +// Should we include a symbol from the index given the completion kind? +// FIXME: Ideally we can filter in the fuzzy find request itself. +bool includeSymbolFromIndex(CodeCompletionContext::Kind Kind, + const Symbol &Sym) { + // Objective-C protocols are only useful in ObjC protocol completions, + // in other places they're confusing, especially when they share the same + // identifier with a class. + if (Sym.SymInfo.Kind == index::SymbolKind::Protocol && + Sym.SymInfo.Lang == index::SymbolLanguage::ObjC) + return Kind == CodeCompletionContext::CCC_ObjCProtocolName; + else if (Kind == CodeCompletionContext::CCC_ObjCProtocolName) + // Don't show anything else in ObjC protocol completions. + return false; + return true; +} + std::future startAsyncFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req) { return runAsync([&Index, Req]() { @@ -1675,14 +1695,6 @@ return Output; } - bool includeSymbolFromIndex(const Symbol &Sym) { - if (CCContextKind == CodeCompletionContext::CCC_ObjCProtocolName) { - return Sym.SymInfo.Lang == index::SymbolLanguage::ObjC && - Sym.SymInfo.Kind == index::SymbolKind::Protocol; - } - return true; - } - SymbolSlab queryIndex() { trace::Span Tracer("Query index"); SPAN_ATTACH(Tracer, "limit", int64_t(Opts.Limit)); @@ -1716,10 +1728,8 @@ // Run the query against the index. SymbolSlab::Builder ResultsBuilder; - if (Opts.Index->fuzzyFind(Req, [&](const Symbol &Sym) { - if (includeSymbolFromIndex(Sym)) - ResultsBuilder.insert(Sym); - })) + if (Opts.Index->fuzzyFind( + Req, [&](const Symbol &Sym) { ResultsBuilder.insert(Sym); })) Incomplete = true; return std::move(ResultsBuilder).build(); } @@ -1783,6 +1793,8 @@ for (const auto &IndexResult : IndexResults) { if (UsedIndexResults.count(&IndexResult)) continue; + if (!includeSymbolFromIndex(CCContextKind, IndexResult)) + continue; AddToBundles(/*SemaResult=*/nullptr, &IndexResult, nullptr); } // Emit identifier results. diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1493,12 +1493,16 @@ class IndexRequestCollector : public SymbolIndex { public: + IndexRequestCollector(std::vector Syms = {}) : Symbols(Syms) {} + bool fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const override { std::unique_lock Lock(Mut); Requests.push_back(Req); ReceivedRequestCV.notify_one(); + for (const auto &Sym : Symbols) + Callback(Sym); return true; } @@ -1533,6 +1537,7 @@ } private: + std::vector Symbols; // We need a mutex to handle async fuzzy find requests. mutable std::condition_variable ReceivedRequestCV; mutable std::mutex Mut; @@ -3214,8 +3219,74 @@ {SymFood, FoodClass, SymFooey}, /*Opts=*/{}, "Foo.m"); - auto C = Results.Completions; - EXPECT_THAT(C, UnorderedElementsAre(named("Food"), named("Fooey"))); + // Should only give protocols for ObjC protocol completions. + EXPECT_THAT(Results.Completions, + UnorderedElementsAre( + AllOf(named("Food"), kind(CompletionItemKind::Interface)), + AllOf(named("Fooey"), kind(CompletionItemKind::Interface)))); + + Results = completions(R"objc( + Fo^ + )objc", + {SymFood, FoodClass, SymFooey}, + /*Opts=*/{}, "Foo.m"); + // Shouldn't give protocols for non protocol completions. + EXPECT_THAT(Results.Completions, + ElementsAre( + AllOf(named("FoodClass"), kind(CompletionItemKind::Class)))); +} + +TEST(CompletionTest, ObjectiveCProtocolFromIndexSpeculation) { + MockFS FS; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, ClangdServer::optsForTest()); + + auto File = testPath("Foo.m"); + Annotations Test(R"cpp( + @protocol Food + @end + id foo; + )cpp"); + runAddDocument(Server, File, Test.code()); + clangd::CodeCompleteOptions Opts = {}; + + Symbol FoodClass = objcClass("FoodClass"); + IndexRequestCollector Requests({FoodClass}); + Opts.Index = &Requests; + + auto CompleteAtPoint = [&](StringRef P) { + return cantFail(runCodeComplete(Server, File, Test.point(P), Opts)).Completions; + }; + + auto C = CompleteAtPoint("1"); + auto Reqs1 = Requests.consumeRequests(1); + ASSERT_EQ(Reqs1.size(), 1u); + EXPECT_THAT(C, + ElementsAre( + AllOf(named("Food"), kind(CompletionItemKind::Interface)))); + + C = CompleteAtPoint("1"); + auto Reqs2 = Requests.consumeRequests(1); + // Speculation succeeded. Used speculative index result, keeping the same + // exact filtering as before to exclude the FoodClass result. + ASSERT_EQ(Reqs2.size(), 1u); + EXPECT_EQ(Reqs2[0], Reqs1[0]); + EXPECT_THAT(C, + ElementsAre( + AllOf(named("Food"), kind(CompletionItemKind::Interface)))); +} + +TEST(CompletionTest, ObjectiveCCategoryFromIndexIgnored) { + Symbol FoodCategory = objcCategory("FoodClass", "Extension"); + auto Results = completions(R"objc( + @interface Foo + @end + @interface Foo (^) + @end + )objc", + {FoodCategory}, + /*Opts=*/{}, "Foo.m"); + EXPECT_THAT(Results.Completions, IsEmpty()); } TEST(CompletionTest, CursorInSnippets) { diff --git a/clang-tools-extra/clangd/unittests/TestIndex.h b/clang-tools-extra/clangd/unittests/TestIndex.h --- a/clang-tools-extra/clangd/unittests/TestIndex.h +++ b/clang-tools-extra/clangd/unittests/TestIndex.h @@ -39,6 +39,8 @@ llvm::StringRef USRPrefix); // Create an @interface or @implementation. Symbol objcClass(llvm::StringRef Name); +// Create an @interface or @implementation category. +Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName); // Create an @protocol. Symbol objcProtocol(llvm::StringRef Name); diff --git a/clang-tools-extra/clangd/unittests/TestIndex.cpp b/clang-tools-extra/clangd/unittests/TestIndex.cpp --- a/clang-tools-extra/clangd/unittests/TestIndex.cpp +++ b/clang-tools-extra/clangd/unittests/TestIndex.cpp @@ -99,6 +99,11 @@ return objcSym(Name, index::SymbolKind::Class, "objc(cs)"); } +Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName) { + std::string USRPrefix = ("objc(cy)" + Name + "@").str(); + return objcSym(CategoryName, index::SymbolKind::Extension, USRPrefix); +} + Symbol objcProtocol(llvm::StringRef Name) { return objcSym(Name, index::SymbolKind::Protocol, "objc(pl)"); }