From bd238fe59cc9d851ed15bcf4f146b6280ca3221b Mon Sep 17 00:00:00 2001 From: ykiko Date: Thu, 9 Apr 2026 16:08:14 +0800 Subject: [PATCH] feat(completion): signature display, underscore filtering, label dedup (#411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Extract function/method signatures from Clang `CodeCompletionString` into `labelDetails.detail` (parameter list) and `labelDetails.description` (return type) - Filter `_`/`__` prefixed internal symbols (e.g. `_Vector_base`, `_Alloc`) unless the user explicitly typed `_` - Fix `completion_kind` isa ordering: `CXXMethodDecl` checked before `FunctionDecl` so methods get correct Kind - Bundle mode: extend overload bundling to Method and Constructor (was Function only) - Bundle mode: deduplicate by label — when the same name appears as Class + Constructor + deduction guide, keep only one (priority: Class > Function > Constructor) - Bundled overloads show `(…) +N overloads` in `labelDetails.detail` instead of `detail` ## Test plan - [x] 12 unit tests covering: signature extraction, return type, overload bundling, underscore filtering, label deduplication, non-bundle mode, method signatures - [x] All 489 unit tests pass - [x] `pixi run format` applied 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit ## Release Notes * **New Features** * Code completion now displays function signatures and return types in completion items * Overloaded functions are bundled together with a count indicator * Internal symbols (underscore-prefixed) are filtered from suggestions unless explicitly typed * Duplicate completion items are deduplicated while preserving higher-priority variants Co-authored-by: Claude Opus 4.6 --- src/feature/code_completion.cpp | 157 +++++++++++++++- tests/unit/feature/code_completion_tests.cpp | 188 +++++++++++++++++-- 2 files changed, 325 insertions(+), 20 deletions(-) diff --git a/src/feature/code_completion.cpp b/src/feature/code_completion.cpp index c6bce6aa..33eb2ed8 100644 --- a/src/feature/code_completion.cpp +++ b/src/feature/code_completion.cpp @@ -53,8 +53,8 @@ auto completion_kind(const clang::NamedDecl* decl) -> protocol::CompletionItemKi return protocol::CompletionItemKind::Module; } - if(llvm::isa(decl)) { - return protocol::CompletionItemKind::Function; + if(llvm::isa(decl)) { + return protocol::CompletionItemKind::Constructor; } if(llvm::isa protocol::CompletionItemKi return protocol::CompletionItemKind::Method; } - if(llvm::isa(decl)) { - return protocol::CompletionItemKind::Constructor; + if(llvm::isa(decl)) { + return protocol::CompletionItemKind::Function; } if(llvm::isa(decl)) { @@ -109,6 +109,65 @@ auto completion_kind(const clang::NamedDecl* decl) -> protocol::CompletionItemKi return protocol::CompletionItemKind::Text; } +/// Extract the function signature (parameter list) from a CodeCompletionString. +/// Returns something like "(int x, float y)" for display in labelDetails.detail. +auto extract_signature(const clang::CodeCompletionString& ccs) -> std::string { + std::string signature; + bool in_parens = false; + + for(const auto& chunk: ccs) { + using CK = clang::CodeCompletionString::ChunkKind; + switch(chunk.Kind) { + case CK::CK_LeftParen: + in_parens = true; + signature += '('; + break; + case CK::CK_RightParen: + signature += ')'; + in_parens = false; + break; + case CK::CK_Placeholder: + case CK::CK_CurrentParameter: + if(in_parens && chunk.Text) { + signature += chunk.Text; + } + break; + case CK::CK_Text: + case CK::CK_Informative: + if(in_parens && chunk.Text) { + signature += chunk.Text; + } + break; + case CK::CK_LeftAngle: + signature += '<'; + in_parens = true; + break; + case CK::CK_RightAngle: + signature += '>'; + in_parens = false; + break; + case CK::CK_Comma: + if(in_parens) { + signature += ", "; + } + break; + default: break; + } + } + + return signature; +} + +/// Extract the return type from a CodeCompletionString. +auto extract_return_type(const clang::CodeCompletionString& ccs) -> std::string { + for(const auto& chunk: ccs) { + if(chunk.Kind == clang::CodeCompletionString::CK_ResultType && chunk.Text) { + return chunk.Text; + } + } + return {}; +} + struct OverloadItem { protocol::CompletionItem item; float score = 0.0F; @@ -159,6 +218,8 @@ public: overloads.reserve(candidate_count); std::unordered_map overload_index; + bool prefix_starts_with_underscore = prefix.spelling.starts_with("_"); + auto build_item = [&](llvm::StringRef label, protocol::CompletionItemKind kind, llvm::StringRef insert) { protocol::CompletionItem item{ @@ -177,11 +238,18 @@ public: auto try_add = [&](llvm::StringRef label, protocol::CompletionItemKind kind, llvm::StringRef insert_text, - llvm::StringRef overload_key) { + llvm::StringRef overload_key, + llvm::StringRef signature = {}, + llvm::StringRef return_type = {}) { if(label.empty()) { return; } + // Filter out _/__ prefixed internal symbols unless user typed _. + if(!prefix_starts_with_underscore && label.starts_with("_")) { + return; + } + auto score = matcher.match(label); if(!score.has_value()) { return; @@ -193,6 +261,16 @@ public: if(inserted) { auto item = build_item(label, kind, insert_text); item.sort_text = std::format("{}", *score); + if(!signature.empty() || !return_type.empty()) { + protocol::CompletionItemLabelDetails details; + if(!signature.empty()) { + details.detail = signature.str(); + } + if(!return_type.empty()) { + details.description = return_type.str(); + } + item.label_details = std::move(details); + } overloads.push_back({ .item = std::move(item), .score = *score, @@ -211,6 +289,16 @@ public: auto item = build_item(label, kind, insert_text); item.sort_text = std::format("{}", *score); + if(!signature.empty() || !return_type.empty()) { + protocol::CompletionItemLabelDetails details; + if(!signature.empty()) { + details.detail = signature.str(); + } + if(!return_type.empty()) { + details.description = return_type.str(); + } + item.label_details = std::move(details); + } collected.push_back(std::move(item)); }; @@ -246,12 +334,28 @@ public: auto kind = completion_kind(declaration); llvm::SmallString<256> qualified_name; - if(options.bundle_overloads && kind == protocol::CompletionItemKind::Function) { + bool is_callable = kind == protocol::CompletionItemKind::Function || + kind == protocol::CompletionItemKind::Method || + kind == protocol::CompletionItemKind::Constructor; + if(options.bundle_overloads && is_callable) { llvm::raw_svector_ostream stream(qualified_name); declaration->printQualifiedName(stream); } - try_add(label, kind, label, qualified_name.str()); + std::string signature; + std::string return_type; + auto* ccs = + candidate.CreateCodeCompletionString(sema, + context, + getAllocator(), + getCodeCompletionTUInfo(), + /*IncludeBriefComments=*/false); + if(ccs) { + signature = extract_signature(*ccs); + return_type = extract_return_type(*ccs); + } + + try_add(label, kind, label, qualified_name.str(), signature, return_type); break; } } @@ -259,11 +363,48 @@ public: for(auto& entry: overloads) { if(entry.count > 1) { - entry.item.detail = "(...)"; + protocol::CompletionItemLabelDetails details; + details.detail = std::format("(…) +{} overloads", entry.count); + entry.item.label_details = std::move(details); } collected.push_back(std::move(entry.item)); } + // In bundle mode, deduplicate by label: when the same name appears as + // both a class and its constructors/deduction guides, keep only the + // highest-priority kind (Class > Function/Method > others). + if(options.bundle_overloads) { + auto kind_priority = [](protocol::CompletionItemKind k) -> int { + switch(k) { + case protocol::CompletionItemKind::Class: + case protocol::CompletionItemKind::Struct: return 3; + case protocol::CompletionItemKind::Function: + case protocol::CompletionItemKind::Method: return 2; + case protocol::CompletionItemKind::Constructor: return 1; + default: return 0; + } + }; + + std::unordered_map label_index; + std::vector deduped; + deduped.reserve(collected.size()); + + for(auto& item: collected) { + auto [it, inserted] = label_index.try_emplace(item.label, deduped.size()); + if(inserted) { + deduped.push_back(std::move(item)); + } else { + auto& existing = deduped[it->second]; + int old_prio = existing.kind.has_value() ? kind_priority(*existing.kind) : 0; + int new_prio = item.kind.has_value() ? kind_priority(*item.kind) : 0; + if(new_prio > old_prio) { + existing = std::move(item); + } + } + } + collected.swap(deduped); + } + output.clear(); output.swap(collected); } diff --git a/tests/unit/feature/code_completion_tests.cpp b/tests/unit/feature/code_completion_tests.cpp index 1334c9c1..7e996abb 100644 --- a/tests/unit/feature/code_completion_tests.cpp +++ b/tests/unit/feature/code_completion_tests.cpp @@ -17,7 +17,7 @@ std::vector items; llvm::IntrusiveRefCntPtr vfs; std::string main_path; -void code_complete(llvm::StringRef code) { +void code_complete(llvm::StringRef code, feature::CodeCompletionOptions options = {}) { vfs = llvm::makeIntrusiveRefCnt(); CompilationParams params; @@ -31,24 +31,58 @@ void code_complete(llvm::StringRef code) { params.completion = {main_path, annotation.offsets.lookup("pos")}; params.add_remapped_file(main_path, annotation.content); - feature::CodeCompletionOptions options = {}; items = feature::code_complete(params, options, feature::PositionEncoding::UTF8); } +auto find_item(llvm::StringRef label) { + return std::ranges::find_if(items, [&](const protocol::CompletionItem& item) { + return item.label == label; + }); +} + TEST_CASE(Score) { code_complete(R"cpp( int foooo(int x); int x = fo$(pos) )cpp"); - auto it = std::ranges::find_if(items, [](const protocol::CompletionItem& item) { - return item.label == "foooo"; - }); + auto it = find_item("foooo"); ASSERT_TRUE(it != items.end()); ASSERT_TRUE(it->kind.has_value()); ASSERT_EQ(*it->kind, protocol::CompletionItemKind::Function); } +TEST_CASE(Signature) { + code_complete(R"cpp( +int foooo(int x, float y); +int x = fo$(pos) +)cpp"); + + auto it = find_item("foooo"); + ASSERT_TRUE(it != items.end()); + ASSERT_TRUE(it->label_details.has_value()); + // label_details.detail should contain the parameter list. + ASSERT_TRUE(it->label_details->detail.has_value()); + auto& sig = *it->label_details->detail; + ASSERT_TRUE(sig.find("int") != std::string::npos); + ASSERT_TRUE(sig.find("float") != std::string::npos); +} + +TEST_CASE(ReturnType) { + code_complete(R"cpp( +double foooo(int x); +int x = fo$(pos) +)cpp"); + + auto it = find_item("foooo"); + ASSERT_TRUE(it != items.end()); + ASSERT_TRUE(it->label_details.has_value()); + // label_details.description should contain the return type. + ASSERT_TRUE(it->label_details->description.has_value()); + auto& ret = *it->label_details->description; + ASSERT_TRUE(ret.find("double") != std::string::npos); +} + TEST_CASE(Snippet) { code_complete(R"cpp( int x = tru$(pos) @@ -65,6 +99,142 @@ int x = fooo$(pos) )cpp"); ASSERT_TRUE(!items.empty()); + // With bundling, there should be exactly one "foooo" item. + auto count = std::ranges::count_if(items, [](const protocol::CompletionItem& item) { + return item.label == "foooo"; + }); + ASSERT_EQ(count, 1); + + auto it = find_item("foooo"); + ASSERT_TRUE(it != items.end()); + // Bundled overload should show count in label_details.detail. + ASSERT_TRUE(it->label_details.has_value()); + ASSERT_TRUE(it->label_details->detail.has_value()); + auto& detail = *it->label_details->detail; + ASSERT_TRUE(detail.find("overload") != std::string::npos); +} + +TEST_CASE(FilterUnderscore) { + code_complete(R"cpp( +int _private_thing; +int public_thing; +int x = pu$(pos) +)cpp"); + + // _private_thing should be filtered when prefix doesn't start with _. + auto it = find_item("_private_thing"); + ASSERT_TRUE(it == items.end()); + + auto it2 = find_item("public_thing"); + ASSERT_TRUE(it2 != items.end()); +} + +TEST_CASE(FilterUnderscoreExplicit) { + code_complete(R"cpp( +int _private_thing; +int x = _p$(pos) +)cpp"); + + // When user types _, underscore-prefixed symbols should appear. + auto it = find_item("_private_thing"); + ASSERT_TRUE(it != items.end()); +} + +TEST_CASE(MethodSignature) { + code_complete(R"cpp( +struct Foo { + int bazzzz(int a, int b); +}; + +void bar() { + Foo f; + f.ba$(pos); +} +)cpp"); + + auto it = find_item("bazzzz"); + ASSERT_TRUE(it != items.end()); + ASSERT_TRUE(it->kind.has_value()); + ASSERT_EQ(*it->kind, protocol::CompletionItemKind::Method); + ASSERT_TRUE(it->label_details.has_value()); + ASSERT_TRUE(it->label_details->detail.has_value()); + auto& sig = *it->label_details->detail; + ASSERT_TRUE(sig.find("int") != std::string::npos); +} + +TEST_CASE(DeduplicateByLabel) { + code_complete(R"cpp( +template +struct Foo { + Foo() {} + Foo(T x) {} + Foo(T x, T y) {} +}; + +template +Foo(T) -> Foo; + +void bar() { + Fo$(pos) +} +)cpp"); + + // In bundle mode, "Foo" should appear exactly once (as Class kind), + // not 3 times (Class + Constructor bundle + deduction guide bundle). + auto count = std::ranges::count_if(items, [](const protocol::CompletionItem& item) { + return item.label == "Foo"; + }); + ASSERT_EQ(count, 1); + + auto it = find_item("Foo"); + ASSERT_TRUE(it != items.end()); + ASSERT_TRUE(it->kind.has_value()); + ASSERT_EQ(*it->kind, protocol::CompletionItemKind::Class); +} + +TEST_CASE(NoBundleOverloads) { + feature::CodeCompletionOptions opts; + opts.bundle_overloads = false; + code_complete(R"cpp( +int foooo(int x); +int foooo(int x, int y); +double foooo(double d); +int x = fooo$(pos) +)cpp", + opts); + + // Without bundling, each overload should be a separate item. + auto count = std::ranges::count_if(items, [](const protocol::CompletionItem& item) { + return item.label == "foooo"; + }); + ASSERT_TRUE(count >= 3); + + // Each should have its own signature in label_details. + for(auto& item: items) { + if(item.label == "foooo") { + ASSERT_TRUE(item.label_details.has_value()); + ASSERT_TRUE(item.label_details->detail.has_value()); + } + } +} + +TEST_CASE(NoBundleNoDeduplicate) { + feature::CodeCompletionOptions opts; + opts.bundle_overloads = false; + code_complete(R"cpp( +int foooo(int x); +int foooo(int x, int y); +double foooo(double d); +int x = fooo$(pos) +)cpp", + opts); + + // Without bundling, deduplication should NOT apply — each overload + // should appear as a separate item. + auto count = std::ranges::count_if(items, [](const protocol::CompletionItem& item) { + return item.label == "foooo"; + }); + ASSERT_TRUE(count >= 3); } TEST_CASE(Unqualified) { @@ -77,14 +247,12 @@ void bar() { fo$(pos) } )cpp"); - - // Legacy parity: keep as smoke case without strict expectation. } TEST_CASE(Functor) { code_complete(R"cpp( struct X { - void operator() () {} + void operator() () {}; }; void bar() { @@ -92,8 +260,6 @@ void bar() { fo$(pos); } )cpp"); - - // Legacy parity: keep as smoke case without strict expectation. } TEST_CASE(Lambda) { @@ -103,8 +269,6 @@ void bar() { fo$(pos); } )cpp"); - - // Legacy parity: keep as smoke case without strict expectation. } }; // TEST_SUITE(CodeCompletion)