diff options
author | Christian Kandeler <christian.kandeler@qt.io> | 2023-05-11 13:16:27 +0200 |
---|---|---|
committer | Christian Kandeler <christian.kandeler@qt.io> | 2023-05-16 10:32:04 +0000 |
commit | 7869c4e8103d1ba6e71b683e94ed7ae8dd04b8c0 (patch) | |
tree | abcfc51495369380df205981db2ad9677e349a9e | |
parent | 9758e7145828e1303a33af3b1ae04c4210b55a28 (diff) | |
download | qt-creator-7869c4e8103d1ba6e71b683e94ed7ae8dd04b8c0.tar.gz |
CppEditor: Add quickfix for creating a member function from use
Fixes: QTCREATORBUG-10356
Change-Id: If7376e766f2817949259591560c74bd6fb0e0cd6
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
-rw-r--r-- | src/plugins/cppeditor/cppquickfix_test.cpp | 116 | ||||
-rw-r--r-- | src/plugins/cppeditor/cppquickfixes.cpp | 253 | ||||
-rw-r--r-- | src/plugins/cppeditor/cppquickfixes.h | 15 |
3 files changed, 311 insertions, 73 deletions
diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index e78667c2fb..f57670c161 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -3968,6 +3968,122 @@ void QuickfixTest::testInsertMemberFromUse_data() "};\n" "void C::setValue(int v) { @m_value = v; }\n"; QTest::addRow("add static member to this (non-inline)") << original << expected; + + original = + "struct S {\n\n};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { m_s.@setValue(v); }\n" + "private:\n" + " S m_s;\n" + "};\n"; + expected = + "struct S {\n\n" + " void setValue(int);\n" + "};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { m_s.setValue(v); }\n" + "private:\n" + " S m_s;\n" + "};\n"; + QTest::addRow("add member function to other struct") << original << expected; + + original = + "struct S {\n\n};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { S::@setValue(v); }\n" + "};\n"; + expected = + "struct S {\n\n" + " static void setValue(int);\n" + "};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { S::setValue(v); }\n" + "};\n"; + QTest::addRow("add static member function to other struct (explicit)") << original << expected; + + original = + "struct S {\n\n};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { m_s.@setValue(v); }\n" + "private:\n" + " static S m_s;\n" + "};\n"; + expected = + "struct S {\n\n" + " static void setValue(int);\n" + "};\n" + "class C {\n" + "public:\n" + " void setValue(int v) { m_s.setValue(v); }\n" + "private:\n" + " static S m_s;\n" + "};\n"; + QTest::addRow("add static member function to other struct (implicit)") << original << expected; + + original = + "class C {\n" + "public:\n" + " void setValue(int v);\n" + "};\n" + "void C::setValue(int v) { this->@setValueInternal(v); }\n"; + expected = + "class C {\n" + "public:\n" + " void setValue(int v);\n" + "private:\n" + " void setValueInternal(int);\n" + "};\n" + "void C::setValue(int v) { this->setValueInternal(v); }\n"; + QTest::addRow("add member function to this (explicit)") << original << expected; + + original = + "class C {\n" + "public:\n" + " void setValue(int v) { @setValueInternal(v); }\n" + "};\n"; + expected = + "class C {\n" + "public:\n" + " void setValue(int v) { setValueInternal(v); }\n" + "private:\n" + " void setValueInternal(int);\n" + "};\n"; + QTest::addRow("add member function to this (implicit)") << original << expected; + + original = + "class C {\n" + "public:\n" + " static int value() { int i = @valueInternal(); return i; }\n" + "};\n"; + expected = + "class C {\n" + "public:\n" + " static int value() { int i = @valueInternal(); return i; }\n" + "private:\n" + " static int valueInternal();\n" + "};\n"; + QTest::addRow("add static member function to this (inline)") << original << expected; + + original = + "class C {\n" + "public:\n" + " static int value();\n" + "};\n" + "int C::value() { return @valueInternal(); }\n"; + expected = + "class C {\n" + "public:\n" + " static int value();\n" + "private:\n" + " static int valueInternal();\n" + "};\n" + "int C::value() { return valueInternal(); }\n"; + QTest::addRow("add static member function to this (non-inline)") << original << expected; } void QuickfixTest::testInsertMemberFromUse() diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 60d799cb5f..85489b877e 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -308,11 +308,14 @@ QString nameString(const NameAST *name) return CppCodeStyleSettings::currentProjectCodeStyleOverview().prettyName(name->name); } -QString declFromExpr(const ExpressionAST *expr, const NameAST *varName, +// FIXME: Needs to consider the scope at the insertion site. +QString declFromExpr(const TypeOrExpr &typeOrExpr, const CallAST *call, const NameAST *varName, const Snapshot &snapshot, const LookupContext &context, const CppRefactoringFilePtr &file) { - const auto getTypeFromUser = [varName]() -> QString { + const auto getTypeFromUser = [varName, call]() -> QString { + if (call) + return {}; const QString typeFromUser = QInputDialog::getText(Core::ICore::dialogParent(), Tr::tr("Provide the type"), Tr::tr("Data type:"), QLineEdit::Normal); @@ -320,29 +323,42 @@ QString declFromExpr(const ExpressionAST *expr, const NameAST *varName, return typeFromUser + ' ' + nameString(varName); return {}; }; + const auto getTypeOfExpr = [&](const ExpressionAST *expr) -> FullySpecifiedType { + TypeOfExpression typeOfExpression; + typeOfExpression.init(file->cppDocument(), snapshot, context.bindings()); + Scope *scope = file->scopeAt(expr->firstToken()); + const QList<LookupItem> result = typeOfExpression( + file->textOf(expr).toUtf8(), scope, TypeOfExpression::Preprocess); + if (result.isEmpty()) + return {}; - TypeOfExpression typeOfExpression; - typeOfExpression.init(file->cppDocument(), snapshot, context.bindings()); - Scope *scope = file->scopeAt(expr->firstToken()); - const QList<LookupItem> result = typeOfExpression( - file->textOf(expr).toUtf8(), scope, TypeOfExpression::Preprocess); - if (result.isEmpty()) - return getTypeFromUser(); + SubstitutionEnvironment env; + env.setContext(context); + env.switchScope(result.first().scope()); + ClassOrNamespace *con = typeOfExpression.context().lookupType(scope); + if (!con) + con = typeOfExpression.context().globalNamespace(); + UseMinimalNames q(con); + env.enter(&q); - SubstitutionEnvironment env; - env.setContext(context); - env.switchScope(result.first().scope()); - ClassOrNamespace *con = typeOfExpression.context().lookupType(scope); - if (!con) - con = typeOfExpression.context().globalNamespace(); - UseMinimalNames q(con); - env.enter(&q); + Control *control = context.bindings()->control().data(); + return rewriteType(result.first().type(), &env, control); + }; - Control *control = context.bindings()->control().data(); - FullySpecifiedType tn = rewriteType(result.first().type(), &env, control); - if (!tn.isValid()) - return getTypeFromUser(); - return CppCodeStyleSettings::currentProjectCodeStyleOverview().prettyType(tn, varName->name); + const Overview oo = CppCodeStyleSettings::currentProjectCodeStyleOverview(); + const FullySpecifiedType type = std::holds_alternative<FullySpecifiedType>(typeOrExpr) + ? std::get<FullySpecifiedType>(typeOrExpr) + : getTypeOfExpr(std::get<const ExpressionAST *>(typeOrExpr)); + if (!call) + return type.isValid() ? oo.prettyType(type, varName->name) : getTypeFromUser(); + + Function func(file->cppDocument()->translationUnit(), 0, varName->name); + for (ExpressionListAST *it = call->expression_list; it; it = it->next) { + Argument * const arg = new Argument(nullptr, 0, nullptr); + arg->setType(getTypeOfExpr(it->value)); + func.addMember(arg); + } + return oo.prettyType(type) + ' ' + oo.prettyType(func.type(), varName->name); } } // anonymous namespace @@ -1654,7 +1670,7 @@ private: if (currentFile->cppDocument()->languageFeatures().cxx11Enabled && settings->useAuto) return "auto " + oo.prettyName(simpleNameAST->name); - return declFromExpr(binaryAST->right_expression, simpleNameAST, snapshot(), + return declFromExpr(binaryAST->right_expression, nullptr, simpleNameAST, snapshot(), context(), currentFile); } @@ -2920,20 +2936,24 @@ public: const CppQuickFixInterface &interface, const Class *theClass, const NameAST *memberName, - const ExpressionAST *initExpr, + const TypeOrExpr &typeOrExpr, + const CallAST *call, InsertionPointLocator::AccessSpec accessSpec, bool makeStatic) : CppQuickFixOperation(interface), - m_class(theClass), m_memberName(memberName), m_initExpr(initExpr), + m_class(theClass), m_memberName(memberName), m_typeOrExpr(typeOrExpr), m_call(call), m_accessSpec(accessSpec), m_makeStatic(makeStatic) { - setDescription(Tr::tr("Add Class Member \"%1\"").arg(nameString(memberName))); + if (call) + setDescription(Tr::tr("Add Member Function \"%1\"").arg(nameString(memberName))); + else + setDescription(Tr::tr("Add Class Member \"%1\"").arg(nameString(memberName))); } private: void perform() override { - QString decl = declFromExpr(m_initExpr, m_memberName, snapshot(), context(), + QString decl = declFromExpr(m_typeOrExpr, m_call, m_memberName, snapshot(), context(), currentFile()); if (decl.isEmpty()) return; @@ -2959,7 +2979,8 @@ private: const Class * const m_class; const NameAST * const m_memberName; - const ExpressionAST * const m_initExpr; + const TypeOrExpr m_typeOrExpr; + const CallAST * m_call; const InsertionPointLocator::AccessSpec m_accessSpec; const bool m_makeStatic; }; @@ -3002,6 +3023,9 @@ void AddDeclarationForUndeclaredIdentifier::collectOperations( const QList<AST *> &path = interface.path(); const CppRefactoringFilePtr &file = interface.currentFile(); for (int index = path.size() - 1; index != -1; --index) { + if (const auto call = path.at(index)->asCall()) + return handleCall(call, interface, result); + // We only trigger if the identifier appears on the left-hand side of an // assignment expression. const auto binExpr = path.at(index)->asBinaryExpression(); @@ -3020,7 +3044,7 @@ void AddDeclarationForUndeclaredIdentifier::collectOperations( && memberAccess->member_name == path.last()) { maybeAddMember(interface, file->scopeAt(memberAccess->firstToken()), file->textOf(memberAccess->base_expression).toUtf8(), - binExpr->right_expression, result); + binExpr->right_expression, nullptr, result); } return; } @@ -3031,43 +3055,8 @@ void AddDeclarationForUndeclaredIdentifier::collectOperations( // In the case of "A::|b = c", add a static member b to A. if (const auto qualName = idExpr->name->asQualifiedName()) { - if (!interface.isCursorOn(qualName->unqualified_name)) - return; - if (qualName->unqualified_name != path.last()) - return; - if (!qualName->nested_name_specifier_list) - return; - - const NameAST * const topLevelName - = qualName->nested_name_specifier_list->value->class_or_namespace_name; - if (!topLevelName) - return; - ClassOrNamespace * const classOrNamespace = interface.context().lookupType( - topLevelName->name, file->scopeAt(qualName->firstToken())); - if (!classOrNamespace) - return; - QList<const Name *> otherNames; - for (auto it = qualName->nested_name_specifier_list->next; it; it = it->next) { - if (!it->value || !it->value->class_or_namespace_name) - return; - otherNames << it->value->class_or_namespace_name->name; - } - - const Class *theClass = nullptr; - if (!otherNames.isEmpty()) { - const Symbol * const symbol = classOrNamespace->lookupInScope(otherNames); - if (!symbol) - return; - theClass = symbol->asClass(); - } else { - theClass = classOrNamespace->rootClass(); - } - if (theClass) { - result << new InsertMemberFromInitializationOp( - interface, theClass, path.last()->asName(), binExpr->right_expression, - InsertionPointLocator::Public, true); - } - return; + return maybeAddStaticMember(interface, qualName, binExpr->right_expression, nullptr, + result); } // For an unqualified access, offer a local declaration and, if we are @@ -3076,12 +3065,92 @@ void AddDeclarationForUndeclaredIdentifier::collectOperations( if (!m_membersOnly) result << new AddLocalDeclarationOp(interface, index, binExpr, simpleName); maybeAddMember(interface, file->scopeAt(idExpr->firstToken()), "this", - binExpr->right_expression, result); + binExpr->right_expression, nullptr, result); return; } } } +void AddDeclarationForUndeclaredIdentifier::handleCall( + const CallAST *call, const CppQuickFixInterface &interface, + TextEditor::QuickFixOperations &result) +{ + if (!call->base_expression) + return; + + // In order to find out the return type, we need to check the context of the call. + // If it is a statement expression, the type is void, if it's a binary expression, + // we assume the type of the other side of the expression, if it's a return statement, + // we use the return type of the surrounding function, and if it's a declaration, + // we use the type of the variable. Other cases are not supported. + const QList<AST *> &path = interface.path(); + const CppRefactoringFilePtr &file = interface.currentFile(); + TypeOrExpr returnTypeOrExpr; + for (auto it = path.rbegin(); it != path.rend(); ++it) { + if ((*it)->asCompoundStatement()) + return; + if ((*it)->asExpressionStatement()) { + returnTypeOrExpr = FullySpecifiedType(new VoidType); + break; + } + if (const auto binExpr = (*it)->asBinaryExpression()) { + returnTypeOrExpr = interface.isCursorOn(binExpr->left_expression) + ? binExpr->right_expression : binExpr->left_expression; + break; + } + if (const auto returnExpr = (*it)->asReturnStatement()) { + for (auto it2 = std::next(it); it2 != path.rend(); ++it2) { + if (const auto func = (*it2)->asFunctionDefinition()) { + if (!func->symbol) + return; + returnTypeOrExpr = func->symbol->returnType(); + break; + } + } + break; + } + if (const auto declarator = (*it)->asDeclarator()) { + if (!interface.isCursorOn(declarator->initializer)) + return; + const auto decl = (*std::next(it))->asSimpleDeclaration(); + if (!decl || !decl->symbols) + return; + if (!decl->symbols->value->type().isValid()) + return; + returnTypeOrExpr = decl->symbols->value->type(); + break; + } + } + + if (std::holds_alternative<const ExpressionAST *>(returnTypeOrExpr) + && !std::get<const ExpressionAST *>(returnTypeOrExpr)) { + return; + } + + // a.f() + if (const auto memberAccess = call->base_expression->asMemberAccess()) { + if (!interface.isCursorOn(memberAccess->member_name)) + return; + maybeAddMember( + interface, file->scopeAt(call->firstToken()), + file->textOf(memberAccess->base_expression).toUtf8(), returnTypeOrExpr, call, result); + } + + const auto idExpr = call->base_expression->asIdExpression(); + if (!idExpr || !idExpr->name) + return; + + // A::f() + if (const auto qualName = idExpr->name->asQualifiedName()) + return maybeAddStaticMember(interface, qualName, returnTypeOrExpr, call, result); + + // f() + if (const auto simpleName = idExpr->name->asSimpleName()) { + maybeAddMember(interface, file->scopeAt(idExpr->firstToken()), "this", + returnTypeOrExpr, call, result); + } +} + bool AddDeclarationForUndeclaredIdentifier::checkForMemberInitializer( const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result) { @@ -3129,13 +3198,13 @@ bool AddDeclarationForUndeclaredIdentifier::checkForMemberInitializer( result << new InsertMemberFromInitializationOp( interface, theClass, memInitializer->name->asSimpleName(), memInitializer->expression, - InsertionPointLocator::Private, false); + nullptr, InsertionPointLocator::Private, false); return false; } void AddDeclarationForUndeclaredIdentifier::maybeAddMember( const CppQuickFixInterface &interface, Scope *scope, const QByteArray &classTypeExpr, - const ExpressionAST *initExpr, TextEditor::QuickFixOperations &result) + const TypeOrExpr &typeOrExpr, const CallAST *call, TextEditor::QuickFixOperations &result) { const QList<AST *> &path = interface.path(); @@ -3199,7 +3268,51 @@ void AddDeclarationForUndeclaredIdentifier::maybeAddMember( } } result << new InsertMemberFromInitializationOp(interface, theClass, path.last()->asName(), - initExpr, accessSpec, needsStatic); + typeOrExpr, call, accessSpec, needsStatic); +} + +void AddDeclarationForUndeclaredIdentifier::maybeAddStaticMember( + const CppQuickFixInterface &interface, const QualifiedNameAST *qualName, + const TypeOrExpr &typeOrExpr, const CallAST *call, TextEditor::QuickFixOperations &result) +{ + const QList<AST *> &path = interface.path(); + + if (!interface.isCursorOn(qualName->unqualified_name)) + return; + if (qualName->unqualified_name != path.last()) + return; + if (!qualName->nested_name_specifier_list) + return; + + const NameAST * const topLevelName + = qualName->nested_name_specifier_list->value->class_or_namespace_name; + if (!topLevelName) + return; + ClassOrNamespace * const classOrNamespace = interface.context().lookupType( + topLevelName->name, interface.currentFile()->scopeAt(qualName->firstToken())); + if (!classOrNamespace) + return; + QList<const Name *> otherNames; + for (auto it = qualName->nested_name_specifier_list->next; it; it = it->next) { + if (!it->value || !it->value->class_or_namespace_name) + return; + otherNames << it->value->class_or_namespace_name->name; + } + + const Class *theClass = nullptr; + if (!otherNames.isEmpty()) { + const Symbol * const symbol = classOrNamespace->lookupInScope(otherNames); + if (!symbol) + return; + theClass = symbol->asClass(); + } else { + theClass = classOrNamespace->rootClass(); + } + if (theClass) { + result << new InsertMemberFromInitializationOp( + interface, theClass, path.last()->asName(), typeOrExpr, call, + InsertionPointLocator::Public, true); + } } class MemberFunctionImplSetting diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index 04c54793e9..61db3ebf53 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -3,9 +3,10 @@ #pragma once -#include "cppeditor_global.h" #include "cppquickfix.h" +#include <variant> + /// /// Adding New Quick Fixes /// @@ -16,6 +17,7 @@ namespace CppEditor { namespace Internal { +using TypeOrExpr = std::variant<const CPlusPlus::ExpressionAST *, CPlusPlus::FullySpecifiedType>; void createCppQuickFixes(); void destroyCppQuickFixes(); @@ -370,14 +372,21 @@ public: private: void collectOperations(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result); + void handleCall(const CPlusPlus::CallAST *call, const CppQuickFixInterface &interface, + TextEditor::QuickFixOperations &result); // Returns whether to still do other checks. bool checkForMemberInitializer(const CppQuickFixInterface &interface, TextEditor::QuickFixOperations &result); void maybeAddMember(const CppQuickFixInterface &interface, CPlusPlus::Scope *scope, - const QByteArray &classTypeExpr, const CPlusPlus::ExpressionAST *initExpr, - TextEditor::QuickFixOperations &result); + const QByteArray &classTypeExpr, const TypeOrExpr &typeOrExpr, + const CPlusPlus::CallAST *call, TextEditor::QuickFixOperations &result); + + void maybeAddStaticMember( + const CppQuickFixInterface &interface, const CPlusPlus::QualifiedNameAST *qualName, + const TypeOrExpr &typeOrExpr, const CPlusPlus::CallAST *call, + TextEditor::QuickFixOperations &result); bool m_membersOnly = false; }; |