From: Younan Zhang Date: Tue, 14 Mar 2023 05:15:34 +0000 (+0800) Subject: [clangd] Refine logic on $0 in completion snippets X-Git-Tag: upstream/17.0.6~14480 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=138adb0980328d6b958a732d6853d0111aa9cb51;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 98fa285..260c44b 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 21f8345..26f8e0e 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::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 6733d02..566756a 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 24d0104..3179810 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 f57f266..25d6a7b 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 329a213..faab625 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -24,11 +24,13 @@ public: 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 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;"); }