LanguageClient: improve clangd function hint

Add a Clangd specific function hint model that alwys highlights the
current parameter based on the number of commas in front of the cursor
position, like the builtin code model. It also correctly closes the
proposal after typing the closing parenthesis.

Fixes: QTCREATORBUG-26346
Fixes: QTCREATORBUG-30489
Change-Id: I09d3ac6856acfe5e0f206d8c3a96dbb561ea2ce7
Reviewed-by: Christian Kandeler <[email protected]>

11 files changed, 166 insertions, 61 deletions

diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp
index 46aa5848f7c..b36ff8a4258 100644
--- a/src/plugins/clangcodemodel/clangdclient.cpp
+++ b/src/plugins/clangcodemodel/clangdclient.cpp
@@ -405,6 +405,7 @@ ClangdClient::ClangdClient(Project *project, const Utils::FilePath &jsonDbDir, c
     setSupportedLanguage(langFilter);
     setActivateDocumentAutomatically(true);
     setCompletionAssistProvider(new ClangdCompletionAssistProvider(this));
+    setFunctionHintAssistProvider(new ClangdFunctionHintProvider(this));
     setQuickFixAssistProvider(new ClangdQuickFixProvider(this));
     symbolSupport().setLimitRenamingToProjects(true);
     symbolSupport().setRenameResultsEnhancer([](const SearchResultItems &symbolOccurrencesInCode) {
diff --git a/src/plugins/clangcodemodel/clangdcompletion.cpp b/src/plugins/clangcodemodel/clangdcompletion.cpp
index a10ba815df3..44520d324de 100644
--- a/src/plugins/clangcodemodel/clangdcompletion.cpp
+++ b/src/plugins/clangcodemodel/clangdcompletion.cpp
@@ -102,13 +102,52 @@ private:
     QElapsedTimer m_timer;
 };
 
+class ClangdFunctionHintProposalModel : public FunctionHintProposalModel
+{
+public:
+    using FunctionHintProposalModel::FunctionHintProposalModel;
+
+private:
+    int activeArgument(const QString &prefix) const override
+    {
+        const int arg = activeArgumenForPrefix(prefix);
+        if (arg < 0)
+            return -1;
+        m_currentArg = arg;
+        return arg;
+    }
+
+    QString text(int index) const override
+    {
+        using Parameters = QList<ParameterInformation>;
+        if (index < 0 || m_sigis.signatures().size() <= index)
+            return {};
+        const SignatureInformation signature = m_sigis.signatures().at(index);
+        QString label = signature.label();
+
+        const QList<QString> parameters = Utils::transform(signature.parameters().value_or(Parameters()),
+                                                           &ParameterInformation::label);
+        if (parameters.size() <= m_currentArg)
+            return label;
+
+        const QString &parameterText = parameters.at(m_currentArg);
+        const int start = label.indexOf(parameterText);
+        const int end = start + parameterText.length();
+        return label.mid(0, start).toHtmlEscaped() + "<b>" + parameterText.toHtmlEscaped() + "</b>"
+               + label.mid(end).toHtmlEscaped();
+    }
+
+    mutable int m_currentArg = 0;
+};
+
 class ClangdFunctionHintProcessor : public FunctionHintProcessor
 {
 public:
-    ClangdFunctionHintProcessor(ClangdClient *client);
+    ClangdFunctionHintProcessor(ClangdClient *client, int basePosition);
 
 private:
     IAssistProposal *perform() override;
+    IFunctionHintProposalModel *createModel(const SignatureHelp &signatureHelp) const override;
 
     ClangdClient * const m_client;
 };
@@ -138,7 +177,8 @@ IAssistProcessor *ClangdCompletionAssistProvider::createProcessor(
     switch (contextAnalyzer.completionAction()) {
     case ClangCompletionContextAnalyzer::PassThroughToLibClangAfterLeftParen:
         qCDebug(clangdLogCompletion) << "creating function hint processor";
-        return new ClangdFunctionHintProcessor(m_client);
+        return new ClangdFunctionHintProcessor(m_client,
+                                               contextAnalyzer.positionForProposal());
     case ClangCompletionContextAnalyzer::CompletePreprocessorDirective:
         qCDebug(clangdLogCompletion) << "creating macro processor";
         return new CustomAssistProcessor(m_client,
@@ -606,8 +646,8 @@ QList<AssistProposalItemInterface *> ClangdCompletionAssistProcessor::generateCo
     return itemGenerator(items);
 }
 
-ClangdFunctionHintProcessor::ClangdFunctionHintProcessor(ClangdClient *client)
-    : FunctionHintProcessor(client)
+ClangdFunctionHintProcessor::ClangdFunctionHintProcessor(ClangdClient *client, int basePosition)
+    : FunctionHintProcessor(client, basePosition)
     , m_client(client)
 {}
 
@@ -621,6 +661,12 @@ IAssistProposal *ClangdFunctionHintProcessor::perform()
     return FunctionHintProcessor::perform();
 }
 
+IFunctionHintProposalModel *ClangdFunctionHintProcessor::createModel(
+    const SignatureHelp &signatureHelp) const
+{
+    return new ClangdFunctionHintProposalModel(signatureHelp);
+}
+
 ClangdCompletionCapabilities::ClangdCompletionCapabilities(const JsonObject &object)
     : TextDocumentClientCapabilities::CompletionCapabilities(object)
 {
@@ -631,4 +677,18 @@ ClangdCompletionCapabilities::ClangdCompletionCapabilities(const JsonObject &obj
     }
 }
 
+ClangdFunctionHintProvider::ClangdFunctionHintProvider(ClangdClient *client)
+    : FunctionHintAssistProvider(client)
+    , m_client(client)
+{}
+
+IAssistProcessor *ClangdFunctionHintProvider::createProcessor(
+    const AssistInterface *interface) const
+{
+    ClangCompletionContextAnalyzer contextAnalyzer(interface->textDocument(),
+                                                   interface->position(), false, {});
+    contextAnalyzer.analyze();
+    return new ClangdFunctionHintProcessor(m_client, contextAnalyzer.positionForProposal());
+}
+
 } // namespace ClangCodeModel::Internal
diff --git a/src/plugins/clangcodemodel/clangdcompletion.h b/src/plugins/clangcodemodel/clangdcompletion.h
index 5dddf6784a8..363fcf5e064 100644
--- a/src/plugins/clangcodemodel/clangdcompletion.h
+++ b/src/plugins/clangcodemodel/clangdcompletion.h
@@ -1,11 +1,10 @@
-
-#include <languageclient/languageclientcompletionassist.h>
 // Copyright (C) 2022 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 
 #pragma once
 
 #include <languageclient/languageclientcompletionassist.h>
+#include <languageclient/languageclientfunctionhint.h>
 #include <languageserverprotocol/clientcapabilities.h>
 
 namespace TextEditor { class IAssistProcessor; }
@@ -37,4 +36,16 @@ public:
     explicit ClangdCompletionCapabilities(const JsonObject &object);
 };
 
+class ClangdFunctionHintProvider : public LanguageClient::FunctionHintAssistProvider
+{
+public:
+    ClangdFunctionHintProvider(ClangdClient *client);
+
+private:
+    TextEditor::IAssistProcessor *createProcessor(
+        const TextEditor::AssistInterface *assistInterface) const override;
+
+    ClangdClient * const m_client;
+};
+
 } // namespace ClangCodeModel::Internal
diff --git a/src/plugins/clangcodemodel/test/clangdtests.cpp b/src/plugins/clangcodemodel/test/clangdtests.cpp
index bfe7ea95368..d4dd092b837 100644
--- a/src/plugins/clangcodemodel/test/clangdtests.cpp
+++ b/src/plugins/clangcodemodel/test/clangdtests.cpp
@@ -29,6 +29,7 @@
 #include <texteditor/blockrange.h>
 #include <texteditor/codeassist/assistproposaliteminterface.h>
 #include <texteditor/codeassist/genericproposal.h>
+#include <texteditor/codeassist/ifunctionhintproposalmodel.h>
 #include <texteditor/codeassist/textdocumentmanipulatorinterface.h>
 #include <texteditor/semantichighlighter.h>
 #include <texteditor/textmark.h>
@@ -1832,12 +1833,12 @@ void ClangdTestCompletion::testFunctionHints()
 
     QVERIFY(proposal);
     QVERIFY(hasItem(proposal, "f() -> void"));
-    QVERIFY(hasItem(proposal, "f(int a) -> void"));
-    QVERIFY(hasItem(proposal, "f(const QString &s) -> void"));
-    QVERIFY(hasItem(proposal, "f(char c, int optional = 3) -> void"));
-    QVERIFY(hasItem(proposal, "f(char c, int optional1 = 3, int optional2 = 3) -> void"));
-    QVERIFY(hasItem(proposal, "f(const TType<QString> *t) -> void"));
-    QVERIFY(hasItem(proposal, "f(bool) -> TType<QString>"));
+    QVERIFY(hasItem(proposal, "f(<b>int a</b>) -&gt; void"));
+    QVERIFY(hasItem(proposal, "f(<b>const QString &amp;s</b>) -&gt; void"));
+    QVERIFY(hasItem(proposal, "f(<b>char c</b>, int optional = 3) -&gt; void"));
+    QVERIFY(hasItem(proposal, "f(<b>char c</b>, int optional1 = 3, int optional2 = 3) -&gt; void"));
+    QVERIFY(hasItem(proposal, "f(<b>const TType&lt;QString&gt; *t</b>) -&gt; void"));
+    QVERIFY(hasItem(proposal, "f(<b>bool</b>) -&gt; TType&lt;QString&gt;"));
 }
 
 void ClangdTestCompletion::testFunctionHintsFiltered()
@@ -1855,7 +1856,6 @@ void ClangdTestCompletion::testFunctionHintsFiltered()
     QVERIFY(proposal);
     QCOMPARE(proposal->size(), 2);
     QVERIFY(hasItem(proposal, "func(const S &amp;s, <b>int j</b>) -&gt; void"));
-    QEXPECT_FAIL("", "QTCREATORBUG-26346", Abort);
     QVERIFY(hasItem(proposal, "func(const S &amp;s, <b>int j</b>, int k) -&gt; void"));
 }
 
@@ -1868,7 +1868,6 @@ void ClangdTestCompletion::testFunctionHintConstructor()
     QVERIFY(!hasItem(proposal, "globalVariable"));
     QVERIFY(!hasItem(proposal, " class"));
     QVERIFY(hasItem(proposal, "Foo(<b>int</b>)"));
-    QEXPECT_FAIL("", "QTCREATORBUG-26346", Abort);
     QVERIFY(hasItem(proposal, "Foo(<b>int</b>, double)"));
 }
 
@@ -2066,7 +2065,8 @@ void ClangdTestCompletion::getProposal(const QString &fileName,
 {
     const TextDocument * const doc = document(fileName);
     QVERIFY(doc);
-    const int pos = doc->document()->toPlainText().indexOf(" /* COMPLETE HERE */");
+    const QString docContent = doc->document()->toPlainText();
+    const int pos = docContent.indexOf(" /* COMPLETE HERE */");
     QVERIFY(pos != -1);
     if (cursorPos)
         *cursorPos = pos;
@@ -2110,6 +2110,13 @@ void ClangdTestCompletion::getProposal(const QString &fileName,
     QVERIFY(timer.isActive());
     QVERIFY(proposal);
     proposalModel = proposal->model();
+    if (auto functionHintModel = proposalModel.dynamicCast<IFunctionHintProposalModel>()) {
+        const int proposalBasePos = proposal->basePosition();
+        // The language client function hint model expects that activeArgument was called before the
+        // text of individual hints is accessed. This is usually done by the proposal widget. But
+        // since we don't have a proposal widget in this test, we have to call it manually.
+        functionHintModel->activeArgument(docContent.mid(proposalBasePos, pos - proposalBasePos));
+    }
     delete proposal;
 
     // The "dot" test files are only used once.
diff --git a/src/plugins/cppeditor/cppcompletionassist.cpp b/src/plugins/cppeditor/cppcompletionassist.cpp
index 8168d72ba1f..03c04765d41 100644
--- a/src/plugins/cppeditor/cppcompletionassist.cpp
+++ b/src/plugins/cppeditor/cppcompletionassist.cpp
@@ -372,27 +372,11 @@ QString CppFunctionHintModel::text(int index) const
 
 int CppFunctionHintModel::activeArgument(const QString &prefix) const
 {
-    int argnr = 0;
-    int parcount = 0;
-    SimpleLexer tokenize;
-    Tokens tokens = tokenize(prefix);
-    for (int i = 0; i < tokens.count(); ++i) {
-        const Token &tk = tokens.at(i);
-        if (tk.is(T_LPAREN))
-            ++parcount;
-        else if (tk.is(T_RPAREN))
-            --parcount;
-        else if (!parcount && tk.is(T_COMMA))
-            ++argnr;
-    }
-
-    if (parcount < 0)
+    const int arg = activeArgumenForPrefix(prefix);
+    if (arg < 0)
         return -1;
-
-    if (argnr != m_currentArg)
-        m_currentArg = argnr;
-
-    return argnr;
+    m_currentArg = arg;
+    return arg;
 }
 
 // ---------------------------
diff --git a/src/plugins/cppeditor/cpptoolsreuse.cpp b/src/plugins/cppeditor/cpptoolsreuse.cpp
index 48fe23244bc..6555010c8f4 100644
--- a/src/plugins/cppeditor/cpptoolsreuse.cpp
+++ b/src/plugins/cppeditor/cpptoolsreuse.cpp
@@ -219,6 +219,28 @@ bool isValidIdentifier(const QString &s)
     return true;
 }
 
+int activeArgumenForPrefix(const QString &prefix)
+{
+    int argnr = 0;
+    int parcount = 0;
+    SimpleLexer tokenize;
+    Tokens tokens = tokenize(prefix);
+    for (int i = 0; i < tokens.count(); ++i) {
+        const Token &tk = tokens.at(i);
+        if (tk.is(T_LPAREN))
+            ++parcount;
+        else if (tk.is(T_RPAREN))
+            --parcount;
+        else if (!parcount && tk.is(T_COMMA))
+            ++argnr;
+    }
+
+    if (parcount < 0)
+        return -1;
+
+    return argnr;
+}
+
 bool isQtKeyword(QStringView text)
 {
     switch (text.length()) {
@@ -859,5 +881,5 @@ void decorateCppEditor(TextEditor::TextEditorWidget *editor)
     editor->setAutoCompleter(new CppAutoCompleter);
 }
 
-} // namespace Internal
+} // Internal
 } // CppEditor
diff --git a/src/plugins/cppeditor/cpptoolsreuse.h b/src/plugins/cppeditor/cpptoolsreuse.h
index 50078bdd791..89bf8961976 100644
--- a/src/plugins/cppeditor/cpptoolsreuse.h
+++ b/src/plugins/cppeditor/cpptoolsreuse.h
@@ -45,6 +45,8 @@ bool CPPEDITOR_EXPORT isValidFirstIdentifierChar(const QChar &ch);
 bool CPPEDITOR_EXPORT isValidIdentifierChar(const QChar &ch);
 bool CPPEDITOR_EXPORT isValidIdentifier(const QString &s);
 
+int CPPEDITOR_EXPORT activeArgumenForPrefix(const QString &prefix);
+
 QStringList CPPEDITOR_EXPORT identifierWordsUnderCursor(const QTextCursor &tc);
 QString CPPEDITOR_EXPORT identifierUnderCursor(QTextCursor *cursor);
 
diff --git a/src/plugins/languageclient/client.cpp b/src/plugins/languageclient/client.cpp
index 90120e3174b..96ba042b84a 100644
--- a/src/plugins/languageclient/client.cpp
+++ b/src/plugins/languageclient/client.cpp
@@ -1630,6 +1630,12 @@ void Client::setCompletionAssistProvider(LanguageClientCompletionAssistProvider
     d->m_clientProviders.completionAssistProvider = provider;
 }
 
+void Client::setFunctionHintAssistProvider(FunctionHintAssistProvider *provider)
+{
+    delete d->m_clientProviders.functionHintProvider;
+    d->m_clientProviders.functionHintProvider = provider;
+}
+
 void Client::setQuickFixAssistProvider(LanguageClientQuickFixProvider *provider)
 {
     delete d->m_clientProviders.quickFixAssistProvider;
diff --git a/src/plugins/languageclient/client.h b/src/plugins/languageclient/client.h
index 65093237085..a881d178242 100644
--- a/src/plugins/languageclient/client.h
+++ b/src/plugins/languageclient/client.h
@@ -39,6 +39,7 @@ class ClientPrivate;
 class DiagnosticManager;
 class DocumentSymbolCache;
 class DynamicCapabilities;
+class FunctionHintAssistProvider;
 class HoverHandler;
 class InterfaceController;
 class LanguageClientCompletionAssistProvider;
@@ -171,6 +172,7 @@ public:
     void setSemanticTokensHandler(const SemanticTokensHandler &handler);
     void setSnippetsGroup(const QString &group);
     void setCompletionAssistProvider(LanguageClientCompletionAssistProvider *provider);
+    void setFunctionHintAssistProvider(FunctionHintAssistProvider *provider);
     void setQuickFixAssistProvider(LanguageClientQuickFixProvider *provider);
     virtual bool supportsDocumentSymbols(const TextEditor::TextDocument *doc) const;
     virtual bool fileBelongsToProject(const Utils::FilePath &filePath) const;
diff --git a/src/plugins/languageclient/languageclientfunctionhint.cpp b/src/plugins/languageclient/languageclientfunctionhint.cpp
index 67f51db90da..c18114392c7 100644
--- a/src/plugins/languageclient/languageclientfunctionhint.cpp
+++ b/src/plugins/languageclient/languageclientfunctionhint.cpp
@@ -17,24 +17,6 @@ using namespace LanguageServerProtocol;
 
 namespace LanguageClient {
 
-class FunctionHintProposalModel : public IFunctionHintProposalModel
-{
-public:
-    explicit FunctionHintProposalModel(SignatureHelp signature)
-        : m_sigis(signature)
-    {}
-    void reset() override {}
-    int size() const override
-    { return m_sigis.signatures().size(); }
-    QString text(int index) const override;
-
-    int activeArgument(const QString &/*prefix*/) const override
-    { return m_sigis.activeParameter().value_or(0); }
-
-private:
-    LanguageServerProtocol::SignatureHelp m_sigis;
-};
-
 QString FunctionHintProposalModel::text(int index) const
 {
     using Parameters = QList<ParameterInformation>;
@@ -62,18 +44,19 @@ QString FunctionHintProposalModel::text(int index) const
            + label.mid(end).toHtmlEscaped();
 }
 
-FunctionHintProcessor::FunctionHintProcessor(Client *client)
+FunctionHintProcessor::FunctionHintProcessor(Client *client, int basePosition)
     : m_client(client)
+    , m_pos(basePosition)
 {}
 
 IAssistProposal *FunctionHintProcessor::perform()
 {
     QTC_ASSERT(m_client, return nullptr);
-    m_pos = interface()->position();
-    QTextCursor cursor(interface()->textDocument());
-    cursor.setPosition(m_pos);
+    if (m_pos < 0)
+        m_pos = interface()->position();
     auto uri = m_client->hostPathToServerUri(interface()->filePath());
-    SignatureHelpRequest request((TextDocumentPositionParams(TextDocumentIdentifier(uri), Position(cursor))));
+    SignatureHelpRequest request(
+        (TextDocumentPositionParams(TextDocumentIdentifier(uri), Position(interface()->cursor()))));
     request.setResponseCallback([this](auto response) { this->handleSignatureResponse(response); });
     m_client->addAssistProcessor(this);
     m_client->sendMessage(request);
@@ -91,6 +74,12 @@ void FunctionHintProcessor::cancel()
     }
 }
 
+IFunctionHintProposalModel *FunctionHintProcessor::createModel(
+    const SignatureHelp &signatureHelp) const
+{
+    return new FunctionHintProposalModel(signatureHelp);
+}
+
 void FunctionHintProcessor::handleSignatureResponse(const SignatureHelpRequest::Response &response)
 {
     QTC_ASSERT(m_client, setAsyncProposalAvailable(nullptr); return);
@@ -107,7 +96,7 @@ void FunctionHintProcessor::handleSignatureResponse(const SignatureHelpRequest::
     if (signatureHelp.signatures().isEmpty()) {
         setAsyncProposalAvailable(nullptr);
     } else {
-        FunctionHintProposalModelPtr model(new FunctionHintProposalModel(signatureHelp));
+        FunctionHintProposalModelPtr model(createModel(signatureHelp));
         setAsyncProposalAvailable(new FunctionHintProposal(m_pos, model));
     }
 }
diff --git a/src/plugins/languageclient/languageclientfunctionhint.h b/src/plugins/languageclient/languageclientfunctionhint.h
index d086d4ccd4d..65d51bec26a 100644
--- a/src/plugins/languageclient/languageclientfunctionhint.h
+++ b/src/plugins/languageclient/languageclientfunctionhint.h
@@ -8,6 +8,7 @@
 #include <languageserverprotocol/languagefeatures.h>
 #include <texteditor/codeassist/completionassistprovider.h>
 #include <texteditor/codeassist/iassistprocessor.h>
+#include <texteditor/codeassist/ifunctionhintproposalmodel.h>
 
 #include <QPointer>
 
@@ -43,13 +44,15 @@ private:
 class LANGUAGECLIENT_EXPORT FunctionHintProcessor : public TextEditor::IAssistProcessor
 {
 public:
-    explicit FunctionHintProcessor(Client *client);
+    explicit FunctionHintProcessor(Client *client, int basePosition = -1);
     TextEditor::IAssistProposal *perform() override;
     bool running() override { return m_currentRequest.has_value(); }
     bool needsRestart() const override { return true; }
     void cancel() override;
 
 private:
+    virtual TextEditor::IFunctionHintProposalModel *createModel(
+        const LanguageServerProtocol::SignatureHelp &signatureHelp) const;
     void handleSignatureResponse(
         const LanguageServerProtocol::SignatureHelpRequest::Response &response);
 
@@ -58,4 +61,22 @@ private:
     int m_pos = -1;
 };
 
+class LANGUAGECLIENT_EXPORT FunctionHintProposalModel
+    : public TextEditor::IFunctionHintProposalModel
+{
+public:
+    explicit FunctionHintProposalModel(LanguageServerProtocol::SignatureHelp signature)
+        : m_sigis(signature)
+    {}
+    void reset() override {}
+    int size() const override { return m_sigis.signatures().size(); }
+    QString text(int index) const override;
+
+    int activeArgument(const QString &/*prefix*/) const override
+    { return m_sigis.activeParameter().value_or(0); }
+
+protected:
+    LanguageServerProtocol::SignatureHelp m_sigis;
+};
+
 } // namespace LanguageClient