From 42edb0dd61a05702370a02d3efcbd5ba32e05e6b Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 14 Mar 2024 10:24:46 +0100 Subject: [PATCH] CppEditor: Make renameIncludes() also work for moved files Task-number: QTCREATORBUG-26545 Change-Id: I0bfe203af8f091562cdd91411dbe502fc5a76956 Reviewed-by: David Schulz Reviewed-by: Reviewed-by: Qt CI Bot --- src/plugins/cppeditor/cppmodelmanager.cpp | 29 ++++-- .../cppeditor/cppmodelmanager_test.cpp | 97 ++++++++++++------- src/plugins/cppeditor/cppmodelmanager_test.h | 1 + src/plugins/projectexplorer/projectmodels.cpp | 4 + .../testdata_renameheaders/header.h | 0 .../testdata_renameheaders/main.cpp | 5 + .../testdata_renameheaders/subdir1/file1.cpp | 3 + .../testdata_renameheaders/subdir1/header1.h | 0 .../testdata_renameheaders/subdir2/file2.cpp | 3 + .../testdata_renameheaders/subdir2/header2.h | 0 .../testdata_renameheaders.pro | 7 ++ 11 files changed, 108 insertions(+), 41 deletions(-) create mode 100644 tests/cppmodelmanager/testdata_renameheaders/header.h create mode 100644 tests/cppmodelmanager/testdata_renameheaders/main.cpp create mode 100644 tests/cppmodelmanager/testdata_renameheaders/subdir1/file1.cpp create mode 100644 tests/cppmodelmanager/testdata_renameheaders/subdir1/header1.h create mode 100644 tests/cppmodelmanager/testdata_renameheaders/subdir2/file2.cpp create mode 100644 tests/cppmodelmanager/testdata_renameheaders/subdir2/header2.h create mode 100644 tests/cppmodelmanager/testdata_renameheaders/testdata_renameheaders.pro diff --git a/src/plugins/cppeditor/cppmodelmanager.cpp b/src/plugins/cppeditor/cppmodelmanager.cpp index 5ea954add45..d5d6842e00f 100644 --- a/src/plugins/cppeditor/cppmodelmanager.cpp +++ b/src/plugins/cppeditor/cppmodelmanager.cpp @@ -1878,16 +1878,15 @@ void CppModelManager::renameIncludes(const QList> 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> 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) { + const FilePath relPathOld = FilePath::fromString(FilePath::calcRelativePath( + 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(), - newFileName); + block.position() + replaceStart + oldString.length(), + newString); file->setChangeSet(changeSet); file->apply(); } diff --git a/src/plugins/cppeditor/cppmodelmanager_test.cpp b/src/plugins/cppeditor/cppmodelmanager_test.cpp index a896563a61f..4ac39de737a 100644 --- a/src/plugins/cppeditor/cppmodelmanager_test.cpp +++ b/src/plugins/cppeditor/cppmodelmanager_test.cpp @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -965,50 +966,80 @@ void ModelManagerTest::testUpdateEditorsAfterProjectUpdate() QCOMPARE(documentBProjectPart->topLevelProject, pi->projectFilePath()); } +void ModelManagerTest::testRenameIncludes_data() +{ + QTest::addColumn("oldRelPath"); + QTest::addColumn("newRelPath"); + QTest::addColumn("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 { - ~ModelManagerGCHelper() { CppModelManager::GC(); } - } GCHelper; - Q_UNUSED(GCHelper) // do not warn about being unused - + // Set up project. 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()); - const QStringList fileNames = {"foo.h", "foo.cpp", "main.cpp"}; - const FilePath oldHeader = FilePath::fromString(workingDir.filePath("foo.h")); - const FilePath newHeader = FilePath::fromString(workingDir.filePath("bar.h")); - const MyTestDataDir testDir(_("testdata_project1")); - - // Copy test files to a temporary directory - QSet sourceFiles; - 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(); + // Verify initial code model state. + const auto makeAbs = [&](const QStringList &relPaths) { + return Utils::transform>(relPaths, [&](const QString &relPath) { + return projectDir.pathAppended(relPath); + }); + }; + const QSet allSources = makeAbs({"main.cpp", "subdir1/file1.cpp", "subdir2/file2.cpp"}); + const QSet allHeaders = makeAbs({"header.h", "subdir1/header1.h", "subdir2/header2.h"}); CPlusPlus::Snapshot snapshot = CppModelManager::snapshot(); - for (const FilePath &sourceFile : std::as_const(sourceFiles)) { - QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet{oldHeader}); + for (const FilePath &srcFile : allSources) { + 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 - CppModelManager::updateSourceFiles(sourceFiles).waitForFinished(); - QCoreApplication::processEvents(); + // Verify new code model state. + QVERIFY(::CppEditor::Tests::waitForSignalOrTimeout( + CppModelManager::instance(), &CppModelManager::sourceFilesRefreshed, 10000)); + QSet incompleteNewHeadersSet = allHeaders; + incompleteNewHeadersSet.remove(oldHeader); + QSet completeNewHeadersSet = incompleteNewHeadersSet; + completeNewHeadersSet << newHeader; + snapshot = CppModelManager::snapshot(); - for (const FilePath &sourceFile : std::as_const(sourceFiles)) { - QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet{newHeader}); + for (const FilePath &srcFile : allSources) { + const QSet &expectedHeaders = srcFile.fileName() == "main.cpp" && !successExpected + ? incompleteNewHeadersSet : completeNewHeadersSet; + QCOMPARE(snapshot.allIncludesForDocument(srcFile), expectedHeaders); } } diff --git a/src/plugins/cppeditor/cppmodelmanager_test.h b/src/plugins/cppeditor/cppmodelmanager_test.h index 26cbefd1c84..14e9c617de9 100644 --- a/src/plugins/cppeditor/cppmodelmanager_test.h +++ b/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(); diff --git a/src/plugins/projectexplorer/projectmodels.cpp b/src/plugins/projectexplorer/projectmodels.cpp index 9d603b47309..c8b7392bf9c 100644 --- a/src/plugins/projectexplorer/projectmodels.cpp +++ b/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> 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) diff --git a/tests/cppmodelmanager/testdata_renameheaders/header.h b/tests/cppmodelmanager/testdata_renameheaders/header.h new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/cppmodelmanager/testdata_renameheaders/main.cpp b/tests/cppmodelmanager/testdata_renameheaders/main.cpp new file mode 100644 index 00000000000..629f20001ff --- /dev/null +++ b/tests/cppmodelmanager/testdata_renameheaders/main.cpp @@ -0,0 +1,5 @@ +#include "header.h" +#include "subdir1/header1.h" +#include + +int main() {} diff --git a/tests/cppmodelmanager/testdata_renameheaders/subdir1/file1.cpp b/tests/cppmodelmanager/testdata_renameheaders/subdir1/file1.cpp new file mode 100644 index 00000000000..e22ce8100c9 --- /dev/null +++ b/tests/cppmodelmanager/testdata_renameheaders/subdir1/file1.cpp @@ -0,0 +1,3 @@ +#include "header1.h" +#include "../header.h" +#include "../subdir2/header2.h" diff --git a/tests/cppmodelmanager/testdata_renameheaders/subdir1/header1.h b/tests/cppmodelmanager/testdata_renameheaders/subdir1/header1.h new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/cppmodelmanager/testdata_renameheaders/subdir2/file2.cpp b/tests/cppmodelmanager/testdata_renameheaders/subdir2/file2.cpp new file mode 100644 index 00000000000..8c83d5ef036 --- /dev/null +++ b/tests/cppmodelmanager/testdata_renameheaders/subdir2/file2.cpp @@ -0,0 +1,3 @@ +#include "header2.h" +#include "../header.h" +#include "../subdir1/header1.h" diff --git a/tests/cppmodelmanager/testdata_renameheaders/subdir2/header2.h b/tests/cppmodelmanager/testdata_renameheaders/subdir2/header2.h new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/cppmodelmanager/testdata_renameheaders/testdata_renameheaders.pro b/tests/cppmodelmanager/testdata_renameheaders/testdata_renameheaders.pro new file mode 100644 index 00000000000..f5f09586fff --- /dev/null +++ b/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