From f1365d99fa517b2c948e2126de8caba64a83722b Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 30 Aug 2023 15:44:23 +0200 Subject: [PATCH] CppEditor: Add quickfix for moving function documentation ... between declaration and definition. Fixes: QTCREATORBUG-13877 Change-Id: If2a8977587ef2ac888e9c9dde5f63d222d96d964 Reviewed-by: David Schulz Reviewed-by: --- src/libs/utils/link.h | 15 +- src/plugins/cppeditor/cppquickfix_test.cpp | 151 +++++++++++++++++++ src/plugins/cppeditor/cppquickfix_test.h | 3 + src/plugins/cppeditor/cppquickfixes.cpp | 161 +++++++++++++++++++++ src/plugins/cppeditor/cppquickfixes.h | 8 + 5 files changed, 333 insertions(+), 5 deletions(-) diff --git a/src/libs/utils/link.h b/src/libs/utils/link.h index 00194654c97..57fb4c896fd 100644 --- a/src/libs/utils/link.h +++ b/src/libs/utils/link.h @@ -38,14 +38,19 @@ public: bool operator==(const Link &other) const { - return targetFilePath == other.targetFilePath - && targetLine == other.targetLine - && targetColumn == other.targetColumn - && linkTextStart == other.linkTextStart - && linkTextEnd == other.linkTextEnd; + return hasSameLocation(other) + && linkTextStart == other.linkTextStart + && linkTextEnd == other.linkTextEnd; } bool operator!=(const Link &other) const { return !(*this == other); } + bool hasSameLocation(const Link &other) const + { + return targetFilePath == other.targetFilePath + && targetLine == other.targetLine + && targetColumn == other.targetColumn; + } + friend size_t qHash(const Link &l, uint seed = 0) { return qHashMulti(seed, l.targetFilePath, l.targetLine, l.targetColumn); } diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 5558e56336c..0c4a07a2c27 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -9181,4 +9181,155 @@ void QuickfixTest::testChangeCommentType() &factory); } +void QuickfixTest::testMoveComments_data() +{ + QTest::addColumn("headers"); + QTest::addColumn("sources"); + + const QByteArrayList headersFuncDecl2Def{R"( +// Function comment +void @aFunction(); +)", R"( +void aFunction(); +)"}; + const QByteArrayList sourcesFuncDecl2Def{R"( +#include "file.h" + +void aFunction() {} +)", R"( +#include "file.h" + +// Function comment +void aFunction() {} +)"}; + QTest::newRow("function: from decl to def") << headersFuncDecl2Def << sourcesFuncDecl2Def; + + const QByteArrayList headersFuncDef2Decl{R"( +void aFunction(); +)", R"( +/* function */ +/* comment */ +void aFunction(); +)"}; + const QByteArrayList sourcesFuncDef2Decl{R"( +#include "file.h" + +/* function */ +/* comment */ +void a@Function() {} +)", R"( +#include "file.h" + +void aFunction() {} +)"}; + QTest::newRow("function: from def to decl") << headersFuncDef2Decl << sourcesFuncDef2Decl; + + const QByteArrayList headersFuncNoDef{R"( +// Function comment +void @aFunction(); +)", R"( +// Function comment +void aFunction(); +)"}; + QTest::newRow("function: no def") << headersFuncNoDef << QByteArrayList(); + + const QByteArrayList headersFuncNoDecl{R"( +// Function comment +inline void @aFunction() {} +)", R"( +// Function comment +inline void aFunction() {} +)"}; + QTest::newRow("function: no decl") << headersFuncNoDecl << QByteArrayList(); + + const QByteArrayList headersFuncTemplateDecl2Def{R"( +// Function comment +template T @aFunction(); + +template inline T aFunction() { return T(); } +)", R"( +template T aFunction(); + +// Function comment +template inline T aFunction() { return T(); } +)"}; + QTest::newRow("function template: from decl to def") << headersFuncTemplateDecl2Def + << QByteArrayList(); + + const QByteArrayList headersFuncTemplateDef2Decl{R"( +template T aFunction(); + +// Function comment +template inline T @aFunction() { return T(); } +)", R"( +// Function comment +template T aFunction(); + +template inline T aFunction() { return T(); } +)"}; + QTest::newRow("function template: from def to decl") << headersFuncTemplateDef2Decl + << QByteArrayList(); + + const QByteArrayList headersMemberDecl2Def{R"( +class C { + // Member function comment + void @aMember(); +)", R"( +class C { + void aMember(); +)"}; + const QByteArrayList sourcesMemberDecl2Def{R"( +#include "file.h" + +void C::aMember() {} +)", R"( +#include "file.h" + +// Member function comment +void C::aMember() {} +)"}; + QTest::newRow("member function: from decl to def") << headersMemberDecl2Def + << sourcesMemberDecl2Def; + + const QByteArrayList headersMemberDef2Decl{R"( +class C { + void aMember(); +)", R"( +class C { + // Member function comment + void aMember(); +)"}; + const QByteArrayList sourcesMemberDef2Decl{R"( +#include "file.h" + +// Member function comment +void C::aMember() {@} +)", R"( +#include "file.h" + +void C::aMember() {} +)"}; + QTest::newRow("member function: from def to decl") << headersMemberDef2Decl + << sourcesMemberDef2Decl; +} + +void QuickfixTest::testMoveComments() +{ + QFETCH(QByteArrayList, headers); + QFETCH(QByteArrayList, sources); + + QList documents; + QCOMPARE(headers.size(), 2); + documents << CppTestDocument::create("file.h", headers.at(0), headers.at(1)); + if (!sources.isEmpty()) { + QCOMPARE(sources.size(), 2); + documents << CppTestDocument::create("file.cpp", sources.at(0), sources.at(1)); + } + MoveFunctionComments factory; + QByteArray failMessage; + if (QByteArray(QTest::currentDataTag()) == "function template: from def to decl") + failMessage = "decl/def switch doesn't work for templates"; + QuickFixOperationTest(documents, &factory, {}, {}, failMessage); +} + } // namespace CppEditor::Internal::Tests diff --git a/src/plugins/cppeditor/cppquickfix_test.h b/src/plugins/cppeditor/cppquickfix_test.h index 50dc29a213b..89f05b0f160 100644 --- a/src/plugins/cppeditor/cppquickfix_test.h +++ b/src/plugins/cppeditor/cppquickfix_test.h @@ -224,6 +224,9 @@ private slots: void testChangeCommentType_data(); void testChangeCommentType(); + + void testMoveComments_data(); + void testMoveComments(); }; } // namespace Tests diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 5bf8269af46..a65c9b61a47 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -9542,6 +9542,166 @@ void ConvertCommentStyle::match(const CppQuickFixInterface &interface, result << new ConvertCommentStyleOp(interface, cursorTokens, kind); } +namespace { +class MoveFunctionCommentsOp : public CppQuickFixOperation +{ +public: + enum class Direction { ToDecl, ToDef }; + MoveFunctionCommentsOp(const CppQuickFixInterface &interface, const Symbol *symbol, + const QList &commentTokens, Direction direction) + : CppQuickFixOperation(interface), m_symbol(symbol), m_commentTokens(commentTokens) + { + setDescription(direction == Direction::ToDecl + ? Tr::tr("Move function documentation to declaration") + : Tr::tr("Move function documentation to definition")); + } + +private: + void perform() override + { + const auto textDoc = const_cast(currentFile()->document()); + const int pos = currentFile()->cppDocument()->translationUnit()->getTokenPositionInDocument( + m_symbol->sourceLocation(), textDoc); + QTextCursor cursor(textDoc); + cursor.setPosition(pos); + const CursorInEditor cursorInEditor(cursor, currentFile()->filePath(), editor(), + editor()->textDocument()); + const auto callback = [symbolLoc = m_symbol->toLink(), comments = m_commentTokens] + (const Link &link) { + moveComments(link, symbolLoc, comments); + }; + CppModelManager::followSymbol(cursorInEditor, callback, true, false); + } + + static void moveComments(const Link &targetLoc, const Link &symbolLoc, + const QList &comments) + { + if (!targetLoc.hasValidTarget() || targetLoc.hasSameLocation(symbolLoc)) + return; + + CppRefactoringChanges changes(CppModelManager::snapshot()); + const CppRefactoringFilePtr sourceFile = changes.file(symbolLoc.targetFilePath); + const CppRefactoringFilePtr targetFile + = targetLoc.targetFilePath == symbolLoc.targetFilePath + ? sourceFile + : changes.file(targetLoc.targetFilePath); + const Document::Ptr &targetCppDoc = targetFile->cppDocument(); + const QList targetAstPath = ASTPath(targetCppDoc)( + targetLoc.targetLine, targetLoc.targetColumn + 1); + if (targetAstPath.isEmpty()) + return; + const AST *targetDeclAst = nullptr; + for (auto it = std::next(std::rbegin(targetAstPath)); + it != std::rend(targetAstPath); ++it) { + AST * const node = *it; + if (node->asDeclaration()) { + targetDeclAst = node; + continue; + } + if (targetDeclAst) + break; + } + if (!targetDeclAst) + return; + const int insertionPos = targetCppDoc->translationUnit()->getTokenPositionInDocument( + targetDeclAst->firstToken(), targetFile->document()); + const TranslationUnit * const sourceTu = sourceFile->cppDocument()->translationUnit(); + const int sourceCommentStartPos = sourceTu->getTokenPositionInDocument( + comments.first(), sourceFile->document()); + const int sourceCommentEndPos = sourceTu->getTokenEndPositionInDocument( + comments.last(), sourceFile->document()); + const QString functionDoc = sourceFile->textOf(sourceCommentStartPos, sourceCommentEndPos); + + // Remove comment plus leading and trailing whitespace, including trailing newline. + const auto removeAtSource = [&](ChangeSet &changeSet) { + int removalPos = sourceCommentStartPos; + const QChar newline(QChar::ParagraphSeparator); + while (true) { + const int prev = removalPos - 1; + if (prev < 0) + break; + const QChar prevChar = sourceFile->charAt(prev); + if (!prevChar.isSpace() || prevChar == newline) + break; + removalPos = prev; + } + int removalEndPos = sourceCommentEndPos; + while (true) { + if (removalEndPos == sourceFile->document()->characterCount()) + break; + const QChar nextChar = sourceFile->charAt(removalEndPos); + if (!nextChar.isSpace()) + break; + ++removalEndPos; + if (nextChar == newline) + break; + } + changeSet.remove(removalPos, removalEndPos); + }; + + ChangeSet targetChangeSet; + targetChangeSet.insert(insertionPos, functionDoc); + targetChangeSet.insert(insertionPos, "\n"); + if (targetFile == sourceFile) + removeAtSource(targetChangeSet); + targetFile->setChangeSet(targetChangeSet); + targetFile->appendIndentRange({insertionPos, insertionPos + int(functionDoc.length())}); + const bool targetFileSuccess = targetFile->apply(); + if (targetFile == sourceFile || !targetFileSuccess) + return; + ChangeSet sourceChangeSet; + removeAtSource(sourceChangeSet); + sourceFile->setChangeSet(sourceChangeSet); + sourceFile->apply(); + } + + const Symbol * const m_symbol; + const QList m_commentTokens; +}; +} // namespace + +void MoveFunctionComments::match(const CppQuickFixInterface &interface, + TextEditor::QuickFixOperations &result) +{ + const QList &astPath = interface.path(); + if (astPath.isEmpty()) + return; + const Symbol *symbol = nullptr; + MoveFunctionCommentsOp::Direction direction = MoveFunctionCommentsOp::Direction::ToDecl; + for (auto it = std::next(std::rbegin(astPath)); it != std::rend(astPath); ++it) { + if (const auto func = (*it)->asFunctionDefinition()) { + symbol = func->symbol; + direction = MoveFunctionCommentsOp::Direction::ToDecl; + break; + } + const auto decl = (*it)->asSimpleDeclaration(); + if (!decl || !decl->declarator_list) + continue; + for (auto it = decl->declarator_list->begin(); + !symbol && it != decl->declarator_list->end(); ++it) { + PostfixDeclaratorListAST * const funcDecls = (*it)->postfix_declarator_list; + if (!funcDecls) + continue; + for (auto it = funcDecls->begin(); it != funcDecls->end(); ++it) { + if (const auto func = (*it)->asFunctionDeclarator()) { + symbol = func->symbol; + direction = MoveFunctionCommentsOp::Direction::ToDef; + break; + } + } + } + + } + if (!symbol) + return; + + if (const QList commentTokens = commentsForDeclaration( + symbol, *interface.textDocument(), interface.currentFile()->cppDocument()); + !commentTokens.isEmpty()) { + result << new MoveFunctionCommentsOp(interface, symbol, commentTokens, direction); + } +} + void createCppQuickFixes() { new AddIncludeForUndefinedIdentifier; @@ -9599,6 +9759,7 @@ void createCppQuickFixes() new RemoveUsingNamespace; new GenerateConstructor; new ConvertCommentStyle; + new MoveFunctionComments; } void destroyCppQuickFixes() diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index 62e1b1f981c..8825e6df911 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -593,5 +593,13 @@ private: TextEditor::QuickFixOperations &result) override; }; +//! Moves function documentation between declaration and implementation. +class MoveFunctionComments : public CppQuickFixFactory +{ +private: + void match(const CppQuickFixInterface &interface, + TextEditor::QuickFixOperations &result) override; +}; + } // namespace Internal } // namespace CppEditor