ClangCodeModel: Do the "Go to Implementation" requests recursively

clangd reports only the first level of overrides.

Change-Id: I4aef7ca548a7a06fc7461994c3b750f9372aa738
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/clangcodemodel/clangdclient.cpp

@@ -146,6 +146,11 @@ public:
         return role() == "declaration" && kind() == "CXXMethod" && arcanaContains("virtual pure");
     }
 
+    bool isPureVirtualDefinition() const
+    {
+        return role() == "declaration" && kind() == "CXXMethod" && arcanaContains("' pure");
+    }
+
     bool mightBeAmbiguousVirtualCall() const
     {
         if (!isMemberFunctionCall())
@@ -458,6 +463,8 @@ public:
             virtualFuncAssistProcessor->cancel();
         for (const MessageId &id : qAsConst(pendingSymbolInfoRequests))
             q->cancelRequest(id);
+        for (const MessageId &id : qAsConst(pendingGotoImplRequests))
+            q->cancelRequest(id);
     }
 
     void closeTempDocuments()
@@ -484,9 +491,11 @@ public:
     const Utils::ProcessLinkCallback callback;
     VirtualFunctionAssistProvider virtualFuncAssistProvider;
     QList<MessageId> pendingSymbolInfoRequests;
+    QList<MessageId> pendingGotoImplRequests;
     const bool openInSplit;
 
     Utils::Link defLink;
+    QList<Utils::Link> allLinks;
     AstNode cursorNode;
     AstNode defLinkNode;
     SymbolDataList symbolsToDisplay;
@@ -512,6 +521,7 @@ public:
     void finishSearch(const ReferencesData &refData, bool canceled);
 
     void handleGotoDefinitionResult();
+    void sendGotoImplementationRequest(const Utils::Link &link);
     void handleGotoImplementationResult(const GotoImplementationRequest::Response &response);
     void handleDocumentInfoResults();
     void closeTempDocuments();
@@ -570,6 +580,7 @@ ClangdClient::~ClangdClient()
     if (d->followSymbolData) {
         d->followSymbolData->openedFiles.clear();
         d->followSymbolData->pendingSymbolInfoRequests.clear();
+        d->followSymbolData->pendingGotoImplRequests.clear();
     }
     delete d;
 }
@@ -939,52 +950,72 @@ void ClangdClient::Private::handleGotoDefinitionResult()
     // Step 2: Get all possible overrides via "Go to Implementation".
     // Note that we have to do this for all member function calls, because
     // we cannot tell here whether the member function is virtual.
-    const TextDocumentIdentifier documentId(followSymbolData->uri);
+    followSymbolData->allLinks << followSymbolData->defLink;
-    const Position pos(followSymbolData->cursor);
+    sendGotoImplementationRequest(followSymbolData->defLink);
-    GotoImplementationRequest req(TextDocumentPositionParams(documentId, pos));
+}
-    req.setResponseCallback([this, id = followSymbolData->id](
+
+void ClangdClient::Private::sendGotoImplementationRequest(const Utils::Link &link)
+{
+    const Position position(link.targetLine - 1, link.targetColumn);
+    const TextDocumentIdentifier documentId(DocumentUri::fromFilePath(link.targetFilePath));
+    GotoImplementationRequest req(TextDocumentPositionParams(documentId, position));
+    req.setResponseCallback([this, id = followSymbolData->id, reqId = req.id()](
                             const GotoImplementationRequest::Response &response) {
+        qCDebug(clangdLog) << "received go to implementation reply";
         if (!followSymbolData || id != followSymbolData->id)
             return;
+        followSymbolData->pendingGotoImplRequests.removeOne(reqId);
         handleGotoImplementationResult(response);
     });
     q->sendContent(req);
+    followSymbolData->pendingGotoImplRequests << req.id();
+    qCDebug(clangdLog) << "sending go to implementation request" << link.targetLine;
 }
 
 void ClangdClient::Private::handleGotoImplementationResult(
         const GotoImplementationRequest::Response &response)
 {
-    if (!response.result()) {
+    if (const Utils::optional<GotoResult> &result = response.result()) {
-        followSymbolData->callback(followSymbolData->defLink);
+        QList<Utils::Link> newLinks;
+        if (const auto ploc = Utils::get_if<Location>(&*result))
+            newLinks = {ploc->toLink()};
+        if (const auto plloc = Utils::get_if<QList<Location>>(&*result))
+            newLinks = Utils::transform(*plloc, &Location::toLink);
+        for (const Utils::Link &link : qAsConst(newLinks)) {
+            if (!followSymbolData->allLinks.contains(link)) {
+                followSymbolData->allLinks << link;
+
+                // We must do this recursively, because clangd reports only the first
+                // level of overrides.
+                sendGotoImplementationRequest(link);
+            }
+        }
+    }
+
+    // We didn't find any further candidates, so jump to the original definition link.
+    if (followSymbolData->allLinks.size() == 1
+            && followSymbolData->pendingGotoImplRequests.isEmpty()) {
+        followSymbolData->callback(followSymbolData->allLinks.first());
         followSymbolData.reset();
         return;
     }
 
-    QList<Utils::Link> links;
+    // As soon as we know that there is more than one candidate, we start the code assist
-    const GotoResult result = response.result().value();
+    // procedure, to let the user know that things are happening.
-    if (const auto ploc = Utils::get_if<Location>(&result))
+    if (followSymbolData->allLinks.size() > 1 && !followSymbolData->virtualFuncAssistProcessor
-        links = {ploc->toLink()};
+            && followSymbolData->isEditorWidgetStillAlive()) {
-    if (const auto plloc = Utils::get_if<QList<Location>>(&result))
-        links = Utils::transform(*plloc, [](const Location &loc) { return loc.toLink(); });
-    if (!links.contains(followSymbolData->defLink))
-        links.prepend(followSymbolData->defLink);
-    if (links.size() == 1) {
-        followSymbolData->callback(links.first());
-        followSymbolData.reset();
-        return;
-    }
-
-    // Step 3: We found more than one possible target.
-    //         Make a symbol info request for each link to get the class names.
-    //         This is the expensive part, so we must start the code assist procedure now.
-    //         Also get the AST for the base declaration, so we can find out whether it's
-    //         pure virtual and mark it accordingly.
-    if (followSymbolData->isEditorWidgetStillAlive()) {
         followSymbolData->editorWidget->invokeTextEditorWidgetAssist(
                     TextEditor::FollowSymbol, &followSymbolData->virtualFuncAssistProvider);
     }
 
-    for (const Utils::Link &link : links) {
+    if (!followSymbolData->pendingGotoImplRequests.isEmpty())
+        return;
+
+    // Step 3: We are done looking for overrides, and we found at least one.
+    //         Make a symbol info request for each link to get the class names.
+    //         Also get the AST for the base declaration, so we can find out whether it's
+    //         pure virtual and mark it accordingly.
+    for (const Utils::Link &link : qAsConst(followSymbolData->allLinks)) {
         if (!q->documentForFilePath(link.targetFilePath)
                 && followSymbolData->openedFiles.insert(link.targetFilePath).second) {
             q->openExtraFile(link.targetFilePath);
@@ -1080,7 +1111,8 @@ void ClangdClient::VirtualFunctionAssistProcessor::finalize()
                     symbol.second, m_data->followSymbolData->openInSplit);
         QString text = symbol.first;
         if (m_data->followSymbolData->defLink == symbol.second
-                && m_data->followSymbolData->defLinkNode.isPureVirtualDeclaration()) {
+                && (m_data->followSymbolData->defLinkNode.isPureVirtualDeclaration()
+                    || m_data->followSymbolData->defLinkNode.isPureVirtualDefinition())) {
             text += " = 0";
         }
         item->setText(text);

src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp

@@ -452,12 +452,9 @@ F2TestCase::F2TestCase(CppEditorAction action,
             QEXPECT_FAIL("QTCREATORBUG-10294_cursorIsAtTheEndOfVirtualFunctionName",
                          "FIXME: clangd behaves differently with cursor at end of function name",
                          Abort);
-            QEXPECT_FAIL("noSiblings_references", "FIXME: clangd traverses only first subclass level",
+            QEXPECT_FAIL("noSiblings_references", "FIXME: check why this fails", Abort);
-                         Abort);
+            QEXPECT_FAIL("noSiblings_pointers", "FIXME: check why this fails", Abort);
-            QEXPECT_FAIL("noSiblings_pointers", "FIXME: clangd traverses only first subclass level",
+            QEXPECT_FAIL("noSiblings_noBaseExpression", "FIXME: check why this fails", Abort);
-                         Abort);
-            QEXPECT_FAIL("noSiblings_noBaseExpression",
-                         "FIXME: clangd traverses only first subclass level", Abort);
             QVERIFY(immediateProposal);
             QVERIFY(finalProposal);
             immediateVirtualSymbolResults = VirtualFunctionTestAssistProvider::itemList(
@@ -516,18 +513,16 @@ F2TestCase::F2TestCase(CppEditorAction action,
         expectedImmediate << OverrideItem(QLatin1String("collecting overrides ..."));
     }
     QCOMPARE(immediateVirtualSymbolResults, expectedImmediate);
-    if (useClangd) {
+    if (useClangd)
-        QEXPECT_FAIL("allOverrides", "FIXME: clangd traverses only first subclass level", Abort);
+        QEXPECT_FAIL("allOverrides from base declaration", "FIXME: check why this fails", Abort);
-        QEXPECT_FAIL("possibleOverrides1", "FIXME: clangd traverses only first subclass level",
-                     Abort);
-        QEXPECT_FAIL("allOverrides from base declaration",
-                     "FIXME: clangd traverses only first subclass level", Abort);
-        QEXPECT_FAIL("itemOrder", "FIXME: clangd traverses only first subclass level", Abort);
-    }
     QCOMPARE(finalVirtualSymbolResults.size(), expectedVirtualFunctionProposal.size());
     if (useClangd) {
-         QEXPECT_FAIL("possibleOverrides2", "FIXME: clangd sometimes goes to decl instead of def",
+        QEXPECT_FAIL("allOverrides", "FIXME: clangd sometimes goes to decl instead of def", Abort);
-                      Abort);
+        QEXPECT_FAIL("possibleOverrides1", "FIXME: clangd sometimes goes to decl instead of def",
+                     Abort);
+        QEXPECT_FAIL("possibleOverrides2", "FIXME: clangd sometimes goes to decl instead of def",
+                     Abort);
+        QEXPECT_FAIL("itemOrder", "FIXME: sort items", Abort);
     }
     QCOMPARE(finalVirtualSymbolResults, expectedVirtualFunctionProposal);
 }