diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index e854df35c41..5efcfe98801 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/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> 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 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(editor); + return textEditor && dynamic_cast( + 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 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(), - [this](IEditor *editor) { - const auto textEditor = qobject_cast(editor); - return textEditor && dynamic_cast( - textEditor->editorWidget()) == followSymbolData->editorWidget; - }); - if (textEditorWidgetStillAlive) { + // 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); } @@ -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( - m_data->followSymbolData->cursor.position(), - items, m_data->followSymbolData->openInSplit)); + const auto finalProposal = new CppTools::VirtualFunctionProposal( + m_data->followSymbolData->cursor.position(), + 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); diff --git a/src/plugins/cppeditor/cppeditorwidget.cpp b/src/plugins/cppeditor/cppeditorwidget.cpp index 3c317ae0639..ad4138420dc 100644 --- a/src/plugins/cppeditor/cppeditorwidget.cpp +++ b/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 diff --git a/src/plugins/cppeditor/cppeditorwidget.h b/src/plugins/cppeditor/cppeditorwidget.h index 2a02c210805..7ed46c55ea3 100644 --- a/src/plugins/cppeditor/cppeditorwidget.h +++ b/src/plugins/cppeditor/cppeditorwidget.h @@ -98,6 +98,12 @@ public: static const QList unselectLeadingWhitespace(const QList &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; diff --git a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp index e6c871f4cdf..37902164169 100644 --- a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp +++ b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp @@ -256,7 +256,7 @@ public: F2TestCase(CppEditorAction action, const QList &testFiles, - const OverrideItemList &expectedVirtualFunctionProposal = OverrideItemList()); + OverrideItemList expectedVirtualFunctionProposal = OverrideItemList()); private: static TestDocumentPtr testFileWithInitialCursorMarker(const QList &testFiles); @@ -269,7 +269,7 @@ private: /// It can be the same document. F2TestCase::F2TestCase(CppEditorAction action, const QList &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); - QVERIFY(CppTools::Tests::waitForSignalOrTimeout(EditorManager::instance(), - &EditorManager::linkOpened, 10000)); + 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; - currentTextEditor->convertPosition(targetTestFile->m_targetCursorPosition, - &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); - QCOMPARE(currentTextEditor->currentColumn(), expectedColumn); + if (expectedColumn != -1) + QCOMPARE(currentTextEditor->currentColumn(), expectedColumn); // qDebug() << immediateVirtualSymbolResults; // qDebug() << finalVirtualSymbolResults; OverrideItemList expectedImmediate; if (!expectedVirtualFunctionProposal.isEmpty()) { - expectedImmediate << expectedVirtualFunctionProposal.first(); - expectedImmediate << OverrideItem(QLatin1String("...searching overrides")); + bool offerBaseDecl = true; + if (useClangd) { + if (tag == "allOverrides from base declaration") { + expectedVirtualFunctionProposal.removeFirst(); + offerBaseDecl = false; + } + } + if (offerBaseDecl) { + OverrideItem first = expectedVirtualFunctionProposal.first(); + if (useClangd) + first.text = ""; + 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); } diff --git a/src/plugins/cpptools/cppeditorwidgetinterface.h b/src/plugins/cpptools/cppeditorwidgetinterface.h index 26b8faa5d60..84939883c86 100644 --- a/src/plugins/cpptools/cppeditorwidgetinterface.h +++ b/src/plugins/cpptools/cppeditorwidgetinterface.h @@ -31,7 +31,10 @@ #include -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 diff --git a/src/plugins/cpptools/cppvirtualfunctionassistprovider.cpp b/src/plugins/cpptools/cppvirtualfunctionassistprovider.cpp index f89b6c5231d..af77a83d0a3 100644 --- a/src/plugins/cpptools/cppvirtualfunctionassistprovider.cpp +++ b/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 items;