Author: Younan Zhang Date: 2023-03-17T17:57:24+08:00 New Revision: 138adb0980328d6b958a732d6853d0111aa9cb51
URL: https://github.com/llvm/llvm-project/commit/138adb0980328d6b958a732d6853d0111aa9cb51 DIFF: https://github.com/llvm/llvm-project/commit/138adb0980328d6b958a732d6853d0111aa9cb51.diff LOG: [clangd] Refine logic on $0 in completion snippets We have a workaround from D128621 that makes $0 no longer being a placeholder to conform a vscode feature. However, we have to refine the logic as it may suppress the last parameter placeholder for constructor of base class because not all patterns of completion are compound statements. This fixes clangd/clangd#1479 Reviewed By: nridge Differential Revision: https://reviews.llvm.org/D145319 Added: Modified: clang-tools-extra/clangd/CodeComplete.cpp clang-tools-extra/clangd/CodeCompletionStrings.cpp clang-tools-extra/clangd/CodeCompletionStrings.h clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 98fa285f71051..260c44b2064b5 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -436,9 +436,8 @@ struct CodeCompletionBuilder { Bundled.emplace_back(); BundledEntry &S = Bundled.back(); if (C.SemaResult) { - bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern; - getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, - &Completion.RequiredQualifier, IsPattern); + getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind, + C.SemaResult->CursorKind, &Completion.RequiredQualifier); if (!C.SemaResult->FunctionCanBeCall) S.SnippetSuffix.clear(); S.ReturnType = getReturnType(*SemaCCS); diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 21f83451f7014..26f8e0e602b2b 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "CodeCompletionStrings.h" +#include "clang-c/Index.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RawCommentList.h" #include "clang/Basic/SourceManager.h" @@ -56,6 +57,26 @@ bool looksLikeDocComment(llvm::StringRef CommentText) { return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos; } +// Determine whether the completion string should be patched +// to replace the last placeholder with $0. +bool shouldPatchPlaceholder0(CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind) { + bool CompletingPattern = ResultKind == CodeCompletionResult::RK_Pattern; + + if (!CompletingPattern) + return false; + + // If the result kind of CodeCompletionResult(CCR) is `RK_Pattern`, it doesn't + // always mean we're completing a chunk of statements. Constructors defined + // in base class, for example, are considered as a type of pattern, with the + // cursor type set to CXCursor_Constructor. + if (CursorKind == CXCursorKind::CXCursor_Constructor || + CursorKind == CXCursorKind::CXCursor_Destructor) + return false; + + return true; +} + } // namespace std::string getDocComment(const ASTContext &Ctx, @@ -95,17 +116,20 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { } void getSignature(const CodeCompletionString &CCS, std::string *Signature, - std::string *Snippet, std::string *RequiredQualifiers, - bool CompletingPattern) { - // Placeholder with this index will be ${0:…} to mark final cursor position. + std::string *Snippet, + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, std::string *RequiredQualifiers) { + // Placeholder with this index will be $0 to mark final cursor position. // Usually we do not add $0, so the cursor is placed at end of completed text. unsigned CursorSnippetArg = std::numeric_limits<unsigned>::max(); - if (CompletingPattern) { - // In patterns, it's best to place the cursor at the last placeholder, to - // handle cases like - // namespace ${1:name} { - // ${0:decls} - // } + + // If the snippet contains a group of statements, we replace the + // last placeholder with $0 to leave the cursor there, e.g. + // namespace ${1:name} { + // ${0:decls} + // } + // We try to identify such cases using the ResultKind and CursorKind. + if (shouldPatchPlaceholder0(ResultKind, CursorKind)) { CursorSnippetArg = llvm::count_if(CCS, [](const CodeCompletionString::Chunk &C) { return C.Kind == CodeCompletionString::CK_Placeholder; @@ -185,7 +209,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, *Snippet += Chunk.Text; break; case CodeCompletionString::CK_Optional: - assert(Chunk.Optional); + assert(Chunk.Optional); // No need to create placeholders for default arguments in Snippet. appendOptionalChunk(*Chunk.Optional, Signature); break; diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index 6733d0231df49..566756ac9c8cc 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -42,12 +42,15 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); /// If set, RequiredQualifiers is the text that must be typed before the name. /// e.g "Base::" when calling a base class member function that's hidden. /// -/// When \p CompletingPattern is true, the last placeholder will be of the form -/// ${0:…}, indicating the cursor should stay there. +/// When \p ResultKind is RK_Pattern, the last placeholder will be $0, +/// indicating the cursor should stay there. +/// Note that for certain \p CursorKind like \p CXCursor_Constructor, $0 won't +/// be emitted in order to avoid overlapping normal parameters. void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, - std::string *RequiredQualifiers = nullptr, - bool CompletingPattern = false); + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, + std::string *RequiredQualifiers = nullptr); /// Assembles formatted documentation for a completion result. This includes /// documentation comments and other relevant information like annotations. diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 24d01043a42d6..3179810b1b185 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -756,7 +756,8 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, *PP, *CompletionAllocator, *CompletionTUInfo); std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; @@ -933,7 +934,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, S.Documentation = Documentation; std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; std::string ReturnType = getReturnType(*CCS); diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index f57f266c3c511..25d6a7b189ed5 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -3450,6 +3450,22 @@ TEST(CompletionTest, CursorInSnippets) { EXPECT_THAT(Results.Completions, Contains(AllOf(named("while_foo"), snippetSuffix("(${1:int a}, ${2:int b})")))); + + Results = completions(R"cpp( + struct Base { + Base(int a, int b) {} + }; + + struct Derived : Base { + Derived() : Base^ + }; + )cpp", + /*IndexSymbols=*/{}, Options); + // Constructors from base classes are a kind of pattern that shouldn't end + // with $0. + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("Base"), + snippetSuffix("(${1:int a}, ${2:int b})")))); } TEST(CompletionTest, WorksWithNullType) { diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp index 329a213c66984..faab6253e2832 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -24,11 +24,13 @@ class CompletionStringTest : public ::testing::Test { protected: void computeSignature(const CodeCompletionString &CCS, - bool CompletingPattern = false) { + CodeCompletionResult::ResultKind ResultKind = + CodeCompletionResult::ResultKind::RK_Declaration) { Signature.clear(); Snippet.clear(); - getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr, - CompletingPattern); + getSignature(CCS, &Signature, &Snippet, ResultKind, + /*CursorKind=*/CXCursorKind::CXCursor_NotImplemented, + /*RequiredQualifiers=*/nullptr); } std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator; @@ -145,11 +147,12 @@ TEST_F(CompletionStringTest, SnippetsInPatterns) { Builder.AddChunk(CodeCompletionString::CK_SemiColon); return *Builder.TakeString(); }; - computeSignature(MakeCCS(), /*CompletingPattern=*/false); + computeSignature(MakeCCS()); EXPECT_EQ(Snippet, " ${1:name} = ${2:target};"); // When completing a pattern, the last placeholder holds the cursor position. - computeSignature(MakeCCS(), /*CompletingPattern=*/true); + computeSignature(MakeCCS(), + /*ResultKind=*/CodeCompletionResult::ResultKind::RK_Pattern); EXPECT_EQ(Snippet, " ${1:name} = $0;"); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
