From bfecefabc05717c29f7b4f572314fabf4d031796 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 27 Oct 2022 14:49:15 +0200 Subject: [PATCH] CppEditor: Let users check for unused functions in (sub-)projects Note that especially in C++, there can be a lot of false positives, especially in template-heavy code bases. We filter out the most notorious offenders, namely: - templates themselves - constructors and destructors - *begin() and *end() - qHash() - main() Since the code model does not know about symbol visibility, the functionality is quite useless for libraries, unless you want to check your test coverage. The procedure is rather slow, but that shouldn't matter so much, as it's something you'll only run "once in a while". Fixes: QTCREATORBUG-6772 Change-Id: If00a537b760a9b0babdda6c848133715c3240155 Reviewed-by: Christian Stenger --- src/libs/cplusplus/FindUsages.cpp | 27 ++- src/libs/cplusplus/FindUsages.h | 6 +- src/plugins/clangcodemodel/clangdclient.cpp | 6 + src/plugins/clangcodemodel/clangdclient.h | 3 + .../clangcodemodel/clangdfindreferences.cpp | 214 ++++++++++++++++-- .../clangcodemodel/clangdfindreferences.h | 11 +- .../clangmodelmanagersupport.cpp | 16 ++ .../clangcodemodel/clangmodelmanagersupport.h | 2 + .../clangcodemodel/test/clangdtests.cpp | 11 +- .../cppbuiltinmodelmanagersupport.cpp | 26 +++ .../cppeditor/cppbuiltinmodelmanagersupport.h | 2 + src/plugins/cppeditor/cppeditorplugin.cpp | 21 ++ src/plugins/cppeditor/cppfindreferences.cpp | 50 ++++ src/plugins/cppeditor/cppfindreferences.h | 4 + src/plugins/cppeditor/cppmodelmanager.cpp | 181 +++++++++++++++ src/plugins/cppeditor/cppmodelmanager.h | 10 +- .../cppeditor/cppmodelmanagersupport.h | 4 + .../cplusplus/findusages/tst_findusages.cpp | 10 +- 18 files changed, 559 insertions(+), 45 deletions(-) diff --git a/src/libs/cplusplus/FindUsages.cpp b/src/libs/cplusplus/FindUsages.cpp index 4928c4f7712..c09ec303423 100644 --- a/src/libs/cplusplus/FindUsages.cpp +++ b/src/libs/cplusplus/FindUsages.cpp @@ -118,14 +118,18 @@ void FindUsages::reportResult(unsigned tokenIndex, const QList &cand lineText = matchingLine(tk); const int len = tk.utf16chars(); - const Usage u(_doc->filePath(), lineText, - getContainingFunction(line, col), getTags(line, col, tokenIndex), - line, col - 1, len); + QString callerName; + const Function * const caller = getContainingFunction(line, col); + if (caller) + callerName = Overview().prettyName(caller->name()); + Usage u(_doc->filePath(), lineText, callerName, getTags(line, col, tokenIndex), + line, col - 1, len); + u.containingFunctionSymbol = caller; _usages.append(u); _references.append(tokenIndex); } -QString FindUsages::getContainingFunction(int line, int column) +Function *FindUsages::getContainingFunction(int line, int column) { const QList astPath = ASTPath(_doc)(line, column); bool hasBlock = false; @@ -135,9 +139,7 @@ QString FindUsages::getContainingFunction(int line, int column) if (const auto func = (*it)->asFunctionDefinition()) { if (!hasBlock) return {}; - if (!func->symbol) - return {}; - return Overview().prettyName(func->symbol->name()); + return func->symbol; } } return {}; @@ -222,7 +224,7 @@ public: // We don't want to classify constructors and destructors as declarations // when listing class usages. if (m_findUsages->_declSymbol->asClass()) - return {}; + return Usage::Tag::ConstructorDestructor; continue; } if (const auto declarator = (*it)->asDeclarator()) { @@ -257,6 +259,10 @@ public: } } } + if (const auto declId = declarator->core_declarator->asDeclaratorId()) { + if (declId->name && declId->name->asOperatorFunctionId()) + tags |= Usage::Tag::Operator; + } } if (const auto decl = (*(it + 1))->asSimpleDeclaration()) { if (tags.toInt() && decl->qt_invokable_token) @@ -268,6 +274,11 @@ public: declarator->initializer, it + 1), it + 1); } + Class *clazz = decl->symbols->value->enclosingClass(); + if (clazz && clazz->name() + && decl->symbols->value->name()->match(clazz->name())) { + return tags |= Usage::Tag::ConstructorDestructor; + } if (const auto func = decl->symbols->value->type()->asFunctionType()) { if (func->isSignal() || func->isSlot() || func->isInvokable()) return tags |= Usage::Tag::MocInvokable; diff --git a/src/libs/cplusplus/FindUsages.h b/src/libs/cplusplus/FindUsages.h index 7430eeb0f1d..fd3681f4665 100644 --- a/src/libs/cplusplus/FindUsages.h +++ b/src/libs/cplusplus/FindUsages.h @@ -25,6 +25,9 @@ public: Override = 1 << 4, MocInvokable = 1 << 5, Template = 1 << 6, + ConstructorDestructor = 1 << 7, + Operator = 1 << 8, + Used = 1 << 9, }; using Tags = QFlags; @@ -37,6 +40,7 @@ public: Utils::FilePath path; QString lineText; QString containingFunction; + const Function *containingFunctionSymbol = nullptr; Tags tags; int line = 0; int col = 0; @@ -65,7 +69,7 @@ protected: void reportResult(unsigned tokenIndex, const Name *name, Scope *scope = nullptr); void reportResult(unsigned tokenIndex, const QList &candidates); Usage::Tags getTags(int line, int column, int tokenIndex); - QString getContainingFunction(int line, int column); + Function *getContainingFunction(int line, int column); bool checkCandidates(const QList &candidates) const; void checkExpression(unsigned startToken, unsigned endToken, Scope *scope = nullptr); diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index 2e61fa1819f..f9118693847 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/src/plugins/clangcodemodel/clangdclient.cpp @@ -519,6 +519,12 @@ void ClangdClient::findUsages(TextDocument *document, const QTextCursor &cursor, requestSymbolInfo(document->filePath(), Range(adjustedCursor).start(), symbolInfoHandler); } +void ClangdClient::checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback) +{ + new ClangdFindReferences(this, link, search, callback); +} + void ClangdClient::handleDiagnostics(const PublishDiagnosticsParams ¶ms) { const DocumentUri &uri = params.uri(); diff --git a/src/plugins/clangcodemodel/clangdclient.h b/src/plugins/clangcodemodel/clangdclient.h index c134bd83e67..c8fd0fe43cf 100644 --- a/src/plugins/clangcodemodel/clangdclient.h +++ b/src/plugins/clangcodemodel/clangdclient.h @@ -14,6 +14,7 @@ #include +namespace Core { class SearchResult; } namespace CppEditor { class CppEditorWidget; } namespace LanguageServerProtocol { class Range; } namespace ProjectExplorer { @@ -52,6 +53,8 @@ public: void findUsages(TextEditor::TextDocument *document, const QTextCursor &cursor, const std::optional &replacement); + void checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback); void followSymbol(TextEditor::TextDocument *document, const QTextCursor &cursor, CppEditor::CppEditorWidget *editorWidget, diff --git a/src/plugins/clangcodemodel/clangdfindreferences.cpp b/src/plugins/clangcodemodel/clangdfindreferences.cpp index 00c9f813d43..1d9e8e2047b 100644 --- a/src/plugins/clangcodemodel/clangdfindreferences.cpp +++ b/src/plugins/clangcodemodel/clangdfindreferences.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -51,6 +52,27 @@ public: QSet fileRenameCandidates; }; +class ClangdFindReferences::CheckUnusedData +{ +public: + CheckUnusedData(ClangdFindReferences *q, const Link &link, SearchResult *search, + const LinkHandler &callback) + : q(q), link(link), linkAsPosition(link.targetLine, link.targetColumn), search(search), + callback(callback) {} + ~CheckUnusedData(); + + ClangdFindReferences * const q; + const Link link; + const Position linkAsPosition; + const QPointer search; + const LinkHandler callback; + QList declDefItems; + bool openedExtraFileForLink = false; + bool declHasUsedTag = false; + bool recursiveCallDetected = false; + bool serverRestarted = false; +}; + class ClangdFindReferences::Private { public: @@ -67,8 +89,7 @@ public: void finishSearch(); void reportAllSearchResultsAndFinish(); void addSearchResultsForFile(const FilePath &file, const ReferencesFileData &fileData); - std::optional getContainingFunctionName(const ClangdAstPath &astPath, - const Range& range); + ClangdAstNode getContainingFunction(const ClangdAstPath &astPath, const Range& range); ClangdFindReferences * const q; QMap fileData; @@ -76,6 +97,7 @@ public: QPointer search; std::optional replacementData; QString searchTerm; + std::optional checkUnusedData; bool canceled = false; bool categorize = false; }; @@ -148,6 +170,60 @@ ClangdFindReferences::ClangdFindReferences(ClangdClient *client, TextDocument *d }); } +ClangdFindReferences::ClangdFindReferences(ClangdClient *client, const Link &link, + SearchResult *search, const LinkHandler &callback) + : QObject(client), d(new Private(this)) +{ + d->checkUnusedData.emplace(this, link, search, callback); + d->categorize = true; + d->search = search; + + if (!client->documentForFilePath(link.targetFilePath)) { + QFile f(link.targetFilePath.toString()); + if (!f.open(QIODevice::ReadOnly)) { + d->finishSearch(); + return; + } + const QString contents = QString::fromUtf8(f.readAll()); + QTextDocument doc(contents); + QTextCursor cursor(&doc); + cursor.setPosition(Text::positionInText(&doc, link.targetLine, link.targetColumn + 1)); + cursor.select(QTextCursor::WordUnderCursor); + d->searchTerm = cursor.selectedText(); + client->openExtraFile(link.targetFilePath, contents); + d->checkUnusedData->openedExtraFileForLink = true; + } + const TextDocumentIdentifier documentId(DocumentUri::fromFilePath(link.targetFilePath)); + const Position pos(link.targetLine - 1, link.targetColumn); + ReferenceParams params(TextDocumentPositionParams(documentId, pos)); + params.setContext(ReferenceParams::ReferenceContext(true)); + FindReferencesRequest request(params); + request.setResponseCallback([self = QPointer(this)] + (const FindReferencesRequest::Response &response) { + if (self) { + const LanguageClientArray locations = response.result().value_or(nullptr); + self->d->handleFindUsagesResult(locations.isNull() ? QList() + : locations.toList()); + } + }); + + client->sendMessage(request, ClangdClient::SendDocUpdates::Ignore); + QObject::connect(d->search, &SearchResult::canceled, this, [this, client, id = request.id()] { + client->cancelRequest(id); + d->canceled = true; + d->finishSearch(); + }); + QObject::connect(d->search, &SearchResult::destroyed, this, [this, client, id = request.id()] { + client->cancelRequest(id); + d->canceled = true; + d->finishSearch(); + }); + connect(client, &ClangdClient::initialized, this, [this] { + d->checkUnusedData->serverRestarted = true; + d->finishSearch(); + }); +} + ClangdFindReferences::~ClangdFindReferences() { delete d; @@ -227,8 +303,12 @@ void ClangdFindReferences::Private::handleFindUsagesResult(const QList } for (auto it = fileData.begin(); it != fileData.end(); ++it) { - const TextDocument * const doc = client()->documentForFilePath(it.key().toFilePath()); - if (!doc) + const FilePath filePath = it.key().toFilePath(); + const TextDocument * const doc = client()->documentForFilePath(filePath); + const bool openExtraFile = !doc && (!checkUnusedData + || !checkUnusedData->openedExtraFileForLink + || checkUnusedData->link.targetFilePath != filePath); + if (openExtraFile) client()->openExtraFile(it.key().toFilePath(), it->fileContent); it->fileContent.clear(); const auto docVariant = doc ? ClangdClient::TextDocOrFile(doc) @@ -246,21 +326,26 @@ void ClangdFindReferences::Private::handleFindUsagesResult(const QList qCDebug(clangdLog) << pendingAstRequests.size() << "AST requests still pending"; addSearchResultsForFile(loc.toFilePath(), data); fileData.remove(loc); - if (pendingAstRequests.isEmpty()) { - qDebug(clangdLog) << "retrieved all ASTs"; + if (pendingAstRequests.isEmpty() && !canceled) { + qCDebug(clangdLog) << "retrieved all ASTs"; finishSearch(); } }; const MessageId reqId = client()->getAndHandleAst( docVariant, astHandler, ClangdClient::AstCallbackMode::AlwaysAsync, {}); pendingAstRequests << reqId; - if (!doc) + if (openExtraFile) client()->closeExtraFile(it.key().toFilePath()); } } void ClangdFindReferences::Private::finishSearch() { + if (checkUnusedData) { + q->deleteLater(); + return; + } + if (!client()->testingEnabled() && search) { search->finishSearch(canceled); search->disconnect(q); @@ -283,24 +368,59 @@ void ClangdFindReferences::Private::finishSearch() void ClangdFindReferences::Private::reportAllSearchResultsAndFinish() { - for (auto it = fileData.begin(); it != fileData.end(); ++it) - addSearchResultsForFile(it.key().toFilePath(), it.value()); + if (!checkUnusedData) { + for (auto it = fileData.begin(); it != fileData.end(); ++it) + addSearchResultsForFile(it.key().toFilePath(), it.value()); + } finishSearch(); } -static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &searchTerm); +static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &searchTerm, + const QStringList &expectedDeclTypes); void ClangdFindReferences::Private::addSearchResultsForFile(const FilePath &file, const ReferencesFileData &fileData) { QList items; qCDebug(clangdLog) << file << "has valid AST:" << fileData.ast.isValid(); + const auto expectedDeclTypes = [this]() -> QStringList { + if (checkUnusedData) + return {"Function", "CXXMethod"}; + return {}; + }(); for (const auto &rangeWithText : fileData.rangesAndLineText) { const Range &range = rangeWithText.first; const ClangdAstPath astPath = getAstPath(fileData.ast, range); - const Usage::Tags usageType = fileData.ast.isValid() ? getUsageType(astPath, searchTerm) - : Usage::Tags(); - + const Usage::Tags usageType = fileData.ast.isValid() + ? getUsageType(astPath, searchTerm, expectedDeclTypes) + : Usage::Tags(); + if (checkUnusedData) { + bool isProperUsage = false; + if (usageType.testFlag(Usage::Tag::Declaration)) { + checkUnusedData->declHasUsedTag = checkUnusedData->declHasUsedTag + || usageType.testFlag(Usage::Tag::Used); + isProperUsage = usageType.testAnyFlags({ + Usage::Tag::Override, Usage::Tag::MocInvokable, + Usage::Tag::ConstructorDestructor, Usage::Tag::Template, + Usage::Tag::Operator}); + } else { + bool isRecursiveCall = false; + if (checkUnusedData->link.targetFilePath == file) { + const ClangdAstNode containingFunction = getContainingFunction(astPath, range); + isRecursiveCall = containingFunction.hasRange() + && containingFunction.range().contains(checkUnusedData->linkAsPosition); + } + checkUnusedData->recursiveCallDetected = checkUnusedData->recursiveCallDetected + || isRecursiveCall; + isProperUsage = !isRecursiveCall; + } + if (isProperUsage) { + qCDebug(clangdLog) << "proper usage at" << rangeWithText.second; + canceled = true; + finishSearch(); + return; + } + } SearchResultItem item; item.setUserData(usageType.toInt()); item.setStyle(CppEditor::colorStyleForUsageType(usageType)); @@ -308,7 +428,18 @@ void ClangdFindReferences::Private::addSearchResultsForFile(const FilePath &file item.setMainRange(SymbolSupport::convertRange(range)); item.setUseTextEditorFont(true); item.setLineText(rangeWithText.second); - item.setContainingFunctionName(getContainingFunctionName(astPath, range)); + if (checkUnusedData) { + if (rangeWithText.second.contains("template<>")) { + // Hack: Function specializations are not detectable in the AST. + canceled = true; + finishSearch(); + return; + } + qCDebug(clangdLog) << "collecting decl/def" << rangeWithText.second; + checkUnusedData->declDefItems << item; + continue; + } + item.setContainingFunctionName(getContainingFunction(astPath, range).detail()); if (search->supportsReplace()) { const bool fileInSession = SessionManager::projectForFile(file); @@ -322,12 +453,12 @@ void ClangdFindReferences::Private::addSearchResultsForFile(const FilePath &file } if (client()->testingEnabled()) emit q->foundReferences(items); - else + else if (!checkUnusedData) search->addResults(items, SearchResult::AddOrdered); } -std::optional ClangdFindReferences::Private::getContainingFunctionName( - const ClangdAstPath &astPath, const Range& range) +ClangdAstNode ClangdFindReferences::Private::getContainingFunction( + const ClangdAstPath &astPath, const Range& range) { const ClangdAstNode* containingFuncNode{nullptr}; const ClangdAstNode* lastCompoundStmtNode{nullptr}; @@ -346,12 +477,13 @@ std::optional ClangdFindReferences::Private::getContainingFunctionName( } if (!containingFuncNode || !containingFuncNode->isValid()) - return std::nullopt; + return {}; - return containingFuncNode->detail(); + return *containingFuncNode; } -static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &searchTerm) +static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &searchTerm, + const QStringList &expectedDeclTypes) { bool potentialWrite = false; bool isFunction = false; @@ -359,6 +491,12 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search QString invokedConstructor; if (path.last().role() == "expression" && path.last().kind() == "CXXConstruct") invokedConstructor = path.last().detail().value_or(QString()); + + // Sometimes (TM), it can happen that none of the AST nodes have a range, + // so path construction fails. + if (path.last().kind() == "TranslationUnit") + return Usage::Tag::Used; + const auto isPotentialWrite = [&] { return potentialWrite && !isFunction; }; const auto isSomeSortOfTemplate = [&](auto declPathIt) { if (declPathIt->kind() == "Function") { @@ -407,8 +545,10 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search if (pathIt->role() == "declaration") { if (symbolIsDataType) return {}; - if (!invokedConstructor.isEmpty() && invokedConstructor == searchTerm) + if (!expectedDeclTypes.isEmpty() && !expectedDeclTypes.contains(pathIt->kind())) return {}; + if (!invokedConstructor.isEmpty() && invokedConstructor == searchTerm) + return Usage::Tag::ConstructorDestructor; if (pathIt->arcanaContains("cinit")) { if (pathIt == path.rbegin()) return {Usage::Tag::Declaration, Usage::Tag::Write}; @@ -421,6 +561,10 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search return Usage::Tag::Read; } Usage::Tags tags = Usage::Tag::Declaration; + if (pathIt->arcanaContains(" used ") || pathIt->arcanaContains(" referenced ")) + tags |= Usage::Tag::Used; + if (pathIt->kind() == "CXXConstructor" || pathIt->kind() == "CXXDestructor") + tags |= Usage::Tag::ConstructorDestructor; const auto children = pathIt->children().value_or(QList()); for (const ClangdAstNode &child : children) { if (child.role() == "attribute") { @@ -432,6 +576,15 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search } if (isSomeSortOfTemplate(pathIt)) tags |= Usage::Tag::Template; + if (pathIt->kind() == "Function" || pathIt->kind() == "CXXMethod") { + const QString detail = pathIt->detail().value_or(QString()); + static const QString opString(QLatin1String("operator")); + if (detail.size() > opString.size() && detail.startsWith(opString) + && !detail.at(opString.size()).isLetterOrNumber() + && detail.at(opString.size()) != '_') { + tags |= Usage::Tag::Operator; + } + } return tags; } if (pathIt->kind() == "MemberInitializer") @@ -447,8 +600,10 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search const bool isBinaryOp = pathIt->kind() == "BinaryOperator"; const bool isOpCall = pathIt->kind() == "CXXOperatorCall"; if (isBinaryOp || isOpCall) { - if (isOpCall && symbolIsDataType) // Constructor invocation. - return {}; + if (isOpCall && symbolIsDataType) { // Constructor invocation. + if (searchTerm == invokedConstructor) + return Usage::Tag::ConstructorDestructor; + return {};} const QString op = pathIt->operatorString(); if (op.endsWith("=") && op != "==") { // Assignment. @@ -473,6 +628,19 @@ static Usage::Tags getUsageType(const ClangdAstPath &path, const QString &search return {}; } +ClangdFindReferences::CheckUnusedData::~CheckUnusedData() +{ + if (!serverRestarted) { + if (openedExtraFileForLink && q->d->client() && q->d->client()->reachable() + && !q->d->client()->documentForFilePath(link.targetFilePath)) { + q->d->client()->closeExtraFile(link.targetFilePath); + } + if (!q->d->canceled && (!declHasUsedTag || recursiveCallDetected) && QTC_GUARD(search)) + search->addResults(declDefItems, SearchResult::AddOrdered); + } + callback(link); +} + class ClangdFindLocalReferences::Private { public: diff --git a/src/plugins/clangcodemodel/clangdfindreferences.h b/src/plugins/clangcodemodel/clangdfindreferences.h index f45a61b25e4..7083a64c631 100644 --- a/src/plugins/clangcodemodel/clangdfindreferences.h +++ b/src/plugins/clangcodemodel/clangdfindreferences.h @@ -5,6 +5,7 @@ #include #include +#include #include @@ -14,6 +15,7 @@ QT_BEGIN_NAMESPACE class QTextCursor; QT_END_NAMESPACE +namespace Core { class SearchResult; } namespace TextEditor { class TextDocument; } namespace ClangCodeModel::Internal { @@ -23,9 +25,11 @@ class ClangdFindReferences : public QObject { Q_OBJECT public: - explicit ClangdFindReferences(ClangdClient *client, TextEditor::TextDocument *document, - const QTextCursor &cursor, const QString &searchTerm, - const std::optional &replacement, bool categorize); + ClangdFindReferences(ClangdClient *client, TextEditor::TextDocument *document, + const QTextCursor &cursor, const QString &searchTerm, + const std::optional &replacement, bool categorize); + ClangdFindReferences(ClangdClient *client, const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback); ~ClangdFindReferences(); signals: @@ -34,6 +38,7 @@ signals: private: class Private; + class CheckUnusedData; Private * const d; }; diff --git a/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp b/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp index 679e41004bf..3454984714e 100644 --- a/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp +++ b/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp @@ -352,6 +352,22 @@ void ClangModelManagerSupport::switchHeaderSource(const Utils::FilePath &filePat CppModelManager::switchHeaderSource(inNextSplit, CppModelManager::Backend::Builtin); } +void ClangModelManagerSupport::checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback) +{ + if (const ProjectExplorer::Project * const project + = ProjectExplorer::SessionManager::projectForFile(link.targetFilePath)) { + if (ClangdClient * const client = clientWithProject(project); + client && client->isFullyIndexed()) { + client->checkUnused(link, search, callback); + return; + } + } + + CppModelManager::instance()->modelManagerSupport( + CppModelManager::Backend::Builtin)->checkUnused(link, search, callback); +} + bool ClangModelManagerSupport::usesClangd(const TextEditor::TextDocument *document) const { return clientForFile(document->filePath()); diff --git a/src/plugins/clangcodemodel/clangmodelmanagersupport.h b/src/plugins/clangcodemodel/clangmodelmanagersupport.h index 3968e9bebab..caf05969f9d 100644 --- a/src/plugins/clangcodemodel/clangmodelmanagersupport.h +++ b/src/plugins/clangcodemodel/clangmodelmanagersupport.h @@ -63,6 +63,8 @@ private: void globalRename(const CppEditor::CursorInEditor &cursor, const QString &replacement) override; void findUsages(const CppEditor::CursorInEditor &cursor) const override; void switchHeaderSource(const Utils::FilePath &filePath, bool inNextSplit) override; + void checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback) override; void onEditorOpened(Core::IEditor *editor); void onCurrentEditorChanged(Core::IEditor *newCurrent); diff --git a/src/plugins/clangcodemodel/test/clangdtests.cpp b/src/plugins/clangcodemodel/test/clangdtests.cpp index 8e3ec84c57c..746a4039f65 100644 --- a/src/plugins/clangcodemodel/test/clangdtests.cpp +++ b/src/plugins/clangcodemodel/test/clangdtests.cpp @@ -251,7 +251,8 @@ void ClangdTestFindReferences::test_data() // Some of these are conceptually questionable, as S is a type and thus we cannot "read from" // or "write to" it. But it probably matches the intuitive user expectation. QTest::newRow("struct type") << "defs.h" << 7 << ItemList{ - makeItem(1, 7, Usage::Tag::Declaration), makeItem(2, 4, Usage::Tag::Declaration), + makeItem(1, 7, Usage::Tag::Declaration), + makeItem(2, 4, (Usage::Tags{Usage::Tag::Declaration, Usage::Tag::ConstructorDestructor})), makeItem(20, 19, Usage::Tags()), makeItem(10, 9, Usage::Tag::WritableRef), makeItem(12, 4, Usage::Tag::Write), makeItem(44, 12, Usage::Tag::Read), makeItem(45, 13, Usage::Tag::Read), makeItem(47, 12, Usage::Tag::Write), @@ -266,7 +267,8 @@ void ClangdTestFindReferences::test_data() makeItem(13, 21, Usage::Tags()), makeItem(32, 8, Usage::Tags())}; QTest::newRow("constructor") << "defs.h" << 627 << ItemList{ - makeItem(31, 4, Usage::Tag::Declaration), makeItem(36, 7, Usage::Tags())}; + makeItem(31, 4, (Usage::Tags{Usage::Tag::Declaration, Usage::Tag::ConstructorDestructor})), + makeItem(36, 7, Usage::Tag::ConstructorDestructor)}; QTest::newRow("subclass") << "defs.h" << 450 << ItemList{ makeItem(20, 7, Usage::Tag::Declaration), makeItem(5, 4, Usage::Tags()), @@ -303,7 +305,10 @@ void ClangdTestFindReferences::test() const SearchResultItem &curExpected = expectedResults.at(i); QCOMPARE(curActual.mainRange().begin.line, curExpected.mainRange().begin.line); QCOMPARE(curActual.mainRange().begin.column, curExpected.mainRange().begin.column); - QCOMPARE(curActual.userData(), curExpected.userData()); + const auto actualTags = Usage::Tags::fromInt(curActual.userData().toInt()) + & ~Usage::Tags(Usage::Tag::Used); + const auto expectedTags = Usage::Tags::fromInt(curExpected.userData().toInt()); + QCOMPARE(actualTags, expectedTags); } } diff --git a/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.cpp b/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.cpp index f46d7ea03af..9d8940b6ed7 100644 --- a/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.cpp +++ b/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.cpp @@ -18,8 +18,11 @@ #include #include #include +#include #include +#include +#include using namespace Core; using namespace TextEditor; @@ -193,4 +196,27 @@ void BuiltinModelManagerSupport::switchHeaderSource(const Utils::FilePath &fileP openEditor(otherFile, inNextSplit); } +void BuiltinModelManagerSupport::checkUnused(const Utils::Link &link, SearchResult *search, + const Utils::LinkHandler &callback) +{ + CPlusPlus::Snapshot snapshot = CppModelManager::instance()->snapshot(); + QFile file(link.targetFilePath.toString()); + if (!file.open(QIODevice::ReadOnly)) + return callback(link); + const QByteArray &contents = file.readAll(); + CPlusPlus::Document::Ptr cppDoc = snapshot.preprocessedDocument(contents, link.targetFilePath); + if (!cppDoc->parse()) + return callback(link); + cppDoc->check(); + snapshot.insert(cppDoc); + QTextDocument doc(QString::fromUtf8(contents)); + QTextCursor cursor(&doc); + cursor.setPosition(Utils::Text::positionInText(&doc, link.targetLine, link.targetColumn + 1)); + Internal::CanonicalSymbol cs(cppDoc, snapshot); + CPlusPlus::Symbol *canonicalSymbol = cs(cursor); + if (!canonicalSymbol || !canonicalSymbol->identifier()) + return callback(link); + CppModelManager::checkForUnusedSymbol(search, link, canonicalSymbol, cs.context(), callback); +} + } // namespace CppEditor::Internal diff --git a/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.h b/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.h index 4a6d56429c5..bd420038066 100644 --- a/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.h +++ b/src/plugins/cppeditor/cppbuiltinmodelmanagersupport.h @@ -41,6 +41,8 @@ private: void globalRename(const CursorInEditor &data, const QString &replacement) override; void findUsages(const CursorInEditor &data) const override; void switchHeaderSource(const Utils::FilePath &filePath, bool inNextSplit) override; + void checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback) override; QScopedPointer m_completionAssistProvider; QScopedPointer m_followSymbol; diff --git a/src/plugins/cppeditor/cppeditorplugin.cpp b/src/plugins/cppeditor/cppeditorplugin.cpp index 867223e15c1..3851e7416e8 100644 --- a/src/plugins/cppeditor/cppeditorplugin.cpp +++ b/src/plugins/cppeditor/cppeditorplugin.cpp @@ -276,6 +276,27 @@ bool CppEditorPlugin::initialize(const QStringList & /*arguments*/, QString *err connect(showPreprocessedInSplitAction, &QAction::triggered, this, [] { CppModelManager::showPreprocessedFile(true); }); + QAction * const findUnusedFunctionsAction = new QAction(tr("Find Unused Functions"), this); + command = ActionManager::registerAction(findUnusedFunctionsAction, + "CppTools.FindUnusedFunctions"); + mcpptools->addAction(command); + connect(findUnusedFunctionsAction, &QAction::triggered, + this, [] { CppModelManager::findUnusedFunctions({}); }); + QAction * const findUnusedFunctionsInSubProjectAction + = new QAction(tr("Find Unused C/C++ Functions"), this); + command = ActionManager::registerAction(findUnusedFunctionsInSubProjectAction, + "CppTools.FindUnusedFunctionsInSubProject"); + for (ActionContainer * const projectContextMenu : { + ActionManager::actionContainer(ProjectExplorer::Constants::M_SUBPROJECTCONTEXT), + ActionManager::actionContainer(ProjectExplorer::Constants::M_PROJECTCONTEXT)}) { + projectContextMenu->addSeparator(ProjectExplorer::Constants::G_PROJECT_TREE); + projectContextMenu->addAction(command, ProjectExplorer::Constants::G_PROJECT_TREE); + } + connect(findUnusedFunctionsInSubProjectAction, &QAction::triggered, this, [] { + if (const Node * const node = ProjectTree::currentNode(); node && node->asFolderNode()) + CppModelManager::findUnusedFunctions(node->directory()); + }); + MacroExpander *expander = globalMacroExpander(); expander->registerVariable("Cpp:LicenseTemplate", tr("The license template."), diff --git a/src/plugins/cppeditor/cppfindreferences.cpp b/src/plugins/cppeditor/cppfindreferences.cpp index dc9cb0fd645..0f0db5b66df 100644 --- a/src/plugins/cppeditor/cppfindreferences.cpp +++ b/src/plugins/cppeditor/cppfindreferences.cpp @@ -784,6 +784,56 @@ void CppFindReferences::renameMacroUses(const CPlusPlus::Macro ¯o, const QSt findMacroUses(macro, textToReplace, true); } +void CppFindReferences::checkUnused(Core::SearchResult *search, const Link &link, + CPlusPlus::Symbol *symbol, + const CPlusPlus::LookupContext &context, + const LinkHandler &callback) +{ + const auto isProperUsage = [symbol](const CPlusPlus::Usage &usage) { + if (!usage.tags.testFlag(CPlusPlus::Usage::Tag::Declaration)) + return usage.containingFunctionSymbol != symbol; + return usage.tags.testAnyFlags({CPlusPlus::Usage::Tag::Override, + CPlusPlus::Usage::Tag::MocInvokable, + CPlusPlus::Usage::Tag::Template, + CPlusPlus::Usage::Tag::Operator, + CPlusPlus::Usage::Tag::ConstructorDestructor}); + }; + const auto watcher = new QFutureWatcher(); + connect(watcher, &QFutureWatcherBase::finished, watcher, + [watcher, link, callback, search, isProperUsage] { + watcher->deleteLater(); + if (watcher->isCanceled()) + return callback(link); + for (int i = 0; i < watcher->future().resultCount(); ++i) { + if (isProperUsage(watcher->resultAt(i))) + return callback(link); + } + for (int i = 0; i < watcher->future().resultCount(); ++i) { + const CPlusPlus::Usage usage = watcher->resultAt(i); + SearchResultItem item; + item.setFilePath(usage.path); + item.setLineText(usage.lineText); + item.setMainRange(usage.line, usage.col, usage.len); + item.setUseTextEditorFont(true); + search->addResult(item); + } + callback(link); + }); + connect(watcher, &QFutureWatcherBase::resultsReadyAt, search, + [watcher, isProperUsage](int first, int end) { + for (int i = first; i < end; ++i) { + if (isProperUsage(watcher->resultAt(i))) { + watcher->cancel(); + break; + } + } + }); + connect(search, &SearchResult::canceled, watcher, [watcher] { watcher->cancel(); }); + connect(search, &SearchResult::destroyed, watcher, [watcher] { watcher->cancel(); }); + watcher->setFuture(Utils::runAsync(m_modelManager->sharedThreadPool(), find_helper, + m_modelManager->workingCopy(), context, symbol, true)); +} + void CppFindReferences::createWatcher(const QFuture &future, SearchResult *search) { auto watcher = new QFutureWatcher(); diff --git a/src/plugins/cppeditor/cppfindreferences.h b/src/plugins/cppeditor/cppfindreferences.h index 14f731ef5a9..34a5a302fd0 100644 --- a/src/plugins/cppeditor/cppfindreferences.h +++ b/src/plugins/cppeditor/cppfindreferences.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -69,6 +70,9 @@ public: void findMacroUses(const CPlusPlus::Macro ¯o); void renameMacroUses(const CPlusPlus::Macro ¯o, const QString &replacement = QString()); + void checkUnused(Core::SearchResult *search, const Utils::Link &link, CPlusPlus::Symbol *symbol, + const CPlusPlus::LookupContext &context, const Utils::LinkHandler &callback); + private: void setupSearch(Core::SearchResult *search); void onReplaceButtonClicked(Core::SearchResult *search, const QString &text, diff --git a/src/plugins/cppeditor/cppmodelmanager.cpp b/src/plugins/cppeditor/cppmodelmanager.cpp index 884e5d31755..a4e0d357b17 100644 --- a/src/plugins/cppeditor/cppmodelmanager.cpp +++ b/src/plugins/cppeditor/cppmodelmanager.cpp @@ -26,12 +26,15 @@ #include "symbolfinder.h" #include "symbolsfindfilter.h" +#include #include #include #include +#include #include #include #include +#include #include #include #include @@ -59,9 +62,11 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -72,10 +77,13 @@ #include #include #include +#include #include #include #include +#include + #if defined(QTCREATOR_WITH_DUMP_AST) && defined(Q_CC_GNU) #define WITH_AST_DUMP #include @@ -456,6 +464,179 @@ void CppModelManager::showPreprocessedFile(bool inNextSplit) compiler->start(); } +class FindUnusedActionsEnabledSwitcher +{ +public: + FindUnusedActionsEnabledSwitcher() + : actions{Core::ActionManager::command("CppTools.FindUnusedFunctions"), + Core::ActionManager::command("CppTools.FindUnusedFunctionsInSubProject")} + { + for (Core::Command * const action : actions) + action->action()->setEnabled(false); + } + ~FindUnusedActionsEnabledSwitcher() + { + for (Core::Command * const action : actions) + action->action()->setEnabled(true); + } +private: + const QList actions; +}; +using FindUnusedActionsEnabledSwitcherPtr = std::shared_ptr; + +static void checkNextFunctionForUnused( + const QPointer &search, + const std::shared_ptr> &findRefsFuture, + const FindUnusedActionsEnabledSwitcherPtr &actionsSwitcher) +{ + if (!search || findRefsFuture->isCanceled()) + return; + QVariantMap data = search->userData().toMap(); + QVariant &remainingLinks = data["remaining"]; + QVariantList remainingLinksList = remainingLinks.toList(); + QVariant &activeLinks = data["active"]; + QVariantList activeLinksList = activeLinks.toList(); + if (remainingLinksList.isEmpty()) { + if (activeLinksList.isEmpty()) { + search->finishSearch(false); + findRefsFuture->reportFinished(); + } + return; + } + const auto link = qvariant_cast(remainingLinksList.takeFirst()); + activeLinksList << QVariant::fromValue(link); + remainingLinks = remainingLinksList; + activeLinks = activeLinksList; + search->setUserData(data); + CppModelManager::instance()->modelManagerSupport(CppModelManager::Backend::Best) + ->checkUnused(link, search, [search, link, findRefsFuture, actionsSwitcher](const Link &) { + if (!search || findRefsFuture->isCanceled()) + return; + const int newProgress = findRefsFuture->progressValue() + 1; + findRefsFuture->setProgressValueAndText(newProgress, Tr::tr("Checked %1 of %2 functions") + .arg(newProgress).arg(findRefsFuture->progressMaximum())); + QVariantMap data = search->userData().toMap(); + QVariant &activeLinks = data["active"]; + QVariantList activeLinksList = activeLinks.toList(); + QTC_CHECK(activeLinksList.removeOne(QVariant::fromValue(link))); + activeLinks = activeLinksList; + search->setUserData(data); + checkNextFunctionForUnused(search, findRefsFuture, actionsSwitcher); + }); +} + +void CppModelManager::findUnusedFunctions(const FilePath &folder) +{ + const auto actionsSwitcher = std::make_shared(); + + // Step 1: Employ locator to find all functions + Core::ILocatorFilter *const functionsFilter + = Utils::findOrDefault(Core::ILocatorFilter::allLocatorFilters(), + Utils::equal(&Core::ILocatorFilter::id, + Id(Constants::FUNCTIONS_FILTER_ID))); + QTC_ASSERT(functionsFilter, return); + const QPointer search + = Core::SearchResultWindow::instance() + ->startNewSearch(tr("Find Unused Functions"), + {}, + {}, + Core::SearchResultWindow::SearchOnly, + Core::SearchResultWindow::PreserveCaseDisabled, + "CppEditor"); + connect(search, &Core::SearchResult::activated, [](const Core::SearchResultItem &item) { + Core::EditorManager::openEditorAtSearchResult(item); + }); + Core::SearchResultWindow::instance()->popup(Core::IOutputPane::ModeSwitch + | Core::IOutputPane::WithFocus); + const auto locatorWatcher = new QFutureWatcher(search); + functionsFilter->prepareSearch({}); + connect(search, &Core::SearchResult::canceled, locatorWatcher, [locatorWatcher] { + locatorWatcher->cancel(); + }); + connect(locatorWatcher, &QFutureWatcher::finished, search, + [locatorWatcher, search, folder, actionsSwitcher] { + locatorWatcher->deleteLater(); + if (locatorWatcher->isCanceled()) { + search->finishSearch(true); + return; + } + Links links; + for (int i = 0; i < locatorWatcher->future().resultCount(); ++i) { + const Core::LocatorFilterEntry &entry = locatorWatcher->resultAt(i); + static const QStringList prefixBlacklist{"main(", "~", "qHash(", "begin()", "end()", + "cbegin()", "cend()", "constBegin()", "constEnd()"}; + if (Utils::anyOf(prefixBlacklist, [&entry](const QString &prefix) { + return entry.displayName.startsWith(prefix); })) { + continue; + } + Link link; + if (entry.internalData.canConvert()) { + link = qvariant_cast(entry.internalData); + } else { + const auto item = qvariant_cast(entry.internalData); + if (item) { + link = Link(FilePath::fromString(item->fileName()), item->line(), + item->column()); + } + } + if (link.hasValidTarget() && link.targetFilePath.isReadableFile() + && (folder.isEmpty() || link.targetFilePath.isChildOf(folder)) + && SessionManager::projectForFile(link.targetFilePath)) { + links << link; + } + } + if (links.isEmpty()) { + search->finishSearch(false); + return; + } + QVariantMap remainingAndActiveLinks; + remainingAndActiveLinks.insert("active", QVariantList()); + remainingAndActiveLinks.insert("remaining", + Utils::transform(links, [](const Link &l) { return QVariant::fromValue(l); + })); + search->setUserData(remainingAndActiveLinks); + const auto findRefsFuture = std::make_shared>(); + Core::FutureProgress *const progress + = Core::ProgressManager::addTask(findRefsFuture->future(), + Tr::tr("Finding Unused Functions"), + "CppEditor.FindUnusedFunctions"); + connect(progress, + &Core::FutureProgress::canceled, + search, + [search, future = std::weak_ptr>(findRefsFuture)] { + search->finishSearch(true); + if (const auto f = future.lock()) { + f->cancel(); + f->reportFinished(); + } + }); + findRefsFuture->setProgressRange(0, links.size()); + connect(search, &Core::SearchResult::canceled, [findRefsFuture] { + findRefsFuture->cancel(); + findRefsFuture->reportFinished(); + }); + + // Step 2: Forward search results one by one to backend to check which functions are unused. + // We keep several requests in flight for decent throughput. + const int inFlightCount = std::min(QThread::idealThreadCount() / 2 + 1, int(links.size())); + for (int i = 0; i < inFlightCount; ++i) + checkNextFunctionForUnused(search, findRefsFuture, actionsSwitcher); + }); + locatorWatcher->setFuture( + Utils::runAsync([functionsFilter](QFutureInterface &future) { + future.reportResults(functionsFilter->matchesFor(future, {})); + })); +} + +void CppModelManager::checkForUnusedSymbol(Core::SearchResult *search, + const Link &link, + CPlusPlus::Symbol *symbol, + const CPlusPlus::LookupContext &context, + const LinkHandler &callback) +{ + instance()->d->m_findReferences->checkUnused(search, link, symbol, context, callback); +} + int argumentPositionOf(const AST *last, const CallAST *callAst) { if (!callAst || !callAst->expression_list) diff --git a/src/plugins/cppeditor/cppmodelmanager.h b/src/plugins/cppeditor/cppmodelmanager.h index 5e5b19a3a1c..181fd91d31c 100644 --- a/src/plugins/cppeditor/cppmodelmanager.h +++ b/src/plugins/cppeditor/cppmodelmanager.h @@ -24,6 +24,7 @@ namespace Core { class IDocument; class IEditor; +class SearchResult; } namespace CPlusPlus { class AST; @@ -181,6 +182,11 @@ public: static void findUsages(const CursorInEditor &data, Backend backend = Backend::Best); static void switchHeaderSource(bool inNextSplit, Backend backend = Backend::Best); static void showPreprocessedFile(bool inNextSplit); + static void findUnusedFunctions(const Utils::FilePath &folder); + static void checkForUnusedSymbol(Core::SearchResult *search, const Utils::Link &link, + CPlusPlus::Symbol *symbol, + const CPlusPlus::LookupContext &context, + const Utils::LinkHandler &callback); static Core::ILocatorFilter *createAuxiliaryCurrentDocumentFilter(); @@ -234,6 +240,8 @@ public: // for VcsBaseSubmitEditor Q_INVOKABLE QSet symbolsInFiles(const QSet &files) const; + ModelManagerSupport *modelManagerSupport(Backend backend) const; + signals: /// Project data might be locked while this is emitted. void aboutToRemoveFiles(const QStringList &files); @@ -281,8 +289,6 @@ private: WorkingCopy buildWorkingCopyList(); - ModelManagerSupport *modelManagerSupport(Backend backend) const; - void ensureUpdated(); QStringList internalProjectFiles() const; ProjectExplorer::HeaderPaths internalHeaderPaths() const; diff --git a/src/plugins/cppeditor/cppmodelmanagersupport.h b/src/plugins/cppeditor/cppmodelmanagersupport.h index 0544cb45323..dcb67b96052 100644 --- a/src/plugins/cppeditor/cppmodelmanagersupport.h +++ b/src/plugins/cppeditor/cppmodelmanagersupport.h @@ -13,6 +13,7 @@ #include +namespace Core { class SearchResult; } namespace TextEditor { class TextDocument; class BaseHoverHandler; @@ -51,6 +52,9 @@ public: virtual void globalRename(const CursorInEditor &data, const QString &replacement) = 0; virtual void findUsages(const CursorInEditor &data) const = 0; virtual void switchHeaderSource(const Utils::FilePath &filePath, bool inNextSplit) = 0; + + virtual void checkUnused(const Utils::Link &link, Core::SearchResult *search, + const Utils::LinkHandler &callback) = 0; }; } // CppEditor namespace diff --git a/tests/auto/cplusplus/findusages/tst_findusages.cpp b/tests/auto/cplusplus/findusages/tst_findusages.cpp index 82dd5564379..1cb09810306 100644 --- a/tests/auto/cplusplus/findusages/tst_findusages.cpp +++ b/tests/auto/cplusplus/findusages/tst_findusages.cpp @@ -1180,12 +1180,12 @@ void tst_FindUsages::templateClass_className() findUsages(classTS); QCOMPARE(findUsages.usages().size(), 7); QCOMPARE(findUsages.usages().at(0).tags, Usage::Tag::Declaration); - QCOMPARE(findUsages.usages().at(1).tags, Usage::Tags()); - QCOMPARE(findUsages.usages().at(2).tags, Usage::Tags()); + QCOMPARE(findUsages.usages().at(1).tags, Usage::Tag::ConstructorDestructor); + QCOMPARE(findUsages.usages().at(2).tags, Usage::Tag::ConstructorDestructor); QCOMPARE(findUsages.usages().at(3).tags, Usage::Tags()); - QCOMPARE(findUsages.usages().at(4).tags, Usage::Tags()); + QCOMPARE(findUsages.usages().at(4).tags, Usage::Tag::ConstructorDestructor); QCOMPARE(findUsages.usages().at(5).tags, Usage::Tags()); - QCOMPARE(findUsages.usages().at(6).tags, Usage::Tags()); + QCOMPARE(findUsages.usages().at(6).tags, Usage::Tag::ConstructorDestructor); } void tst_FindUsages::templateFunctionParameters() @@ -2324,7 +2324,7 @@ int main() find(structS); QCOMPARE(find.usages().size(), 18); QCOMPARE(find.usages().at(0).tags, Usage::Tag::Declaration); - QCOMPARE(find.usages().at(1).tags, Usage::Tags()); + QCOMPARE(find.usages().at(1).tags, Usage::Tag::ConstructorDestructor); QCOMPARE(find.usages().at(2).tags, Usage::Tags()); QCOMPARE(find.usages().at(3).tags, Usage::Tags()); QCOMPARE(find.usages().at(4).tags, Usage::Tags());