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 @@ -291,7 +291,8 @@ // computed from the first candidate, in the constructor. // Others vary per candidate, so add() must be called for remaining candidates. struct CodeCompletionBuilder { - CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C, + CodeCompletionBuilder(ASTContext *ASTCtx, DeclContext *SemaDeclCtx, + const CompletionCandidate &C, CodeCompletionString *SemaCCS, llvm::ArrayRef QueryScopes, const IncludeInserter &Includes, @@ -299,13 +300,15 @@ CodeCompletionContext::Kind ContextKind, const CodeCompleteOptions &Opts, bool IsUsingDeclaration, tok::TokenKind NextTokenKind) - : ASTCtx(ASTCtx), + : ASTCtx(ASTCtx), SemaDeclCtx(SemaDeclCtx), EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), - IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { + ContextKind(ContextKind), IsUsingDeclaration(IsUsingDeclaration), + NextTokenKind(NextTokenKind) { Completion.Deprecated = true; // cleared by any non-deprecated overload. add(C, SemaCCS); if (C.SemaResult) { assert(ASTCtx); + assert(SemaDeclCtx); Completion.Origin |= SymbolOrigin::AST; Completion.Name = std::string(llvm::StringRef(SemaCCS->getTypedText())); Completion.FilterText = SemaCCS->getAllTypedText(); @@ -412,6 +415,8 @@ getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, &Completion.RequiredQualifier, IsPattern); S.ReturnType = getReturnType(*SemaCCS); + maybeClearSnippetSuffixForMethodFunctionPointer(*C.SemaResult, + S.SnippetSuffix); } else if (C.IndexResult) { S.Signature = std::string(C.IndexResult->Signature); S.SnippetSuffix = std::string(C.IndexResult->CompletionSnippetSuffix); @@ -570,11 +575,57 @@ return "(…)"; } + // Clear snippet when completing a non-static member function (and not via + // dot/arrow member access) and we're not inside that class' scope + void clearSnippetSuffixWhenNotInsideClassScope( + const CodeCompletionResult &SemaResult, std::string &SnippetSuffix) { + if (SemaResult.Kind != CodeCompletionResult::RK_Declaration || + // This check makes sure we only consider Foo::^ but not foo->^ here + ContextKind != CodeCompletionContext::Kind::CCC_Symbol) + return; + + const auto *CompletedMethod = + llvm::dyn_cast(SemaResult.getDeclaration()); + if (!CompletedMethod || CompletedMethod->isStatic()) + return; + + const auto *CurrentClassScope = [&]() -> const CXXRecordDecl * { + for (DeclContext *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent()) { + const auto *CtxMethod = llvm::dyn_cast(Ctx); + if (CtxMethod && !CtxMethod->getParent()->isLambda()) { + return CtxMethod->getParent(); + } + } + return nullptr; + }(); + + bool InsideCompletedClassScope = + CurrentClassScope && + (CurrentClassScope == CompletedMethod->getParent() || + CurrentClassScope->isDerivedFrom(CompletedMethod->getParent())); + if (InsideCompletedClassScope) + return; + + SnippetSuffix.clear(); + } + + // Some heuristics for when we don't want to add a snippet when completing + // a member function + void maybeClearSnippetSuffixForMethodFunctionPointer( + const CodeCompletionResult &SemaResult, std::string &SnippetSuffix) { + clearSnippetSuffixWhenNotInsideClassScope(SemaResult, SnippetSuffix); + // TODO Implement 2nd heuristic mentioned in + // https://github.com/clangd/clangd/issues/968#issuecomment-1002242704 + } + // ASTCtx can be nullptr if not run with sema. ASTContext *ASTCtx; + // SemaDeclCtx can be nullptr if not run with sema. + DeclContext *SemaDeclCtx; CodeCompletion Completion; llvm::SmallVector Bundled; bool EnableFunctionArgSnippets; + CodeCompletionContext::Kind ContextKind; // No snippets will be generated for using declarations and when the function // arguments are already present. bool IsUsingDeclaration; @@ -1934,7 +1985,8 @@ : nullptr; if (!Builder) Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, - Item, SemaCCS, QueryScopes, *Inserter, FileName, + Recorder ? Recorder->CCSema->CurContext : nullptr, Item, + SemaCCS, QueryScopes, *Inserter, FileName, CCContextKind, Opts, IsUsingDeclaration, NextTokenKind); else Builder->add(Item, SemaCCS); 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 @@ -500,6 +500,63 @@ snippetSuffix("(${1:int i}, ${2:const float f})"))); } +TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) { + clangd::CodeCompleteOptions Opts; + Opts.EnableSnippets = true; + + Annotations Code(R"cpp( + struct Foo { + static int staticMethod(); + int method() const; + Foo() { + this->$keepSnippet^ + $keepSnippet^ + Foo::$keepSnippet^ + } + }; + + struct Derived : Foo { + Derived() { + Foo::$keepSnippet^ + } + }; + + struct OtherClass { + OtherClass() { + Foo f; + f.$keepSnippet^ + &Foo::$noSnippet^ + } + }; + + int main() { + Foo f; + f.$keepSnippet^ + &Foo::$noSnippet^ + } + )cpp"); + auto TU = TestTU::withCode(Code.code()); + + for (const auto &P : Code.points("noSnippet")) { + auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("method"), snippetSuffix("")))); + } + + for (const auto &P : Code.points("keepSnippet")) { + auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("method"), snippetSuffix("()")))); + } + + // static method will always keep the snippet + for (const auto &P : Code.points()) { + auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("staticMethod"), snippetSuffix("()")))); + } +} + TEST(CompletionTest, NoSnippetsInUsings) { clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true;