From c95b324b79cb66a2dd0e128d8b41331af6e17c3c Mon Sep 17 00:00:00 2001
From: Lorenz Haas <lykurg@gmail.com>
Date: Tue, 4 Jun 2013 17:37:24 +0200
Subject: [PATCH] CppEditor: Same insert position for quick fixes
Now definitions are inserted at the same position inside the
implementation file for MoveFuncDefOutside and InsertDefFromDecl.
Task-number: QTCREATORBUG-9389
Change-Id: If823ffd15ec39a7bc2edb53519380cb9cabb4c55
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com>
---
src/plugins/cppeditor/cppeditorplugin.h | 3 +-
src/plugins/cppeditor/cppquickfix_test.cpp | 76 ++++++++++----
src/plugins/cppeditor/cppquickfixes.cpp | 114 ++++++++++++---------
3 files changed, 125 insertions(+), 68 deletions(-)
diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h
index a18c528e2a9..c0fb4094c69 100644
--- a/src/plugins/cppeditor/cppeditorplugin.h
+++ b/src/plugins/cppeditor/cppeditorplugin.h
@@ -186,7 +186,8 @@ private slots:
void test_quickfix_AddIncludeForUndefinedIdentifier_veryFirstIncludeCStyleCommentOnTop();
void test_quickfix_MoveFuncDefOutside_MemberFuncToCpp();
- void test_quickfix_MoveFuncDefOutside_MemberFuncOutside();
+ void test_quickfix_MoveFuncDefOutside_MemberFuncOutside1();
+ void test_quickfix_MoveFuncDefOutside_MemberFuncOutside2();
void test_quickfix_MoveFuncDefOutside_MemberFuncToCppNS();
void test_quickfix_MoveFuncDefOutside_MemberFuncToCppNSUsing();
void test_quickfix_MoveFuncDefOutside_MemberFuncOutsideWithNs();
diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp
index e93bf76267b..1ce9ed1aa7f 100644
--- a/src/plugins/cppeditor/cppquickfix_test.cpp
+++ b/src/plugins/cppeditor/cppquickfix_test.cpp
@@ -760,15 +760,14 @@ void CppEditorPlugin::test_quickfix_InsertDefFromDecl_basic()
"struct Foo\n"
"{\n"
" Foo();@\n"
- "};\n"
- "\n"
- ;
+ "};\n";
const QByteArray expected = original +
+ "\n"
+ "\n"
"Foo::Foo()\n"
"{\n\n"
"}\n"
- "\n\n"
- ;
+ "\n";
InsertDefFromDecl factory;
TestCase data(original, expected);
@@ -826,6 +825,7 @@ void CppEditorPlugin::test_quickfix_InsertDefFromDecl_headerSource_basic2()
" Foo()@;\n"
"};\n";
expected = original +
+ "\n"
"\n"
"Foo::Foo()\n"
"{\n\n"
@@ -1768,8 +1768,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncToCpp()
// Source File
original =
- "#include \"file.h\"\n"
- "\n";
+ "#include \"file.h\"\n";
expected =
"#include \"file.h\"\n"
"\n"
@@ -1787,7 +1786,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncToCpp()
}
/// Check: Move definition outside class
-void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside()
+void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside1()
{
QByteArray original =
"class Foo {\n"
@@ -1814,7 +1813,6 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside()
" return 1;\n"
"}\n"
"\n"
- "\n"
"void Foo::f4() {}\n\n";
MoveFuncDefOutside factory;
@@ -1822,6 +1820,50 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside()
data.run(&factory);
}
+/// Check: Move definition outside class
+void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside2()
+{
+ QList<TestDocumentPtr> testFiles;
+ QByteArray original;
+ QByteArray expected;
+
+ // Header File
+ original =
+ "class Foo {\n"
+ " void f1();\n"
+ " int f2@()\n"
+ " {\n"
+ " return 1;\n"
+ " }\n"
+ " void f3();\n"
+ "};\n";
+ expected =
+ "class Foo {\n"
+ " void f1();\n"
+ " int f2();\n"
+ " void f3();\n"
+ "};\n"
+ "\n"
+ "\n"
+ "int Foo::f2()\n"
+ "{\n"
+ " return 1;\n"
+ "}\n\n";
+ testFiles << TestDocument::create(original, expected, QLatin1String("file.h"));
+
+ // Source File
+ original =
+ "#include \"file.h\"\n"
+ "void Foo::f1() {}\n"
+ "void Foo::f3() {}\n";
+ expected = original + "\n";
+ testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
+
+ MoveFuncDefOutside factory;
+ TestCase data(testFiles);
+ data.run(&factory, 1);
+}
+
/// Check: Move definition from header to cpp (with namespace).
void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncToCppNS()
{
@@ -1849,8 +1891,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncToCppNS()
// Source File
original =
- "#include \"file.h\"\n"
- "\n";
+ "#include \"file.h\"\n";
expected =
"#include \"file.h\"\n"
"\n"
@@ -1895,8 +1936,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncToCppNSUsing()
// Source File
original =
"#include \"file.h\"\n"
- "using namespace MyNs;\n"
- "\n";
+ "using namespace MyNs;\n";
expected =
"#include \"file.h\"\n"
"using namespace MyNs;\n"
@@ -1934,7 +1974,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutsideWithNs()
"int Foo::number() const\n"
"{\n"
" return 5;\n"
- "}"
+ "}\n"
"\n}\n";
MoveFuncDefOutside factory;
@@ -1962,8 +2002,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_FreeFuncToCpp()
// Source File
original =
- "#include \"file.h\"\n"
- "\n";
+ "#include \"file.h\"\n";
expected =
"#include \"file.h\"\n"
"\n"
@@ -2004,8 +2043,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_FreeFuncToCppNS()
// Source File
original =
- "#include \"file.h\"\n"
- "\n";
+ "#include \"file.h\"\n";
expected =
"#include \"file.h\"\n"
"\n"
@@ -2053,6 +2091,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_CtorWithInitialization1()
expected =
"#include \"file.h\"\n"
"\n"
+ "\n"
"Foo::Foo() : a(42), b(3.141) {}\n"
"\n";
testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp"));
@@ -2096,6 +2135,7 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_CtorWithInitialization2()
expected =
"#include \"file.h\"\n"
"\n"
+ "\n"
"Foo::Foo() : member(2)\n"
"{\n"
"}\n"
diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp
index ac629d14fa2..50aaf7b494b 100644
--- a/src/plugins/cppeditor/cppquickfixes.cpp
+++ b/src/plugins/cppeditor/cppquickfixes.cpp
@@ -143,6 +143,37 @@ enum DefPos {
DefPosImplementationFile
};
+InsertionLocation insertLocationForMethodDefinition(Symbol *symbol,
+ CppRefactoringChanges& refactoring,
+ const QString& fileName)
+{
+ QTC_ASSERT(symbol, return InsertionLocation());
+
+ // Try to find optimal location
+ const InsertionPointLocator locator(refactoring);
+ const QList<InsertionLocation> list = locator.methodDefinition(symbol, symbol->asDeclaration(),
+ fileName);
+ for (int i = 0; i < list.count(); ++i) {
+ InsertionLocation location = list.at(i);
+ if (location.isValid() && location.fileName() == fileName) {
+ return location;
+ break;
+ }
+ }
+
+ // ...failed, so return location at end of file
+ CppRefactoringFilePtr file = refactoring.file(fileName);
+ const QTextDocument *doc = file->document();
+ int pos = qMax(0, doc->characterCount() - 1);
+
+ //TODO watch for matching namespace
+ //TODO watch for moc-includes
+
+ unsigned line, column;
+ file->lineAndColumn(pos, &line, &column);
+ return InsertionLocation(fileName, QLatin1String("\n\n"), QLatin1String("\n"), line, column);
+}
+
inline bool isQtStringLiteral(const QByteArray &id)
{
return id == "QLatin1String" || id == "QLatin1Literal" || id == "QStringLiteral";
@@ -2549,11 +2580,6 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
if (func->isSignal())
return;
- InsertDefOperation *op = 0;
- bool isHeaderFile = false;
- const QString cppFileName = correspondingHeaderOrSource(
- interface->fileName(), &isHeaderFile);
-
// Check if there is already a definition
CppTools::SymbolFinder symbolFinder;
if (symbolFinder.findMatchingDefinition(decl, interface->snapshot(),
@@ -2561,18 +2587,21 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
return;
}
+ InsertDefOperation *op = 0;
+ bool isHeaderFile = false;
+ const QString cppFileName = correspondingHeaderOrSource(
+ interface->fileName(), &isHeaderFile);
+ InsertionLocation loc;
+
// Insert Position: Implementation File
if (isHeaderFile && !cppFileName.isEmpty()) {
CppRefactoringChanges refactoring(interface->snapshot());
- InsertionPointLocator locator(refactoring);
- foreach (const InsertionLocation &loc,
- locator.methodDefinition(decl)) {
- if (loc.isValid()) {
- op = new InsertDefOperation(interface, decl, loc,
- DefPosImplementationFile);
- result.append(CppQuickFixOperation::Ptr(op));
- break;
- }
+ loc = insertLocationForMethodDefinition(decl, refactoring,
+ cppFileName);
+ if (loc.isValid()) {
+ op = new InsertDefOperation(interface, decl, loc,
+ DefPosImplementationFile);
+ result.append(CppQuickFixOperation::Ptr(op));
}
}
@@ -2581,8 +2610,8 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
unsigned line, column;
if (func->enclosingClass() == 0) {
file->lineAndColumn(file->endOf(simpleDecl), &line, &column);
- InsertionLocation loc(interface->fileName(), QLatin1String(""),
- QLatin1String(""), line, column);
+ loc = InsertionLocation(interface->fileName(), QLatin1String(""),
+ QLatin1String(""), line, column);
op = new InsertDefOperation(interface, decl, loc,
DefPosInsideClass, true);
result.append(CppQuickFixOperation::Ptr(op));
@@ -2590,25 +2619,19 @@ void InsertDefFromDecl::match(const CppQuickFixInterface &interface, QuickFixOpe
}
// Insert Position: Outside Class
- --idx;
- for (; idx >= 0; --idx) {
- AST *node = path.at(idx);
- if (ClassSpecifierAST *ast = node->asClassSpecifier()) {
- file->lineAndColumn(file->endOf(ast), &line, &column);
- InsertionLocation loc(interface->fileName(),
- QLatin1String("\n\n"), QLatin1String(""),
- line, column + 1); // include ';'
- op = new InsertDefOperation(interface, decl, loc,
- DefPosOutsideClass);
- result.append(CppQuickFixOperation::Ptr(op));
- break;
- }
+ CppRefactoringChanges refactoring(interface->snapshot());
+ loc = insertLocationForMethodDefinition(decl, refactoring,
+ interface->fileName());
+ if (loc.isValid()) {
+ op = new InsertDefOperation(interface, decl, loc,
+ DefPosOutsideClass);
+ result.append(CppQuickFixOperation::Ptr(op));
}
// Insert Position: Inside Class
file->lineAndColumn(file->endOf(simpleDecl), &line, &column);
- InsertionLocation loc(interface->fileName(), QLatin1String(""),
- QLatin1String(""), line, column);
+ loc = InsertionLocation(interface->fileName(), QLatin1String(""),
+ QLatin1String(""), line, column);
op = new InsertDefOperation(interface, decl, loc, DefPosInsideClass);
result.append(CppQuickFixOperation::Ptr(op));
return;
@@ -3726,32 +3749,25 @@ public:
{
CppRefactoringChanges refactoring(snapshot());
CppRefactoringFilePtr fromFile = refactoring.file(m_headerFileName);
- CppRefactoringFilePtr toFile;
- int insertPos = 0;
- Scope *scopeAtInsertPos = 0;
+ CppRefactoringFilePtr toFile = (m_type == MoveOutside) ? fromFile
+ : refactoring.file(m_cppFileName);
// Determine file, insert position and scope
- if (m_type == MoveOutside) {
- toFile = fromFile;
- insertPos = toFile->endOf(m_classAST);
- scopeAtInsertPos = m_func->enclosingScope()->enclosingScope();
- } else {
- toFile = refactoring.file(m_cppFileName);
- const QTextDocument *doc = toFile->document();
- insertPos = qMax(0, doc->characterCount() - 1);
- scopeAtInsertPos = toFile->cppDocument()->scopeAt(doc->lineCount(),
- doc->lastBlock().length());
- }
+ InsertionLocation l = insertLocationForMethodDefinition(m_func, refactoring,
+ toFile->fileName());
+ const QString prefix = l.prefix();
+ const QString suffix = l.suffix();
+ const int insertPos = toFile->position(l.line(), l.column());
+ Scope *scopeAtInsertPos = toFile->cppDocument()->scopeAt(l.line(), l.column());
// construct definition
const QString funcDec = getDefinitionSignature(assistInterface(), m_func, toFile,
scopeAtInsertPos);
- QString funcDef = QLatin1String("\n") + funcDec;
+ QString funcDef = prefix + funcDec;
const int startPosition = fromFile->endOf(m_funcDef->declarator);
const int endPosition = fromFile->endOf(m_funcDef->function_body);
- funcDef += fromFile->textOf(startPosition, endPosition) + QLatin1String("\n");
- if (m_cppFileName.isEmpty() || !m_insideHeader)
- funcDef = QLatin1String("\n") + funcDef;
+ funcDef += fromFile->textOf(startPosition, endPosition);
+ funcDef += suffix;
// insert definition at new position
ChangeSet cppChanges;
--
GitLab