qmlls: move common module code to base class

Move boilerplate code that is duplicated (or will be duplicated when
the find usages module will be added in a later commit) into the
qqmlbasemodule and BaseRequest class.
Also rename the BaseRequest members to start with "m_".

Task-number: QTBUG-100084
Task-number: QTBUG-111415
Change-Id: I69ce175a9438feb599c23f13803a673c460c5f3f
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

5 files changed, 81 insertions, 120 deletions

diff --git a/src/qmlls/qqmlbasemodule_p.h b/src/qmlls/qqmlbasemodule_p.h
index 49e201a82e..e73e44f0d7 100644
--- a/src/qmlls/qqmlbasemodule_p.h
+++ b/src/qmlls/qqmlbasemodule_p.h
@@ -17,6 +17,7 @@
 
 #include "qlanguageserver_p.h"
 #include "qqmlcodemodel_p.h"
+#include "qqmllsutils_p.h"
 
 #include <QObject>
 #include <unordered_map>
@@ -33,9 +34,9 @@ struct BaseRequest
     // Request is received: mark it with the current version of the textDocument.
     // Then, wait for the codemodel to finish creating a snapshot version that is newer or equal to
     // the textDocument version at request-received-time.
-    int minVersion;
-    Parameters parameters;
-    Response response;
+    int m_minVersion;
+    Parameters m_parameters;
+    Response m_response;
 
     bool fillFrom(QmlLsp::OpenDocument doc, const Parameters &params, Response &&response);
 };
@@ -47,17 +48,19 @@ struct QQmlBaseModule : public QLanguageServerModule
     using RequestResponse = typename RequestType::Response;
     using RequestPointer = std::unique_ptr<RequestType>;
     using RequestPointerArgument = RequestPointer &&;
+    using BaseT = QQmlBaseModule<RequestType>;
 
     QQmlBaseModule(QmlLsp::QQmlCodeModel *codeModel);
     ~QQmlBaseModule();
 
-    bool addRequestAndCheckForUpdate(const QmlLsp::OpenDocument,
-                                     const RequestParameters &parameters,
-                                     RequestResponse &&response);
-    void processPending(const QByteArray &url);
+    void requestHandler(const RequestParameters &parameters, RequestResponse &&response);
+    decltype(auto) getRequestHandler();
     // processes a request in a different thread.
     virtual void process(RequestPointerArgument toBeProcessed) = 0;
 
+public Q_SLOTS:
+    void updatedSnapshot(const QByteArray &uri);
+
 protected:
     QMutex m_pending_mutex;
     std::unordered_multimap<QString, RequestPointer> m_pending;
@@ -66,11 +69,19 @@ protected:
 
 template<typename Parameters, typename Response>
 bool BaseRequest<Parameters, Response>::fillFrom(QmlLsp::OpenDocument doc, const Parameters &params,
-                                                 Response &&resp)
+                                                 Response &&response)
 {
     Q_UNUSED(doc);
-    parameters = params;
-    response = std::move(resp);
+    m_parameters = params;
+    m_response = std::move(response);
+
+    if (!doc.textDocument)
+        return false;
+
+    {
+        QMutexLocker l(doc.textDocument->mutex());
+        m_minVersion = doc.textDocument->version().value_or(0);
+    }
     return true;
 }
 
@@ -78,6 +89,8 @@ template<typename RequestType>
 QQmlBaseModule<RequestType>::QQmlBaseModule(QmlLsp::QQmlCodeModel *codeModel)
     : m_codeModel(codeModel)
 {
+    QObject::connect(m_codeModel, &QmlLsp::QQmlCodeModel::updatedSnapshot, this,
+                     &QQmlBaseModule<RequestType>::updatedSnapshot);
 }
 
 template<typename RequestType>
@@ -87,38 +100,48 @@ QQmlBaseModule<RequestType>::~QQmlBaseModule()
     m_pending.clear(); // empty the m_pending while the mutex is hold
 }
 
-// make the registerHandlers method easier to write
 template<typename RequestType>
-bool QQmlBaseModule<RequestType>::addRequestAndCheckForUpdate(QmlLsp::OpenDocument doc,
-                                                              const RequestParameters &parameters,
-                                                              RequestResponse &&response)
+decltype(auto) QQmlBaseModule<RequestType>::getRequestHandler()
+{
+    auto handler = [this](const QByteArray &, const RequestParameters &parameters,
+                          RequestResponse &&response) {
+        requestHandler(parameters, std::move(response));
+    };
+    return handler;
+}
+
+template<typename RequestType>
+void QQmlBaseModule<RequestType>::requestHandler(const RequestParameters &parameters,
+                                                 RequestResponse &&response)
 {
     auto req = std::make_unique<RequestType>();
-    const bool requestIsValid = req->fillFrom(doc, parameters, std::move(response));
-    if (!requestIsValid) {
-        req->response.sendErrorResponse(0, "Received invalid request", parameters);
-        return false;
+    QmlLsp::OpenDocument doc = m_codeModel->openDocumentByUrl(
+            QQmlLSUtils::lspUriToQmlUrl(parameters.textDocument.uri));
+
+    if (!req->fillFrom(doc, parameters, std::move(response))) {
+        req->m_response.sendErrorResponse(0, "Received invalid request", parameters);
+        return;
     }
-    const int minVersion = req->minVersion;
+    const int minVersion = req->m_minVersion;
     {
         QMutexLocker l(&m_pending_mutex);
-        m_pending.insert({ QString::fromUtf8(req->parameters.textDocument.uri), std::move(req) });
+        m_pending.insert({ QString::fromUtf8(req->m_parameters.textDocument.uri), std::move(req) });
     }
-    const bool requireSnapshotUpdate =
-            doc.snapshot.docVersion && *doc.snapshot.docVersion >= minVersion;
-    return requireSnapshotUpdate;
+
+    if (doc.snapshot.docVersion && *doc.snapshot.docVersion >= minVersion)
+        updatedSnapshot(QQmlLSUtils::lspUriToQmlUrl(parameters.textDocument.uri));
 }
 
-// make the updatedSnapshot method easier to write
 template<typename RequestType>
-void QQmlBaseModule<RequestType>::processPending(const QByteArray &url)
+void QQmlBaseModule<RequestType>::updatedSnapshot(const QByteArray &url)
 {
     QmlLsp::OpenDocumentSnapshot doc = m_codeModel->snapshotByUrl(url);
     std::vector<RequestPointer> toCompl;
     {
         QMutexLocker l(&m_pending_mutex);
         for (auto [it, end] = m_pending.equal_range(QString::fromUtf8(url)); it != end;) {
-            if (auto &[key, value] = *it; doc.docVersion && value->minVersion <= *doc.docVersion) {
+            if (auto &[key, value] = *it;
+                doc.docVersion && value->m_minVersion <= *doc.docVersion) {
                 toCompl.push_back(std::move(value));
                 it = m_pending.erase(it);
             } else {
diff --git a/src/qmlls/qqmlcompletionsupport.cpp b/src/qmlls/qqmlcompletionsupport.cpp
index 51f2c1dec4..b5bb16678e 100644
--- a/src/qmlls/qqmlcompletionsupport.cpp
+++ b/src/qmlls/qqmlcompletionsupport.cpp
@@ -21,7 +21,10 @@ Q_LOGGING_CATEGORY(complLog, "qt.languageserver.completions")
 bool CompletionRequest::fillFrom(QmlLsp::OpenDocument doc, const Parameters &params,
                                  Response &&response)
 {
-    BaseRequest<Parameters, Response>::fillFrom(doc, params, std::move(response));
+    // do not call BaseRequest::fillFrom() to avoid taking the Mutex twice and getting an
+    // inconsistent state.
+    m_parameters = params;
+    m_response = std::move(response);
 
     if (!doc.textDocument)
         return false;
@@ -32,36 +35,19 @@ bool CompletionRequest::fillFrom(QmlLsp::OpenDocument doc, const Parameters &par
         targetVersion = doc.textDocument->version();
         code = doc.textDocument->toPlainText();
     }
-    minVersion = (targetVersion ? *targetVersion : 0);
+    m_minVersion = (targetVersion ? *targetVersion : 0);
 
     return true;
 }
 
 void QmlCompletionSupport::registerHandlers(QLanguageServer *, QLanguageServerProtocol *protocol)
 {
-    protocol->registerCompletionRequestHandler([this](const QByteArray &,
-                                                      const RequestParameters &cParams,
-                                                      RequestResponse &&response) {
-        QmlLsp::OpenDocument doc = m_codeModel->openDocumentByUrl(
-                QQmlLSUtils::lspUriToQmlUrl(cParams.textDocument.uri));
-
-        const bool needsUpdate = addRequestAndCheckForUpdate(doc, cParams, std::move(response));
-
-        if (needsUpdate)
-            updatedSnapshot(QQmlLSUtils::lspUriToQmlUrl(cParams.textDocument.uri));
-    });
+    protocol->registerCompletionRequestHandler(getRequestHandler());
     protocol->registerCompletionItemResolveRequestHandler(
             [](const QByteArray &, const CompletionItem &cParams,
                LSPResponse<CompletionItem> &&response) { response.sendResponse(cParams); });
 }
 
-QmlCompletionSupport::QmlCompletionSupport(QmlLsp::QQmlCodeModel *codeModel)
-    : QQmlBaseModule(codeModel)
-{
-    QObject::connect(m_codeModel, &QmlLsp::QQmlCodeModel::updatedSnapshot, this,
-                     &QmlCompletionSupport::updatedSnapshot);
-}
-
 QString QmlCompletionSupport::name() const
 {
     return u"QmlCompletionSupport"_s;
@@ -79,22 +65,18 @@ void QmlCompletionSupport::setupCapabilities(
     serverCapabilities.capabilities.completionProvider = cOptions;
 }
 
-void QmlCompletionSupport::updatedSnapshot(const QByteArray &url)
-{
-    processPending(url);
-}
-
 void QmlCompletionSupport::process(RequestPointerArgument req)
 {
-    QmlLsp::OpenDocumentSnapshot doc = m_codeModel->snapshotByUrl(req->parameters.textDocument.uri);
+    QmlLsp::OpenDocumentSnapshot doc =
+            m_codeModel->snapshotByUrl(req->m_parameters.textDocument.uri);
     req->sendCompletions(doc);
 }
 
 QString CompletionRequest::urlAndPos() const
 {
-    return QString::fromUtf8(parameters.textDocument.uri) + u":"
-            + QString::number(parameters.position.line) + u":"
-            + QString::number(parameters.position.character);
+    return QString::fromUtf8(m_parameters.textDocument.uri) + u":"
+            + QString::number(m_parameters.position.line) + u":"
+            + QString::number(m_parameters.position.character);
 }
 
 // finds the filter string, the base (for fully qualified accesses) and the whole string
@@ -186,7 +168,7 @@ enum class ImportCompletionType { None, Module, Version };
 void CompletionRequest::sendCompletions(QmlLsp::OpenDocumentSnapshot &doc)
 {
     QList<CompletionItem> res = completions(doc);
-    response.sendResponse(res);
+    m_response.sendResponse(res);
 }
 
 static QList<CompletionItem> importCompletions(DomItem &file, const CompletionContextStrings &ctx)
@@ -459,24 +441,24 @@ QList<CompletionItem> CompletionRequest::completions(QmlLsp::OpenDocumentSnapsho
     QList<CompletionItem> res;
     if (!doc.validDoc) {
         qCWarning(complLog) << "No valid document for completions  for "
-                            << QString::fromUtf8(parameters.textDocument.uri);
+                            << QString::fromUtf8(m_parameters.textDocument.uri);
         // try to add some import and global completions?
         return res;
     }
-    if (!doc.docVersion || *doc.docVersion < minVersion) {
+    if (!doc.docVersion || *doc.docVersion < m_minVersion) {
         qCWarning(complLog) << "sendCompletions on older doc version";
-    } else if (!doc.validDocVersion || *doc.validDocVersion < minVersion) {
+    } else if (!doc.validDocVersion || *doc.validDocVersion < m_minVersion) {
         qCWarning(complLog) << "using outdated valid doc, position might be incorrect";
     }
     DomItem file = doc.validDoc.fileObject(QQmlJS::Dom::GoTo::MostLikely);
     // clear reference cache to resolve latest versions (use a local env instead?)
     if (std::shared_ptr<DomEnvironment> envPtr = file.environment().ownerAs<DomEnvironment>())
         envPtr->clearReferenceCache();
-    qsizetype pos = QQmlLSUtils::textOffsetFrom(code, parameters.position.line,
-                                                parameters.position.character);
+    qsizetype pos = QQmlLSUtils::textOffsetFrom(code, m_parameters.position.line,
+                                                m_parameters.position.character);
     CompletionContextStrings ctx(code, pos);
-    auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, parameters.position.line,
-                                                         parameters.position.character
+    auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, m_parameters.position.line,
+                                                         m_parameters.position.character
                                                                  - ctx.filterChars().size());
     if (itemsFound.size() > 1) {
         QStringList paths;
@@ -488,9 +470,9 @@ QList<CompletionItem> CompletionRequest::completions(QmlLsp::OpenDocumentSnapsho
     DomItem currentItem;
     if (!itemsFound.isEmpty())
         currentItem = itemsFound.first().domItem;
-    qCDebug(complLog) << "Completion at " << urlAndPos() << " " << parameters.position.line << ":"
-                      << parameters.position.character << "offset:" << pos << "base:" << ctx.base()
-                      << "filter:" << ctx.filterChars()
+    qCDebug(complLog) << "Completion at " << urlAndPos() << " " << m_parameters.position.line << ":"
+                      << m_parameters.position.character << "offset:" << pos
+                      << "base:" << ctx.base() << "filter:" << ctx.filterChars()
                       << "lastVersion:" << (doc.docVersion ? (*doc.docVersion) : -1)
                       << "validVersion:" << (doc.validDocVersion ? (*doc.validDocVersion) : -1)
                       << "in" << currentItem.internalKindStr() << currentItem.canonicalPath();
diff --git a/src/qmlls/qqmlcompletionsupport_p.h b/src/qmlls/qqmlcompletionsupport_p.h
index 6036d169d4..9b4a3e781f 100644
--- a/src/qmlls/qqmlcompletionsupport_p.h
+++ b/src/qmlls/qqmlcompletionsupport_p.h
@@ -42,14 +42,12 @@ class QmlCompletionSupport : public QQmlBaseModule<CompletionRequest>
 {
     Q_OBJECT
 public:
-    QmlCompletionSupport(QmlLsp::QQmlCodeModel *codeModel);
+    using BaseT::BaseT;
     QString name() const override;
     void registerHandlers(QLanguageServer *server, QLanguageServerProtocol *protocol) override;
     void setupCapabilities(const QLspSpecification::InitializeParams &clientInfo,
                            QLspSpecification::InitializeResult &) override;
     void process(RequestPointerArgument req) override;
-public Q_SLOTS:
-    void updatedSnapshot(const QByteArray &uri);
 };
 
 #endif // QMLCOMPLETIONSUPPORT_P_H
diff --git a/src/qmlls/qqmlgototypedefinitionsupport.cpp b/src/qmlls/qqmlgototypedefinitionsupport.cpp
index da2e663d90..d3f384f33b 100644
--- a/src/qmlls/qqmlgototypedefinitionsupport.cpp
+++ b/src/qmlls/qqmlgototypedefinitionsupport.cpp
@@ -10,25 +10,8 @@
 QT_USE_NAMESPACE
 using namespace Qt::StringLiterals;
 
-bool TypeDefinitionRequest::fillFrom(QmlLsp::OpenDocument doc, const Parameters &params,
-                                     Response &&response)
-{
-    BaseRequest<Parameters, Response>::fillFrom(doc, params, std::move(response));
-
-    if (!doc.textDocument)
-        return false;
-
-    std::optional<int> targetVersion;
-    {
-        QMutexLocker l(doc.textDocument->mutex());
-        targetVersion = doc.textDocument->version();
-    }
-    minVersion = (targetVersion ? *targetVersion : 0);
-    return true;
-}
-
 QmlGoToTypeDefinitionSupport::QmlGoToTypeDefinitionSupport(QmlLsp::QQmlCodeModel *codeModel)
-    : QQmlBaseModule<TypeDefinitionRequest>(codeModel)
+    : BaseT(codeModel)
 {
 }
 
@@ -50,43 +33,32 @@ void QmlGoToTypeDefinitionSupport::setupCapabilities(
 void QmlGoToTypeDefinitionSupport::registerHandlers(QLanguageServer *,
                                                     QLanguageServerProtocol *protocol)
 {
-    protocol->registerTypeDefinitionRequestHandler([this](const QByteArray &,
-                                                          const RequestParameters &parameters,
-                                                          RequestResponse &&response) {
-        QmlLsp::OpenDocument doc = m_codeModel->openDocumentByUrl(
-                QQmlLSUtils::lspUriToQmlUrl(parameters.textDocument.uri));
-
-        bool checkUpdate = addRequestAndCheckForUpdate(doc, parameters, std::move(response));
-
-        if (checkUpdate)
-            updatedSnapshot(QQmlLSUtils::lspUriToQmlUrl(parameters.textDocument.uri));
-    });
+    protocol->registerTypeDefinitionRequestHandler(getRequestHandler());
 }
 
 void QmlGoToTypeDefinitionSupport::process(RequestPointerArgument request)
 {
     QList<QLspSpecification::Location> results;
-    QScopeGuard onExit([&results, &request]() { request->response.sendResponse(results); });
+    QScopeGuard onExit([&results, &request]() { request->m_response.sendResponse(results); });
 
     QmlLsp::OpenDocument doc;
     {
         doc = m_codeModel->openDocumentByUrl(
-                QQmlLSUtils::lspUriToQmlUrl(request->parameters.textDocument.uri));
+                QQmlLSUtils::lspUriToQmlUrl(request->m_parameters.textDocument.uri));
     }
 
     QQmlJS::Dom::DomItem file = doc.snapshot.validDoc.fileObject(QQmlJS::Dom::GoTo::MostLikely);
     // clear reference cache to resolve latest versions (use a local env instead?)
     if (auto envPtr = file.environment().ownerAs<QQmlJS::Dom::DomEnvironment>())
         envPtr->clearReferenceCache();
-    auto filePtr = file.as<QQmlJS::Dom::QmlFile>();
-    if (!filePtr || !filePtr->isValid()) {
+    if (!file) {
         qWarning() << u"Could not find file in Dom Environment from Codemodel :"_s
                    << doc.snapshot.doc.toString();
         return;
     }
 
-    auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, request->parameters.position.line,
-                                                         request->parameters.position.character);
+    auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, request->m_parameters.position.line,
+                                                         request->m_parameters.position.character);
     if (itemsFound.size() == 0) {
         qWarning() << u"Could not find any items at given text location."_s;
         return;
@@ -135,8 +107,3 @@ void QmlGoToTypeDefinitionSupport::process(RequestPointerArgument request)
 
     results.append(l);
 }
-
-void QmlGoToTypeDefinitionSupport::updatedSnapshot(const QByteArray &url)
-{
-    processPending(url);
-}
diff --git a/src/qmlls/qqmlgototypedefinitionsupport_p.h b/src/qmlls/qqmlgototypedefinitionsupport_p.h
index a0e62c57db..2e87d5faba 100644
--- a/src/qmlls/qqmlgototypedefinitionsupport_p.h
+++ b/src/qmlls/qqmlgototypedefinitionsupport_p.h
@@ -20,15 +20,9 @@
 #include "qqmlbasemodule_p.h"
 
 struct TypeDefinitionRequest
-    : public BaseRequest<
-              QLspSpecification::TypeDefinitionParams,
-              QLspSpecification::LSPPartialResponse<
-                      std::variant<QLspSpecification::Location, QList<QLspSpecification::Location>,
-                                   QList<QLspSpecification::LocationLink>, std::nullptr_t>,
-                      std::variant<QList<QLspSpecification::Location>,
-                                   QList<QLspSpecification::LocationLink>>>>
+    : public BaseRequest<QLspSpecification::TypeDefinitionParams,
+                         QLspSpecification::Responses::TypeDefinitionResponseType>
 {
-    bool fillFrom(QmlLsp::OpenDocument doc, const Parameters &params, Response &&response);
 };
 
 class QmlGoToTypeDefinitionSupport : public QQmlBaseModule<TypeDefinitionRequest>
@@ -47,9 +41,6 @@ public:
     void typeDefinitionRequestHandler(const QByteArray &,
                                       const QLspSpecification::TypeDefinitionParams &params,
                                       TypeDefinitionRequest::Response &&response);
-
-public Q_SLOTS:
-    void updatedSnapshot(const QByteArray &uri);
 };
 
 #endif // QMLGOTOTYPEDEFINITIONSUPPORT_P_H