CppEditor: Make renameIncludes() also work for moved files

Task-number: QTCREATORBUG-26545
Change-Id: I0bfe203af8f091562cdd91411dbe502fc5a76956
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/plugins/cppeditor/cppmodelmanager.cpp

@@ -1878,16 +1878,15 @@ void CppModelManager::renameIncludes(const QList<std::pair<FilePath, FilePath>>
         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())
-            continue;
-
         const TextEditor::PlainRefactoringFileFactory changes;
 
         QString oldFileName = oldFilePath.fileName();
         QString newFileName = newFilePath.fileName();
         const bool isUiFile = oldFilePath.suffix() == "ui" && newFilePath.suffix() == "ui";
+        const bool moved = oldFilePath.absolutePath() != newFilePath.absolutePath();
         if (isUiFile) {
+            if (moved)
+                return; // This is out of scope.
             oldFileName = "ui_" + oldFilePath.baseName() + ".h";
             newFileName = "ui_" + newFilePath.baseName() + ".h";
         }
@@ -1925,12 +1924,26 @@ void CppModelManager::renameIncludes(const QList<std::pair<FilePath, FilePath>>
 
             TextEditor::RefactoringFilePtr file = changes.file(includingFileNew);
             const QTextBlock &block = file->document()->findBlockByNumber(loc.second - 1);
-            const int replaceStart = block.text().indexOf(oldFileName);
+            const FilePath relPathOld = FilePath::fromString(FilePath::calcRelativePath(
-            if (replaceStart > -1) {
+                oldFilePath.toString(), includingFileOld.parentDir().toString()));
+            const FilePath relPathNew = FilePath::fromString(FilePath::calcRelativePath(
+                newFilePath.toString(), includingFileNew.parentDir().toString()));
+            int replaceStart = block.text().indexOf(relPathOld.toString());
+            QString oldString;
+            QString newString;
+            if (isUiFile || replaceStart == -1) {
+                replaceStart = block.text().indexOf(oldFileName);
+                oldString = oldFileName;
+                newString = newFileName;
+            } else {
+                oldString = relPathOld.toString();
+                newString = relPathNew.toString();
+            }
+            if (replaceStart > -1 && oldString != newString) {
                 ChangeSet changeSet;
                 changeSet.replace(block.position() + replaceStart,
-                                  block.position() + replaceStart + oldFileName.length(),
+                                  block.position() + replaceStart + oldString.length(),
-                                  newFileName);
+                                  newString);
                 file->setChangeSet(changeSet);
                 file->apply();
             }

src/plugins/cppeditor/cppmodelmanager_test.cpp

@@ -17,6 +17,7 @@
 
 #include <cplusplus/LookupContext.h>
 
+#include <projectexplorer/kitmanager.h>
 #include <projectexplorer/projectexplorer.h>
 #include <projectexplorer/projectmanager.h>
 #include <projectexplorer/projectnodes.h>
@@ -965,50 +966,80 @@ void ModelManagerTest::testUpdateEditorsAfterProjectUpdate()
     QCOMPARE(documentBProjectPart->topLevelProject, pi->projectFilePath());
 }
 
+void ModelManagerTest::testRenameIncludes_data()
+{
+    QTest::addColumn<QString>("oldRelPath");
+    QTest::addColumn<QString>("newRelPath");
+    QTest::addColumn<bool>("successExpected");
+
+    QTest::addRow("rename in place 1")
+        << "subdir1/header1.h" << "subdir1/header1_renamed.h" << true;
+    QTest::addRow("rename in place 2")
+        << "subdir2/header2.h" << "subdir2/header2_renamed.h" << true;
+    QTest::addRow("rename in place 3") << "header.h" << "header_renamed.h" << true;
+    QTest::addRow("move up") << "subdir1/header1.h" << "header1_moved.h" << true;
+    QTest::addRow("move up (breaks build)") << "subdir2/header2.h" << "header2_moved.h" << false;
+    QTest::addRow("move down") << "header.h" << "subdir1/header_moved.h" << true;
+    QTest::addRow("move across") << "subdir1/header1.h" << "subdir2/header1_moved.h" << true;
+    QTest::addRow("move across (breaks build)")
+        << "subdir2/header2.h" << "subdir1/header2_moved.h" << false;
+}
+
 void ModelManagerTest::testRenameIncludes()
 {
-    struct ModelManagerGCHelper {
+    // Set up project.
-        ~ModelManagerGCHelper() { CppModelManager::GC(); }
-    } GCHelper;
-    Q_UNUSED(GCHelper) // do not warn about being unused
-
     TemporaryDir tmpDir;
     QVERIFY(tmpDir.isValid());
+    const MyTestDataDir sourceDir("testdata_renameheaders");
+    const FilePath srcFilePath = FilePath::fromString(sourceDir.path());
+    const FilePath projectDir = tmpDir.filePath().pathAppended(srcFilePath.fileName());
+    const auto copyResult = srcFilePath.copyRecursively(projectDir);
+    if (!copyResult)
+        qDebug() << copyResult.error();
+    QVERIFY(copyResult);
+    Kit * const kit  = Utils::findOr(KitManager::kits(), nullptr, [](const Kit *k) {
+        return k->isValid() && !k->hasWarning() && k->value("QtSupport.QtInformation").isValid();
+    });
+    if (!kit)
+        QSKIP("The test requires at least one valid kit with a valid Qt");
+    const FilePath projectFile = projectDir.pathAppended(projectDir.fileName() + ".pro");
+    ProjectOpenerAndCloser projectMgr;
+    QVERIFY(projectMgr.open(projectFile, true, kit));
 
-    const QDir workingDir(tmpDir.path());
+    // Verify initial code model state.
-    const QStringList fileNames = {"foo.h", "foo.cpp", "main.cpp"};
+    const auto makeAbs = [&](const QStringList &relPaths) {
-    const FilePath oldHeader = FilePath::fromString(workingDir.filePath("foo.h"));
+        return Utils::transform<QSet<FilePath>>(relPaths, [&](const QString &relPath) {
-    const FilePath newHeader = FilePath::fromString(workingDir.filePath("bar.h"));
+            return projectDir.pathAppended(relPath);
-    const MyTestDataDir testDir(_("testdata_project1"));
+        });
-
+    };
-    // Copy test files to a temporary directory
+    const QSet<FilePath> allSources = makeAbs({"main.cpp", "subdir1/file1.cpp", "subdir2/file2.cpp"});
-    QSet<FilePath> sourceFiles;
+    const QSet<FilePath> allHeaders = makeAbs({"header.h", "subdir1/header1.h", "subdir2/header2.h"});
-    for (const QString &fileName : std::as_const(fileNames)) {
-        const QString &file = workingDir.filePath(fileName);
-        QVERIFY(QFile::copy(testDir.file(fileName), file));
-        // Saving source file names for the model manager update,
-        // so we can update just the relevant files.
-        if (ProjectFile::classify(file) == ProjectFile::CXXSource)
-            sourceFiles.insert(FilePath::fromString(file));
-    }
-
-    // Update the c++ model manager and check for the old includes
-    CppModelManager::updateSourceFiles(sourceFiles).waitForFinished();
-    QCoreApplication::processEvents();
     CPlusPlus::Snapshot snapshot = CppModelManager::snapshot();
-    for (const FilePath &sourceFile : std::as_const(sourceFiles)) {
+    for (const FilePath &srcFile : allSources) {
-        QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet<FilePath>{oldHeader});
+        QCOMPARE(snapshot.allIncludesForDocument(srcFile), allHeaders);
     }
 
-    // Renaming the header
+    // Rename the header.
+    QFETCH(QString, oldRelPath);
+    QFETCH(QString, newRelPath);
+    QFETCH(bool, successExpected);
+    const FilePath oldHeader = projectDir.pathAppended(oldRelPath);
+    const FilePath newHeader = projectDir.pathAppended(newRelPath);
     QVERIFY(ProjectExplorerPlugin::renameFile(oldHeader, newHeader));
 
-    // Update the c++ model manager again and check for the new includes
+    // Verify new code model state.
-    CppModelManager::updateSourceFiles(sourceFiles).waitForFinished();
+    QVERIFY(::CppEditor::Tests::waitForSignalOrTimeout(
-    QCoreApplication::processEvents();
+        CppModelManager::instance(), &CppModelManager::sourceFilesRefreshed, 10000));
+    QSet<FilePath> incompleteNewHeadersSet = allHeaders;
+    incompleteNewHeadersSet.remove(oldHeader);
+    QSet<FilePath> completeNewHeadersSet = incompleteNewHeadersSet;
+    completeNewHeadersSet << newHeader;
+
     snapshot = CppModelManager::snapshot();
-    for (const FilePath &sourceFile : std::as_const(sourceFiles)) {
+    for (const FilePath &srcFile : allSources) {
-        QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet<FilePath>{newHeader});
+        const QSet<FilePath> &expectedHeaders = srcFile.fileName() == "main.cpp" && !successExpected
+            ? incompleteNewHeadersSet : completeNewHeadersSet;
+        QCOMPARE(snapshot.allIncludesForDocument(srcFile), expectedHeaders);
     }
 }
 

src/plugins/cppeditor/cppmodelmanager_test.h

@@ -28,6 +28,7 @@ private slots:
     void testDefinesPerEditor();
     void testUpdateEditorsAfterProjectUpdate();
     void testPrecompiledHeaders();
+    void testRenameIncludes_data();
     void testRenameIncludes();
     void testRenameIncludesInEditor();
     void testDocumentsAndRevisions();

src/plugins/projectexplorer/projectmodels.cpp

@@ -802,6 +802,10 @@ bool FlatModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int r
             if (vcsAddPossible && !targetVcs.vcs->vcsAdd(targetFile))
                 failedVcsOp << targetFile;
         }
+        QList<std::pair<FilePath, FilePath>> renameList;
+        for (int i = 0; i < filesToAdd.size(); ++i)
+            renameList.emplaceBack(filesToRemove.at(i), filesToAdd.at(i));
+        emit ProjectExplorerPlugin::instance()->filesRenamed(renameList);
         const RemovedFilesFromProject result
                 = sourceProjectNode->removeFiles(filesToRemove, &failedRemoveFromProject);
         if (result == RemovedFilesFromProject::Wildcard)

tests/cppmodelmanager/testdata_renameheaders/main.cpp

@@ -0,0 +1,5 @@
+#include "header.h"
+#include "subdir1/header1.h"
+#include <header2.h>
+
+int main() {}

tests/cppmodelmanager/testdata_renameheaders/subdir1/file1.cpp

@@ -0,0 +1,3 @@
+#include "header1.h"
+#include "../header.h"
+#include "../subdir2/header2.h"

tests/cppmodelmanager/testdata_renameheaders/subdir2/file2.cpp

@@ -0,0 +1,3 @@
+#include "header2.h"
+#include "../header.h"
+#include "../subdir1/header1.h"

tests/cppmodelmanager/testdata_renameheaders/testdata_renameheaders.pro

@@ -0,0 +1,7 @@
+TEMPLATE = app
+TARGET = testdata_renameheaders
+INCLUDEPATH += subdir2
+CONFIG += no_include_pwd
+
+HEADERS = header.h subdir1/header1.h subdir2/header2.h
+SOURCES = main.cpp subdir1/file1.cpp subdir2/file2.cpp