CppEditor: Run test cases for virtual functions also with clangd

This uncovered some bugs, of which we fixed the ones that we could do
something about.

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

src/plugins/clangcodemodel/clangdclient.cpp

@@ -146,6 +146,20 @@ public:
         return role() == "declaration" && kind() == "CXXMethod" && arcanaContains("virtual pure");
     }
 
+    bool mightBeAmbiguousVirtualCall() const
+    {
+        if (!isMemberFunctionCall())
+            return false;
+        const Utils::optional<QList<AstNode>> childList = children();
+        if (!childList)
+            return true;
+        for (const AstNode &c : qAsConst(*childList)) {
+            if (c.detailIs("UncheckedDerivedToBase"))
+                return false;
+        }
+        return true;
+    }
+
     QString type() const
     {
         const Utils::optional<QString> arcanaString = arcana();
@@ -410,6 +424,8 @@ private:
 
     TextEditor::IAssistProposal *immediateProposal(const TextEditor::AssistInterface *) override;
 
+    TextEditor::IAssistProposal *immediateProposalImpl() const;
+
     ClangdClient::Private *m_data = nullptr;
 };
 
@@ -451,6 +467,15 @@ public:
         openedFiles.clear();
     }
 
+    bool isEditorWidgetStillAlive() const
+    {
+        return Utils::anyOf(EditorManager::visibleEditors(), [this](IEditor *editor) {
+            const auto textEditor = qobject_cast<TextEditor::BaseTextEditor *>(editor);
+            return textEditor && dynamic_cast<CppTools::CppEditorWidgetInterface *>(
+                    textEditor->editorWidget()) == editorWidget;
+        });
+    }
+
     ClangdClient * const q;
     const quint64 id;
     const QTextCursor cursor;
@@ -463,6 +488,7 @@ public:
 
     Utils::Link defLink;
     AstNode cursorNode;
+    AstNode defLinkNode;
     SymbolDataList symbolsToDisplay;
     std::set<Utils::FilePath> openedFiles;
     VirtualFunctionAssistProcessor *virtualFuncAssistProcessor = nullptr;
@@ -881,7 +907,7 @@ void ClangdClient::followSymbol(
                                     Range(cursor)));
     astRequest.setResponseCallback([this, id = d->followSymbolData->id](
                                    const AstRequest::Response &response) {
-        qCDebug(clangdLog) << "received ast response";
+        qCDebug(clangdLog) << "received ast response for cursor";
         if (!d->followSymbolData || d->followSymbolData->id != id)
             return;
         const auto result = response.result();
@@ -903,7 +929,7 @@ void ClangdClient::Private::handleGotoDefinitionResult()
     qCDebug(clangdLog) << "handling go to definition result";
 
     // No dis-ambiguation necessary. Call back with the link and finish.
-    if (!followSymbolData->cursorNode.isMemberFunctionCall()
+    if (!followSymbolData->cursorNode.mightBeAmbiguousVirtualCall()
             && !followSymbolData->cursorNode.isPureVirtualDeclaration()) {
         followSymbolData->callback(followSymbolData->defLink);
         followSymbolData.reset();
@@ -951,13 +977,9 @@ void ClangdClient::Private::handleGotoImplementationResult(
     // 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.
-    const bool textEditorWidgetStillAlive = Utils::anyOf(EditorManager::visibleEditors(),
+    //         Also get the AST for the base declaration, so we can find out whether it's
-                                                         [this](IEditor *editor) {
+    //         pure virtual and mark it accordingly.
-        const auto textEditor = qobject_cast<TextEditor::BaseTextEditor *>(editor);
+    if (followSymbolData->isEditorWidgetStillAlive()) {
-        return textEditor && dynamic_cast<CppTools::CppEditorWidgetInterface *>(
-                textEditor->editorWidget()) == followSymbolData->editorWidget;
-    });
-    if (textEditorWidgetStillAlive) {
         followSymbolData->editorWidget->invokeTextEditorWidgetAssist(
                     TextEditor::FollowSymbol, &followSymbolData->virtualFuncAssistProvider);
     }
@@ -989,13 +1011,35 @@ void ClangdClient::Private::handleGotoImplementationResult(
                 }
             }
             followSymbolData->pendingSymbolInfoRequests.removeOne(reqId);
-            if (followSymbolData->pendingSymbolInfoRequests.isEmpty())
+            if (followSymbolData->pendingSymbolInfoRequests.isEmpty()
+                    && followSymbolData->defLinkNode.isValid()) {
                 handleDocumentInfoResults();
+            }
         });
         followSymbolData->pendingSymbolInfoRequests << req.id();
         qCDebug(clangdLog) << "sending symbol info request";
         q->sendContent(req);
     }
+
+    const DocumentUri defLinkUri
+            = DocumentUri::fromFilePath(followSymbolData->defLink.targetFilePath);
+    const Position defLinkPos(followSymbolData->defLink.targetLine - 1,
+                              followSymbolData->defLink.targetColumn);
+    AstRequest astRequest(AstParams(TextDocumentIdentifier(defLinkUri),
+                                    Range(defLinkPos, defLinkPos)));
+    astRequest.setResponseCallback([this, id = followSymbolData->id](
+                                   const AstRequest::Response &response) {
+        qCDebug(clangdLog) << "received ast response for def link";
+        if (!followSymbolData || followSymbolData->id != id)
+            return;
+        const auto result = response.result();
+        if (result)
+            followSymbolData->defLinkNode = *result;
+        if (followSymbolData->pendingSymbolInfoRequests.isEmpty())
+            handleDocumentInfoResults();
+    });
+    qCDebug(clangdLog) << "sending ast request for def link";
+    q->sendContent(astRequest);
 }
 
 void ClangdClient::Private::handleDocumentInfoResults()
@@ -1034,12 +1078,24 @@ void ClangdClient::VirtualFunctionAssistProcessor::finalize()
     for (const SymbolData &symbol : qAsConst(m_data->followSymbolData->symbolsToDisplay)) {
         const auto item = new CppTools::VirtualFunctionProposalItem(
                     symbol.second, m_data->followSymbolData->openInSplit);
-        item->setText(symbol.first);
+        QString text = symbol.first;
+        if (m_data->followSymbolData->defLink == symbol.second
+                && m_data->followSymbolData->defLinkNode.isPureVirtualDeclaration()) {
+            text += " = 0";
+        }
+        item->setText(text);
         items << item;
     }
-    setAsyncProposalAvailable(new CppTools::VirtualFunctionProposal(
+    const auto finalProposal = new CppTools::VirtualFunctionProposal(
                 m_data->followSymbolData->cursor.position(),
-                                  items, m_data->followSymbolData->openInSplit));
+                items, m_data->followSymbolData->openInSplit);
+    if (m_data->followSymbolData->isEditorWidgetStillAlive()
+            && m_data->followSymbolData->editorWidget->inTestMode) {
+        m_data->followSymbolData->editorWidget->setProposals(immediateProposalImpl(),
+                                                             finalProposal);
+    } else {
+        setAsyncProposalAvailable(finalProposal);
+    }
 
     m_data->followSymbolData->virtualFuncAssistProcessor = nullptr;
     m_data->followSymbolData.reset();
@@ -1048,6 +1104,16 @@ void ClangdClient::VirtualFunctionAssistProcessor::finalize()
 
 TextEditor::IAssistProposal *ClangdClient::VirtualFunctionAssistProcessor::immediateProposal(
         const TextEditor::AssistInterface *)
+{
+    if (m_data->followSymbolData->isEditorWidgetStillAlive()
+            && m_data->followSymbolData->editorWidget->inTestMode) {
+        return nullptr;
+    }
+    return immediateProposalImpl();
+}
+
+TextEditor::IAssistProposal *
+ClangdClient::VirtualFunctionAssistProcessor::immediateProposalImpl() const
 {
     QTC_ASSERT(m_data && m_data->followSymbolData, return nullptr);
 

src/plugins/cppeditor/cppeditorwidget.cpp

@@ -278,6 +278,15 @@ void CppEditorWidget::finalizeInitializationAfterDuplication(TextEditorWidget *o
                 d->m_cppEditorDocument->parseContextModel().areMultipleAvailable());
 }
 
+void CppEditorWidget::setProposals(const TextEditor::IAssistProposal *immediateProposal,
+                                   const TextEditor::IAssistProposal *finalProposal)
+{
+    QTC_ASSERT(inTestMode, return);
+#ifdef WITH_TESTS
+    emit proposalsReady(immediateProposal, finalProposal);
+#endif
+}
+
 CppEditorWidget::~CppEditorWidget() = default;
 
 CppEditorDocument *CppEditorWidget::cppEditorDocument() const

src/plugins/cppeditor/cppeditorwidget.h

@@ -98,6 +98,12 @@ public:
     static const QList<QTextEdit::ExtraSelection>
     unselectLeadingWhitespace(const QList<QTextEdit::ExtraSelection> &selections);
 
+#ifdef WITH_TESTS
+signals:
+    void proposalsReady(const TextEditor::IAssistProposal *immediateProposal,
+                        const TextEditor::IAssistProposal *finalProposal);
+#endif
+
 protected:
     bool event(QEvent *e) override;
     void contextMenuEvent(QContextMenuEvent *) override;
@@ -136,6 +142,9 @@ private:
     void finalizeInitialization() override;
     void finalizeInitializationAfterDuplication(TextEditorWidget *other) override;
 
+    void setProposals(const TextEditor::IAssistProposal *immediateProposal,
+                      const TextEditor::IAssistProposal *finalProposal) override;
+
     unsigned documentRevision() const;
 
     QMenu *createRefactorMenu(QWidget *parent) const;

src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp

@@ -256,7 +256,7 @@ public:
 
     F2TestCase(CppEditorAction action,
                const QList<TestDocumentPtr> &testFiles,
-               const OverrideItemList &expectedVirtualFunctionProposal = OverrideItemList());
+               OverrideItemList expectedVirtualFunctionProposal = OverrideItemList());
 
 private:
     static TestDocumentPtr testFileWithInitialCursorMarker(const QList<TestDocumentPtr> &testFiles);
@@ -269,7 +269,7 @@ private:
 /// It can be the same document.
 F2TestCase::F2TestCase(CppEditorAction action,
                        const QList<TestDocumentPtr> &testFiles,
-                       const OverrideItemList &expectedVirtualFunctionProposal)
+                       OverrideItemList expectedVirtualFunctionProposal)
 {
     QVERIFY(succeededSoFar());
 
@@ -287,10 +287,6 @@ F2TestCase::F2TestCase(CppEditorAction action,
     const QString tag = QLatin1String(QTest::currentDataTag());
     const bool useClangd = CppTools::codeModelSettings()->useClangd();
     if (useClangd) {
-        if (curTestName == "test_FollowSymbolUnderCursor_virtualFunctionCall"
-                || curTestName == "test_FollowSymbolUnderCursor_virtualFunctionCall_multipleDocuments") {
-            QSKIP("TODO: Add test infrastructure for this");
-        }
         if (curTestName == "test_FollowSymbolUnderCursor_QObject_connect"
                 || curTestName == "test_FollowSymbolUnderCursor_QObject_oldStyleConnect") {
             QSKIP("TODO: Implement fall-back");
@@ -398,7 +394,7 @@ F2TestCase::F2TestCase(CppEditorAction action,
         if (!builtinFollowSymbol) {
             if (curTestName == "test_FollowSymbolUnderCursor_QTCREATORBUG7903")
                 QSKIP((curTestName + " is not supported by Clang FollowSymbol").toLatin1());
-
+            widget->inTestMode = true;
             widget->openLinkUnderCursor();
             break;
         }
@@ -432,8 +428,43 @@ F2TestCase::F2TestCase(CppEditorAction action,
     if (useClangd) {
         QEXPECT_FAIL("infiniteLoopLocalTypedef_QTCREATORBUG-11999",
                      "clangd bug: Go to definition does not return", Abort);
+        if (expectedVirtualFunctionProposal.size() <= 1) {
             QVERIFY(CppTools::Tests::waitForSignalOrTimeout(EditorManager::instance(),
                                                             &EditorManager::linkOpened, 10000));
+        } else {
+            QTimer t;
+            QEventLoop l;
+            t.setSingleShot(true);
+            QObject::connect(&t, &QTimer::timeout, &l, &QEventLoop::quit);
+            const IAssistProposal *immediateProposal = nullptr;
+            const IAssistProposal *finalProposal = nullptr;
+            QObject::connect(initialTestFile->m_editorWidget, &CppEditorWidget::proposalsReady,
+                             [&](const IAssistProposal *i, const IAssistProposal *f) {
+                immediateProposal = i;
+                finalProposal = f;
+                l.quit();
+            });
+            t.start(10000);
+            l.exec();
+            QEXPECT_FAIL("possibleOverrides2",
+                         "FIXME: clangd behaves differently with cursor at end of function name",
+                         Abort);
+            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",
+                         Abort);
+            QEXPECT_FAIL("noSiblings_pointers", "FIXME: clangd traverses only first subclass level",
+                         Abort);
+            QEXPECT_FAIL("noSiblings_noBaseExpression",
+                         "FIXME: clangd traverses only first subclass level", Abort);
+            QVERIFY(immediateProposal);
+            QVERIFY(finalProposal);
+            immediateVirtualSymbolResults = VirtualFunctionTestAssistProvider::itemList(
+                        immediateProposal->model());
+            finalVirtualSymbolResults = VirtualFunctionTestAssistProvider::itemList(
+                        finalProposal->model());
+        }
     } else {
         QCoreApplication::processEvents();
     }
@@ -445,8 +476,14 @@ F2TestCase::F2TestCase(CppEditorAction action,
 
     QCOMPARE(currentTextEditor->document()->filePath().toString(), targetTestFile->filePath());
     int expectedLine, expectedColumn;
+    if (useClangd && expectedVirtualFunctionProposal.size() == 1) {
+        expectedLine = expectedVirtualFunctionProposal.first().line;
+        expectedColumn = -1;
+        expectedVirtualFunctionProposal.clear();
+    } else {
         currentTextEditor->convertPosition(targetTestFile->m_targetCursorPosition,
                                            &expectedLine, &expectedColumn);
+    }
 //    qDebug() << "Expected line:" << expectedLine;
 //    qDebug() << "Expected column:" << expectedColumn;
 
@@ -456,16 +493,42 @@ F2TestCase::F2TestCase(CppEditorAction action,
     }
 
     QCOMPARE(currentTextEditor->currentLine(), expectedLine);
+    if (expectedColumn != -1)
         QCOMPARE(currentTextEditor->currentColumn(), expectedColumn);
 
 //    qDebug() << immediateVirtualSymbolResults;
 //    qDebug() << finalVirtualSymbolResults;
     OverrideItemList expectedImmediate;
     if (!expectedVirtualFunctionProposal.isEmpty()) {
-        expectedImmediate << expectedVirtualFunctionProposal.first();
+        bool offerBaseDecl = true;
-        expectedImmediate << OverrideItem(QLatin1String("...searching overrides"));
+        if (useClangd) {
+            if (tag == "allOverrides from base declaration") {
+                expectedVirtualFunctionProposal.removeFirst();
+                offerBaseDecl = false;
+            }
+        }
+        if (offerBaseDecl) {
+            OverrideItem first = expectedVirtualFunctionProposal.first();
+            if (useClangd)
+                first.text = "<base declaration>";
+            expectedImmediate << first;
+        }
+        expectedImmediate << OverrideItem(QLatin1String("collecting overrides ..."));
     }
     QCOMPARE(immediateVirtualSymbolResults, expectedImmediate);
+    if (useClangd) {
+        QEXPECT_FAIL("allOverrides", "FIXME: clangd traverses only first subclass level", 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",
+                      Abort);
+    }
     QCOMPARE(finalVirtualSymbolResults, expectedVirtualFunctionProposal);
 }
 

src/plugins/cpptools/cppeditorwidgetinterface.h

@@ -31,7 +31,10 @@
 
 #include <QString>
 
-namespace TextEditor { class IAssistProvider; }
+namespace TextEditor {
+class IAssistProposal;
+class IAssistProvider;
+}
 
 namespace CppTools {
 
@@ -46,6 +49,11 @@ public:
 
     virtual void invokeTextEditorWidgetAssist(TextEditor::AssistKind assistKind,
                                               TextEditor::IAssistProvider *provider) = 0;
+
+    virtual void setProposals(const TextEditor::IAssistProposal *immediateProposal,
+                              const TextEditor::IAssistProposal *finalProposal) = 0;
+
+    bool inTestMode = false;
 };
 
 } // namespace CppTools

src/plugins/cpptools/cppvirtualfunctionassistprovider.cpp

@@ -113,7 +113,7 @@ public:
 
         auto *hintItem = new VirtualFunctionProposalItem(Utils::Link());
         hintItem->setText(QCoreApplication::translate("VirtualFunctionsAssistProcessor",
-                                                      "...searching overrides"));
+                                                      "collecting overrides ..."));
         hintItem->setOrder(-1000);
 
         QList<AssistProposalItemInterface *> items;