https://github.com/HighCommander4 created https://github.com/llvm/llvm-project/pull/111322
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 >From b0c222e3a67c6ec320ae3582d8034c5562e8fe94 Mon Sep 17 00:00:00 2001 From: MK-Alias <imnotreadingt...@maininator.com> Date: Sun, 6 Oct 2024 20:19:55 -0400 Subject: [PATCH] [clangd] Add ArgumentLists config option under Completion 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 --- clang-tools-extra/clangd/ClangdServer.cpp | 1 + clang-tools-extra/clangd/CodeComplete.cpp | 26 +++++++++++++------ clang-tools-extra/clangd/CodeComplete.h | 9 ++++--- clang-tools-extra/clangd/Config.h | 14 ++++++++++ clang-tools-extra/clangd/ConfigCompile.cpp | 15 +++++++++++ clang-tools-extra/clangd/ConfigFragment.h | 8 ++++++ clang-tools-extra/clangd/ConfigYAML.cpp | 4 +++ clang-tools-extra/clangd/tool/ClangdMain.cpp | 18 +++++++++---- .../clangd/unittests/CodeCompleteTests.cpp | 25 ++++++++++++++++-- 9 files changed, 101 insertions(+), 19 deletions(-) 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
