feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

CppEditor: Let users check for unused functions in (sub-)projects

Browse Source 
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 <christian.stenger@qt.io>
This commit is contained in:
Christian Kandeler
2022-10-27 14:49:15 +02:00
parent 43b21595e9
commit bfecefabc0
18 changed files with 559 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

214
src/plugins/clangcodemodel/clangdfindreferences.cpp
View File

@@ -23,6 +23,7 @@
#include <utils/filepath.h>
#include <QCheckBox>
#include <QFile>
#include <QMap>
#include <QSet>
@@ -51,6 +52,27 @@ public:
QSet<Utils::FilePath> 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<SearchResult> search;
const LinkHandler callback;
QList<SearchResultItem> 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<QString> getContainingFunctionName(const ClangdAstPath &astPath,
const Range& range);
ClangdAstNode getContainingFunction(const ClangdAstPath &astPath, const Range& range);
ClangdFindReferences * const q;
QMap<DocumentUri, ReferencesFileData> fileData;
@@ -76,6 +97,7 @@ public:
QPointer<SearchResult> search;
std::optional<ReplacementData> replacementData;
QString searchTerm;
std::optional<CheckUnusedData> 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<Location> locations = response.result().value_or(nullptr);
self->d->handleFindUsagesResult(locations.isNull() ? QList<Location>()
: 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<Location>
}
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<Location>
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<SearchResultItem> 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<QString> 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<QString> 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<ClangdAstNode>());
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: