feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

C++ Refactoring: Fix the include macros in header files after renaming

Browse Source 
Fixes: QTCREATORBUG-4686
Change-Id: If22078bb183910941d8e2a94b0e8629baa2fa8de
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
This commit is contained in:
Miklos Marton
2020-05-08 18:06:26 +02:00
committed by Miklós Márton
parent 795ccfbffc
commit 3299239095
9 changed files with 289 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

160
src/plugins/coreplugin/fileutils.cpp
View File

@@ -29,10 +29,12 @@
#include <coreplugin/documentmanager.h>
#include <coreplugin/icore.h>
#include <coreplugin/iversioncontrol.h>
#include <coreplugin/messagemanager.h>
#include <coreplugin/vcsmanager.h>
#include <utils/consoleprocess.h>
#include <utils/environment.h>
#include <utils/hostosinfo.h>
#include <utils/textfileformat.h>
#include <utils/unixutils.h>
#include <QApplication>
@@ -40,6 +42,9 @@
#include <QFileInfo>
#include <QMessageBox>
#include <QPushButton>
#include <QRegularExpression>
#include <QTextStream>
#include <QTextCodec>
#include <QWidget>
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

7
src/plugins/coreplugin/fileutils.h
View File

@@ -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

72
src/plugins/cpptools/cppmodelmanager_test.cpp
View File

@@ -1039,7 +1039,7 @@ void CppToolsPlugin::test_modelmanager_renameIncludes()
QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet<QString>() << 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<QString>() << oldHeader);
QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet<QString>() << 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<QString>() << newHeader);
QCOMPARE(snapshot.allIncludesForDocument(sourceFile), QSet<QString>() << renamedHeaderWithPragmaOnce);
}
void CppToolsPlugin::test_modelmanager_documentsAndRevisions()

9
src/plugins/projectexplorer/projectexplorer.cpp
View File

@@ -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

10
tests/cppmodelmanager/testdata_project1/baz.h Normal file
View File

@@ -0,0 +1,10 @@
#ifndef BAZ_H
#define BAZ_H
class Baz {
public:
Baz() {}
};
#endif // BAZ_H

10
tests/cppmodelmanager/testdata_project1/baz2.h Normal file
View File

@@ -0,0 +1,10 @@
#ifndef __BAZ2_H__
#define __BAZ2_H__
class Baz {
public:
Baz() {}
};
#endif /* __BAZ2_H__ */

14
tests/cppmodelmanager/testdata_project1/baz3.h Normal file
View File

@@ -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 */

10
tests/cppmodelmanager/testdata_project2/foobar2000.h Normal file
View File

@@ -0,0 +1,10 @@
#ifndef FOOBAR2000_H
#define FOOBAR2000_H
class Baz {
public:
Baz() {}
};
#endif // FOOBAR2000_H

10
tests/cppmodelmanager/testdata_project2/foobar4000.h Normal file
View File

@@ -0,0 +1,10 @@
#ifndef __FOOBAR4000_H__
#define __FOOBAR4000_H__
class Baz {
public:
Baz() {}
};
#endif /* __FOOBAR4000_H__ */