From 3299239095e4eff5b94604841491712a2fbf78ca Mon Sep 17 00:00:00 2001 From: Miklos Marton Date: Fri, 8 May 2020 18:06:26 +0200 Subject: [PATCH] C++ Refactoring: Fix the include macros in header files after renaming Fixes: QTCREATORBUG-4686 Change-Id: If22078bb183910941d8e2a94b0e8629baa2fa8de Reviewed-by: Christian Kandeler --- src/plugins/coreplugin/fileutils.cpp | 160 +++++++++++++++++- src/plugins/coreplugin/fileutils.h | 7 +- src/plugins/cpptools/cppmodelmanager_test.cpp | 72 +++++++- .../projectexplorer/projectexplorer.cpp | 9 +- tests/cppmodelmanager/testdata_project1/baz.h | 10 ++ .../cppmodelmanager/testdata_project1/baz2.h | 10 ++ .../cppmodelmanager/testdata_project1/baz3.h | 14 ++ .../testdata_project2/foobar2000.h | 10 ++ .../testdata_project2/foobar4000.h | 10 ++ 9 files changed, 289 insertions(+), 13 deletions(-) create mode 100644 tests/cppmodelmanager/testdata_project1/baz.h create mode 100644 tests/cppmodelmanager/testdata_project1/baz2.h create mode 100644 tests/cppmodelmanager/testdata_project1/baz3.h create mode 100644 tests/cppmodelmanager/testdata_project2/foobar2000.h create mode 100644 tests/cppmodelmanager/testdata_project2/foobar4000.h diff --git a/src/plugins/coreplugin/fileutils.cpp b/src/plugins/coreplugin/fileutils.cpp index 53f2e7c7b3e..ee58091a2be 100644 --- a/src/plugins/coreplugin/fileutils.cpp +++ b/src/plugins/coreplugin/fileutils.cpp @@ -29,10 +29,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -40,6 +42,9 @@ #include #include #include +#include +#include +#include #include using namespace Utils; @@ -178,7 +183,8 @@ static inline bool fileSystemRenameFile(const QString &orgFilePath, return QFile::rename(orgFilePath, newFilePath); } -bool FileUtils::renameFile(const QString &orgFilePath, const QString &newFilePath) +bool FileUtils::renameFile(const QString &orgFilePath, const QString &newFilePath, + HandleIncludeGuards handleGuards) { if (orgFilePath == newFilePath) return false; @@ -195,7 +201,159 @@ bool FileUtils::renameFile(const QString &orgFilePath, const QString &newFilePat // yeah we moved, tell the filemanager about it DocumentManager::renamedFile(orgFilePath, newFilePath); } + + if (result && handleGuards == HandleIncludeGuards::Yes) { + QFileInfo fi(orgFilePath); + bool headerUpdateSuccess = updateHeaderFileGuardAfterRename(newFilePath, fi.baseName()); + if (!headerUpdateSuccess) { + Core::MessageManager::write(QCoreApplication::translate( + "Core::FileUtils", + "Failed to rename the include guard in file \"%1\".") + .arg(newFilePath)); + } + } + return result; } +bool FileUtils::updateHeaderFileGuardAfterRename(const QString &headerPath, + const QString &oldHeaderBaseName) +{ + bool ret = true; + QFile headerFile(headerPath); + if (!headerFile.open(QFile::ReadOnly | QFile::Text)) + return false; + + QRegularExpression guardConditionRegExp( + QString("(#ifndef)(\\s*)(_*)%1_H(_*)(\\s*)").arg(oldHeaderBaseName.toUpper())); + QRegularExpression guardDefineRegexp, guardCloseRegExp; + QRegularExpressionMatch guardConditionMatch, guardDefineMatch, guardCloseMatch; + int guardStartLine = -1; + int guardCloseLine = -1; + + QByteArray data = headerFile.readAll(); + headerFile.close(); + + const auto headerFileTextFormat = Utils::TextFileFormat::detect(data); + QTextStream inStream(&data); + int lineCounter = 0; + QString line; + while (!inStream.atEnd()) { + // use trimmed line to get rid from the maunder leading spaces + inStream.readLineInto(&line); + line = line.trimmed(); + if (line == QStringLiteral("#pragma once")) { + // if pragma based guard found skip reading the whole file + break; + } + + if (guardStartLine == -1) { + // we are still looking for the guard condition + guardConditionMatch = guardConditionRegExp.match(line); + if (guardConditionMatch.hasMatch()) { + guardDefineRegexp.setPattern(QString("(#define\\s*%1)%2(_H%3\\s*)") + .arg(guardConditionMatch.captured(3), + oldHeaderBaseName.toUpper(), + guardConditionMatch.captured(4))); + // read the next line for the guard define + line = inStream.readLine(); + if (!inStream.atEnd()) { + guardDefineMatch = guardDefineRegexp.match(line); + if (guardDefineMatch.hasMatch()) { + // if a proper guard define present in the next line store the line number + guardCloseRegExp + .setPattern( + QString("(#endif\\s*)(\\/\\/|\\/\\*)(\\s*%1)%2(_H%3\\s*)((\\*\\/)?)") + .arg( + guardConditionMatch.captured(3), + oldHeaderBaseName.toUpper(), + guardConditionMatch.captured(4))); + guardStartLine = lineCounter; + lineCounter++; + } + } else { + // it the line after the guard opening is not something what we expect + // then skip the whole guard replacing process + break; + } + } + } else { + // guard start found looking for the guard closing endif + guardCloseMatch = guardCloseRegExp.match(line); + if (guardCloseMatch.hasMatch()) { + guardCloseLine = lineCounter; + break; + } + } + lineCounter++; + } + + if (guardStartLine != -1) { + // At least the guard have been found -> + // copy the contents of the header to a temporary file with the updated guard lines + inStream.seek(0); + + QFileInfo fi(headerFile); + const auto guardCondition = QString("#ifndef%1%2%3_H%4%5").arg( + guardConditionMatch.captured(2), + guardConditionMatch.captured(3), + fi.baseName().toUpper(), + guardConditionMatch.captured(4), + guardConditionMatch.captured(5)); + // guardDefineRegexp.setPattern(QString("(#define\\s*%1)%2(_H%3\\s*)") + const auto guardDefine = QString("%1%2%3").arg( + guardDefineMatch.captured(1), + fi.baseName().toUpper(), + guardDefineMatch.captured(2)); + const auto guardClose = QString("%1%2%3%4%5%6").arg( + guardCloseMatch.captured(1), + guardCloseMatch.captured(2), + guardCloseMatch.captured(3), + fi.baseName().toUpper(), + guardCloseMatch.captured(4), + guardCloseMatch.captured(5)); + + QFile tmpHeader(headerPath + ".tmp"); + if (tmpHeader.open(QFile::WriteOnly)) { + const auto lineEnd = + headerFileTextFormat.lineTerminationMode + == Utils::TextFileFormat::LFLineTerminator + ? QStringLiteral("\n") : QStringLiteral("\r\n"); + QTextStream outStream(&tmpHeader); + if (headerFileTextFormat.codec == nullptr) + outStream.setCodec("UTF-8"); + else + outStream.setCodec(headerFileTextFormat.codec->name()); + int lineCounter = 0; + while (!inStream.atEnd()) { + inStream.readLineInto(&line); + if (lineCounter == guardStartLine) { + outStream << guardCondition << lineEnd; + outStream << guardDefine << lineEnd; + inStream.readLine(); + lineCounter++; + } else if (lineCounter == guardCloseLine) { + outStream << guardClose << lineEnd; + } else { + outStream << line << lineEnd; + } + lineCounter++; + } + tmpHeader.close(); + } else { + // if opening the temp file failed report error + ret = false; + } + } + + if (ret && guardStartLine != -1) { + // if the guard was found (and updated updated properly) swap the temp and the target file + ret = QFile::remove(headerPath); + if (ret) + ret = QFile::rename(headerPath + ".tmp", headerPath); + } + + return ret; +} + } // namespace Core diff --git a/src/plugins/coreplugin/fileutils.h b/src/plugins/coreplugin/fileutils.h index 0067d1d7285..a2dbb5a2e25 100644 --- a/src/plugins/coreplugin/fileutils.h +++ b/src/plugins/coreplugin/fileutils.h @@ -35,6 +35,8 @@ namespace Utils { class Environment; } namespace Core { +enum class HandleIncludeGuards { No, Yes }; + struct CORE_EXPORT FileUtils { // Helpers for common directory browser options. @@ -48,7 +50,10 @@ struct CORE_EXPORT FileUtils static QString msgTerminalWithAction(); // File operations aware of version control and file system case-insensitiveness static void removeFile(const QString &filePath, bool deleteFromFS); - static bool renameFile(const QString &from, const QString &to); + static bool renameFile(const QString &from, const QString &to, + HandleIncludeGuards handleGuards = HandleIncludeGuards::No); + // This method is used to refactor the include guards in the renamed headers + static bool updateHeaderFileGuardAfterRename(const QString &headerPath, const QString &oldHeaderBaseName); }; } // namespace Core diff --git a/src/plugins/cpptools/cppmodelmanager_test.cpp b/src/plugins/cpptools/cppmodelmanager_test.cpp index 73da04531f8..ea40d84ea0d 100644 --- a/src/plugins/cpptools/cppmodelmanager_test.cpp +++ b/src/plugins/cpptools/cppmodelmanager_test.cpp @@ -1039,7 +1039,7 @@ void CppToolsPlugin::test_modelmanager_renameIncludes() QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet() << oldHeader); // Renaming the header - QVERIFY(Core::FileUtils::renameFile(oldHeader, newHeader)); + QVERIFY(Core::FileUtils::renameFile(oldHeader, newHeader, Core::HandleIncludeGuards::Yes)); // Update the c++ model manager again and check for the new includes modelManager->updateSourceFiles(sourceFiles).waitForFinished(); @@ -1060,9 +1060,15 @@ void CppToolsPlugin::test_modelmanager_renameIncludesInEditor() QVERIFY(tmpDir.isValid()); const QDir workingDir(tmpDir.path()); - const QStringList fileNames = {"foo.h", "foo.cpp", "main.cpp"}; - const QString oldHeader(workingDir.filePath(_("foo.h"))); - const QString newHeader(workingDir.filePath(_("bar.h"))); + const QStringList fileNames = {"baz.h", "baz2.h", "baz3.h", "foo.h", "foo.cpp", "main.cpp"}; + const QString headerWithPragmaOnce(workingDir.filePath(_("foo.h"))); + const QString renamedHeaderWithPragmaOnce(workingDir.filePath(_("bar.h"))); + const QString headerWithNormalGuard(workingDir.filePath(_("baz.h"))); + const QString renamedHeaderWithNormalGuard(workingDir.filePath(_("foobar2000.h"))); + const QString headerWithUnderscoredGuard(workingDir.filePath(_("baz2.h"))); + const QString renamedHeaderWithUnderscoredGuard(workingDir.filePath(_("foobar4000.h"))); + const QString headerWithMalformedGuard(workingDir.filePath(_("baz3.h"))); + const QString renamedHeaderWithMalformedGuard(workingDir.filePath(_("foobar5000.h"))); const QString mainFile(workingDir.filePath(_("main.cpp"))); CppModelManager *modelManager = CppModelManager::instance(); const MyTestDataDir testDir(_("testdata_project1")); @@ -1086,7 +1092,7 @@ void CppToolsPlugin::test_modelmanager_renameIncludesInEditor() QCoreApplication::processEvents(); CPlusPlus::Snapshot snapshot = modelManager->snapshot(); foreach (const QString &sourceFile, sourceFiles) - QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet() << oldHeader); + QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet() << headerWithPragmaOnce); // Open a file in the editor QCOMPARE(Core::DocumentModel::openedDocuments().size(), 0); @@ -1100,8 +1106,58 @@ void CppToolsPlugin::test_modelmanager_renameIncludesInEditor() QVERIFY(modelManager->isCppEditor(editor)); QVERIFY(modelManager->workingCopy().contains(mainFile)); - // Renaming the header - QVERIFY(Core::FileUtils::renameFile(oldHeader, newHeader)); + // Test the renaming of a header file where a pragma once guard is present + QVERIFY(Core::FileUtils::renameFile(headerWithPragmaOnce, renamedHeaderWithPragmaOnce, + Core::HandleIncludeGuards::Yes)); + + // Test the renaming the header with include guard: + // The contents should match the foobar2000.h in the testdata_project2 project + QVERIFY(Core::FileUtils::renameFile(headerWithNormalGuard, renamedHeaderWithNormalGuard, + Core::HandleIncludeGuards::Yes)); + + const MyTestDataDir testDir2(_("testdata_project2")); + QFile foobar2000Header(testDir2.file("foobar2000.h")); + QVERIFY(foobar2000Header.open(QFile::ReadOnly)); + const auto foobar2000HeaderContents = foobar2000Header.readAll(); + foobar2000Header.close(); + + QFile renamedHeader(renamedHeaderWithNormalGuard); + QVERIFY(renamedHeader.open(QFile::ReadOnly)); + auto renamedHeaderContents = renamedHeader.readAll(); + renamedHeader.close(); + QCOMPARE(renamedHeaderContents, foobar2000HeaderContents); + + // Test the renaming the header with underscore pre/suffixed include guard: + // The contents should match the foobar2000.h in the testdata_project2 project + QVERIFY(Core::FileUtils::renameFile(headerWithUnderscoredGuard, renamedHeaderWithUnderscoredGuard, + Core::HandleIncludeGuards::Yes)); + + QFile foobar4000Header(testDir2.file("foobar4000.h")); + QVERIFY(foobar4000Header.open(QFile::ReadOnly)); + const auto foobar4000HeaderContents = foobar4000Header.readAll(); + foobar4000Header.close(); + + renamedHeader.setFileName(renamedHeaderWithUnderscoredGuard); + QVERIFY(renamedHeader.open(QFile::ReadOnly)); + renamedHeaderContents = renamedHeader.readAll(); + renamedHeader.close(); + QCOMPARE(renamedHeaderContents, foobar4000HeaderContents); + + // test the renaming of a header with a malformed guard to verify we do not make + // accidental refactors + renamedHeader.setFileName(headerWithMalformedGuard); + QVERIFY(renamedHeader.open(QFile::ReadOnly)); + auto originalMalformedGuardContents = renamedHeader.readAll(); + renamedHeader.close(); + + QVERIFY(Core::FileUtils::renameFile(headerWithMalformedGuard, renamedHeaderWithMalformedGuard, + Core::HandleIncludeGuards::Yes)); + + renamedHeader.setFileName(renamedHeaderWithMalformedGuard); + QVERIFY(renamedHeader.open(QFile::ReadOnly)); + renamedHeaderContents = renamedHeader.readAll(); + renamedHeader.close(); + QCOMPARE(renamedHeaderContents, originalMalformedGuardContents); // Update the c++ model manager again and check for the new includes TestCase::waitForProcessedEditorDocument(mainFile); @@ -1109,7 +1165,7 @@ void CppToolsPlugin::test_modelmanager_renameIncludesInEditor() QCoreApplication::processEvents(); snapshot = modelManager->snapshot(); foreach (const QString &sourceFile, sourceFiles) - QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet() << newHeader); + QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet() << renamedHeaderWithPragmaOnce); } void CppToolsPlugin::test_modelmanager_documentsAndRevisions() diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp index 1926d931e00..8df37279b85 100644 --- a/src/plugins/projectexplorer/projectexplorer.cpp +++ b/src/plugins/projectexplorer/projectexplorer.cpp @@ -3772,8 +3772,11 @@ void ProjectExplorerPlugin::renameFile(Node *node, const QString &newFilePath) if (oldFilePath == newFilePath) return; + auto handleGuards = Core::HandleIncludeGuards::No; + if (node->asFileNode() && node->asFileNode()->fileType() == FileType::Header) + handleGuards = Core::HandleIncludeGuards::Yes; if (!folderNode->canRenameFile(oldFilePath, newFilePath)) { - QTimer::singleShot(0, [oldFilePath, newFilePath, projectFileName] { + QTimer::singleShot(0, [oldFilePath, newFilePath, projectFileName, handleGuards] { int res = QMessageBox::question(ICore::dialogParent(), tr("Project Editing Failed"), tr("The project file %1 cannot be automatically changed.\n\n" @@ -3782,13 +3785,13 @@ void ProjectExplorerPlugin::renameFile(Node *node, const QString &newFilePath) .arg(QDir::toNativeSeparators(oldFilePath)) .arg(QDir::toNativeSeparators(newFilePath))); if (res == QMessageBox::Yes) { - QTC_CHECK(Core::FileUtils::renameFile(oldFilePath, newFilePath)); + QTC_CHECK(Core::FileUtils::renameFile(oldFilePath, newFilePath, handleGuards)); } }); return; } - if (Core::FileUtils::renameFile(oldFilePath, newFilePath)) { + if (Core::FileUtils::renameFile(oldFilePath, newFilePath, handleGuards)) { // Tell the project plugin about rename if (!folderNode->renameFile(oldFilePath, newFilePath)) { const QString renameFileError diff --git a/tests/cppmodelmanager/testdata_project1/baz.h b/tests/cppmodelmanager/testdata_project1/baz.h new file mode 100644 index 00000000000..9ddc881e140 --- /dev/null +++ b/tests/cppmodelmanager/testdata_project1/baz.h @@ -0,0 +1,10 @@ +#ifndef BAZ_H +#define BAZ_H + +class Baz { +public: + Baz() {} +}; + +#endif // BAZ_H + diff --git a/tests/cppmodelmanager/testdata_project1/baz2.h b/tests/cppmodelmanager/testdata_project1/baz2.h new file mode 100644 index 00000000000..9f3e0cb3be7 --- /dev/null +++ b/tests/cppmodelmanager/testdata_project1/baz2.h @@ -0,0 +1,10 @@ +#ifndef __BAZ2_H__ +#define __BAZ2_H__ + +class Baz { +public: + Baz() {} +}; + +#endif /* __BAZ2_H__ */ + diff --git a/tests/cppmodelmanager/testdata_project1/baz3.h b/tests/cppmodelmanager/testdata_project1/baz3.h new file mode 100644 index 00000000000..f32c7010ad6 --- /dev/null +++ b/tests/cppmodelmanager/testdata_project1/baz3.h @@ -0,0 +1,14 @@ +#ifndef BAZ2_H +#define __BAZ2_H + +/* + * This is a test file with a malformed include guard + * to test that the renaming does not touch the guard in similar cases at all. + */ +class Baz { +public: + Baz() {} +}; + +#endif /* __BAZ2_H */ + diff --git a/tests/cppmodelmanager/testdata_project2/foobar2000.h b/tests/cppmodelmanager/testdata_project2/foobar2000.h new file mode 100644 index 00000000000..39ca9457b64 --- /dev/null +++ b/tests/cppmodelmanager/testdata_project2/foobar2000.h @@ -0,0 +1,10 @@ +#ifndef FOOBAR2000_H +#define FOOBAR2000_H + +class Baz { +public: + Baz() {} +}; + +#endif // FOOBAR2000_H + diff --git a/tests/cppmodelmanager/testdata_project2/foobar4000.h b/tests/cppmodelmanager/testdata_project2/foobar4000.h new file mode 100644 index 00000000000..373ab576ff1 --- /dev/null +++ b/tests/cppmodelmanager/testdata_project2/foobar4000.h @@ -0,0 +1,10 @@ +#ifndef __FOOBAR4000_H__ +#define __FOOBAR4000_H__ + +class Baz { +public: + Baz() {} +}; + +#endif /* __FOOBAR4000_H__ */ +