[clang-tools-extra] e09c750 - [clangd][ObjC] Improve completions for protocols + category names
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 8 08:31:05 PDT 2022
Author: David Goldman
Date: 2022-09-08T11:30:26-04:00
New Revision: e09c75049854fee251e99c4c3c55f3f391f52a10
URL: https://github.com/llvm/llvm-project/commit/e09c75049854fee251e99c4c3c55f3f391f52a10
DIFF: https://github.com/llvm/llvm-project/commit/e09c75049854fee251e99c4c3c55f3f391f52a10.diff
LOG: [clangd][ObjC] Improve completions for protocols + category names
- Render protocols as interfaces to differentiate them from classes
since a protocol and class can have the same name. Take this one step
further though, and only recommend protocols in ObjC protocol completions.
- Properly call `includeSymbolFromIndex` even with a cached
speculative fuzzy find request
- Don't use the index to provide completions for category names,
symbols there don't make sense
Differential Revision: https://reviews.llvm.org/D132962
Added:
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/TestIndex.cpp
clang-tools-extra/clangd/unittests/TestIndex.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index f10235894190..9d9b0e748153 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -94,10 +94,14 @@ CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
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
diff erentiation 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 @@ bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
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 @@ bool allowIndex(CodeCompletionContext &CC) {
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<SymbolSlab> startAsyncFuzzyFind(const SymbolIndex &Index,
const FuzzyFindRequest &Req) {
return runAsync<SymbolSlab>([&Index, Req]() {
@@ -1675,14 +1695,6 @@ class CodeCompleteFlow {
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,11 +1728,8 @@ class CodeCompleteFlow {
// Run the query against the index.
SymbolSlab::Builder ResultsBuilder;
- if (Opts.Index->fuzzyFind(Req, [&](const Symbol &Sym) {
- if (includeSymbolFromIndex(Sym))
- ResultsBuilder.insert(Sym);
- }))
- Incomplete = true;
+ Incomplete |= Opts.Index->fuzzyFind(
+ Req, [&](const Symbol &Sym) { ResultsBuilder.insert(Sym); });
return std::move(ResultsBuilder).build();
}
@@ -1783,6 +1792,8 @@ class CodeCompleteFlow {
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
index 5050ab203b8d..f2d8328afdbb 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1493,12 +1493,16 @@ TEST(SignatureHelpTest, StalePreamble) {
class IndexRequestCollector : public SymbolIndex {
public:
+ IndexRequestCollector(std::vector<Symbol> Syms = {}) : Symbols(Syms) {}
+
bool
fuzzyFind(const FuzzyFindRequest &Req,
llvm::function_ref<void(const Symbol &)> Callback) const override {
std::unique_lock<std::mutex> Lock(Mut);
Requests.push_back(Req);
ReceivedRequestCV.notify_one();
+ for (const auto &Sym : Symbols)
+ Callback(Sym);
return true;
}
@@ -1533,6 +1537,7 @@ class IndexRequestCollector : public SymbolIndex {
}
private:
+ std::vector<Symbol> Symbols;
// We need a mutex to handle async fuzzy find requests.
mutable std::condition_variable ReceivedRequestCV;
mutable std::mutex Mut;
@@ -3208,14 +3213,74 @@ TEST(CompletionTest, ObjectiveCProtocolFromIndex) {
Symbol FoodClass = objcClass("FoodClass");
Symbol SymFood = objcProtocol("Food");
Symbol SymFooey = objcProtocol("Fooey");
+ auto Results = completions("id<Foo^>", {SymFood, FoodClass, SymFooey},
+ /*Opts=*/{}, "Foo.m");
+
+ // 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("Fo^", {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$1^> foo;
+ Foo$2^ bar;
+ )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("2");
+ auto Reqs2 = Requests.consumeRequests(1);
+ // Speculation succeeded. Used speculative index result, but filtering now to
+ // now include FoodClass.
+ ASSERT_EQ(Reqs2.size(), 1u);
+ EXPECT_EQ(Reqs2[0], Reqs1[0]);
+ EXPECT_THAT(C, ElementsAre(AllOf(named("FoodClass"),
+ kind(CompletionItemKind::Class))));
+}
+
+TEST(CompletionTest, ObjectiveCCategoryFromIndexIgnored) {
+ Symbol FoodCategory = objcCategory("FoodClass", "Extension");
auto Results = completions(R"objc(
- id<Foo^>
+ @interface Foo
+ @end
+ @interface Foo (^)
+ @end
)objc",
- {SymFood, FoodClass, SymFooey},
+ {FoodCategory},
/*Opts=*/{}, "Foo.m");
-
- auto C = Results.Completions;
- EXPECT_THAT(C, UnorderedElementsAre(named("Food"), named("Fooey")));
+ EXPECT_THAT(Results.Completions, IsEmpty());
}
TEST(CompletionTest, CursorInSnippets) {
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.cpp b/clang-tools-extra/clangd/unittests/TestIndex.cpp
index 6f7bd3aac207..c247a9c2e90c 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ b/clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -99,6 +99,11 @@ Symbol objcClass(llvm::StringRef Name) {
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)");
}
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.h b/clang-tools-extra/clangd/unittests/TestIndex.h
index 6a4d2cb5cdd0..0cd8a713c31d 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.h
+++ b/clang-tools-extra/clangd/unittests/TestIndex.h
@@ -39,6 +39,8 @@ Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,
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);
More information about the cfe-commits
mailing list