diff --git a/src/plugins/cpptools/cpppointerdeclarationformatter.cpp b/src/plugins/cpptools/cpppointerdeclarationformatter.cpp index ba0fa4c60fa..a40316f290b 100644 --- a/src/plugins/cpptools/cpppointerdeclarationformatter.cpp +++ b/src/plugins/cpptools/cpppointerdeclarationformatter.cpp @@ -32,12 +32,24 @@ #include +#include #include #define DEBUG_OUTPUT 0 -#define CHECK_RV(cond, err, r) if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << (err); return r; } -#define CHECK_R(cond, err) if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << (err); return; } -#define CHECK_C(cond, err) if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << (err); continue; } + +#if DEBUG_OUTPUT +# include +# ifdef __GNUC__ +# include +# endif +#endif + +#define CHECK_RV(cond, err, r) \ + if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << "Discarded:" << (err); return r; } +#define CHECK_R(cond, err) \ + if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << "Discarded:" << (err); return; } +#define CHECK_C(cond, err) \ + if (!(cond)) { if (DEBUG_OUTPUT) qDebug() << "Discarded:" << (err); continue; } using namespace CppTools; @@ -122,6 +134,11 @@ PointerDeclarationFormatter::PointerDeclarationFormatter( bool PointerDeclarationFormatter::visit(SimpleDeclarationAST *ast) { CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); + + const unsigned tokenKind = tokenAt(ast->firstToken()).kind(); + const bool astIsOk = tokenKind != T_CLASS && tokenKind != T_STRUCT && tokenKind != T_ENUM; + CHECK_RV(astIsOk, "Nothing to do for class/struct/enum", true); DeclaratorListAST *declaratorList = ast->declarator_list; CHECK_RV(declaratorList, "No declarator list", true); @@ -151,7 +168,7 @@ bool PointerDeclarationFormatter::visit(SimpleDeclarationAST *ast) // Specify activation range int lastActivationToken = 0; - Range range; + TokenRange range; // (2) Handle function declaration's return type if (symbol->type()->asFunctionType()) { PostfixDeclaratorListAST *pfDeclaratorList = declarator->postfix_declarator_list; @@ -179,7 +196,7 @@ bool PointerDeclarationFormatter::visit(SimpleDeclarationAST *ast) firstActivationToken = declarator->firstToken(); } - range.start = m_cppRefactoringFile->startOf(firstActivationToken); + range.start = firstActivationToken; // (1) Handle 'normal' declarations. } else { @@ -191,15 +208,16 @@ bool PointerDeclarationFormatter::visit(SimpleDeclarationAST *ast) declarator->firstToken(), &foundBegin); CHECK_RV(foundBegin, "Declaration without attributes not supported", true); - range.start = m_cppRefactoringFile->startOf(firstActivationToken); + range.start = firstActivationToken; } else { - range.start = m_cppRefactoringFile->startOf(declarator); + range.start = declarator->firstToken(); } lastActivationToken = declarator->equal_token ? declarator->equal_token - 1 : declarator->lastToken() - 1; } - range.end = m_cppRefactoringFile->endOf(lastActivationToken); + + range.end = lastActivationToken; checkAndRewrite(symbol, range, charactersToRemove); } @@ -209,6 +227,9 @@ bool PointerDeclarationFormatter::visit(SimpleDeclarationAST *ast) /*! Handle return types of function definitions */ bool PointerDeclarationFormatter::visit(FunctionDefinitionAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); + DeclaratorAST *declarator = ast->declarator; CHECK_RV(declarator, "No declarator", true); CHECK_RV(declarator->ptr_operator_list, "No Pointer or references", true); @@ -230,8 +251,7 @@ bool PointerDeclarationFormatter::visit(FunctionDefinitionAST *ast) lastActivationToken, &foundBegin); CHECK_RV(foundBegin, "Declaration without attributes not supported", true); - Range range(m_cppRefactoringFile->startOf(firstActivationToken), - m_cppRefactoringFile->endOf(lastActivationToken)); + TokenRange range(firstActivationToken, lastActivationToken); checkAndRewrite(symbol, range); return true; @@ -240,6 +260,9 @@ bool PointerDeclarationFormatter::visit(FunctionDefinitionAST *ast) /*! Handle parameters in function declarations and definitions */ bool PointerDeclarationFormatter::visit(ParameterDeclarationAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); + DeclaratorAST *declarator = ast->declarator; CHECK_RV(declarator, "No declarator", true); CHECK_RV(declarator->ptr_operator_list, "No Pointer or references", true); @@ -249,8 +272,7 @@ bool PointerDeclarationFormatter::visit(ParameterDeclarationAST *ast) const int lastActivationToken = ast->equal_token ? ast->equal_token - 1 : ast->lastToken() - 1; - Range range(m_cppRefactoringFile->startOf(ast), - m_cppRefactoringFile->endOf(lastActivationToken)); + TokenRange range(ast->firstToken(), lastActivationToken); checkAndRewrite(symbol, range); return true; @@ -259,6 +281,9 @@ bool PointerDeclarationFormatter::visit(ParameterDeclarationAST *ast) /*! Handle declaration in foreach statement */ bool PointerDeclarationFormatter::visit(ForeachStatementAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); + DeclaratorAST *declarator = ast->declarator; CHECK_RV(declarator, "No declarator", true); CHECK_RV(declarator->ptr_operator_list, "No Pointer or references", true); @@ -272,8 +297,7 @@ bool PointerDeclarationFormatter::visit(ForeachStatementAST *ast) const int lastActivationToken = declarator->equal_token ? declarator->equal_token - 1 : declarator->lastToken() - 1; - Range range(m_cppRefactoringFile->startOf(firstSpecifier), - m_cppRefactoringFile->endOf(lastActivationToken)); + TokenRange range(firstSpecifier->firstToken(), lastActivationToken); checkAndRewrite(symbol, range); return true; @@ -281,18 +305,24 @@ bool PointerDeclarationFormatter::visit(ForeachStatementAST *ast) bool PointerDeclarationFormatter::visit(IfStatementAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); processIfWhileForStatement(ast->condition, ast->symbol); return true; } bool PointerDeclarationFormatter::visit(WhileStatementAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); processIfWhileForStatement(ast->condition, ast->symbol); return true; } bool PointerDeclarationFormatter::visit(ForStatementAST *ast) { + CHECK_RV(ast, "Invalid AST", true); + printCandidate(ast); processIfWhileForStatement(ast->condition, ast->symbol); return true; } @@ -330,8 +360,7 @@ void PointerDeclarationFormatter::processIfWhileForStatement(ExpressionAST *expr } // Specify activation range - Range range(m_cppRefactoringFile->startOf(condition), - m_cppRefactoringFile->endOf(declarator->equal_token - 1)); + TokenRange range(condition->firstToken(), declarator->equal_token - 1); checkAndRewrite(symbol, range); } @@ -343,13 +372,23 @@ void PointerDeclarationFormatter::processIfWhileForStatement(ExpressionAST *expr \param symbol the symbol to be rewritten \param range the substitution range in the file */ -void PointerDeclarationFormatter::checkAndRewrite(Symbol *symbol, Range range, +void PointerDeclarationFormatter::checkAndRewrite(Symbol *symbol, TokenRange tokenRange, unsigned charactersToRemove) { - CHECK_R(range.start >= 0 && range.end > 0, "Range invalid"); - CHECK_R(range.start < range.end, "Range invalid"); + CHECK_R(tokenRange.end > 0, "TokenRange invalid1"); + CHECK_R(tokenRange.start < tokenRange.end, "TokenRange invalid2"); CHECK_R(symbol, "No symbol"); + // Check for expanded tokens + for (unsigned token = tokenRange.start; token <= tokenRange.end; ++token) + CHECK_R(! tokenAt(token).expanded(), "Token is expanded"); + + Range range(m_cppRefactoringFile->startOf(tokenRange.start), + m_cppRefactoringFile->endOf(tokenRange.end)); + + CHECK_R(range.start >= 0 && range.end > 0, "ChangeRange invalid1"); + CHECK_R(range.start < range.end, "ChangeRange invalid2"); + // Check range with respect to cursor position / selection if (m_cursorHandling == RespectCursor) { const QTextCursor cursor = m_cppRefactoringFile->cursor(); @@ -374,10 +413,14 @@ void PointerDeclarationFormatter::checkAndRewrite(Symbol *symbol, Range range, // Does the rewritten declaration (part) differs from the original source (part)? QString rewrittenDeclaration = rewriteDeclaration(type, symbol->name()); rewrittenDeclaration.remove(0, charactersToRemove); + CHECK_R(originalDeclaration != rewrittenDeclaration, "Rewritten is same as original"); + CHECK_R(rewrittenDeclaration.contains(QLatin1Char('&')) + || rewrittenDeclaration.contains(QLatin1Char('*')), + "No pointer or references in rewritten declaration"); if (DEBUG_OUTPUT) { - qDebug("Rewritten: \"%s\" --> \"%s\"", originalDeclaration.toLatin1().constData(), + qDebug("==> Rewritten: \"%s\" --> \"%s\"", originalDeclaration.toLatin1().constData(), rewrittenDeclaration.toLatin1().constData()); } @@ -415,3 +458,22 @@ QString PointerDeclarationFormatter::rewriteDeclaration(FullySpecifiedType type, return m_overview.prettyType(type, QLatin1String(identifier)); } + +void PointerDeclarationFormatter::printCandidate(AST *ast) +{ +#if DEBUG_OUTPUT + QString tokens; + for (unsigned token = ast->firstToken(); token < ast->lastToken(); token++) + tokens += QString::fromLatin1(tokenAt(token).spell()) + QLatin1Char(' '); + +# ifdef __GNUC__ + QByteArray name = abi::__cxa_demangle(typeid(*ast).name(), 0, 0, 0) + 11; + name.truncate(name.length() - 3); +# else + QByteArray name = typeid(*ast).name(); +# endif + qDebug("--> Candidate: %s: %s", name.constData(), qPrintable(tokens)); +#else + Q_UNUSED(ast) +#endif // DEBUG_OUTPUT +} diff --git a/src/plugins/cpptools/cpppointerdeclarationformatter.h b/src/plugins/cpptools/cpppointerdeclarationformatter.h index e43a58bc499..cc0e1b8d7f1 100644 --- a/src/plugins/cpptools/cpppointerdeclarationformatter.h +++ b/src/plugins/cpptools/cpppointerdeclarationformatter.h @@ -106,9 +106,18 @@ protected: bool visit(ForeachStatementAST *ast); private: + class TokenRange { + public: + TokenRange() : start(0), end(0) {} + TokenRange(unsigned start, unsigned end) : start(start), end(end) {} + unsigned start; + unsigned end; + }; + void processIfWhileForStatement(ExpressionAST *expression, Symbol *symbol); - void checkAndRewrite(Symbol *symbol, Range range, unsigned charactersToRemove = 0); + void checkAndRewrite(Symbol *symbol, TokenRange range, unsigned charactersToRemove = 0); QString rewriteDeclaration(FullySpecifiedType type, const Name *name) const; + void printCandidate(AST *ast); const CppRefactoringFilePtr m_cppRefactoringFile; const Overview &m_overview; diff --git a/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp b/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp index dbffa14c68e..092a2efdd26 100644 --- a/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp +++ b/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp @@ -575,3 +575,113 @@ void CppToolsPlugin::test_format_pointerdeclaration_multiple_matches_data() " return 0;\n" "}\n"; } + +void CppToolsPlugin::test_format_pointerdeclaration_macros() +{ + QFETCH(QString, source); + QFETCH(QString, reformattedSource); + + TestEnvironment env(source.toLatin1(), Document::ParseTranlationUnit); + AST *ast = env.translationUnit->ast(); + QVERIFY(ast); + + env.applyFormatting(ast, PointerDeclarationFormatter::RespectCursor); + + QCOMPARE(env.textDocument->toPlainText(), reformattedSource); +} + +void CppToolsPlugin::test_format_pointerdeclaration_macros_data() +{ + QTest::addColumn("source"); + QTest::addColumn("reformattedSource"); + + QString source; + + source = QLatin1String( + "#define FOO int*\n" + "FOO @bla;\n"); + QTest::newRow("macro-in-simple-declaration") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "FOO @f();\n"); + QTest::newRow("macro-in-function-declaration-returntype") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "int f(@FOO a);\n"); + QTest::newRow("macro-in-function-declaration-param") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "FOO @f() {};\n"); + QTest::newRow("macro-in-function-definition-returntype") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "int f(FOO @a) {};\n"); + QTest::newRow("macro-in-function-definition-param") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "while (FOO @s = 0);\n"); + QTest::newRow("macro-in-if-while-for") + << source << stripCursor(source); + + source = QLatin1String( + "#define FOO int*\n" + "foreach (FOO @s, list);\n"); + QTest::newRow("macro-in-foreach") + << source << stripCursor(source); + + // The bug was that we got "Reformat to 'QMetaObject staticMetaObject'" + // at the cursor position below, which was completelty wrong. + QTest::newRow("wrong-reformat-suggestion") + << + "#define Q_OBJECT \\\n" + "public: \\\n" + " template inline void qt_check_for_QOBJECT_macro(T &_q_argument) \\\n" + " { int i = qYouForgotTheQ_OBJECT_Macro(this, &_q_argument); i = i; } \\\n" + " QMetaObject staticMetaObject; \\\n" + " void *qt_metacast(const char *); \\\n" + " static inline QString tr(const char *s, const char *f = 0); \\\n" + " \n" + "class KitInformation\n" + "{\n" + " Q_OBJECT\n" + "public:\n" + " typedef QPair Item;\n" + " \n" + " Core::Id dataId(); // the higher the closer to top.\n" + " \n" + " unsigned int priority() = 0;\n" + " \n" + " QVariant defaultValue(Kit@*) = 0;\n" + "};\n" + << + "#define Q_OBJECT \\\n" + "public: \\\n" + " template inline void qt_check_for_QOBJECT_macro(T &_q_argument) \\\n" + " { int i = qYouForgotTheQ_OBJECT_Macro(this, &_q_argument); i = i; } \\\n" + " QMetaObject staticMetaObject; \\\n" + " void *qt_metacast(const char *); \\\n" + " static inline QString tr(const char *s, const char *f = 0); \\\n" + " \n" + "class KitInformation\n" + "{\n" + " Q_OBJECT\n" + "public:\n" + " typedef QPair Item;\n" + " \n" + " Core::Id dataId(); // the higher the closer to top.\n" + " \n" + " unsigned int priority() = 0;\n" + " \n" + " QVariant defaultValue(Kit *) = 0;\n" + "};\n"; +} diff --git a/src/plugins/cpptools/cpptoolsplugin.h b/src/plugins/cpptools/cpptoolsplugin.h index a26ae0ef271..9bb89b67075 100644 --- a/src/plugins/cpptools/cpptoolsplugin.h +++ b/src/plugins/cpptools/cpptoolsplugin.h @@ -130,6 +130,8 @@ private slots: void test_format_pointerdeclaration_multiple_declarators_data(); void test_format_pointerdeclaration_multiple_matches(); void test_format_pointerdeclaration_multiple_matches_data(); + void test_format_pointerdeclaration_macros(); + void test_format_pointerdeclaration_macros_data(); void test_modelmanager_paths(); void test_modelmanager_framework_headers();