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 <david.schulz@qt.io>

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<QString> 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<std::pair<FilePath, FilePath>> &oldAndNewPaths)
 {
-    if (oldFilePath.isEmpty() || newFilePath.isEmpty())
+    for (const auto &[oldFilePath, newFilePath] : oldAndNewPaths) {
-        return;
+        if (oldFilePath.isEmpty() || newFilePath.isEmpty())
-
-    // 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<Snapshot::IncludeLocation> 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)
             continue;
 
-        TextEditor::RefactoringFilePtr file = changes.file(filePath);
+        // We just want to handle renamings so return when the file was actually moved.
-        const QTextBlock &block = file->document()->findBlockByNumber(loc.second - 1);
+        if (oldFilePath.absolutePath() != newFilePath.absolutePath())
-        const int replaceStart = block.text().indexOf(oldFileName);
+            continue;
-        if (replaceStart > -1) {
+
-            ChangeSet changeSet;
+        const TextEditor::PlainRefactoringFileFactory changes;
-            changeSet.replace(block.position() + replaceStart,
+
-                              block.position() + replaceStart + oldFileName.length(),
+        QString oldFileName = oldFilePath.fileName();
-                              newFileName);
+        QString newFileName = newFilePath.fileName();
-            file->setChangeSet(changeSet);
+        const bool isUiFile = oldFilePath.suffix() == "ui" && newFilePath.suffix() == "ui";
-            file->apply();
+        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<Snapshot::IncludeLocation> 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();
+            }
         }
     }
 }

src/plugins/cppeditor/cppmodelmanager.h

@@ -22,6 +22,7 @@
 
 #include <functional>
 #include <memory>
+#include <utility>
 
 namespace Core {
 class IDocument;
@@ -244,8 +245,6 @@ public:
 
     static QSet<QString> internalTargets(const Utils::FilePath &filePath);
 
-    static void renameIncludes(const Utils::FilePath &oldFilePath, const Utils::FilePath &newFilePath);
-
     // for VcsBaseSubmitEditor
     Q_INVOKABLE QSet<QString> symbolsInFiles(const QSet<Utils::FilePath> &files) const;
 
@@ -299,6 +298,9 @@ private:
 
     static void dumpModelManagerConfiguration(const QString &logFileId);
     static void initCppTools();
+
+    static void renameIncludes(const QList<std::pair<Utils::FilePath,
+                                                     Utils::FilePath>> &oldAndNewPaths);
 };
 
 } // CppEditor

src/plugins/projectexplorer/projectexplorer.cpp

@@ -2466,6 +2466,18 @@ void ProjectExplorerPlugin::showOutputPaneForRunControl(RunControl *runControl)
     dd->showOutputPaneForRunControl(runControl);
 }
 
+QList<std::pair<FilePath, FilePath>> ProjectExplorerPlugin::renameFiles(
+    const QList<std::pair<Node *, Utils::FilePath>> &nodesAndNewFilePaths)
+{
+    QList<std::pair<FilePath, FilePath>> 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<std::pair<FilePath, FilePath>>
+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 {
+        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);
-        });
     }
+
+    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<std::pair<FilePath, FilePath>> 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,

src/plugins/projectexplorer/projectexplorer.h

@@ -13,6 +13,8 @@
 
 #include <QPair>
 
+#include <optional>
+
 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 QList<std::pair<Utils::FilePath, Utils::FilePath>>
-    static void renameFile(Node *node, const QString &newFilePath);
+    renameFiles(const QList<std::pair<Node *, Utils::FilePath>> &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<std::pair<Utils::FilePath, Utils::FilePath>> &oldAndNewPaths);
+
 private:
     static bool coreAboutToClose();
     void handleCommandLineArguments(const QStringList &arguments);
+    static std::optional<std::pair<Utils::FilePath, Utils::FilePath>>
+    renameFile(Node *node, const QString &newFilePath);
 
 #ifdef WITH_TESTS
 private slots:

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) {
+    QList<std::pair<Node *, FilePath>> renameList;
-        ProjectExplorerPlugin::renameFile(std::get<0>(f), std::get<2>(f).toString());
+    for (const auto &f : toRename)
-        emit renamed(std::get<1>(f), std::get<2>(f));
+        renameList << std::make_pair(std::get<0>(f), std::get<2>(f));
-    }
+    const QList<std::pair<FilePath, FilePath>> renamedList
+        = ProjectExplorerPlugin::renameFiles(renameList);
+    for (const auto &[oldFilePath, newFilePath] : renamedList)
+        emit renamed(oldFilePath, newFilePath);
     return true;
 }