ClangCodeModel: Move finding local references to a dedicated class

Change-Id: I5be08b58f4481df462f0ec664687a2c9f141f8b6
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/clangcodemodel/clangdclient.cpp

@@ -192,28 +192,6 @@ static BaseClientInterface *clientInterface(Project *project, const Utils::FileP
     return interface;
 }
 
-class LocalRefsData {
-public:
-    LocalRefsData(quint64 id, TextDocument *doc, const QTextCursor &cursor,
-                  CppEditor::RenameCallback &&callback)
-        : id(id), document(doc), cursor(cursor), callback(std::move(callback)),
-          uri(DocumentUri::fromFilePath(doc->filePath())), revision(doc->document()->revision())
-    {}
-
-    ~LocalRefsData()
-    {
-        if (callback)
-            callback({}, {}, revision);
-    }
-
-    const quint64 id;
-    const QPointer<TextDocument> document;
-    const QTextCursor cursor;
-    CppEditor::RenameCallback callback;
-    const DocumentUri uri;
-    const int revision;
-};
-
 class DiagnosticsCapabilities : public JsonObject
 {
 public:
@@ -345,7 +323,7 @@ public:
     const CppEditor::ClangdSettings::Data settings;
     ClangdFollowSymbol *followSymbol = nullptr;
     ClangdSwitchDeclDef *switchDeclDef = nullptr;
-    Utils::optional<LocalRefsData> localRefsData;
+    ClangdFindLocalReferences *findLocalRefs = nullptr;
     Utils::optional<QVersionNumber> versionNumber;
 
     QHash<TextDocument *, HighlightingData> highlightingData;
@@ -355,7 +333,6 @@ public:
     VersionedDataCache<const TextDocument *, ClangdAstNode> astCache;
     VersionedDataCache<Utils::FilePath, ClangdAstNode> externalAstCache;
     TaskTimer highlightingTimer{"highlighting"};
-    quint64 nextJobId = 0;
     bool isFullyIndexed = false;
     bool isTesting = false;
 };
@@ -950,81 +927,23 @@ void ClangdClient::findLocalUsages(TextDocument *document, const QTextCursor &cu
     qCDebug(clangdLog) << "local references requested" << document->filePath()
                        << (cursor.blockNumber() + 1) << (cursor.positionInBlock() + 1);
 
-    d->localRefsData.emplace(++d->nextJobId, document, cursor, std::move(callback));
+    if (d->findLocalRefs) {
+        d->findLocalRefs->disconnect(this);
+        d->findLocalRefs->deleteLater();
+        d->findLocalRefs = nullptr;
+    }
+
     const QString searchTerm = d->searchTermFromCursor(cursor);
     if (searchTerm.isEmpty()) {
-        d->localRefsData.reset();
+        callback({}, {}, document->document()->revision());
         return;
     }
 
-    // Step 1: Go to definition
+    d->findLocalRefs = new ClangdFindLocalReferences(this, document, cursor, callback);
-    const auto gotoDefCallback = [this, id = d->localRefsData->id](const Utils::Link &link) {
+    connect(d->findLocalRefs, &ClangdFindLocalReferences::done, this, [this] {
-        qCDebug(clangdLog) << "received go to definition response" << link.targetFilePath
+        d->findLocalRefs->deleteLater();
-                           << link.targetLine << (link.targetColumn + 1);
+        d->findLocalRefs = nullptr;
-        if (!d->localRefsData || id != d->localRefsData->id)
+    });
-            return;
-        if (!link.hasValidTarget()) {
-            d->localRefsData.reset();
-            return;
-        }
-
-        // Step 2: Get AST and check whether it's a local variable.
-        const auto astHandler = [this, link, id](const ClangdAstNode &ast, const MessageId &) {
-            qCDebug(clangdLog) << "received ast response";
-            if (!d->localRefsData || id != d->localRefsData->id)
-                return;
-            if (!ast.isValid() || !d->localRefsData->document) {
-                d->localRefsData.reset();
-                return;
-            }
-
-            const Position linkPos(link.targetLine - 1, link.targetColumn);
-            const ClangdAstPath astPath = getAstPath(ast, linkPos);
-            bool isVar = false;
-            for (auto it = astPath.rbegin(); it != astPath.rend(); ++it) {
-                if (it->role() == "declaration"
-                        && (it->kind() == "Function" || it->kind() == "CXXMethod"
-                            || it->kind() == "CXXConstructor" || it->kind() == "CXXDestructor"
-                            || it->kind() == "Lambda")) {
-                    if (!isVar)
-                        break;
-
-                    // Step 3: Find references.
-                    qCDebug(clangdLog) << "finding references for local var";
-                    symbolSupport().findUsages(d->localRefsData->document,
-                                               d->localRefsData->cursor,
-                                               [this, id](const QList<Location> &locations) {
-                        qCDebug(clangdLog) << "found" << locations.size() << "local references";
-                        if (!d->localRefsData || id != d->localRefsData->id)
-                            return;
-                        const Utils::Links links = Utils::transform(locations, &Location::toLink);
-
-                        // The callback only uses the symbol length, so we just create a dummy.
-                        // Note that the calculation will be wrong for identifiers with
-                        // embedded newlines, but we've never supported that.
-                        QString symbol;
-                        if (!locations.isEmpty()) {
-                            const Range r = locations.first().range();
-                            symbol = QString(r.end().character() - r.start().character(), 'x');
-                        }
-                        d->localRefsData->callback(symbol, links, d->localRefsData->revision);
-                        d->localRefsData->callback = {};
-                        d->localRefsData.reset();
-                    });
-                    return;
-                }
-                if (!isVar && it->role() == "declaration"
-                        && (it->kind() == "Var" || it->kind() == "ParmVar")) {
-                    isVar = true;
-                }
-            }
-            d->localRefsData.reset();
-        };
-        qCDebug(clangdLog) << "sending ast request for link";
-        d->getAndHandleAst(d->localRefsData->document, astHandler,
-                           AstCallbackMode::SyncIfPossible);
-    };
-    symbolSupport().findLinkAt(document, cursor, std::move(gotoDefCallback), true);
 }
 
 void ClangdClient::gatherHelpItemForTooltip(const HoverRequest::Response &hoverResponse,

src/plugins/clangcodemodel/clangdfindreferences.cpp

@@ -460,6 +460,135 @@ static Usage::Type getUsageType(const ClangdAstPath &path)
     return Usage::Type::Other;
 }
 
+class ClangdFindLocalReferences::Private
+{
+public:
+    Private(ClangdFindLocalReferences *q, TextDocument *document, const QTextCursor &cursor,
+            const RenameCallback &callback)
+        : q(q), document(document), cursor(cursor), callback(callback),
+          uri(DocumentUri::fromFilePath(document->filePath())),
+          revision(document->document()->revision())
+    {}
+
+    ClangdClient *client() const { return qobject_cast<ClangdClient *>(q->parent()); }
+    void findDefinition();
+    void getDefinitionAst(const Link &link);
+    void checkDefinitionAst(const ClangdAstNode &ast);
+    void handleReferences(const QList<Location> &references);
+    void finish();
+
+    ClangdFindLocalReferences * const q;
+    const QPointer<TextDocument> document;
+    const QTextCursor cursor;
+    RenameCallback callback;
+    const DocumentUri uri;
+    const int revision;
+    Link defLink;
+};
+
+ClangdFindLocalReferences::ClangdFindLocalReferences(
+        ClangdClient *client, TextDocument *document, const QTextCursor &cursor,
+        const RenameCallback &callback)
+    : QObject(client), d(new Private(this, document, cursor, callback))
+{
+    d->findDefinition();
+}
+
+ClangdFindLocalReferences::~ClangdFindLocalReferences()
+{
+    delete d;
+}
+
+void ClangdFindLocalReferences::Private::findDefinition()
+{
+    const auto linkHandler = [sentinel = QPointer(q), this](const Link &l) {
+        if (sentinel)
+            getDefinitionAst(l);
+    };
+    client()->symbolSupport().findLinkAt(document, cursor, linkHandler, true);
+}
+
+void ClangdFindLocalReferences::Private::getDefinitionAst(const Link &link)
+{
+    qCDebug(clangdLog) << "received go to definition response" << link.targetFilePath
+                       << link.targetLine << (link.targetColumn + 1);
+
+    if (!link.hasValidTarget() || !document) {
+        finish();
+        return;
+    }
+
+    defLink = link;
+    qCDebug(clangdLog) << "sending ast request for link";
+    const auto astHandler = [sentinel = QPointer(q), this]
+            (const ClangdAstNode &ast, const MessageId &) {
+        if (sentinel)
+            checkDefinitionAst(ast);
+    };
+    client()->getAndHandleAst(document, astHandler, ClangdClient::AstCallbackMode::SyncIfPossible,
+                              {});
+}
+
+void ClangdFindLocalReferences::Private::checkDefinitionAst(const ClangdAstNode &ast)
+{
+    qCDebug(clangdLog) << "received ast response";
+    if (!ast.isValid() || !document) {
+        finish();
+        return;
+    }
+
+    const Position linkPos(defLink.targetLine - 1, defLink.targetColumn);
+    const ClangdAstPath astPath = getAstPath(ast, linkPos);
+    bool isVar = false;
+    for (auto it = astPath.rbegin(); it != astPath.rend(); ++it) {
+        if (it->role() == "declaration"
+                && (it->kind() == "Function" || it->kind() == "CXXMethod"
+                    || it->kind() == "CXXConstructor" || it->kind() == "CXXDestructor"
+                    || it->kind() == "Lambda")) {
+            if (!isVar)
+                break;
+
+            qCDebug(clangdLog) << "finding references for local var";
+            const auto refsHandler = [sentinel = QPointer(q), this](const QList<Location> &refs) {
+                if (sentinel)
+                    handleReferences(refs);
+            };
+            client()->symbolSupport().findUsages(document, cursor, refsHandler);
+            return;
+        }
+        if (!isVar && it->role() == "declaration"
+                && (it->kind() == "Var" || it->kind() == "ParmVar")) {
+            isVar = true;
+        }
+    }
+    finish();
+}
+
+void ClangdFindLocalReferences::Private::handleReferences(const QList<Location> &references)
+{
+    qCDebug(clangdLog) << "found" << references.size() << "local references";
+    const Utils::Links links = Utils::transform(references, &Location::toLink);
+
+    // The callback only uses the symbol length, so we just create a dummy.
+    // Note that the calculation will be wrong for identifiers with
+    // embedded newlines, but we've never supported that.
+    QString symbol;
+    if (!references.isEmpty()) {
+        const Range r = references.first().range();
+        symbol = QString(r.end().character() - r.start().character(), 'x');
+    }
+    callback(symbol, links, revision);
+    callback = {};
+    finish();
+}
+
+void ClangdFindLocalReferences::Private::finish()
+{
+    if (callback)
+        callback({}, {}, revision);
+    emit q->done();
+}
+
 } // namespace ClangCodeModel::Internal
 
 Q_DECLARE_METATYPE(ClangCodeModel::Internal::ReplacementData)

src/plugins/clangcodemodel/clangdfindreferences.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include <coreplugin/find/searchresultitem.h>
+#include <cppeditor/cursorineditor.h>
 #include <utils/optional.h>
 
 #include <QObject>
@@ -57,4 +58,21 @@ private:
     Private * const d;
 };
 
+class ClangdFindLocalReferences : public QObject
+{
+    Q_OBJECT
+public:
+    explicit ClangdFindLocalReferences(ClangdClient *client, TextEditor::TextDocument *document,
+                                       const QTextCursor &cursor,
+                                       const CppEditor::RenameCallback &callback);
+    ~ClangdFindLocalReferences();
+
+signals:
+    void done();
+
+private:
+    class Private;
+    Private * const d;
+};
+
 } // namespace ClangCodeModel::Internal