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>

5 files changed, 111 insertions, 69 deletions

diff --git a/src/plugins/cppeditor/cppmodelmanager.cpp b/src/plugins/cppeditor/cppmodelmanager.cpp
index d601bbcf26..67470745bb 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<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())
-        return;
+    for (const auto &[oldFilePath, newFilePath] : oldAndNewPaths) {
+        if (oldFilePath.isEmpty() || newFilePath.isEmpty())
+            continue;
 
-    // We just want to handle renamings so return when the file was actually moved.
-    if (oldFilePath.absolutePath() != newFilePath.absolutePath())
-        return;
+        // We just want to handle renamings so return when the file was actually moved.
+        if (oldFilePath.absolutePath() != newFilePath.absolutePath())
+            continue;
 
-    const TextEditor::PlainRefactoringFileFactory changes;
+        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;
+        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<Snapshot::IncludeLocation> locations = snapshot().includeLocationsOfDocument(
-        isUiFile ? FilePath::fromString(oldFileName) : oldFilePath);
-    for (const Snapshot::IncludeLocation &loc : locations) {
-        const FilePath filePath = loc.first->filePath();
+        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(filePath) != productNodeForUiFile)
-            continue;
+            // 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(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();
+            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 a229d98957..0fe9e151ae 100644
--- a/src/plugins/cppeditor/cppmodelmanager.h
+++ b/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
diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index 469d9b9b2f..6d95819f36 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<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 {
-        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<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,
diff --git a/src/plugins/projectexplorer/projectexplorer.h b/src/plugins/projectexplorer/projectexplorer.h
index eec4fae860..ced2d4e0f0 100644
--- a/src/plugins/projectexplorer/projectexplorer.h
+++ b/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 void renameFile(Node *node, const QString &newFilePath);
+    static QList<std::pair<Utils::FilePath, Utils::FilePath>>
+    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:
diff --git a/src/plugins/projectexplorer/projectmodels.cpp b/src/plugins/projectexplorer/projectmodels.cpp
index f824a19cc9..dcbd3a89c8 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<std::pair<Node *, FilePath>> renameList;
+    for (const auto &f : toRename)
+        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;
 }