Commit f3186690 authored by Nikolai Kosjar's avatar Nikolai Kosjar Committed by Erik Verbruggen
Browse files

CppEditor: Improve finding position for new includes



...by detecting include groups (separated by new lines, include types
and same dir prefix).

Task-number: QTCREATORBUG-9317
Change-Id: I73e80fdc715104901cb2d4f5b15b4cab5d04d305
Reviewed-by: default avatarErik Verbruggen <erik.verbruggen@digia.com>
parent e3bc84c4
......@@ -162,14 +162,28 @@ private slots:
void test_quickfix_InsertDeclFromDef();
void test_quickfix_AddIncludeForUndefinedIdentifier_detectIncludeGroupsByNewLines();
void test_quickfix_AddIncludeForUndefinedIdentifier_detectIncludeGroupsByIncludeDir();
void test_quickfix_AddIncludeForUndefinedIdentifier_detectIncludeGroupsByIncludeType();
void test_quickfix_AddIncludeForUndefinedIdentifier_normal();
void test_quickfix_AddIncludeForUndefinedIdentifier_ignoremoc();
void test_quickfix_AddIncludeForUndefinedIdentifier_sortingTop();
void test_quickfix_AddIncludeForUndefinedIdentifier_sortingMiddle();
void test_quickfix_AddIncludeForUndefinedIdentifier_sortingBottom();
void test_quickfix_AddIncludeForUndefinedIdentifier_appendToUnsorted();
void test_quickfix_AddIncludeForUndefinedIdentifier_firstLocalIncludeAtFront();
void test_quickfix_AddIncludeForUndefinedIdentifier_firstGlobalIncludeAtBack();
void test_quickfix_AddIncludeForUndefinedIdentifier_preferGroupWithLongerMatchingPrefix();
void test_quickfix_AddIncludeForUndefinedIdentifier_newGroupIfOnlyDifferentIncludeDirs();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedDirsSorted();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedDirsUnsorted();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedIncludeTypes1();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedIncludeTypes2();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedIncludeTypes3();
void test_quickfix_AddIncludeForUndefinedIdentifier_mixedIncludeTypes4();
void test_quickfix_AddIncludeForUndefinedIdentifier_noinclude();
void test_quickfix_AddIncludeForUndefinedIdentifier_noincludeComment01();
void test_quickfix_AddIncludeForUndefinedIdentifier_noincludeComment02();
void test_quickfix_AddIncludeForUndefinedIdentifier_veryFirstIncludeCppStyleCommentOnTop();
void test_quickfix_AddIncludeForUndefinedIdentifier_veryFirstIncludeCStyleCommentOnTop();
void test_quickfix_MoveFuncDefOutside_MemberFuncToCpp();
void test_quickfix_MoveFuncDefOutside_MemberFuncOutside();
......
This diff is collapsed.
......@@ -40,6 +40,7 @@
#include <cpptools/cpppointerdeclarationformatter.h>
#include <cpptools/cpptoolsconstants.h>
#include <cpptools/cpptoolsreuse.h>
#include <cpptools/includeutils.h>
#include <cpptools/insertionpointlocator.h>
#include <cpptools/symbolfinder.h>
......@@ -182,6 +183,37 @@ Class *isMemberFunction(const LookupContext &context, Function *function)
return 0;
}
// Given include is e.g. "afile.h" or <afile.h> (quotes/angle brackets included!).
void insertNewIncludeDirective(const QString &include, CppRefactoringFilePtr file)
{
// Find optimal position
using namespace IncludeUtils;
LineForNewIncludeDirective finder(file->document(), file->cppDocument()->includes(),
LineForNewIncludeDirective::IgnoreMocIncludes,
LineForNewIncludeDirective::AutoDetect);
unsigned newLinesToPrepend = 0;
unsigned newLinesToAppend = 0;
const int insertLine = finder(include, &newLinesToPrepend, &newLinesToAppend);
QTC_ASSERT(insertLine >= 1, return);
const int insertPosition = file->position(insertLine, 1);
QTC_ASSERT(insertPosition >= 0, return);
// Construct text to insert
const QString includeLine = QLatin1String("#include ") + include + QLatin1Char('\n');
QString prependedNewLines, appendedNewLines;
while (newLinesToAppend--)
appendedNewLines += QLatin1String("\n");
while (newLinesToPrepend--)
prependedNewLines += QLatin1String("\n");
const QString textToInsert = prependedNewLines + includeLine + appendedNewLines;
// Insert
ChangeSet changes;
changes.insert(insertPosition, textToInsert);
file->setChangeSet(changes);
file->apply();
}
} // anonymous namespace
namespace {
......@@ -1479,24 +1511,8 @@ public:
if (best.isEmpty())
best = headerFile;
int pos = currentFile->startOf(1);
unsigned currentLine = currentFile->cursor().blockNumber() + 1;
unsigned bestLine = 0;
foreach (const Document::Include &incl,
assistInterface()->semanticInfo().doc->includes()) {
if (incl.line() < currentLine)
bestLine = incl.line();
}
if (bestLine)
pos = currentFile->document()->findBlockByNumber(bestLine).position();
ChangeSet changes;
changes.insert(pos, QLatin1String("#include <")
+ QFileInfo(best).fileName() + QLatin1String(">\n"));
currentFile->setChangeSet(changes);
currentFile->apply();
const QString include = QString::fromLatin1("<%1>").arg(QFileInfo(best).fileName());
insertNewIncludeDirective(include, currentFile);
}
}
......@@ -1731,108 +1747,21 @@ void ConvertToCamelCase::match(const CppQuickFixInterface &interface, QuickFixOp
}
}
namespace {
class AddIncludeForUndefinedIdentifierOp: public CppQuickFixOperation
AddIncludeForUndefinedIdentifierOp::AddIncludeForUndefinedIdentifierOp(
const CppQuickFixInterface &interface, int priority, const QString &include)
: CppQuickFixOperation(interface, priority)
, m_include(include)
{
public:
AddIncludeForUndefinedIdentifierOp(const CppQuickFixInterface &interface, int priority,
const QString &include)
: CppQuickFixOperation(interface, priority)
, m_include(include)
{
setDescription(QApplication::translate("CppTools::QuickFix",
"Add #include %1").arg(m_include));
}
void perform()
{
CppRefactoringChanges refactoring(snapshot());
CppRefactoringFilePtr file = refactoring.file(fileName());
QList<Document::Include> includes = file->cppDocument()->includes();
if (!includes.isEmpty()) {
QHash<QString, unsigned> includePositions;
foreach (const Document::Include &include, includes) {
const QString fileName = include.unresolvedFileName();
if (fileName.endsWith(QLatin1String(".moc")))
continue;
includePositions.insert(fileName, include.line());
}
if (!includePositions.isEmpty()) {
const QString include = m_include.mid(1, m_include.length() - 2);
QList<QString> keys = includePositions.keys();
keys << include;
qSort(keys);
const int pos = keys.indexOf(include);
ChangeSet changes;
if (pos + 1 != keys.count()) {
const int insertPos = qMax(0, file->position(
includePositions.value(keys.at(pos + 1)), 1));
changes.insert(insertPos,
QLatin1String("#include ") + m_include + QLatin1String("\n"));
} else {
const int insertPos = qMax(0, file->position(includePositions.value(
keys.at(pos - 1)) + 1, 1) - 1);
changes.insert(insertPos, QLatin1String("\n#include ") + m_include);
}
file->setChangeSet(changes);
file->apply();
return;
}
}
// No includes or no matching include, find possible first/multi line comment
int insertPos = 0;
QTextBlock block = file->document()->firstBlock();
while (block.isValid()) {
const QString trimmedText = block.text().trimmed();
// Only skip the first comment!
if (trimmedText.startsWith(QLatin1String("/*"))) {
do {
const int pos = block.text().indexOf(QLatin1String("*/"));
if (pos > -1) {
insertPos = block.position() + pos + 2;
break;
}
block = block.next();
} while (block.isValid());
break;
} else if (trimmedText.startsWith(QLatin1String("//"))) {
block = block.next();
while (block.isValid()) {
if (!block.text().trimmed().startsWith(QLatin1String("//"))) {
insertPos = block.position() - 1;
break;
}
block = block.next();
}
break;
}
if (!trimmedText.isEmpty())
break;
block = block.next();
}
ChangeSet changes;
if (insertPos != 0)
changes.insert(insertPos, QLatin1String("\n\n#include ") + m_include);
else
changes.insert(insertPos, QString::fromLatin1("#include %1\n\n").arg(m_include));
file->setChangeSet(changes);
file->apply();
}
setDescription(QApplication::translate("CppTools::QuickFix", "Add #include %1").arg(m_include));
}
private:
QString m_include;
};
void AddIncludeForUndefinedIdentifierOp::perform()
{
CppRefactoringChanges refactoring(snapshot());
CppRefactoringFilePtr file = refactoring.file(fileName());
} // anonymous namespace
insertNewIncludeDirective(m_include, file);
}
void AddIncludeForUndefinedIdentifier::match(const CppQuickFixInterface &interface,
QuickFixOperations &result)
......
......@@ -71,6 +71,18 @@ public:
void match(const CppQuickFixInterface &interface, QuickFixOperations &result);
};
// Exposed for tests
class AddIncludeForUndefinedIdentifierOp: public CppQuickFixOperation
{
public:
AddIncludeForUndefinedIdentifierOp(const CppQuickFixInterface &interface, int priority,
const QString &include);
void perform();
private:
QString m_include;
};
/*!
Can be triggered on a class forward declaration to add the matching #include.
......@@ -493,6 +505,7 @@ public:
/*!
Insert (pure) virtual functions of a base class.
Exposed for tests.
*/
class InsertVirtualMethodsDialog : public QDialog
{
......
......@@ -124,6 +124,12 @@ public:
return m_includePaths;
}
// Use this *only* for auto tests
void setIncludePaths(const QStringList &includePaths)
{
m_includePaths = includePaths;
}
QStringList frameworkPaths()
{
ensureUpdated();
......
......@@ -45,7 +45,8 @@ HEADERS += completionsettingspage.h \
builtinindexingsupport.h \
cpppointerdeclarationformatter.h \
cppprojectfile.h \
cpppreprocessor.h
cpppreprocessor.h \
includeutils.h
SOURCES += completionsettingspage.cpp \
cppclassesfilter.cpp \
......@@ -89,7 +90,8 @@ SOURCES += completionsettingspage.cpp \
builtinindexingsupport.cpp \
cpppointerdeclarationformatter.cpp \
cppprojectfile.cpp \
cpppreprocessor.cpp
cpppreprocessor.cpp \
includeutils.cpp
FORMS += completionsettingspage.ui \
cppfilesettingspage.ui \
......
......@@ -107,7 +107,9 @@ QtcPlugin {
"builtinindexingsupport.cpp",
"builtinindexingsupport.h",
"cpppreprocessor.cpp",
"cpppreprocessor.h"
"cpppreprocessor.h",
"includeutils.cpp",
"includeutils.h"
]
Group {
......
/****************************************************************************
**
** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
** Contact: http://www.qt-project.org/legal
**
** This file is part of Qt Creator.
**
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and Digia. For licensing terms and
** conditions see http://qt.digia.com/licensing. For further information
** use the contact form at http://qt.digia.com/contact-us.
**
** GNU Lesser General Public License Usage
** Alternatively, this file may be used under the terms of the GNU Lesser
** General Public License version 2.1 as published by the Free Software
** Foundation and appearing in the file LICENSE.LGPL included in the
** packaging of this file. Please review the following information to
** ensure the GNU Lesser General Public License version 2.1 requirements
** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
**
** In addition, as a special exception, Digia gives you certain additional
** rights. These rights are described in the Digia Qt LGPL Exception
** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
**
****************************************************************************/
#include "includeutils.h"
#include <cplusplus/pp-engine.h>
#include <cplusplus/PreprocessorClient.h>
#include <cplusplus/PreprocessorEnvironment.h>
#include <utils/stringutils.h>
#include <QDir>
#include <QFileInfo>
#include <QStringList>
#include <QTextBlock>
#include <QTextDocument>
using namespace CPlusPlus;
using namespace CppTools;
using namespace CppTools::IncludeUtils;
using namespace Utils;
static bool includeLineLessThan(const Include &left, const Include &right)
{ return left.line() < right.line(); }
static bool includeFileNamelessThen(const Include & left, const Include & right)
{ return left.unresolvedFileName() < right.unresolvedFileName(); }
LineForNewIncludeDirective::LineForNewIncludeDirective(const QTextDocument *textDocument,
QList<Document::Include> includes,
MocIncludeMode mocIncludeMode,
IncludeStyle includeStyle)
: m_textDocument(textDocument)
, m_includeStyle(includeStyle)
{
// Ignore *.moc includes if requested
if (mocIncludeMode == IgnoreMocIncludes) {
foreach (const Document::Include &include, includes) {
if (!include.unresolvedFileName().endsWith(QLatin1String(".moc")))
m_includes << include;
}
} else {
m_includes = includes;
}
// TODO: Remove this filter loop once FastPreprocessor::sourceNeeded does not add
// extra includes anymore.
for (int i = m_includes.count() - 1; i >= 0; --i) {
if (!QFileInfo(m_includes.at(i).resolvedFileName()).isAbsolute())
m_includes.removeAt(i);
}
// Detect include style
if (m_includeStyle == AutoDetect) {
unsigned timesIncludeStyleChanged = 0;
if (m_includes.isEmpty() || m_includes.size() == 1) {
m_includeStyle = LocalBeforeGlobal; // Fallback
} else {
for (int i = 1, size = m_includes.size(); i < size; ++i) {
if (m_includes.at(i - 1).type() != m_includes.at(i).type()) {
if (++timesIncludeStyleChanged > 1)
break;
}
}
if (timesIncludeStyleChanged == 1) {
m_includeStyle = m_includes.first().type() == Client::IncludeLocal
? LocalBeforeGlobal
: GlobalBeforeLocal;
} else {
m_includeStyle = LocalBeforeGlobal; // Fallback
}
}
}
}
int LineForNewIncludeDirective::operator()(const QString &newIncludeFileName,
unsigned *newLinesToPrepend,
unsigned *newLinesToAppend)
{
if (newLinesToPrepend)
*newLinesToPrepend = false;
if (newLinesToAppend)
*newLinesToAppend = false;
const QString pureIncludeFileName = newIncludeFileName.mid(1, newIncludeFileName.length() - 2);
const CPlusPlus::Client::IncludeType newIncludeType =
newIncludeFileName.startsWith(QLatin1Char('"')) ? Client::IncludeLocal
: Client::IncludeGlobal;
// Handle no includes
if (m_includes.empty()) {
unsigned insertLine = 0;
QTextBlock block = m_textDocument->firstBlock();
while (block.isValid()) {
const QString trimmedText = block.text().trimmed();
// Only skip the first comment!
if (trimmedText.startsWith(QLatin1String("/*"))) {
do {
const int pos = block.text().indexOf(QLatin1String("*/"));
if (pos > -1) {
insertLine = block.blockNumber() + 2;
break;
}
block = block.next();
} while (block.isValid());
break;
} else if (trimmedText.startsWith(QLatin1String("//"))) {
block = block.next();
while (block.isValid()) {
if (!block.text().trimmed().startsWith(QLatin1String("//"))) {
insertLine = block.blockNumber() + 1;
break;
}
block = block.next();
}
break;
}
if (!trimmedText.isEmpty())
break;
block = block.next();
}
if (insertLine == 0) {
if (newLinesToAppend)
*newLinesToAppend += 1;
insertLine = 1;
} else {
if (newLinesToPrepend)
*newLinesToPrepend = 1;
}
return insertLine;
}
typedef QList<IncludeGroup> IncludeGroups;
const IncludeGroups groupsNewline = IncludeGroup::detectIncludeGroupsByNewLines(m_includes);
const bool includeAtTop
= (newIncludeType == Client::IncludeLocal && m_includeStyle == LocalBeforeGlobal)
|| (newIncludeType == Client::IncludeGlobal && m_includeStyle == GlobalBeforeLocal);
IncludeGroup bestGroup = includeAtTop ? groupsNewline.first() : groupsNewline.last();
IncludeGroups groupsMatchingIncludeType = getGroupsByIncludeType(groupsNewline, newIncludeType);
if (groupsMatchingIncludeType.isEmpty()) {
const IncludeGroups groupsMixedIncludeType
= IncludeGroup::filterMixedIncludeGroups(groupsNewline);
// case: The new include goes into an own include group
if (groupsMixedIncludeType.isEmpty()) {
return includeAtTop
? IncludeGroup::lineForPrependedIncludeGroup(groupsNewline, newLinesToAppend)
: IncludeGroup::lineForAppendedIncludeGroup(groupsNewline, newLinesToPrepend);
// case: add to mixed group
} else {
const IncludeGroup bestMixedGroup = groupsMixedIncludeType.last(); // TODO: flaterize
const IncludeGroups groupsIncludeType
= IncludeGroup::detectIncludeGroupsByIncludeType(bestMixedGroup.includes());
groupsMatchingIncludeType = getGroupsByIncludeType(groupsIncludeType, newIncludeType);
// Avoid extra new lines for include groups which are not separated by new lines
newLinesToPrepend = 0;
newLinesToAppend = 0;
}
}
IncludeGroups groupsSameIncludeDir;
IncludeGroups groupsMixedIncludeDirs;
foreach (const IncludeGroup &group, groupsMatchingIncludeType) {
if (group.hasCommonIncludeDir())
groupsSameIncludeDir << group;
else
groupsMixedIncludeDirs << group;
}
IncludeGroups groupsMatchingIncludeDir;
foreach (const IncludeGroup &group, groupsSameIncludeDir) {
if (group.commonIncludeDir() == IncludeGroup::includeDir(pureIncludeFileName))
groupsMatchingIncludeDir << group;
}
// case: There are groups with a matching include dir, insert the new include
// at the best position of the best group
if (!groupsMatchingIncludeDir.isEmpty()) {
// The group with the longest common matching prefix is the best group
int longestPrefixSoFar = 0;
foreach (const IncludeGroup &group, groupsMatchingIncludeDir) {
const int groupPrefixLength = group.commonPrefix().length();
if (groupPrefixLength >= longestPrefixSoFar) {
bestGroup = group;
longestPrefixSoFar = groupPrefixLength;
}
}
} else {
// case: The new include goes into an own include group
if (groupsMixedIncludeDirs.isEmpty()) {
if (includeAtTop) {
return groupsSameIncludeDir.isEmpty()
? IncludeGroup::lineForPrependedIncludeGroup(groupsNewline, newLinesToAppend)
: IncludeGroup::lineForAppendedIncludeGroup(groupsSameIncludeDir, newLinesToPrepend);
} else {
return IncludeGroup::lineForAppendedIncludeGroup(groupsNewline, newLinesToPrepend);
}
// case: The new include is inserted at the best position of the best
// group with mixed include dirs
} else {
IncludeGroups groupsIncludeDir;
foreach (const IncludeGroup &group, groupsMixedIncludeDirs) {
groupsIncludeDir.append(
IncludeGroup::detectIncludeGroupsByIncludeDir(group.includes()));
}
IncludeGroup localBestIncludeGroup = IncludeGroup(QList<Include>());
foreach (const IncludeGroup &group, groupsIncludeDir) {
if (group.commonIncludeDir() == IncludeGroup::includeDir(pureIncludeFileName))
localBestIncludeGroup = group;
}
if (!localBestIncludeGroup.isEmpty()) {
bestGroup = localBestIncludeGroup;
} else {
bestGroup = groupsMixedIncludeDirs.last();
}
}
}
return bestGroup.lineForNewInclude(pureIncludeFileName, newIncludeType);
}
QList<IncludeGroup> LineForNewIncludeDirective::getGroupsByIncludeType(
const QList<IncludeGroup> &groups, IncludeType includeType)
{
return includeType == Client::IncludeLocal
? IncludeGroup::filterIncludeGroups(groups, Client::IncludeLocal)
: IncludeGroup::filterIncludeGroups(groups, Client::IncludeGlobal);
}
/// includes will be modified!
QList<IncludeGroup> IncludeGroup::detectIncludeGroupsByNewLines(QList<Document::Include> &includes)
{
// Sort by line
qSort(includes.begin(), includes.end(), includeLineLessThan);
// Create groups
QList<IncludeGroup> result;
unsigned lastLine = 0;
QList<Include> currentIncludes;
bool isFirst = true;
foreach (const Include &include, includes) {
// First include...
if (isFirst) {
isFirst = false;
currentIncludes << include;
}
// Include belongs to current group
else if (lastLine + 1 == include.line()) {
currentIncludes << include;
}
// Include is member of new group
else {
result << IncludeGroup(currentIncludes);
currentIncludes.clear();
currentIncludes << include;
}
lastLine = include.line();
}
if (!currentIncludes.isEmpty())