From 02825b589443836eff985e61a03af8be28a2f17d Mon Sep 17 00:00:00 2001 From: Lorenz Haas <lykurg@gmail.com> Date: Thu, 23 May 2013 20:59:02 +0200 Subject: [PATCH] CppEditor: New quick fix "Optimize For Loop" Rewrites post increment/decrement operators (++ and --) as pre increment/decrement operators and moves non string/numeric literals and non id expressions from loops condition to loops initializer. Change-Id: Id95334b6df6fcaa9af436cc1d2d0982d38bf8fe2 Reviewed-by: Leena Miettinen <riitta-leena.miettinen@digia.com> Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com> --- doc/src/editors/creator-editors.qdoc | 15 ++ src/plugins/cppeditor/cppeditorplugin.h | 10 ++ src/plugins/cppeditor/cppquickfix_test.cpp | 90 ++++++++++ src/plugins/cppeditor/cppquickfixes.cpp | 185 +++++++++++++++++++++ src/plugins/cppeditor/cppquickfixes.h | 10 ++ 5 files changed, 310 insertions(+) diff --git a/doc/src/editors/creator-editors.qdoc b/doc/src/editors/creator-editors.qdoc index 21936ed244e..ecdc9bb54f2 100644 --- a/doc/src/editors/creator-editors.qdoc +++ b/doc/src/editors/creator-editors.qdoc @@ -1978,6 +1978,21 @@ \image qtcreator-refactoring-virtual-function-dialog.png \li Class or base class name + \row + \li Optimize for-Loop + \li Rewrites post increment operators as pre increment operators and post decrement + operators as pre decrement operators. It also moves other than string or numeric + literals and id expressions from the condition of a for loop to its initializer. + For example, rewrites: + + \code + for (int i = 0; i < 3 * 2; i++) + \endcode + as + \code + for (int i = 0, total = 3 * 2; i < total; ++i) + \endcode + \li for \endtable diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h index cbb34643a6d..2d53b3656b8 100644 --- a/src/plugins/cppeditor/cppeditorplugin.h +++ b/src/plugins/cppeditor/cppeditorplugin.h @@ -252,6 +252,16 @@ private slots: void test_quickfix_InsertVirtualMethods_notrigger_allImplemented(); void test_quickfix_InsertVirtualMethods_BaseClassInNamespace(); + void test_quickfix_OptimizeForLoop_postcrement(); + void test_quickfix_OptimizeForLoop_condition(); + void test_quickfix_OptimizeForLoop_flipedCondition(); + void test_quickfix_OptimizeForLoop_alterVariableName(); + void test_quickfix_OptimizeForLoop_optimizeBoth(); + void test_quickfix_OptimizeForLoop_emptyInitializer(); + void test_quickfix_OptimizeForLoop_wrongInitializer(); + void test_quickfix_OptimizeForLoop_noTriggerNumeric1(); + void test_quickfix_OptimizeForLoop_noTriggerNumeric2(); + // The following tests depend on the projects that are loaded on startup // and will be skipped in case no projects are loaded. void test_openEachFile(); diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 4b7a0dcb8d6..4d7fd9af0d6 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -3768,3 +3768,93 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_BaseClassInNamespace() TestCase data(testFiles); data.run(&factory); } + +/// Check: optimize postcrement +void CppEditorPlugin::test_quickfix_OptimizeForLoop_postcrement() +{ + const QByteArray original = "void foo() {f@or (int i = 0; i < 3; i++) {}}\n"; + const QByteArray expected = "void foo() {for (int i = 0; i < 3; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: optimize condition +void CppEditorPlugin::test_quickfix_OptimizeForLoop_condition() +{ + const QByteArray original = "void foo() {f@or (int i = 0; i < 3 + 5; ++i) {}}\n"; + const QByteArray expected = "void foo() {for (int i = 0, total = 3 + 5; i < total; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: optimize fliped condition +void CppEditorPlugin::test_quickfix_OptimizeForLoop_flipedCondition() +{ + const QByteArray original = "void foo() {f@or (int i = 0; 3 + 5 > i; ++i) {}}\n"; + const QByteArray expected = "void foo() {for (int i = 0, total = 3 + 5; total > i; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: if "total" used, create other name. +void CppEditorPlugin::test_quickfix_OptimizeForLoop_alterVariableName() +{ + const QByteArray original = "void foo() {f@or (int i = 0, total = 0; i < 3 + 5; ++i) {}}\n"; + const QByteArray expected = "void foo() {for (int i = 0, total = 0, totalX = 3 + 5; i < totalX; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: optimize postcrement and condition +void CppEditorPlugin::test_quickfix_OptimizeForLoop_optimizeBoth() +{ + const QByteArray original = "void foo() {f@or (int i = 0; i < 3 + 5; i++) {}}\n"; + const QByteArray expected = "void foo() {for (int i = 0, total = 3 + 5; i < total; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: empty initializier +void CppEditorPlugin::test_quickfix_OptimizeForLoop_emptyInitializer() +{ + const QByteArray original = "int i; void foo() {f@or (; i < 3 + 5; ++i) {}}\n"; + const QByteArray expected = "int i; void foo() {for (int total = 3 + 5; i < total; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: wrong initializier type -> no trigger +void CppEditorPlugin::test_quickfix_OptimizeForLoop_wrongInitializer() +{ + const QByteArray original = "int i; void foo() {f@or (double a = 0; i < 3 + 5; ++i) {}}\n"; + const QByteArray expected = "int i; void foo() {f@or (double a = 0; i < 3 + 5; ++i) {}}\n\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: No trigger when numeric +void CppEditorPlugin::test_quickfix_OptimizeForLoop_noTriggerNumeric1() +{ + const QByteArray original = "void foo() {fo@r (int i = 0; i < 3; ++i) {}}\n"; + const QByteArray expected = original + "\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} + +/// Check: No trigger when numeric +void CppEditorPlugin::test_quickfix_OptimizeForLoop_noTriggerNumeric2() +{ + const QByteArray original = "void foo() {fo@r (int i = 0; i < -3; ++i) {}}\n"; + const QByteArray expected = original + "\n"; + OptimizeForLoop factory; + TestCase data(original, expected); + data.run(&factory); +} diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index a33a45c7c4e..fd01a487f4b 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -131,6 +131,8 @@ void CppEditor::Internal::registerQuickFixes(ExtensionSystem::IPlugin *plugIn) plugIn->addAutoReleasedObject(new AssignToLocalVariable); plugIn->addAutoReleasedObject(new InsertVirtualMethods); + + plugIn->addAutoReleasedObject(new OptimizeForLoop); } // In the following anonymous namespace all functions are collected, which could be of interest for @@ -5051,4 +5053,187 @@ void InsertVirtualMethods::match(const CppQuickFixInterface &interface, QuickFix delete op; } +namespace { + +class OptimizeForLoopOperation: public CppQuickFixOperation +{ +public: + OptimizeForLoopOperation(const CppQuickFixInterface &interface, const ForStatementAST *forAst, + const bool optimizePostcrement, const ExpressionAST *expression, + const FullySpecifiedType type) + : CppQuickFixOperation(interface) + , m_forAst(forAst) + , m_optimizePostcrement(optimizePostcrement) + , m_expression(expression) + , m_type(type) + { + setDescription(QApplication::translate("CppTools::QuickFix", "Optimize for-Loop")); + } + + void perform() + { + QTC_ASSERT(m_forAst, return); + + const QString filename = assistInterface()->currentFile()->fileName(); + const CppRefactoringChanges refactoring(assistInterface()->snapshot()); + const CppRefactoringFilePtr file = refactoring.file(filename); + ChangeSet change; + + // Optimize post (in|de)crement operator to pre (in|de)crement operator + if (m_optimizePostcrement && m_forAst->expression) { + PostIncrDecrAST *incrdecr = m_forAst->expression->asPostIncrDecr(); + if (incrdecr && incrdecr->base_expression && incrdecr->incr_decr_token) { + change.flip(file->range(incrdecr->base_expression), + file->range(incrdecr->incr_decr_token)); + } + } + + // Optimize Condition + int renamePos = -1; + if (m_expression) { + QString varName = QLatin1String("total"); + + if (file->textOf(m_forAst->initializer).length() == 1) { + Overview oo = CppCodeStyleSettings::currentProjectCodeStyleOverview(); + const QString typeAndName = oo.prettyType(m_type, varName); + renamePos = file->endOf(m_forAst->initializer) - 1 + typeAndName.length(); + change.insert(file->endOf(m_forAst->initializer) - 1, // "-1" because of ";" + typeAndName + QLatin1String(" = ") + file->textOf(m_expression)); + } else { + // Check if varName is already used + if (DeclarationStatementAST *ds = m_forAst->initializer->asDeclarationStatement()) { + if (DeclarationAST *decl = ds->declaration) { + if (SimpleDeclarationAST *sdecl = decl->asSimpleDeclaration()) { + for (;;) { + bool match = false; + for (DeclaratorListAST *it = sdecl->declarator_list; it; + it = it->next) { + if (file->textOf(it->value->core_declarator) == varName) { + varName += QLatin1Char('X'); + match = true; + break; + } + } + if (!match) + break; + } + } + } + } + + renamePos = file->endOf(m_forAst->initializer) + 1 + varName.length(); + change.insert(file->endOf(m_forAst->initializer) - 1, // "-1" because of ";" + QLatin1String(", ") + varName + QLatin1String(" = ") + + file->textOf(m_expression)); + } + + ChangeSet::Range exprRange(file->startOf(m_expression), file->endOf(m_expression)); + change.replace(exprRange, varName); + } + + file->setChangeSet(change); + file->apply(); + + // Select variable name and trigger symbol rename + if (renamePos != -1) { + QTextCursor c = file->cursor(); + c.setPosition(renamePos); + assistInterface()->editor()->setTextCursor(c); + assistInterface()->editor()->renameSymbolUnderCursor(); + c.select(QTextCursor::WordUnderCursor); + assistInterface()->editor()->setTextCursor(c); + } + } + +private: + const ForStatementAST *m_forAst; + const bool m_optimizePostcrement; + const ExpressionAST *m_expression; + const FullySpecifiedType m_type; +}; + +} // anonymous namespace + +void OptimizeForLoop::match(const CppQuickFixInterface &interface, QuickFixOperations &result) +{ + const QList<AST *> path = interface->path(); + ForStatementAST *forAst = 0; + if (!path.isEmpty()) + forAst = path.last()->asForStatement(); + if (!forAst || !interface->isCursorOn(forAst)) + return; + + // Check for optimizing a postcrement + const CppRefactoringFilePtr file = interface->currentFile(); + bool optimizePostcrement = false; + if (forAst->expression) { + if (PostIncrDecrAST *incrdecr = forAst->expression->asPostIncrDecr()) { + const Token t = file->tokenAt(incrdecr->incr_decr_token); + if (t.is(T_PLUS_PLUS) || t.is(T_MINUS_MINUS)) + optimizePostcrement = true; + } + } + + // Check for optimizing condition + bool optimizeCondition = false; + FullySpecifiedType conditionType; + ExpressionAST *conditionExpression = 0; + if (forAst->initializer && forAst->condition) { + if (BinaryExpressionAST *binary = forAst->condition->asBinaryExpression()) { + // Get the expression against which we should evaluate + IdExpressionAST *conditionId = binary->left_expression->asIdExpression(); + if (conditionId) { + conditionExpression = binary->right_expression; + } else { + conditionId = binary->right_expression->asIdExpression(); + conditionExpression = binary->left_expression; + } + + if (conditionId && conditionExpression + && !(conditionExpression->asNumericLiteral() + || conditionExpression->asStringLiteral() + || conditionExpression->asIdExpression() + || conditionExpression->asUnaryExpression())) { + // Determine type of for initializer + FullySpecifiedType initializerType; + if (DeclarationStatementAST *stmt = forAst->initializer->asDeclarationStatement()) { + if (stmt->declaration) { + if (SimpleDeclarationAST *decl = stmt->declaration->asSimpleDeclaration()) { + if (decl->symbols) { + if (Symbol *symbol = decl->symbols->value) + initializerType = symbol->type(); + } + } + } + } + + // Determine type of for condition + TypeOfExpression typeOfExpression; + typeOfExpression.init(interface->semanticInfo().doc, interface->snapshot(), + interface->context().bindings()); + typeOfExpression.setExpandTemplates(true); + Scope *scope = file->scopeAt(conditionId->firstToken()); + const QList<LookupItem> conditionItems = typeOfExpression( + conditionId, interface->semanticInfo().doc, scope); + if (!conditionItems.isEmpty()) + conditionType = conditionItems.first().type(); + + if (conditionType.isValid() + && (file->textOf(forAst->initializer) == QLatin1String(";") + || initializerType == conditionType)) { + optimizeCondition = true; + } + } + } + } + + if (optimizePostcrement || optimizeCondition) { + OptimizeForLoopOperation *op + = new OptimizeForLoopOperation(interface, forAst, optimizePostcrement, + (optimizeCondition) ? conditionExpression : 0, + conditionType); + result.append(QuickFixOperation::Ptr(op)); + } +} + #include "cppquickfixes.moc" diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index 9cc84c6b5da..e218bd02fa1 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -584,6 +584,16 @@ private: InsertVirtualMethodsDialog *m_dialog; }; +/*! + Optimizes a for loop to avoid permanent condition check and forces to use preincrement + or predecrement operators in the expression of the for loop. + */ +class OptimizeForLoop : public CppQuickFixFactory +{ +public: + void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result); +}; + } // namespace Internal } // namespace CppEditor -- GitLab