From 69571427a303fea5c417206555df17572d12b001 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 9 Jan 2024 15:52:29 +0100 Subject: [PATCH] CppEditor: Fix renaming include directives after bulk renamings E.g. when renaming a class, the C++ model manager needs to know about both renamings (.h and .cpp) at the same time. This way, it is able to open the correct including file, even though the code model likely still refers to the old, now-gone file path. Fixes: QTCREATORBUG-30154 Change-Id: I583e77a4920c7cc3e642d7bebba46d734ae6bbe0 Reviewed-by: David Schulz --- src/plugins/cppeditor/cppmodelmanager.cpp | 113 ++++++++++-------- src/plugins/cppeditor/cppmodelmanager.h | 6 +- .../projectexplorer/projectexplorer.cpp | 47 +++++--- src/plugins/projectexplorer/projectexplorer.h | 11 +- src/plugins/projectexplorer/projectmodels.cpp | 11 +- 5 files changed, 115 insertions(+), 73 deletions(-) diff --git a/src/plugins/cppeditor/cppmodelmanager.cpp b/src/plugins/cppeditor/cppmodelmanager.cpp index d601bbcf266..67470745bb9 100644 --- a/src/plugins/cppeditor/cppmodelmanager.cpp +++ b/src/plugins/cppeditor/cppmodelmanager.cpp @@ -1017,7 +1017,7 @@ CppModelManager::CppModelManager() connect(EditorManager::instance(), &EditorManager::currentEditorChanged, this, &CppModelManager::onCurrentEditorChanged); - connect(DocumentManager::instance(), &DocumentManager::allDocumentsRenamed, + connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::filesRenamed, this, &CppModelManager::renameIncludes); connect(ICore::instance(), &ICore::coreAboutToClose, @@ -1860,59 +1860,70 @@ QSet CppModelManager::internalTargets(const FilePath &filePath) return targets; } -void CppModelManager::renameIncludes(const FilePath &oldFilePath, const FilePath &newFilePath) +// Note: This function assumes that neither the project tree nor the code model are aware of +// the renamings yet, i.e. they still refer to the old file paths. +void CppModelManager::renameIncludes(const QList> &oldAndNewPaths) { - if (oldFilePath.isEmpty() || newFilePath.isEmpty()) - return; - - // We just want to handle renamings so return when the file was actually moved. - if (oldFilePath.absolutePath() != newFilePath.absolutePath()) - return; - - const TextEditor::PlainRefactoringFileFactory changes; - - QString oldFileName = oldFilePath.fileName(); - QString newFileName = newFilePath.fileName(); - const bool isUiFile = oldFilePath.suffix() == "ui" && newFilePath.suffix() == "ui"; - if (isUiFile) { - oldFileName = "ui_" + oldFilePath.baseName() + ".h"; - newFileName = "ui_" + newFilePath.baseName() + ".h"; - } - static const auto getProductNode = [](const FilePath &filePath) -> const Node * { - const Node * const fileNode = ProjectTree::nodeForFile(filePath); - if (!fileNode) - return nullptr; - const ProjectNode *productNode = fileNode->parentProjectNode(); - while (productNode && !productNode->isProduct()) - productNode = productNode->parentProjectNode(); - if (!productNode) - productNode = fileNode->getProject()->rootProjectNode(); - return productNode; - }; - const Node * const productNodeForUiFile = isUiFile ? getProductNode(oldFilePath) : nullptr; - if (isUiFile && !productNodeForUiFile) - return; - - const QList locations = snapshot().includeLocationsOfDocument( - isUiFile ? FilePath::fromString(oldFileName) : oldFilePath); - for (const Snapshot::IncludeLocation &loc : locations) { - const FilePath filePath = loc.first->filePath(); - - // Larger projects can easily have more than one ui file with the same name. - // Replace only if ui file and including source file belong to the same product. - if (isUiFile && getProductNode(filePath) != productNodeForUiFile) + for (const auto &[oldFilePath, newFilePath] : oldAndNewPaths) { + if (oldFilePath.isEmpty() || newFilePath.isEmpty()) continue; - TextEditor::RefactoringFilePtr file = changes.file(filePath); - const QTextBlock &block = file->document()->findBlockByNumber(loc.second - 1); - const int replaceStart = block.text().indexOf(oldFileName); - if (replaceStart > -1) { - ChangeSet changeSet; - changeSet.replace(block.position() + replaceStart, - block.position() + replaceStart + oldFileName.length(), - newFileName); - file->setChangeSet(changeSet); - file->apply(); + // We just want to handle renamings so return when the file was actually moved. + if (oldFilePath.absolutePath() != newFilePath.absolutePath()) + continue; + + const TextEditor::PlainRefactoringFileFactory changes; + + QString oldFileName = oldFilePath.fileName(); + QString newFileName = newFilePath.fileName(); + const bool isUiFile = oldFilePath.suffix() == "ui" && newFilePath.suffix() == "ui"; + if (isUiFile) { + oldFileName = "ui_" + oldFilePath.baseName() + ".h"; + newFileName = "ui_" + newFilePath.baseName() + ".h"; + } + static const auto getProductNode = [](const FilePath &filePath) -> const Node * { + const Node * const fileNode = ProjectTree::nodeForFile(filePath); + if (!fileNode) + return nullptr; + const ProjectNode *productNode = fileNode->parentProjectNode(); + while (productNode && !productNode->isProduct()) + productNode = productNode->parentProjectNode(); + if (!productNode) + productNode = fileNode->getProject()->rootProjectNode(); + return productNode; + }; + const Node * const productNodeForUiFile = isUiFile ? getProductNode(oldFilePath) : nullptr; + if (isUiFile && !productNodeForUiFile) + continue; + + const QList locations = snapshot().includeLocationsOfDocument( + isUiFile ? FilePath::fromString(oldFileName) : oldFilePath); + for (const Snapshot::IncludeLocation &loc : locations) { + const FilePath includingFileOld = loc.first->filePath(); + FilePath includingFileNew = includingFileOld; + for (const auto &pathPair : oldAndNewPaths) { + if (includingFileNew == pathPair.first) { + includingFileNew = pathPair.second; + break; + } + } + + // Larger projects can easily have more than one ui file with the same name. + // Replace only if ui file and including source file belong to the same product. + if (isUiFile && getProductNode(includingFileOld) != productNodeForUiFile) + continue; + + TextEditor::RefactoringFilePtr file = changes.file(includingFileNew); + const QTextBlock &block = file->document()->findBlockByNumber(loc.second - 1); + const int replaceStart = block.text().indexOf(oldFileName); + if (replaceStart > -1) { + ChangeSet changeSet; + changeSet.replace(block.position() + replaceStart, + block.position() + replaceStart + oldFileName.length(), + newFileName); + file->setChangeSet(changeSet); + file->apply(); + } } } } diff --git a/src/plugins/cppeditor/cppmodelmanager.h b/src/plugins/cppeditor/cppmodelmanager.h index a229d98957a..0fe9e151ae4 100644 --- a/src/plugins/cppeditor/cppmodelmanager.h +++ b/src/plugins/cppeditor/cppmodelmanager.h @@ -22,6 +22,7 @@ #include #include +#include namespace Core { class IDocument; @@ -244,8 +245,6 @@ public: static QSet internalTargets(const Utils::FilePath &filePath); - static void renameIncludes(const Utils::FilePath &oldFilePath, const Utils::FilePath &newFilePath); - // for VcsBaseSubmitEditor Q_INVOKABLE QSet symbolsInFiles(const QSet &files) const; @@ -299,6 +298,9 @@ private: static void dumpModelManagerConfiguration(const QString &logFileId); static void initCppTools(); + + static void renameIncludes(const QList> &oldAndNewPaths); }; } // CppEditor diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp index 469d9b9b2f9..6d95819f36a 100644 --- a/src/plugins/projectexplorer/projectexplorer.cpp +++ b/src/plugins/projectexplorer/projectexplorer.cpp @@ -2466,6 +2466,18 @@ void ProjectExplorerPlugin::showOutputPaneForRunControl(RunControl *runControl) dd->showOutputPaneForRunControl(runControl); } +QList> ProjectExplorerPlugin::renameFiles( + const QList> &nodesAndNewFilePaths) +{ + QList> renamedFiles; + for (const auto &[node, newFilePath] : nodesAndNewFilePaths) { + if (const auto res = renameFile(node, newFilePath.toString())) + renamedFiles << *res; + } + emit instance()->filesRenamed(renamedFiles); + return renamedFiles; +} + void ProjectExplorerPluginPrivate::startRunControl(RunControl *runControl) { m_outputPane.createNewOutputWindow(runControl); @@ -3859,17 +3871,18 @@ void ProjectExplorerPluginPrivate::handleRenameFile() } } -void ProjectExplorerPlugin::renameFile(Node *node, const QString &newFileName) +std::optional> +ProjectExplorerPlugin::renameFile(Node *node, const QString &newFileName) { const FilePath oldFilePath = node->filePath().absoluteFilePath(); FolderNode *folderNode = node->parentFolderNode(); - QTC_ASSERT(folderNode, return); + QTC_ASSERT(folderNode, return {}); const QString projectFileName = folderNode->managingProject()->filePath().fileName(); const FilePath newFilePath = FilePath::fromString(newFileName); if (oldFilePath == newFilePath) - return; + return {}; const HandleIncludeGuards handleGuards = canTryToRenameIncludeGuards(node); if (!folderNode->canRenameFile(oldFilePath, newFilePath)) { @@ -3886,11 +3899,13 @@ void ProjectExplorerPlugin::renameFile(Node *node, const QString &newFileName) QTC_CHECK(Core::FileUtils::renameFile(oldFilePath, newFilePath, handleGuards)); } }); - return; + return {}; } if (Core::FileUtils::renameFile(oldFilePath, newFilePath, handleGuards)) { // Tell the project plugin about rename + // TODO: We might want to separate this into an extra step to make bulk renamings safer; + // see CppModelManager::renameIncludes(). if (!folderNode->renameFile(oldFilePath, newFilePath)) { const QString renameFileError = Tr::tr("The file %1 was renamed to %2, but the project " "file %3 could not be automatically changed.") @@ -3904,15 +3919,17 @@ void ProjectExplorerPlugin::renameFile(Node *node, const QString &newFileName) renameFileError); }); } - } else { - const QString renameFileError = Tr::tr("The file %1 could not be renamed %2.") - .arg(oldFilePath.toUserOutput()) - .arg(newFilePath.toUserOutput()); - - QTimer::singleShot(0, m_instance, [renameFileError] { - QMessageBox::warning(ICore::dialogParent(), Tr::tr("Cannot Rename File"), renameFileError); - }); + return std::make_pair(oldFilePath, newFilePath); } + + const QString renameFileError = Tr::tr("The file %1 could not be renamed %2.") + .arg(oldFilePath.toUserOutput()) + .arg(newFilePath.toUserOutput()); + + QTimer::singleShot(0, m_instance, [renameFileError] { + QMessageBox::warning(ICore::dialogParent(), Tr::tr("Cannot Rename File"), renameFileError); + }); + return {}; } void ProjectExplorerPluginPrivate::handleSetStartupProject() @@ -4048,7 +4065,7 @@ void ProjectExplorerPlugin::renameFilesForSymbol(const QString &oldSymbolName, const QString &newSymbolName, const FilePaths &files, bool preferLowerCaseFileNames) { static const auto isAllLowerCase = [](const QString &text) { return text.toLower() == text; }; - + QList> renamedFiles; for (const FilePath &file : files) { Node * const node = ProjectTree::nodeForFile(file); if (!node) @@ -4079,8 +4096,10 @@ void ProjectExplorerPlugin::renameFilesForSymbol(const QString &oldSymbolName, const QString newFilePath = file.absolutePath().toString() + '/' + newBaseName + '.' + file.completeSuffix(); - renameFile(node, newFilePath); + if (const auto res = renameFile(node, newFilePath)) + renamedFiles << *res; } + emit instance()->filesRenamed(renamedFiles); } void ProjectManager::registerProjectCreator(const QString &mimeType, diff --git a/src/plugins/projectexplorer/projectexplorer.h b/src/plugins/projectexplorer/projectexplorer.h index eec4fae8601..ced2d4e0f09 100644 --- a/src/plugins/projectexplorer/projectexplorer.h +++ b/src/plugins/projectexplorer/projectexplorer.h @@ -13,6 +13,8 @@ #include +#include + QT_BEGIN_NAMESPACE class QPoint; class QThreadPool; @@ -123,8 +125,9 @@ public: static void startRunControl(RunControl *runControl); static void showOutputPaneForRunControl(RunControl *runControl); - // internal public for FlatModel - static void renameFile(Node *node, const QString &newFilePath); + static QList> + renameFiles(const QList> &nodesAndNewFilePaths); + static QStringList projectFilePatterns(); static bool isProjectFile(const Utils::FilePath &filePath); static RecentProjectsEntries recentProjects(); @@ -175,9 +178,13 @@ signals: void runActionsUpdated(); + void filesRenamed(const QList> &oldAndNewPaths); + private: static bool coreAboutToClose(); void handleCommandLineArguments(const QStringList &arguments); + static std::optional> + renameFile(Node *node, const QString &newFilePath); #ifdef WITH_TESTS private slots: diff --git a/src/plugins/projectexplorer/projectmodels.cpp b/src/plugins/projectexplorer/projectmodels.cpp index f824a19cc91..dcbd3a89c89 100644 --- a/src/plugins/projectexplorer/projectmodels.cpp +++ b/src/plugins/projectexplorer/projectmodels.cpp @@ -324,10 +324,13 @@ bool FlatModel::setData(const QModelIndex &index, const QVariant &value, int rol } } - for (const auto &f : toRename) { - ProjectExplorerPlugin::renameFile(std::get<0>(f), std::get<2>(f).toString()); - emit renamed(std::get<1>(f), std::get<2>(f)); - } + QList> renameList; + for (const auto &f : toRename) + renameList << std::make_pair(std::get<0>(f), std::get<2>(f)); + const QList> renamedList + = ProjectExplorerPlugin::renameFiles(renameList); + for (const auto &[oldFilePath, newFilePath] : renamedList) + emit renamed(oldFilePath, newFilePath); return true; }