[clang-tools-extra] [clangd] Add ArgumentLists config option under Completion (PR #111322)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 6 17:26:41 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clangd
Author: Nathan Ridge (HighCommander4)
<details>
<summary>Changes</summary>
The new config option is a more flexible version of --function-arg-placeholders, allowing users more detailed control of what is inserted in argument list position when clangd completes the name of a function in a function call context.
Fixes https://github.com/llvm/llvm-project/issues/63565
---
Full diff: https://github.com/llvm/llvm-project/pull/111322.diff
9 Files Affected:
- (modified) clang-tools-extra/clangd/ClangdServer.cpp (+1)
- (modified) clang-tools-extra/clangd/CodeComplete.cpp (+18-8)
- (modified) clang-tools-extra/clangd/CodeComplete.h (+5-4)
- (modified) clang-tools-extra/clangd/Config.h (+14)
- (modified) clang-tools-extra/clangd/ConfigCompile.cpp (+15)
- (modified) clang-tools-extra/clangd/ConfigFragment.h (+8)
- (modified) clang-tools-extra/clangd/ConfigYAML.cpp (+4)
- (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+13-5)
- (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+23-2)
``````````diff
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index e910a80ba0bae9..9b38be04e7ddd7 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -451,6 +451,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
CodeCompleteOpts.MainFileSignals = IP->Signals;
CodeCompleteOpts.AllScopes = Config::current().Completion.AllScopes;
+ CodeCompleteOpts.ArgumentLists = Config::current().Completion.ArgumentLists;
// FIXME(ibiryukov): even if Preamble is non-null, we may want to check
// both the old and the new version in case only one of them matches.
CodeCompleteResult Result = clangd::codeComplete(
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 89eee392837af4..adca462b53f0a0 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -350,8 +351,7 @@ struct CodeCompletionBuilder {
CodeCompletionContext::Kind ContextKind,
const CodeCompleteOptions &Opts,
bool IsUsingDeclaration, tok::TokenKind NextTokenKind)
- : ASTCtx(ASTCtx),
- EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
+ : ASTCtx(ASTCtx), ArgumentLists(Opts.ArgumentLists),
IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
Completion.Deprecated = true; // cleared by any non-deprecated overload.
add(C, SemaCCS, ContextKind);
@@ -561,6 +561,15 @@ struct CodeCompletionBuilder {
}
std::string summarizeSnippet() const {
+ /// localize ArgumentLists tests for better readability
+ const bool None = ArgumentLists == Config::ArgumentListsPolicy::None;
+ const bool Open =
+ ArgumentLists == Config::ArgumentListsPolicy::OpenDelimiter;
+ const bool Delim = ArgumentLists == Config::ArgumentListsPolicy::Delimiters;
+ const bool Full =
+ ArgumentLists == Config::ArgumentListsPolicy::FullPlaceholders ||
+ (!None && !Open && !Delim); // <-- failsafe: Full is default
+
if (IsUsingDeclaration)
return "";
auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>();
@@ -568,7 +577,7 @@ struct CodeCompletionBuilder {
// All bundles are function calls.
// FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
// we need to complete 'forward<$1>($0)'.
- return "($0)";
+ return None ? "" : (Open ? "(" : "($0)");
if (Snippet->empty())
return "";
@@ -607,7 +616,7 @@ struct CodeCompletionBuilder {
return "";
}
}
- if (EnableFunctionArgSnippets)
+ if (Full)
return *Snippet;
// Replace argument snippets with a simplified pattern.
@@ -622,9 +631,9 @@ struct CodeCompletionBuilder {
bool EmptyArgs = llvm::StringRef(*Snippet).ends_with("()");
if (Snippet->front() == '<')
- return EmptyArgs ? "<$1>()$0" : "<$1>($0)";
+ return None ? "" : (Open ? "<" : (EmptyArgs ? "<$1>()$0" : "<$1>($0)"));
if (Snippet->front() == '(')
- return EmptyArgs ? "()" : "($0)";
+ return None ? "" : (Open ? "(" : (EmptyArgs ? "()" : "($0)"));
return *Snippet; // Not an arg snippet?
}
// 'CompletionItemKind::Interface' matches template type aliases.
@@ -638,7 +647,7 @@ struct CodeCompletionBuilder {
// e.g. Foo<${1:class}>.
if (llvm::StringRef(*Snippet).ends_with("<>"))
return "<>"; // can happen with defaulted template arguments.
- return "<$0>";
+ return None ? "" : (Open ? "<" : "<$0>");
}
return *Snippet;
}
@@ -654,7 +663,8 @@ struct CodeCompletionBuilder {
ASTContext *ASTCtx;
CodeCompletion Completion;
llvm::SmallVector<BundledEntry, 1> Bundled;
- bool EnableFunctionArgSnippets;
+ /// the way argument lists are handled.
+ Config::ArgumentListsPolicy ArgumentLists;
// No snippets will be generated for using declarations and when the function
// arguments are already present.
bool IsUsingDeclaration;
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
index a7c1ae95dcbf49..2628f9edafe85d 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -17,6 +17,7 @@
#include "ASTSignals.h"
#include "Compiler.h"
+#include "Config.h"
#include "Protocol.h"
#include "Quality.h"
#include "index/Index.h"
@@ -96,10 +97,6 @@ struct CodeCompleteOptions {
/// '->' on member access etc.
bool IncludeFixIts = false;
- /// Whether to generate snippets for function arguments on code-completion.
- /// Needs snippets to be enabled as well.
- bool EnableFunctionArgSnippets = true;
-
/// Whether to include index symbols that are not defined in the scopes
/// visible from the code completion point. This applies in contexts without
/// explicit scope qualifiers.
@@ -107,6 +104,10 @@ struct CodeCompleteOptions {
/// Such completions can insert scope qualifiers.
bool AllScopes = false;
+ /// The way argument list on calls '()' and generics '<>' are handled.
+ Config::ArgumentListsPolicy ArgumentLists =
+ Config::ArgumentListsPolicy::FullPlaceholders;
+
/// Whether to use the clang parser, or fallback to text-based completion
/// (using identifiers in the current file and symbol indexes).
enum CodeCompletionParse {
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 41143b9ebc8d27..8fcbc5c33469fa 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -126,11 +126,25 @@ struct Config {
std::vector<std::string> FullyQualifiedNamespaces;
} Style;
+ /// controls the completion options for argument lists.
+ enum class ArgumentListsPolicy {
+ /// nothing, no argument list and also NO Delimiters "()" or "<>".
+ None,
+ /// open, only opening delimiter "(" or "<".
+ OpenDelimiter,
+ /// empty pair of delimiters "()" or "<>".
+ Delimiters,
+ /// full name of both type and variable.
+ FullPlaceholders,
+ };
+
/// Configures code completion feature.
struct {
/// Whether code completion includes results that are not visible in current
/// scopes.
bool AllScopes = true;
+ /// controls the completion options for argument lists.
+ ArgumentListsPolicy ArgumentLists = ArgumentListsPolicy::FullPlaceholders;
} Completion;
/// Configures hover feature.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index f32f674443ffeb..58610a5b87922d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -622,6 +622,21 @@ struct FragmentCompiler {
C.Completion.AllScopes = AllScopes;
});
}
+ if (F.ArgumentLists) {
+ if (auto Val =
+ compileEnum<Config::ArgumentListsPolicy>("ArgumentLists",
+ *F.ArgumentLists)
+ .map("None", Config::ArgumentListsPolicy::None)
+ .map("OpenDelimiter",
+ Config::ArgumentListsPolicy::OpenDelimiter)
+ .map("Delimiters", Config::ArgumentListsPolicy::Delimiters)
+ .map("FullPlaceholders",
+ Config::ArgumentListsPolicy::FullPlaceholders)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Completion.ArgumentLists = *Val;
+ });
+ }
}
void compile(Fragment::HoverBlock &&F) {
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index f3e51a9b6dbc4b..fc1b45f5d4c3e9 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -32,6 +32,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H
+#include "Config.h"
#include "ConfigProvider.h"
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/SourceMgr.h"
@@ -308,6 +309,13 @@ struct Fragment {
/// Whether code completion should include suggestions from scopes that are
/// not visible. The required scope prefix will be inserted.
std::optional<Located<bool>> AllScopes;
+ /// How to present the argument list between '()' and '<>':
+ /// valid values are enum Config::ArgumentListsPolicy values:
+ /// None: Nothing at all
+ /// OpenDelimiter: only opening delimiter "(" or "<"
+ /// Delimiters: empty pair of delimiters "()" or "<>"
+ /// FullPlaceholders: full name of both type and parameter
+ std::optional<Located<std::string>> ArgumentLists;
};
CompletionBlock Completion;
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 3e9b6a07d3b325..bcdda99eeed67a 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -230,6 +230,10 @@ class Parser {
if (auto AllScopes = boolValue(N, "AllScopes"))
F.AllScopes = *AllScopes;
});
+ Dict.handle("ArgumentLists", [&](Node &N) {
+ if (auto ArgumentLists = scalarValue(N, "ArgumentLists"))
+ F.ArgumentLists = *ArgumentLists;
+ });
Dict.parse(N);
}
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 3a5449ac8c7994..0bf2916278b639 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -242,13 +242,13 @@ opt<std::string> FallbackStyle{
init(clang::format::DefaultFallbackStyle),
};
-opt<bool> EnableFunctionArgSnippets{
+opt<int> EnableFunctionArgSnippets{
"function-arg-placeholders",
cat(Features),
- desc("When disabled, completions contain only parentheses for "
- "function calls. When enabled, completions also contain "
+ desc("When disabled (0), completions contain only parentheses for "
+ "function calls. When enabled (1), completions also contain "
"placeholders for method parameters"),
- init(CodeCompleteOptions().EnableFunctionArgSnippets),
+ init(-1),
};
opt<CodeCompleteOptions::IncludeInsertion> HeaderInsertion{
@@ -650,6 +650,7 @@ class FlagsConfigProvider : public config::Provider {
std::optional<Config::CDBSearchSpec> CDBSearch;
std::optional<Config::ExternalIndexSpec> IndexSpec;
std::optional<Config::BackgroundPolicy> BGPolicy;
+ std::optional<Config::ArgumentListsPolicy> ArgumentLists;
// If --compile-commands-dir arg was invoked, check value and override
// default path.
@@ -694,6 +695,12 @@ class FlagsConfigProvider : public config::Provider {
BGPolicy = Config::BackgroundPolicy::Skip;
}
+ if (EnableFunctionArgSnippets >= 0) {
+ ArgumentLists = EnableFunctionArgSnippets
+ ? Config::ArgumentListsPolicy::FullPlaceholders
+ : Config::ArgumentListsPolicy::Delimiters;
+ }
+
Frag = [=](const config::Params &, Config &C) {
if (CDBSearch)
C.CompileFlags.CDBSearch = *CDBSearch;
@@ -701,6 +708,8 @@ class FlagsConfigProvider : public config::Provider {
C.Index.External = *IndexSpec;
if (BGPolicy)
C.Index.Background = *BGPolicy;
+ if (ArgumentLists)
+ C.Completion.ArgumentLists = *ArgumentLists;
if (AllScopesCompletion.getNumOccurrences())
C.Completion.AllScopes = AllScopesCompletion;
@@ -916,7 +925,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.CodeComplete.IncludeIndicator.Insert.clear();
Opts.CodeComplete.IncludeIndicator.NoInsert.clear();
}
- Opts.CodeComplete.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
Opts.CodeComplete.RunParser = CodeCompletionParse;
Opts.CodeComplete.RankingModel = RankingModel;
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 96d1ee1f0add73..a89f4997362265 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -11,6 +11,7 @@
#include "ClangdServer.h"
#include "CodeComplete.h"
#include "Compiler.h"
+#include "Config.h"
#include "Feature.h"
#include "Matchers.h"
#include "Protocol.h"
@@ -2595,10 +2596,10 @@ TEST(SignatureHelpTest, DynamicIndexDocumentation) {
ElementsAre(AllOf(sig("foo() -> int"), sigDoc("Member doc"))));
}
-TEST(CompletionTest, CompletionFunctionArgsDisabled) {
+TEST(CompletionTest, ArgumentListsPolicy) {
CodeCompleteOptions Opts;
Opts.EnableSnippets = true;
- Opts.EnableFunctionArgSnippets = false;
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::Delimiters;
{
auto Results = completions(
@@ -2670,6 +2671,26 @@ TEST(CompletionTest, CompletionFunctionArgsDisabled) {
EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
named("FOO"), snippetSuffix("($0)"))));
}
+ {
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::None;
+ auto Results = completions(
+ R"cpp(
+ void xfoo(int x, int y);
+ void f() { xfo^ })cpp",
+ {}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(named("xfoo"), snippetSuffix(""))));
+ }
+ {
+ Opts.ArgumentLists = Config::ArgumentListsPolicy::OpenDelimiter;
+ auto Results = completions(
+ R"cpp(
+ void xfoo(int x, int y);
+ void f() { xfo^ })cpp",
+ {}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(named("xfoo"), snippetSuffix("("))));
+ }
}
TEST(CompletionTest, SuggestOverrides) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/111322
More information about the cfe-commits
mailing list