From 8f4c4b41f6dc93dd7afb1a349484a08588604056 Mon Sep 17 00:00:00 2001 From: Lorenz Haas Date: Fri, 7 Jun 2013 16:44:08 +0200 Subject: [PATCH] CppTools: Fix nextToSurroundingDefinitions Now it is checked if the adjacent declaration is also defined and one can define the destination file. Change-Id: Ifff59c49fc2ab3e2f36f41df42ae4b7d7ff8dd35 Reviewed-by: Nikolai Kosjar --- src/plugins/cppeditor/cppquickfix_test.cpp | 30 ++-- src/plugins/cpptools/cppcodegen_test.cpp | 147 ++++++++++++++++++ src/plugins/cpptools/cpptoolsplugin.h | 2 + .../cpptools/insertionpointlocator.cpp | 51 ++++-- src/plugins/cpptools/insertionpointlocator.h | 3 +- 5 files changed, 205 insertions(+), 28 deletions(-) diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index c67dce2680..e93bf76267 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -1791,21 +1791,31 @@ void CppEditorPlugin::test_quickfix_MoveFuncDefOutside_MemberFuncOutside() { QByteArray original = "class Foo {\n" - " inline int numbe@r() const\n" - " {\n" - " return 5;\n" - " }\n" - "};"; + " void f1();\n" + " inline int f2@() const\n" + " {\n" + " return 1;\n" + " }\n" + " void f3();\n" + " void f4();\n" + "};\n" + "\n" + "void Foo::f4() {}\n"; QByteArray expected = "class Foo {\n" - " inline int number() const;\n" + " void f1();\n" + " inline int f2@() const;\n" + " void f3();\n" + " void f4();\n" "};\n" "\n" - "int Foo::number() const\n" + "int Foo::f2() const\n" "{\n" - " return 5;\n" - "}" - "\n\n"; + " return 1;\n" + "}\n" + "\n" + "\n" + "void Foo::f4() {}\n\n"; MoveFuncDefOutside factory; TestCase data(original, expected); diff --git a/src/plugins/cpptools/cppcodegen_test.cpp b/src/plugins/cpptools/cppcodegen_test.cpp index e4bea891a4..7344abe7dd 100644 --- a/src/plugins/cpptools/cppcodegen_test.cpp +++ b/src/plugins/cpptools/cppcodegen_test.cpp @@ -621,3 +621,150 @@ void CppToolsPlugin::test_codegen_definition_middle_member() QCOMPARE(loc.prefix(), QLatin1String("\n\n")); QCOMPARE(loc.suffix(), QString()); } + +void CppToolsPlugin::test_codegen_definition_middle_member_surrounded_by_undefined() +{ + const QByteArray srcText = "\n" + "class Foo\n" // line 1 + "{\n" + "void foo();\n" // line 3 + "void bar();\n" // line 4 + "void baz();\n" // line 5 + "void car();\n" // line 6 + "};\n" + "\n"; + + const QByteArray dstText = QString::fromLatin1( + "\n" + "#include \"%1/file.h\"\n" // line 1 + "int x;\n" + "\n" + "void Foo::car()\n" // line 4 + "{\n" + "\n" + "}\n" + "\n" + "int y;\n").arg(QDir::tempPath()).toLatin1(); + + Document::Ptr src = Document::create(QDir::tempPath() + QLatin1String("/file.h")); + Utils::FileSaver srcSaver(src->fileName()); + srcSaver.write(srcText); + srcSaver.finalize(); + src->setUtf8Source(srcText); + src->parse(); + src->check(); + QCOMPARE(src->diagnosticMessages().size(), 0); + QCOMPARE(src->globalSymbolCount(), 1U); + + Document::Ptr dst = Document::create(QDir::tempPath() + QLatin1String("/file.cpp")); + dst->addIncludeFile(Document::Include(QLatin1String("file.h"), src->fileName(), 1, + Client::IncludeLocal)); + Utils::FileSaver dstSaver(dst->fileName()); + dstSaver.write(dstText); + dstSaver.finalize(); + dst->setUtf8Source(dstText); + dst->parse(); + dst->check(); + QCOMPARE(dst->diagnosticMessages().size(), 0); + QCOMPARE(dst->globalSymbolCount(), 3U); + + Snapshot snapshot; + snapshot.insert(src); + snapshot.insert(dst); + + Class *foo = src->globalSymbolAt(0)->asClass(); + QVERIFY(foo); + QCOMPARE(foo->line(), 1U); + QCOMPARE(foo->column(), 7U); + QCOMPARE(foo->memberCount(), 4U); + Declaration *decl = foo->memberAt(1)->asDeclaration(); + QVERIFY(decl); + QCOMPARE(decl->line(), 4U); + QCOMPARE(decl->column(), 6U); + + CppRefactoringChanges changes(snapshot); + InsertionPointLocator find(changes); + QList locList = find.methodDefinition(decl); + QVERIFY(locList.size() == 1); + InsertionLocation loc = locList.first(); + QCOMPARE(loc.fileName(), dst->fileName()); + QCOMPARE(loc.line(), 4U); + QCOMPARE(loc.column(), 1U); + QCOMPARE(loc.prefix(), QString()); + QCOMPARE(loc.suffix(), QLatin1String("\n\n")); +} + +void CppToolsPlugin::test_codegen_definition_member_specific_file() +{ + const QByteArray srcText = "\n" + "class Foo\n" // line 1 + "{\n" + "void foo();\n" // line 3 + "void bar();\n" // line 4 + "void baz();\n" // line 5 + "};\n" + "\n" + "void Foo::bar()\n" + "{\n" + "\n" + "}\n"; + + const QByteArray dstText = QString::fromLatin1( + "\n" + "#include \"%1/file.h\"\n" // line 1 + "int x;\n" + "\n" + "void Foo::foo()\n" // line 4 + "{\n" + "\n" + "}\n" // line 7 + "\n" + "int y;\n").arg(QDir::tempPath()).toLatin1(); + + Document::Ptr src = Document::create(QDir::tempPath() + QLatin1String("/file.h")); + Utils::FileSaver srcSaver(src->fileName()); + srcSaver.write(srcText); + srcSaver.finalize(); + src->setUtf8Source(srcText); + src->parse(); + src->check(); + QCOMPARE(src->diagnosticMessages().size(), 0); + QCOMPARE(src->globalSymbolCount(), 2U); + + Document::Ptr dst = Document::create(QDir::tempPath() + QLatin1String("/file.cpp")); + dst->addIncludeFile(Document::Include(QLatin1String("file.h"), src->fileName(), 1, + Client::IncludeLocal)); + Utils::FileSaver dstSaver(dst->fileName()); + dstSaver.write(dstText); + dstSaver.finalize(); + dst->setUtf8Source(dstText); + dst->parse(); + dst->check(); + QCOMPARE(dst->diagnosticMessages().size(), 0); + QCOMPARE(dst->globalSymbolCount(), 3U); + + Snapshot snapshot; + snapshot.insert(src); + snapshot.insert(dst); + + Class *foo = src->globalSymbolAt(0)->asClass(); + QVERIFY(foo); + QCOMPARE(foo->line(), 1U); + QCOMPARE(foo->column(), 7U); + QCOMPARE(foo->memberCount(), 3U); + Declaration *decl = foo->memberAt(2)->asDeclaration(); + QVERIFY(decl); + QCOMPARE(decl->line(), 5U); + QCOMPARE(decl->column(), 6U); + + CppRefactoringChanges changes(snapshot); + InsertionPointLocator find(changes); + QList locList = find.methodDefinition(decl, true, dst->fileName()); + QVERIFY(locList.size() == 1); + InsertionLocation loc = locList.first(); + QCOMPARE(loc.fileName(), dst->fileName()); + QCOMPARE(loc.line(), 7U); + QCOMPARE(loc.column(), 2U); + QCOMPARE(loc.prefix(), QLatin1String("\n\n")); + QCOMPARE(loc.suffix(), QString()); +} diff --git a/src/plugins/cpptools/cpptoolsplugin.h b/src/plugins/cpptools/cpptoolsplugin.h index d0dadeb2c8..4c983bab6c 100644 --- a/src/plugins/cpptools/cpptoolsplugin.h +++ b/src/plugins/cpptools/cpptoolsplugin.h @@ -82,6 +82,8 @@ private slots: void test_codegen_definition_first_member(); void test_codegen_definition_last_member(); void test_codegen_definition_middle_member(); + void test_codegen_definition_middle_member_surrounded_by_undefined(); + void test_codegen_definition_member_specific_file(); void test_completion_forward_declarations_present(); void test_completion_inside_parentheses_c_style_conversion(); diff --git a/src/plugins/cpptools/insertionpointlocator.cpp b/src/plugins/cpptools/insertionpointlocator.cpp index f7118a2cfe..5170203463 100644 --- a/src/plugins/cpptools/insertionpointlocator.cpp +++ b/src/plugins/cpptools/insertionpointlocator.cpp @@ -470,7 +470,9 @@ static Declaration *isNonVirtualFunctionDeclaration(Symbol *s) return declaration; } -static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const CppRefactoringChanges &changes) +static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, + const CppRefactoringChanges &changes, + const QString& destinationFile) { InsertionLocation noResult; Class *klass = declaration->enclosingClass(); @@ -489,35 +491,47 @@ static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const if (declIndex == -1) return noResult; - // scan preceding declarations for a function declaration + // scan preceding declarations for a function declaration (and see if it is defined) + CppTools::SymbolFinder symbolFinder; + Function *definitionFunction = 0; QString prefix, suffix; Declaration *surroundingFunctionDecl = 0; for (int i = declIndex - 1; i >= 0; --i) { Symbol *s = klass->memberAt(i); surroundingFunctionDecl = isNonVirtualFunctionDeclaration(s); - if (surroundingFunctionDecl) { - prefix = QLatin1String("\n\n"); - break; + if (!surroundingFunctionDecl) + continue; + if ((definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl, + changes.snapshot()))) + { + if (destinationFile.isEmpty() || destinationFile == QString::fromUtf8( + definitionFunction->fileName(), definitionFunction->fileNameLength())) { + prefix = QLatin1String("\n\n"); + break; + } + definitionFunction = 0; } } - if (!surroundingFunctionDecl) { + if (!definitionFunction) { // try to find one below for (unsigned i = declIndex + 1; i < klass->memberCount(); ++i) { Symbol *s = klass->memberAt(i); surroundingFunctionDecl = isNonVirtualFunctionDeclaration(s); - if (surroundingFunctionDecl) { - suffix = QLatin1String("\n\n"); - break; + if (!surroundingFunctionDecl) + continue; + if ((definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl, + changes.snapshot()))) + { + if (destinationFile.isEmpty() || destinationFile == QString::fromUtf8( + definitionFunction->fileName(), definitionFunction->fileNameLength())) { + suffix = QLatin1String("\n\n"); + break; + } + definitionFunction = 0; } } - if (!surroundingFunctionDecl) - return noResult; } - // find the declaration's definition - CppTools::SymbolFinder symbolFinder; - Function *definitionFunction = symbolFinder.findMatchingDefinition(surroundingFunctionDecl, - changes.snapshot()); if (!definitionFunction) return noResult; @@ -546,7 +560,8 @@ static InsertionLocation nextToSurroundingDefinitions(Symbol *declaration, const } QList InsertionPointLocator::methodDefinition(Symbol *declaration, - bool useSymbolFinder) const + bool useSymbolFinder, + const QString &destinationFile) const { QList result; if (!declaration) @@ -558,7 +573,9 @@ QList InsertionPointLocator::methodDefinition(Symbol *declara return result; } - const InsertionLocation location = nextToSurroundingDefinitions(declaration, m_refactoringChanges); + const InsertionLocation location = nextToSurroundingDefinitions(declaration, + m_refactoringChanges, + destinationFile); if (location.isValid()) { result += location; return result; diff --git a/src/plugins/cpptools/insertionpointlocator.h b/src/plugins/cpptools/insertionpointlocator.h index f5ea46d26d..1d7065b389 100644 --- a/src/plugins/cpptools/insertionpointlocator.h +++ b/src/plugins/cpptools/insertionpointlocator.h @@ -99,7 +99,8 @@ public: AccessSpec xsSpec) const; QList methodDefinition(CPlusPlus::Symbol *declaration, - bool useSymbolFinder = true) const; + bool useSymbolFinder = true, + const QString &destinationFile = QString()) const; private: CppRefactoringChanges m_refactoringChanges; -- GitLab