dgoldman updated this revision to Diff 457049.
dgoldman added a comment.
Add tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132962/new/
https://reviews.llvm.org/D132962
Files:
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
Index: clang-tools-extra/clangd/unittests/TestIndex.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestIndex.h
+++ 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);
Index: clang-tools-extra/clangd/unittests/TestIndex.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ 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)");
}
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1493,12 +1493,16 @@
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 @@
}
private:
+ std::vector<Symbol> 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$1^> 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) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ 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<SymbolSlab> startAsyncFuzzyFind(const SymbolIndex &Index,
const FuzzyFindRequest &Req) {
return runAsync<SymbolSlab>([&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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
