From 837e951b29240d28b4c5849acae04d2ebbcd03f1 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 7 Feb 2024 14:12:11 +0100 Subject: [PATCH] CppEditor: Add curly braces for more control statements Fixes: QTCREATORBUG-24542 Change-Id: I3e0893e1c736730d94e2c9ab2baa0aa580393fe4 Reviewed-by: Reviewed-by: Christian Stenger Reviewed-by: Qt CI Bot --- src/plugins/cppeditor/cppquickfix_test.cpp | 96 ++++++++++++++++-- src/plugins/cppeditor/cppquickfix_test.h | 1 + src/plugins/cppeditor/cppquickfixes.cpp | 109 ++++++++++++++------- src/plugins/cppeditor/cppquickfixes.h | 6 +- 4 files changed, 166 insertions(+), 46 deletions(-) diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 48c9bbc68af..56ae513a8a1 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -8229,29 +8229,105 @@ void QuickfixTest::testExtractLiteralAsParameterNotTriggeringForInvalidCode() QuickFixOperationTest(testDocuments, &factory); } -void QuickfixTest::testAddCurlyBraces() +void QuickfixTest::testAddCurlyBraces_data() { - QList testDocuments; - const QByteArray original = R"delim( + QTest::addColumn("original"); + QTest::addColumn("expected"); + + QByteArray original = R"delim( void MyObject::f() { @if (true) emit mySig(); -} -)delim"; - const QByteArray expected = R"delim( +})delim"; + QByteArray expected = R"delim( void MyObject::f() { if (true) { emit mySig(); } +})delim"; + QTest::newRow("if") << original << expected; + + original = R"delim( +void MyObject::f() +{ + @while (true) + emit mySig(); +})delim"; + expected = R"delim( +void MyObject::f() +{ + while (true) { + emit mySig(); + } +})delim"; + QTest::newRow("while") << original << expected; + + original = R"delim( +void MyObject::f() +{ + @for (int i = 0; i < 10; ++i) + emit mySig(); +})delim"; + expected = R"delim( +void MyObject::f() +{ + for (int i = 0; i < 10; ++i) { + emit mySig(); + } +})delim"; + QTest::newRow("for") << original << expected; + + original = R"delim( +void MyObject::f() +{ + @for (int i : list) + emit mySig(); +})delim"; + expected = R"delim( +void MyObject::f() +{ + for (int i : list) { + emit mySig(); + } +})delim"; + QTest::newRow("range-based for") << original << expected; + + original = R"delim( +void MyObject::f() +{ + @do + emit mySig(); + while (true); +})delim"; + expected = R"delim( +void MyObject::f() +{ + do { + emit mySig(); + } while (true); +})delim"; + QTest::newRow("do") << original << expected; + + original = R"delim( +void MyObject::f() +{ + @do { + emit mySig(); + } while (true); +})delim"; + expected.clear(); + QTest::newRow("already has braces") << original << expected; } -)delim"; - testDocuments << CppTestDocument::create("file.cpp", original, expected); - AddBracesToIf factory; - QuickFixOperationTest(testDocuments, &factory); +void QuickfixTest::testAddCurlyBraces() +{ + QFETCH(QByteArray, original); + QFETCH(QByteArray, expected); + AddBracesToControlStatement factory; + QuickFixOperationTest({CppTestDocument::create("file.cpp", original, expected)}, &factory); } void QuickfixTest::testConvertQt4ConnectConnectOutOfClass() diff --git a/src/plugins/cppeditor/cppquickfix_test.h b/src/plugins/cppeditor/cppquickfix_test.h index 4d734c9c09c..917abe19c73 100644 --- a/src/plugins/cppeditor/cppquickfix_test.h +++ b/src/plugins/cppeditor/cppquickfix_test.h @@ -209,6 +209,7 @@ private slots: void testExtractLiteralAsParameterMemberFunctionSeparateFiles(); void testExtractLiteralAsParameterNotTriggeringForInvalidCode(); + void testAddCurlyBraces_data(); void testAddCurlyBraces(); void testRemoveUsingNamespace_data(); diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 90563ccd33d..bf3f1375117 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -764,13 +764,49 @@ void SplitSimpleDeclaration::doMatch(const CppQuickFixInterface &interface, } namespace { +template Statement *asControlStatement(AST *node) +{ + if constexpr (std::is_same_v) + return node->asIfStatement(); + if constexpr (std::is_same_v) + return node->asWhileStatement(); + if constexpr (std::is_same_v) + return node->asForStatement(); + if constexpr (std::is_same_v) + return node->asRangeBasedForStatement(); + if constexpr (std::is_same_v) + return node->asDoStatement(); + return nullptr; +} -class AddBracesToIfOp: public CppQuickFixOperation +template +int triggerToken(const Statement *statement) +{ + if constexpr (std::is_same_v) + return statement->if_token; + if constexpr (std::is_same_v) + return statement->while_token; + if constexpr (std::is_same_v) + return statement->do_token; + if constexpr (std::is_same_v + || std::is_same_v) { + return statement->for_token; + } +} + +template +int tokenToInsertOpeningBraceAfter(const Statement *statement) +{ + if constexpr (std::is_same_v) + return statement->do_token; + return statement->rparen_token; +} + +template class AddBracesToControlStatementOp : public CppQuickFixOperation { public: - AddBracesToIfOp(const CppQuickFixInterface &interface, int priority, - const IfStatementAST *statement) - : CppQuickFixOperation(interface, priority) + AddBracesToControlStatementOp(const CppQuickFixInterface &interface, const Statement *statement) + : CppQuickFixOperation(interface, 0) , _statement(statement) { setDescription(Tr::tr("Add Curly Braces")); @@ -783,51 +819,58 @@ public: ChangeSet changes; - const int start = currentFile->endOf(_statement->rparen_token); + const int start = currentFile->endOf(tokenToInsertOpeningBraceAfter(_statement)); changes.insert(start, QLatin1String(" {")); - const int end = currentFile->endOf(_statement->statement->lastToken() - 1); - changes.insert(end, QLatin1String("\n}")); + if constexpr (std::is_same_v) { + const int end = currentFile->startOf(_statement->while_token); + changes.insert(end, QLatin1String("} ")); + } else { + const int end = currentFile->endOf(_statement->statement->lastToken() - 1); + changes.insert(end, QLatin1String("\n}")); + } + // TODO: For if statements, also bracify all else cases. + // Also check all else cases in the factory. currentFile->setChangeSet(changes); currentFile->apply(); } private: - const IfStatementAST * const _statement; + const Statement * const _statement; }; } // anonymous namespace -void AddBracesToIf::doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) +template +bool checkControlStatementsHelper(const CppQuickFixInterface &interface, QuickFixOperations &result) { - const QList &path = interface.path(); - if (path.isEmpty()) - return; + Statement * const statement = asControlStatement(interface.path().last()); + if (!statement) + return false; - // show when we're on the 'if' of an if statement - int index = path.size() - 1; - IfStatementAST *ifStatement = path.at(index)->asIfStatement(); - if (ifStatement && interface.isCursorOn(ifStatement->if_token) && ifStatement->statement - && !ifStatement->statement->asCompoundStatement()) { - result << new AddBracesToIfOp(interface, index, ifStatement); - return; + if (interface.isCursorOn(triggerToken(statement)) && statement->statement + && !statement->statement->asCompoundStatement()) { + result << new AddBracesToControlStatementOp(interface, statement); } + return true; +} - // or if we're on the statement contained in the if - // ### This may not be such a good idea, consider nested ifs... - for (; index != -1; --index) { - IfStatementAST *ifStatement = path.at(index)->asIfStatement(); - if (ifStatement && ifStatement->statement - && interface.isCursorOn(ifStatement->statement) - && !ifStatement->statement->asCompoundStatement()) { - result << new AddBracesToIfOp(interface, index, ifStatement); - return; - } - } +template +void checkControlStatements(const CppQuickFixInterface &interface, QuickFixOperations &result) +{ + (... || checkControlStatementsHelper(interface, result)); +} - // ### This could very well be extended to the else branch - // and other nodes entirely. +void AddBracesToControlStatement::doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) +{ + if (interface.path().isEmpty()) + return; + checkControlStatements(interface, result); } namespace { @@ -9969,7 +10012,7 @@ void createCppQuickFixes() new SplitIfStatement; new SplitSimpleDeclaration; - new AddBracesToIf; + new AddBracesToControlStatement; new RearrangeParamDeclarationList; new ReformatPointerDeclaration; diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index 99904ffcb19..10f82c9dda3 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -287,7 +287,7 @@ public: }; /*! - Add curly braces to a if statement that doesn't already contain a + Add curly braces to a control statement that doesn't already contain a compound statement. I.e. if (a) @@ -297,9 +297,9 @@ public: b; } - Activates on: the if + Activates on: the keyword */ -class AddBracesToIf: public CppQuickFixFactory +class AddBracesToControlStatement : public CppQuickFixFactory { public: void doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) override;