From a6c8158484931c41e026b265297a1f2ae8a6e2c7 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Wed, 11 Sep 2013 12:00:07 +0200 Subject: [PATCH] CppEditor: implement ExtractLiteralAsParameter quickfix Task-number: QTCREATORBUG-9617 Change-Id: I6c6313746b837775bab665bb7019a2adf0b0f286 Reviewed-by: Nikolai Kosjar --- doc/src/editors/creator-editors.qdoc | 6 + src/plugins/cppeditor/cppeditorplugin.h | 8 + src/plugins/cppeditor/cppquickfix_test.cpp | 183 ++++++++++++ src/plugins/cppeditor/cppquickfixes.cpp | 321 +++++++++++++++++++++ src/plugins/cppeditor/cppquickfixes.h | 11 + 5 files changed, 529 insertions(+) diff --git a/doc/src/editors/creator-editors.qdoc b/doc/src/editors/creator-editors.qdoc index ecdc9bb54f2..65e8045f1ab 100644 --- a/doc/src/editors/creator-editors.qdoc +++ b/doc/src/editors/creator-editors.qdoc @@ -1834,6 +1834,12 @@ code with a call to the new method. Enter a name for the method in the \gui {Extract Function Refactoring} dialog. \li Block of code selected + \row + \li Extract Constant as Function Parameter + \li Replaces the selected literal and all its occurrences with the function parameter + \c{newParameter}. The parameter \c{newParameter} will have the original literal as the + default value. + \li Block of code selected \row \li Add Local Declaration \li diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h index 389ccd43e65..74e031fc3fc 100644 --- a/src/plugins/cppeditor/cppeditorplugin.h +++ b/src/plugins/cppeditor/cppeditorplugin.h @@ -250,6 +250,14 @@ private slots: void test_quickfix_AssignToLocalVariable_noReturnFunc2(); void test_quickfix_AssignToLocalVariable_noSignatureMatch(); + void test_quickfix_ExtractLiteralAsParameter_typeDeduction_data(); + void test_quickfix_ExtractLiteralAsParameter_typeDeduction(); + void test_quickfix_ExtractLiteralAsParameter_freeFunction(); + void test_quickfix_ExtractLiteralAsParameter_freeFunction_separateFiles(); + void test_quickfix_ExtractLiteralAsParameter_memberFunction(); + void test_quickfix_ExtractLiteralAsParameter_memberFunction_separateFiles(); + void test_quickfix_ExtractLiteralAsParameter_memberFunctionInline(); + void test_quickfix_InsertVirtualMethods_onlyDecl(); void test_quickfix_InsertVirtualMethods_onlyDeclWithoutVirtual(); void test_quickfix_InsertVirtualMethods_Access(); diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index c20e2570c39..7c4051d389e 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -3326,6 +3326,189 @@ void CppEditorPlugin::test_quickfix_AssignToLocalVariable_noSignatureMatch() data.run(&factory); } +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_typeDeduction_data() +{ + QTest::addColumn("typeString"); + QTest::addColumn("literal"); + QTest::newRow("int") + << QByteArray("int ") << QByteArray("156"); + QTest::newRow("unsigned int") + << QByteArray("unsigned int ") << QByteArray("156u"); + QTest::newRow("long") + << QByteArray("long ") << QByteArray("156l"); + QTest::newRow("unsigned long") + << QByteArray("unsigned long ") << QByteArray("156ul"); + QTest::newRow("long long") + << QByteArray("long long ") << QByteArray("156ll"); + QTest::newRow("unsigned long long") + << QByteArray("unsigned long long ") << QByteArray("156ull"); + QTest::newRow("float") + << QByteArray("float ") << QByteArray("3.14159f"); + QTest::newRow("double") + << QByteArray("double ") << QByteArray("3.14159"); + QTest::newRow("long double") + << QByteArray("long double ") << QByteArray("3.14159L"); + QTest::newRow("bool") + << QByteArray("bool ") << QByteArray("true"); + QTest::newRow("bool") + << QByteArray("bool ") << QByteArray("false"); + QTest::newRow("char") + << QByteArray("char ") << QByteArray("'X'"); + QTest::newRow("wchar_t") + << QByteArray("wchar_t ") << QByteArray("L'X'"); + QTest::newRow("char16_t") + << QByteArray("char16_t ") << QByteArray("u'X'"); + QTest::newRow("char32_t") + << QByteArray("char32_t ") << QByteArray("U'X'"); + QTest::newRow("const char *") + << QByteArray("const char *") << QByteArray("\"narf\""); + QTest::newRow("const wchar_t *") + << QByteArray("const wchar_t *") << QByteArray("L\"narf\""); + QTest::newRow("const char16_t *") + << QByteArray("const char16_t *") << QByteArray("u\"narf\""); + QTest::newRow("const char32_t *") + << QByteArray("const char32_t *") << QByteArray("U\"narf\""); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_typeDeduction() +{ + QFETCH(QByteArray, typeString); + QFETCH(QByteArray, literal); + const QByteArray original = QByteArray("void foo() {return @") + literal + QByteArray(";}"); + const QByteArray expected = QByteArray("void foo(") + typeString + QByteArray("newParameter = ") + + literal + QByteArray(") {return newParameter;}\n"); + + if (literal == "3.14159") { + qWarning("Literal 3.14159 is wrongly reported as int. Skipping."); + return; + } else if (literal == "3.14159L") { + qWarning("Literal 3.14159L is wrongly reported as long. Skipping."); + return; + } + + ExtractLiteralAsParameter factory; + TestCase data(original, expected); + data.run(&factory); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_freeFunction() +{ + const QByteArray original = + "void foo(const char *a, long b = 1)\n" + "{return 1@56 + 123 + 156;}"; + const QByteArray expected = + "void foo(const char *a, long b = 1, int newParameter = 156)\n" + "{return newParameter + 123 + newParameter;}\n"; + + ExtractLiteralAsParameter factory; + TestCase data(original, expected); + data.run(&factory); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_freeFunction_separateFiles() +{ + QList testFiles; + QByteArray original; + QByteArray expected; + + // Header File + original = + "void foo(const char *a, long b = 1);"; + expected = + "void foo(const char *a, long b = 1, int newParameter = 156);\n"; + testFiles << TestDocument::create(original, expected, QLatin1String("file.h")); + + // Source File + original = + "void foo(const char *a, long b)\n" + "{return 1@56 + 123 + 156;}"; + expected = + "void foo(const char *a, long b, int newParameter)\n" + "{return newParameter + 123 + newParameter;}\n"; + testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp")); + + ExtractLiteralAsParameter factory; + TestCase data(testFiles); + data.run(&factory); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_memberFunction() +{ + const QByteArray original = + "class Narf {\n" + "public:\n" + " int zort();\n" + "};\n\n" + "int Narf::zort()\n" + "{ return 15@5 + 1; }"; + const QByteArray expected = + "class Narf {\n" + "public:\n" + " int zort(int newParameter = 155);\n" + "};\n\n" + "int Narf::zort(int newParameter)\n" + "{ return newParameter + 1; }\n"; + + ExtractLiteralAsParameter factory; + TestCase data(original, expected); + data.run(&factory); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_memberFunction_separateFiles() +{ + QList testFiles; + QByteArray original; + QByteArray expected; + + // Header File + original = + "class Narf {\n" + "public:\n" + " int zort();\n" + "};"; + expected = + "class Narf {\n" + "public:\n" + " int zort(int newParameter = 155);\n" + "};\n"; + testFiles << TestDocument::create(original, expected, QLatin1String("file.h")); + + // Source File + original = + "#include \"file.h\"\n\n" + "int Narf::zort()\n" + "{ return 15@5 + 1; }"; + expected = + "#include \"file.h\"\n\n" + "int Narf::zort(int newParameter)\n" + "{ return newParameter + 1; }\n"; + testFiles << TestDocument::create(original, expected, QLatin1String("file.cpp")); + + ExtractLiteralAsParameter factory; + TestCase data(testFiles); + data.run(&factory); +} + +void CppEditorPlugin::test_quickfix_ExtractLiteralAsParameter_memberFunctionInline() +{ + const QByteArray original = + "class Narf {\n" + "public:\n" + " int zort()\n" + " { return 15@5 + 1; }\n" + "};"; + const QByteArray expected = + "class Narf {\n" + "public:\n" + " int zort(int newParameter = 155)\n" + " { return newParameter + 1; }\n" + "};\n"; + + ExtractLiteralAsParameter factory; + TestCase data(original, expected); + data.run(&factory); +} + /// Check: Insert only declarations void CppEditorPlugin::test_quickfix_InsertVirtualMethods_onlyDecl() { diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index fd01a487f4b..a6d9dd1900b 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -121,6 +121,7 @@ void CppEditor::Internal::registerQuickFixes(ExtensionSystem::IPlugin *plugIn) plugIn->addAutoReleasedObject(new ApplyDeclDefLinkChanges); plugIn->addAutoReleasedObject(new ExtractFunction); + plugIn->addAutoReleasedObject(new ExtractLiteralAsParameter); plugIn->addAutoReleasedObject(new GenerateGetterSetter); plugIn->addAutoReleasedObject(new InsertDeclFromDef); plugIn->addAutoReleasedObject(new InsertDefFromDecl); @@ -3554,6 +3555,326 @@ void ExtractFunction::match(const CppQuickFixInterface &interface, QuickFixOpera namespace { +struct ReplaceLiteralsResult +{ + Token token; + QString literalText; +}; + +template +class ReplaceLiterals : private ASTVisitor +{ +public: + ReplaceLiterals(const CppRefactoringFilePtr &file, ChangeSet *changes, T *literal) + : ASTVisitor(file->cppDocument()->translationUnit()), m_file(file), m_changes(changes), + m_literal(literal) + { + m_result.token = m_file->tokenAt(literal->firstToken()); + m_literalTokenText = m_result.token.spell(); + m_result.literalText = QLatin1String(m_literalTokenText); + if (m_result.token.isCharLiteral()) { + m_result.literalText.prepend(QLatin1Char('\'')); + m_result.literalText.append(QLatin1Char('\'')); + if (m_result.token.kind() == T_WIDE_CHAR_LITERAL) + m_result.literalText.prepend(QLatin1Char('L')); + else if (m_result.token.kind() == T_UTF16_CHAR_LITERAL) + m_result.literalText.prepend(QLatin1Char('u')); + else if (m_result.token.kind() == T_UTF32_CHAR_LITERAL) + m_result.literalText.prepend(QLatin1Char('U')); + } else if (m_result.token.isStringLiteral()) { + m_result.literalText.prepend(QLatin1Char('"')); + m_result.literalText.append(QLatin1Char('"')); + if (m_result.token.kind() == T_WIDE_STRING_LITERAL) + m_result.literalText.prepend(QLatin1Char('L')); + else if (m_result.token.kind() == T_UTF16_STRING_LITERAL) + m_result.literalText.prepend(QLatin1Char('u')); + else if (m_result.token.kind() == T_UTF32_STRING_LITERAL) + m_result.literalText.prepend(QLatin1Char('U')); + } + } + + ReplaceLiteralsResult apply(AST *ast) + { + ast->accept(this); + return m_result; + } + +private: + bool visit(T *ast) + { + if (ast != m_literal + && strcmp(m_file->tokenAt(ast->firstToken()).spell(), m_literalTokenText) != 0) { + return true; + } + int start, end; + m_file->startAndEndOf(ast->firstToken(), &start, &end); + m_changes->replace(start, end, QLatin1String("newParameter")); + return true; + } + + const CppRefactoringFilePtr &m_file; + ChangeSet *m_changes; + T *m_literal; + const char *m_literalTokenText; + ReplaceLiteralsResult m_result; +}; + +class ExtractLiteralAsParameterOp : public CppQuickFixOperation +{ +public: + ExtractLiteralAsParameterOp(const CppQuickFixInterface &interface, int priority, + ExpressionAST *literal, FunctionDefinitionAST *function) + : CppQuickFixOperation(interface, priority), + m_literal(literal), + m_functionDefinition(function) + { + setDescription(QApplication::translate("CppTools::QuickFix", + "Extract Constant as Function Parameter")); + } + + struct FoundDeclaration + { + FoundDeclaration() + : ast(0) + {} + + FunctionDeclaratorAST *ast; + CppRefactoringFilePtr file; + }; + + FoundDeclaration findDeclaration(const CppRefactoringChanges &refactoring, + FunctionDefinitionAST *ast) + { + FoundDeclaration result; + Function *func = ast->symbol; + QString declFileName; + if (Class *matchingClass = isMemberFunction(assistInterface()->context(), func)) { + // Dealing with member functions + const QualifiedNameId *qName = func->name()->asQualifiedNameId(); + for (Symbol *s = matchingClass->find(qName->identifier()); s; s = s->next()) { + if (!s->name() + || !qName->identifier()->isEqualTo(s->identifier()) + || !s->type()->isFunctionType() + || !s->type().isEqualTo(func->type()) + || s->isFunction()) { + continue; + } + + declFileName = QString::fromUtf8(matchingClass->fileName(), + matchingClass->fileNameLength()); + + result.file = refactoring.file(declFileName); + ASTPath astPath(result.file->cppDocument()); + const QList path = astPath(s->line(), s->column()); + SimpleDeclarationAST *simpleDecl; + for (int idx = 0; idx < path.size(); ++idx) { + AST *node = path.at(idx); + simpleDecl = node->asSimpleDeclaration(); + if (simpleDecl) { + if (simpleDecl->symbols && !simpleDecl->symbols->next) { + result.ast = functionDeclarator(simpleDecl); + return result; + } + } + } + + if (simpleDecl) + break; + } + } else if (Namespace *matchingNamespace + = isNamespaceFunction(assistInterface()->context(), func)) { + // Dealing with free functions and inline member functions. + bool isHeaderFile; + declFileName = correspondingHeaderOrSource(fileName(), &isHeaderFile); + if (!QFile::exists(declFileName)) + return FoundDeclaration(); + result.file = refactoring.file(declFileName); + if (!result.file) + return FoundDeclaration(); + const LookupContext lc(result.file->cppDocument(), snapshot()); + const QList candidates = lc.lookup(func->name(), matchingNamespace); + for (int i = 0; i < candidates.size(); ++i) { + if (Symbol *s = candidates.at(i).declaration()) { + if (s->asDeclaration()) { + ASTPath astPath(result.file->cppDocument()); + const QList path = astPath(s->line(), s->column()); + for (int idx = 0; idx < path.size(); ++idx) { + AST *node = path.at(idx); + SimpleDeclarationAST *simpleDecl = node->asSimpleDeclaration(); + if (simpleDecl) { + result.ast = functionDeclarator(simpleDecl); + return result; + } + } + } + } + } + } + return result; + } + + void perform() + { + FunctionDeclaratorAST *functionDeclaratorOfDefinition + = functionDeclarator(m_functionDefinition); + const CppRefactoringChanges refactoring(snapshot()); + const CppRefactoringFilePtr currentFile = refactoring.file(fileName()); + deduceTypeNameOfLiteral(currentFile->cppDocument()); + + ChangeSet changes; + if (NumericLiteralAST *concreteLiteral = m_literal->asNumericLiteral()) { + m_literalInfo = ReplaceLiterals(currentFile, &changes, + concreteLiteral) + .apply(m_functionDefinition->function_body); + } else if (StringLiteralAST *concreteLiteral = m_literal->asStringLiteral()) { + m_literalInfo = ReplaceLiterals(currentFile, &changes, + concreteLiteral) + .apply(m_functionDefinition->function_body); + } else if (BoolLiteralAST *concreteLiteral = m_literal->asBoolLiteral()) { + m_literalInfo = ReplaceLiterals(currentFile, &changes, + concreteLiteral) + .apply(m_functionDefinition->function_body); + } + const FoundDeclaration functionDeclaration + = findDeclaration(refactoring, m_functionDefinition); + appendFunctionParameter(functionDeclaratorOfDefinition, currentFile, &changes, + !functionDeclaration.ast); + if (functionDeclaration.ast) { + if (currentFile->fileName() != functionDeclaration.file->fileName()) { + ChangeSet declChanges; + appendFunctionParameter(functionDeclaration.ast, functionDeclaration.file, &declChanges, + true); + functionDeclaration.file->setChangeSet(declChanges); + functionDeclaration.file->apply(); + } else { + appendFunctionParameter(functionDeclaration.ast, currentFile, &changes, + true); + } + } + currentFile->setChangeSet(changes); + currentFile->apply(); + } + +private: + bool hasParameters(FunctionDeclaratorAST *ast) const + { + return ast->parameter_declaration_clause + && ast->parameter_declaration_clause->parameter_declaration_list + && ast->parameter_declaration_clause->parameter_declaration_list->value; + } + + void deduceTypeNameOfLiteral(const Document::Ptr &document) + { + TypeOfExpression typeOfExpression; + typeOfExpression.init(document, snapshot()); + Overview overview; + Scope *scope = m_functionDefinition->symbol->enclosingScope(); + const QList items = typeOfExpression(m_literal, document, scope); + if (!items.isEmpty()) + m_typeName = overview.prettyType(items.first().type()); + } + + QString parameterDeclarationTextToInsert(FunctionDeclaratorAST *ast) const + { + QString str; + if (hasParameters(ast)) + str = QLatin1String(", "); + str += m_typeName; + if (!m_typeName.endsWith(QLatin1Char('*'))) + str += QLatin1Char(' '); + str += QLatin1String("newParameter"); + return str; + } + + FunctionDeclaratorAST *functionDeclarator(SimpleDeclarationAST *ast) const + { + for (DeclaratorListAST *decls = ast->declarator_list; decls; decls = decls->next) { + FunctionDeclaratorAST * const functionDeclaratorAST = functionDeclarator(decls->value); + if (functionDeclaratorAST) + return functionDeclaratorAST; + } + return 0; + } + + FunctionDeclaratorAST *functionDeclarator(DeclaratorAST *ast) const + { + for (PostfixDeclaratorListAST *pds = ast->postfix_declarator_list; pds; pds = pds->next) { + FunctionDeclaratorAST *funcdecl = pds->value->asFunctionDeclarator(); + if (funcdecl) + return funcdecl; + } + return 0; + } + + FunctionDeclaratorAST *functionDeclarator(FunctionDefinitionAST *ast) const + { + return functionDeclarator(ast->declarator); + } + + void appendFunctionParameter(FunctionDeclaratorAST *ast, const CppRefactoringFileConstPtr &file, + ChangeSet *changes, bool addDefaultValue) + { + if (!ast) + return; + if (m_declarationInsertionString.isEmpty()) + m_declarationInsertionString = parameterDeclarationTextToInsert(ast); + QString insertion = m_declarationInsertionString; + if (addDefaultValue) + insertion += QLatin1String(" = ") + m_literalInfo.literalText; + changes->insert(file->startOf(ast->rparen_token), insertion); + } + + ExpressionAST *m_literal; + FunctionDefinitionAST *m_functionDefinition; + QString m_typeName; + QString m_declarationInsertionString; + ReplaceLiteralsResult m_literalInfo; +}; + +} // anonymous namespace + +void ExtractLiteralAsParameter::match(const CppQuickFixInterface &interface, + QuickFixOperations &result) +{ + const QList &path = interface->path(); + if (path.count() < 2) + return; + + AST * const lastAst = path.last(); + ExpressionAST *literal; + if (!((literal = lastAst->asNumericLiteral()) + || (literal = lastAst->asStringLiteral()) + || (literal = lastAst->asBoolLiteral()))) { + return; + } + + FunctionDefinitionAST *function; + int i = path.count() - 2; + while (!(function = path.at(i)->asFunctionDefinition())) { + // Ignore literals in lambda expressions for now. + if (path.at(i)->asLambdaExpression()) + return; + if (--i < 0) + return; + } + + FunctionDeclaratorAST *functionDeclarator + = function->declarator->postfix_declarator_list->value->asFunctionDeclarator(); + if (functionDeclarator + && functionDeclarator->parameter_declaration_clause + && functionDeclarator->parameter_declaration_clause->dot_dot_dot_token) { + // Do not handle functions with ellipsis parameter. + return; + } + + const int priority = path.size() - 1; + QuickFixOperation::Ptr op( + new ExtractLiteralAsParameterOp(interface, priority, literal, function)); + result.append(op); +} + +namespace { + class InsertQtPropertyMembersOp: public CppQuickFixOperation { public: diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index e218bd02fa1..7d7aa058b01 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -456,6 +456,17 @@ public: void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result); }; +/*! + Extracts the selected constant and converts it to a parameter of the current function. + + Activates on numeric, bool, character, or string literal in the function body. + */ +class ExtractLiteralAsParameter : public CppQuickFixFactory +{ +public: + void match(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result); +}; + /*! Adds getter and setter functions for a member variable */