LanguageClient: remove codeassist proposal handler

Removes noise from the interface. Using specialized clangd processors
that redirect the proposals for testing purposes is an overall cleaner
solution.

Change-Id: Ia4f3d4720765f0533c5530a2a5c7db3cc971e0be
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/clangcodemodel/clangdclient.cpp

@@ -681,9 +681,11 @@ public:
 class DoxygenAssistProcessor : public TextEditor::IAssistProcessor
 {
 public:
-    DoxygenAssistProcessor(int position, unsigned completionOperator,
+    DoxygenAssistProcessor(ClangdClient *client, int position, unsigned completionOperator)
-                           const ProposalHandler &handler)
+        : m_client(client)
-        : m_position(position), m_completionOperator(completionOperator), m_handler(handler) {}
+        , m_position(position)
+        , m_completionOperator(completionOperator)
+    {}
 
 private:
     TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *) override
@@ -699,16 +701,16 @@ private:
         TextEditor::GenericProposalModelPtr model(new TextEditor::GenericProposalModel);
         model->loadContent(completions);
         const auto proposal = new TextEditor::GenericProposal(m_position, model);
-        if (m_handler) {
+        if (m_client->testingEnabled()) {
-            m_handler(proposal);
+            emit m_client->proposalReady(proposal);
             return nullptr;
         }
         return proposal;
     }
 
+    ClangdClient * const m_client;
     const int m_position;
     const unsigned m_completionOperator;
-    const ProposalHandler m_handler;
 };
 
 class ClangdClient::Private
@@ -770,6 +772,67 @@ public:
     }
 };
 
+QList<LanguageServerProtocol::CompletionItem> completionItemsTransformer(
+    const Utils::FilePath &filePath,
+    const QString &content,
+    int pos,
+    const QList<CompletionItem> &items)
+{
+    qCDebug(clangdLog) << "received" << items.count() << "completions";
+
+    // If there are signals among the candidates, we employ the built-in code model to find out
+    // whether the cursor was on the second argument of a (dis)connect() call.
+    // If so, we offer only signals, as nothing else makes sense in that context.
+    static const auto criterion = [](const CompletionItem &ci) {
+        const Utils::optional<MarkupOrString> doc = ci.documentation();
+        if (!doc)
+            return false;
+        QString docText;
+        if (Utils::holds_alternative<QString>(*doc))
+            docText = Utils::get<QString>(*doc);
+        else if (Utils::holds_alternative<MarkupContent>(*doc))
+            docText = Utils::get<MarkupContent>(*doc).content();
+        return docText.contains("Annotation: qt_signal");
+    };
+    if (pos != -1 && Utils::anyOf(items, criterion)
+        && CppEditor::CppModelManager::instance()->positionRequiresSignal(filePath.toString(),
+                                                                          content.toUtf8(),
+                                                                          pos)) {
+        return Utils::filtered(items, criterion);
+    }
+    return items;
+};
+
+class ClangdClient::ClangdCompletionAssistProcessor : public LanguageClientCompletionAssistProcessor
+{
+public:
+    ClangdCompletionAssistProcessor(ClangdClient *client, const QString &snippetsGroup)
+        : LanguageClientCompletionAssistProcessor(client,
+                                                  &completionItemsTransformer,
+                                                  &applyCompletionItem,
+                                                  snippetsGroup)
+        , m_client(client)
+    {
+    }
+
+private:
+    TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *interface) override
+    {
+        if (m_client->d->isTesting) {
+            setAsyncCompletionAvailableHandler([this](TextEditor::IAssistProposal *proposal) {
+                emit m_client->proposalReady(proposal);
+            });
+        }
+        return LanguageClientCompletionAssistProcessor::perform(interface);
+    }
+
+    static void applyCompletionItem(const CompletionItem &item,
+                                    TextEditor::TextDocumentManipulatorInterface &manipulator,
+                                    QChar typedChar);
+
+    ClangdClient * const m_client;
+};
+
 class ClangdClient::ClangdCompletionAssistProvider : public LanguageClientCompletionAssistProvider
 {
 public:
@@ -783,9 +846,7 @@ private:
     bool isActivationCharSequence(const QString &sequence) const override;
     bool isContinuationChar(const QChar &c) const override;
 
-    void applyCompletionItem(const CompletionItem &item,
+    ClangdClient * const m_client;
-                             TextEditor::TextDocumentManipulatorInterface &manipulator,
-                             QChar typedChar);
 };
 
 ClangdClient::ClangdClient(Project *project, const Utils::FilePath &jsonDbDir)
@@ -1081,13 +1142,11 @@ void ClangdClient::Private::findUsages(TextEditor::TextDocument *document,
 void ClangdClient::enableTesting()
 {
     d->isTesting = true;
-    setCompletionProposalHandler([this](TextEditor::IAssistProposal *proposal) {
+}
-        QMetaObject::invokeMethod(this, [this, proposal] { emit proposalReady(proposal); },
+
-            Qt::QueuedConnection);
+bool ClangdClient::testingEnabled() const
-    });
+{
-    setFunctionHintProposalHandler([this](TextEditor::IAssistProposal *proposal) {
+    return d->isTesting;
-        emit proposalReady(proposal);
-    });
 }
 
 void ClangdClient::Private::handleFindUsagesResult(quint64 key, const QList<Location> &locations)
@@ -2664,39 +2723,32 @@ QString ClangdDiagnostic::category() const
     return typedValue<QString>("category");
 }
 
+class ClangdClient::ClangdFunctionHintProcessor : public FunctionHintProcessor
+{
+public:
+    ClangdFunctionHintProcessor(ClangdClient *client)
+        : FunctionHintProcessor(client)
+        , m_client(client)
+    {}
+
+private:
+    TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *interface) override
+    {
+        if (m_client->d->isTesting) {
+            setAsyncCompletionAvailableHandler([this](TextEditor::IAssistProposal *proposal) {
+                emit m_client->proposalReady(proposal);
+            });
+        }
+        return FunctionHintProcessor::perform(interface);
+    }
+
+    ClangdClient * const m_client;
+};
+
 ClangdClient::ClangdCompletionAssistProvider::ClangdCompletionAssistProvider(ClangdClient *client)
     : LanguageClientCompletionAssistProvider(client)
-{
+    , m_client(client)
-    setItemsTransformer([](const Utils::FilePath &filePath, const QString &content,
+{}
-                        int pos,  const QList<CompletionItem> &items) {
-        qCDebug(clangdLog) << "received" << items.count() << "completions";
-
-        // If there are signals among the candidates, we employ the built-in code model to find out
-        // whether the cursor was on the second argument of a (dis)connect() call.
-        // If so, we offer only signals, as nothing else makes sense in that context.
-        static const auto criterion = [](const CompletionItem &ci) {
-            const Utils::optional<MarkupOrString> doc = ci.documentation();
-            if (!doc)
-                return false;
-            QString docText;
-            if (Utils::holds_alternative<QString>(*doc))
-                docText = Utils::get<QString>(*doc);
-            else if (Utils::holds_alternative<MarkupContent>(*doc))
-                docText = Utils::get<MarkupContent>(*doc).content();
-            return docText.contains("Annotation: qt_signal");
-        };
-        if (pos != -1 && Utils::anyOf(items, criterion) && CppEditor::CppModelManager::instance()
-                ->positionRequiresSignal(filePath.toString(), content.toUtf8(), pos)) {
-            return Utils::filtered(items, criterion);
-        }
-        return items;
-    });
-
-    setApplyHelper([this](const CompletionItem &item,
-                   TextEditor::TextDocumentManipulatorInterface &manipulator, QChar typedChar) {
-        applyCompletionItem(item, manipulator, typedChar);
-    });
-}
 
 TextEditor::IAssistProcessor *ClangdClient::ClangdCompletionAssistProvider::createProcessor(
     const TextEditor::AssistInterface *assistInterface) const
@@ -2704,20 +2756,21 @@ TextEditor::IAssistProcessor *ClangdClient::ClangdCompletionAssistProvider::crea
     ClangCompletionContextAnalyzer contextAnalyzer(assistInterface->textDocument(),
                                                    assistInterface->position(), false, {});
     contextAnalyzer.analyze();
-    client()->setSnippetsGroup(
-        contextAnalyzer.addSnippets() ? CppEditor::Constants::CPP_SNIPPETS_GROUP_ID : QString());
     switch (contextAnalyzer.completionAction()) {
     case ClangCompletionContextAnalyzer::PassThroughToLibClangAfterLeftParen:
         qCDebug(clangdLog) << "completion changed to function hint";
-        return new FunctionHintProcessor(client(), proposalHandler());
+        return new ClangdFunctionHintProcessor(m_client);
     case ClangCompletionContextAnalyzer::CompleteDoxygenKeyword:
-        return new DoxygenAssistProcessor(contextAnalyzer.positionForProposal(),
+        return new DoxygenAssistProcessor(m_client,
-                                          contextAnalyzer.completionOperator(),
+                                          contextAnalyzer.positionForProposal(),
-                                          proposalHandler());
+                                          contextAnalyzer.completionOperator());
     default:
         break;
     }
-    return LanguageClientCompletionAssistProvider::createProcessor(assistInterface);
+    const QString snippetsGroup = contextAnalyzer.addSnippets()
+                                      ? CppEditor::Constants::CPP_SNIPPETS_GROUP_ID
+                                      : QString();
+    return new ClangdCompletionAssistProcessor(m_client, snippetsGroup);
 }
 
 bool ClangdClient::ClangdCompletionAssistProvider::isActivationCharSequence(const QString &sequence) const
@@ -2747,7 +2800,7 @@ bool ClangdClient::ClangdCompletionAssistProvider::isContinuationChar(const QCha
     return CppEditor::isValidIdentifierChar(c);
 }
 
-void ClangdClient::ClangdCompletionAssistProvider::applyCompletionItem(
+void ClangdClient::ClangdCompletionAssistProcessor::applyCompletionItem(
         const CompletionItem &item, TextEditor::TextDocumentManipulatorInterface &manipulator,
         QChar typedChar)
 {

src/plugins/clangcodemodel/clangdclient.h

@@ -77,6 +77,7 @@ public:
             const LanguageServerProtocol::DocumentUri &uri);
 
     void enableTesting();
+    bool testingEnabled() const;
 
 signals:
     void indexingFinished();
@@ -96,6 +97,8 @@ private:
     class FollowSymbolData;
     class VirtualFunctionAssistProcessor;
     class VirtualFunctionAssistProvider;
+    class ClangdFunctionHintProcessor;
+    class ClangdCompletionAssistProcessor;
     class ClangdCompletionAssistProvider;
     Private * const d;
 };

src/plugins/clangcodemodel/test/clangdtests.cpp

@@ -77,6 +77,7 @@ using Range = std::tuple<int, int, int>;
 } // namespace ClangCodeModel
 
 Q_DECLARE_METATYPE(ClangCodeModel::Internal::Tests::Range)
+Q_DECLARE_METATYPE(IAssistProposal *)
 
 namespace ClangCodeModel {
 namespace Internal {
@@ -1885,7 +1886,7 @@ void ClangdTestCompletion::getProposal(const QString &fileName,
     connect(client(), &ClangdClient::proposalReady, &loop, [&proposal, &loop](IAssistProposal *p) {
         proposal = p;
         loop.quit();
-    });
+    }, Qt::QueuedConnection);
     editor->editorWidget()->invokeAssist(Completion, nullptr);
     timer.start(5000);
     loop.exec();

src/plugins/languageclient/client.cpp

@@ -1047,22 +1047,6 @@ SymbolStringifier Client::symbolStringifier() const
     return m_symbolStringifier;
 }
 
-void Client::setCompletionProposalHandler(const ProposalHandler &handler)
-{
-    if (const auto provider = qobject_cast<LanguageClientCompletionAssistProvider *>(
-                m_clientProviders.completionAssistProvider)) {
-        provider->setProposalHandler(handler);
-    }
-}
-
-void Client::setFunctionHintProposalHandler(const ProposalHandler &handler)
-{
-    if (const auto provider = qobject_cast<FunctionHintAssistProvider *>(
-                m_clientProviders.functionHintProvider)) {
-        provider->setProposalHandler(handler);
-    }
-}
-
 void Client::setSnippetsGroup(const QString &group)
 {
     if (const auto provider = qobject_cast<LanguageClientCompletionAssistProvider *>(

src/plugins/languageclient/client.h

@@ -185,8 +185,6 @@ public:
     void setSemanticTokensHandler(const SemanticTokensHandler &handler);
     void setSymbolStringifier(const LanguageServerProtocol::SymbolStringifier &stringifier);
     LanguageServerProtocol::SymbolStringifier symbolStringifier() const;
-    void setCompletionProposalHandler(const ProposalHandler &handler);
-    void setFunctionHintProposalHandler(const ProposalHandler &handler);
     void setSnippetsGroup(const QString &group);
     void setCompletionAssistProvider(LanguageClientCompletionAssistProvider *provider);
 

src/plugins/languageclient/languageclientcompletionassist.cpp

@@ -287,12 +287,15 @@ public:
     int m_pos = -1;
 };
 
-
+LanguageClientCompletionAssistProcessor::LanguageClientCompletionAssistProcessor(
-LanguageClientCompletionAssistProcessor::LanguageClientCompletionAssistProcessor(Client *client,
+    Client *client,
-    const CompletionItemsTransformer &itemsTransformer, const CompletionApplyHelper &applyHelper,
+    const CompletionItemsTransformer &itemsTransformer,
-    const ProposalHandler &proposalHandler, const QString &snippetsGroup)
+    const CompletionApplyHelper &applyHelper,
-    : m_client(client), m_itemsTransformer(itemsTransformer), m_applyHelper(applyHelper),
+    const QString &snippetsGroup)
-      m_proposalHandler(proposalHandler), m_snippetsGroup(snippetsGroup)
+    : m_client(client)
+    , m_itemsTransformer(itemsTransformer)
+    , m_applyHelper(applyHelper)
+    , m_snippetsGroup(snippetsGroup)
 {}
 
 LanguageClientCompletionAssistProcessor::~LanguageClientCompletionAssistProcessor()
@@ -427,9 +430,6 @@ void LanguageClientCompletionAssistProcessor::handleCompletionResponse(
     proposal->m_pos = m_pos;
     proposal->setFragile(true);
     proposal->setSupportsPrefix(false);
-    if (m_proposalHandler)
-        m_proposalHandler(proposal);
-    else
     setAsyncProposalAvailable(proposal);
     m_client->removeAssistProcessor(this);
     qCDebug(LOGLSPCOMPLETION) << QTime::currentTime() << " : "
@@ -444,8 +444,10 @@ LanguageClientCompletionAssistProvider::LanguageClientCompletionAssistProvider(C
 IAssistProcessor *LanguageClientCompletionAssistProvider::createProcessor(
     const AssistInterface *) const
 {
-    return new LanguageClientCompletionAssistProcessor(m_client, m_itemsTransformer, m_applyHelper,
+    return new LanguageClientCompletionAssistProcessor(m_client,
-                                                       m_proposalHandler, m_snippetsGroup);
+                                                       m_itemsTransformer,
+                                                       m_applyHelper,
+                                                       m_snippetsGroup);
 }
 
 IAssistProvider::RunType LanguageClientCompletionAssistProvider::runType() const

src/plugins/languageclient/languageclientcompletionassist.h

@@ -53,7 +53,6 @@ using CompletionItemsTransformer = std::function<QList<LanguageServerProtocol::C
 using CompletionApplyHelper = std::function<void(
         const LanguageServerProtocol::CompletionItem &,
         TextEditor::TextDocumentManipulatorInterface &, QChar)>;
-using ProposalHandler = std::function<void(TextEditor::IAssistProposal *)>;
 
 class LANGUAGECLIENT_EXPORT LanguageClientCompletionAssistProvider
     : public TextEditor::CompletionAssistProvider
@@ -72,20 +71,17 @@ public:
 
     void setTriggerCharacters(const Utils::optional<QList<QString>> triggerChars);
 
-    void setProposalHandler(const ProposalHandler &handler) { m_proposalHandler = handler; }
     void setSnippetsGroup(const QString &group) { m_snippetsGroup = group; }
 
 protected:
     void setItemsTransformer(const CompletionItemsTransformer &transformer);
     void setApplyHelper(const CompletionApplyHelper &applyHelper);
     Client *client() const { return m_client; }
-    const ProposalHandler &proposalHandler() const { return m_proposalHandler; }
 
 private:
     QList<QString> m_triggerChars;
     CompletionItemsTransformer m_itemsTransformer;
     CompletionApplyHelper m_applyHelper;
-    ProposalHandler m_proposalHandler;
     QString m_snippetsGroup;
     int m_activationCharSequenceLength = 0;
     Client *m_client = nullptr; // not owned
@@ -98,7 +94,6 @@ public:
     LanguageClientCompletionAssistProcessor(Client *client,
                                             const CompletionItemsTransformer &itemsTransformer,
                                             const CompletionApplyHelper &applyHelper,
-                                            const ProposalHandler &proposalHandler,
                                             const QString &snippetsGroup);
     ~LanguageClientCompletionAssistProcessor() override;
     TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *interface) override;
@@ -116,7 +111,6 @@ private:
     QMetaObject::Connection m_postponedUpdateConnection;
     const CompletionItemsTransformer m_itemsTransformer;
     const CompletionApplyHelper m_applyHelper;
-    const ProposalHandler m_proposalHandler;
     const QString m_snippetsGroup;
     int m_pos = -1;
     int m_basePos = -1;

src/plugins/languageclient/languageclientfunctionhint.cpp

@@ -81,9 +81,8 @@ QString FunctionHintProposalModel::text(int index) const
            + label.mid(end).toHtmlEscaped();
 }
 
-FunctionHintProcessor::FunctionHintProcessor(Client *client, const ProposalHandler &proposalHandler)
+FunctionHintProcessor::FunctionHintProcessor(Client *client)
     : m_client(client)
-    , m_proposalHandler(proposalHandler)
 {}
 
 IAssistProposal *FunctionHintProcessor::perform(const AssistInterface *interface)
@@ -117,26 +116,18 @@ void FunctionHintProcessor::handleSignatureResponse(const SignatureHelpRequest::
     m_client->removeAssistProcessor(this);
     auto result = response.result().value_or(LanguageClientValue<SignatureHelp>());
     if (result.isNull()) {
-        processProposal(nullptr);
+        setAsyncProposalAvailable(nullptr);
         return;
     }
     const SignatureHelp &signatureHelp = result.value();
     if (signatureHelp.signatures().isEmpty()) {
-        processProposal(nullptr);
+        setAsyncProposalAvailable(nullptr);
     } else {
         FunctionHintProposalModelPtr model(new FunctionHintProposalModel(signatureHelp));
-        processProposal(new FunctionHintProposal(m_pos, model));
+        setAsyncProposalAvailable(new FunctionHintProposal(m_pos, model));
     }
 }
 
-void FunctionHintProcessor::processProposal(IAssistProposal *proposal)
-{
-    if (m_proposalHandler)
-        m_proposalHandler(proposal);
-    else
-        setAsyncProposalAvailable(proposal);
-}
-
 FunctionHintAssistProvider::FunctionHintAssistProvider(Client *client)
     : CompletionAssistProvider(client)
     , m_client(client)
@@ -145,7 +136,7 @@ FunctionHintAssistProvider::FunctionHintAssistProvider(Client *client)
 TextEditor::IAssistProcessor *FunctionHintAssistProvider::createProcessor(
     const AssistInterface *) const
 {
-    return new FunctionHintProcessor(m_client, m_proposalHandler);
+    return new FunctionHintProcessor(m_client);
 }
 
 IAssistProvider::RunType FunctionHintAssistProvider::runType() const

src/plugins/languageclient/languageclientfunctionhint.h

@@ -40,8 +40,6 @@ namespace LanguageClient {
 
 class Client;
 
-using ProposalHandler = std::function<void(TextEditor::IAssistProposal *)>;
-
 class LANGUAGECLIENT_EXPORT FunctionHintAssistProvider : public TextEditor::CompletionAssistProvider
 {
     Q_OBJECT
@@ -58,11 +56,8 @@ public:
 
     void setTriggerCharacters(const Utils::optional<QList<QString>> &triggerChars);
 
-    void setProposalHandler(const ProposalHandler &handler) { m_proposalHandler = handler; }
-
 private:
     QList<QString> m_triggerChars;
-    ProposalHandler m_proposalHandler;
     int m_activationCharSequenceLength = 0;
     Client *m_client = nullptr; // not owned
 };
@@ -70,7 +65,7 @@ private:
 class LANGUAGECLIENT_EXPORT FunctionHintProcessor : public TextEditor::IAssistProcessor
 {
 public:
-    explicit FunctionHintProcessor(Client *client, const ProposalHandler &proposalHandler);
+    explicit FunctionHintProcessor(Client *client);
     TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *interface) override;
     bool running() override { return m_currentRequest.has_value(); }
     bool needsRestart() const override { return true; }
@@ -79,10 +74,8 @@ public:
 private:
     void handleSignatureResponse(
         const LanguageServerProtocol::SignatureHelpRequest::Response &response);
-    void processProposal(TextEditor::IAssistProposal *proposal);
 
     QPointer<Client> m_client;
-    const ProposalHandler m_proposalHandler;
     Utils::optional<LanguageServerProtocol::MessageId> m_currentRequest;
     int m_pos = -1;
 };