From 8da53e65873facd0dab7cf54adc19cfbc3e337ba Mon Sep 17 00:00:00 2001 From: Orgad Shaneh <orgad.shaneh@audiocodes.com> Date: Tue, 26 Nov 2013 23:16:52 +0200 Subject: [PATCH] CppEditor: Check only pure virtual functions by default Task-number: QTCREATORBUG-10154 Change-Id: Iec1b895e3f06d9d6ae36f19f6c8048c78faac514 Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com> --- src/plugins/cppeditor/cppquickfix_test.cpp | 205 +++++++++++++++++---- src/plugins/cppeditor/cppquickfixes.cpp | 136 +++++++------- src/plugins/cppeditor/cppquickfixes.h | 6 +- 3 files changed, 242 insertions(+), 105 deletions(-) diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 9f3975843ca..1a87985f17c 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -3571,14 +3571,14 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public Bas@eA {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "\n" @@ -3593,14 +3593,14 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeOnlyDeclarations << false << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public Bas@eA {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "\n" @@ -3615,38 +3615,38 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "protected:\n" - " virtual int b();\n" + " virtual int b() = 0;\n" "private:\n" - " virtual int c();\n" + " virtual int c() = 0;\n" "public slots:\n" - " virtual int d();\n" + " virtual int d() = 0;\n" "protected slots:\n" - " virtual int e();\n" + " virtual int e() = 0;\n" "private slots:\n" - " virtual int f();\n" + " virtual int f() = 0;\n" "signals:\n" - " virtual int g();\n" + " virtual int g() = 0;\n" "};\n\n" "class Der@ived : public BaseA {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "protected:\n" - " virtual int b();\n" + " virtual int b() = 0;\n" "private:\n" - " virtual int c();\n" + " virtual int c() = 0;\n" "public slots:\n" - " virtual int d();\n" + " virtual int d() = 0;\n" "protected slots:\n" - " virtual int e();\n" + " virtual int e() = 0;\n" "private slots:\n" - " virtual int f();\n" + " virtual int f() = 0;\n" "signals:\n" - " virtual int g();\n" + " virtual int g() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "\n" @@ -3673,56 +3673,57 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class BaseB : public BaseA {\n" "public:\n" - " virtual int b();\n" + " virtual int b() = 0;\n" "};\n\n" "class Der@ived : public BaseB {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class BaseB : public BaseA {\n" "public:\n" - " virtual int b();\n" + " virtual int b() = 0;\n" "};\n\n" "class Der@ived : public BaseB {\n" "\n" - " // BaseB interface\n" - "public:\n" - " virtual int b();\n" - "\n" " // BaseA interface\n" "public:\n" " virtual int a();\n" + "\n" + " // BaseB interface\n" + "public:\n" + " virtual int b();\n" "};\n" ); + // Check: Do not insert reimplemented functions twice. QTest::newRow("SuperclassOverride") << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class BaseB : public BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class Der@ived : public BaseB {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class BaseB : public BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class Der@ived : public BaseB {\n" "\n" @@ -3783,14 +3784,14 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeInsideClass << true << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public Bas@eA {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "\n" @@ -3807,14 +3808,14 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() << InsertVirtualMethodsDialog::ModeOutsideClass << true << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public Bas@eA {\n" "};" ) << _( "class BaseA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "\n" @@ -3836,7 +3837,7 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() "};\n\n" "class Derived : public Bas@eA {\n" "public:\n" - " virtual int virtualFuncA();\n" + " virtual int virtualFuncA() = 0;\n" "};" ) << _( "class BaseA {\n" @@ -3845,9 +3846,135 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data() "};\n\n" "class Derived : public Bas@eA {\n" "public:\n" + " virtual int virtualFuncA() = 0;\n" + "};\n" + ); + + // Check: One pure, one not + QTest::newRow("Some_Pure") + << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( + "class BaseA {\n" + "public:\n" + " virtual int virtualFuncA() = 0;\n" + " virtual int virtualFuncB();\n" + "};\n\n" + "class Derived : public Bas@eA {\n" + "};" + ) << _( + "class BaseA {\n" + "public:\n" + " virtual int virtualFuncA() = 0;\n" + " virtual int virtualFuncB();\n" + "};\n\n" + "class Derived : public BaseA {\n" + "\n" + " // BaseA interface\n" + "public:\n" " virtual int virtualFuncA();\n" "};\n" ); + + // Check: Pure function in derived class + QTest::newRow("Pure_in_Derived") + << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( + "class BaseA {\n" + "public:\n" + " virtual int a();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "};" + ) << _( + "class BaseA {\n" + "public:\n" + " virtual int a();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "\n" + " // BaseA interface\n" + "public:\n" + " virtual int a();\n" + "};\n" + ); + + // Check: One pure function in base class, one in derived + QTest::newRow("Pure_in_Base_And_Derived") + << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( + "class BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + " virtual int b();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int b() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "};" + ) << _( + "class BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + " virtual int b();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int b() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "\n" + " // BaseA interface\n" + "public:\n" + " virtual int a();\n" + " virtual int b();\n" + "};\n" + ); + + // Check: One pure function in base class, two in derived + QTest::newRow("Pure_in_Base_And_Derived_2") + << InsertVirtualMethodsDialog::ModeOnlyDeclarations << true << _( + "class BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + " virtual int b();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int b() = 0;\n" + " virtual int c() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "};" + ) << _( + "class BaseA {\n" + "public:\n" + " virtual int a() = 0;\n" + " virtual int b();\n" + "};\n\n" + "class BaseB : public BaseA {\n" + "public:\n" + " virtual int b() = 0;\n" + " virtual int c() = 0;\n" + "};\n\n" + "class Der@ived : public BaseB {\n" + "\n" + " // BaseA interface\n" + "public:\n" + " virtual int a();\n" + " virtual int b();\n" + "\n" + " // BaseB interface\n" + "public:\n" + " virtual int c();\n" + "};\n" + ); } void CppEditorPlugin::test_quickfix_InsertVirtualMethods() @@ -3873,7 +4000,7 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_implementationFile() original = "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class Derived : public Bas@eA {\n" "public:\n" @@ -3882,7 +4009,7 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_implementationFile() expected = "class BaseA {\n" "public:\n" - " virtual int a();\n" + " virtual int a() = 0;\n" "};\n\n" "class Derived : public BaseA {\n" "public:\n" @@ -3921,7 +4048,7 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_BaseClassInNamespace() "namespace BaseNS {\n" "class Base {\n" "public:\n" - " virtual BaseEnum a(BaseEnum e);\n" + " virtual BaseEnum a(BaseEnum e) = 0;\n" "};\n" "}\n" "class Deri@ved : public BaseNS::Base {\n" @@ -3933,7 +4060,7 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_BaseClassInNamespace() "namespace BaseNS {\n" "class Base {\n" "public:\n" - " virtual BaseEnum a(BaseEnum e);\n" + " virtual BaseEnum a(BaseEnum e) = 0;\n" "};\n" "}\n" "class Deri@ved : public BaseNS::Base {\n" diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 92c4615a43f..e2fd697ce2b 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -32,6 +32,7 @@ #include "cppeditor.h" #include "cppfunctiondecldeflink.h" #include "cppquickfixassistant.h" +#include "cppvirtualfunctionassistprovider.h" #include <coreplugin/icore.h> @@ -40,6 +41,7 @@ #include <cpptools/cpppointerdeclarationformatter.h> #include <cpptools/cpptoolsconstants.h> #include <cpptools/cpptoolsreuse.h> +#include <cpptools/functionutils.h> #include <cpptools/includeutils.h> #include <cpptools/insertionpointlocator.h> #include <cpptools/symbolfinder.h> @@ -4690,7 +4692,6 @@ public: , m_insertPosDecl(0) , m_insertPosOutside(0) , m_functionCount(0) - , m_implementedFunctionCount(0) { setDescription(QCoreApplication::translate( "CppEditor::QuickFix", "Insert Virtual Functions of Base Classes")); @@ -4742,7 +4743,7 @@ public: && (clazz = interface->context().lookupType(symbol)) && !visitedBaseClasses.contains(clazz) && !baseClasses.contains(base)) { - baseClasses << base; + baseClasses.prepend(base); baseClassQueue.enqueue(clazz); } } @@ -4755,36 +4756,36 @@ public: printer.showFunctionSignatures = true; const TextEditor::FontSettings &fs = TextEditor::TextEditorSettings::fontSettings(); const Format formatReimpFunc = fs.formatFor(C_DISABLED_CODE); + QHash<const Function *, QStandardItem *> virtualFunctions; foreach (const Class *clazz, baseClasses) { QStandardItem *itemBase = new QStandardItem(printer.prettyName(clazz->name())); - itemBase->setData(false, InsertVirtualMethodsDialog::Implemented); + itemBase->setData(false, InsertVirtualMethodsDialog::Reimplemented); itemBase->setData(qVariantFromValue((void *) clazz), InsertVirtualMethodsDialog::ClassOrFunction); - const QString baseClassName = printer.prettyName(clazz->name()); - const Qt::CheckState funcItemsCheckState = (baseClassName != QLatin1String("QObject") - && baseClassName != QLatin1String("QWidget") - && baseClassName != QLatin1String("QPaintDevice")) - ? Qt::Checked : Qt::Unchecked; for (Scope::iterator it = clazz->firstMember(); it != clazz->lastMember(); ++it) { if (const Function *func = (*it)->type()->asFunctionType()) { // Filter virtual destructors if (func->name()->asDestructorNameId()) continue; - if (!func->isVirtual()) + const Function *firstVirtual = 0; + const bool isVirtual = FunctionUtils::isVirtualFunction( + func, interface->context(), &firstVirtual); + if (!isVirtual) continue; // Filter OQbject's // - virtual const QMetaObject *metaObject() const; // - virtual void *qt_metacast(const char *); // - virtual int qt_metacall(QMetaObject::Call, int, void **); - if (baseClassName == QLatin1String("QObject")) { - if (printer.prettyName(func->name()) == QLatin1String("metaObject")) - continue; - if (printer.prettyName(func->name()) == QLatin1String("qt_metacast")) - continue; - if (printer.prettyName(func->name()) == QLatin1String("qt_metacall")) + if (printer.prettyName(firstVirtual->enclosingClass()->name()) + == QLatin1String("QObject")) { + const QString funcName = printer.prettyName(func->name()); + if (funcName == QLatin1String("metaObject") + || funcName == QLatin1String("qt_metacast") + || funcName == QLatin1String("qt_metacall")) { continue; + } } // Do not implement existing functions inside target class @@ -4802,76 +4803,83 @@ public: } } - // Do not show when reimplemented from an other class - bool funcReimplemented = false; - for (int i = baseClasses.count() - 1; i >= 0; --i) { - const Class *prevClass = baseClasses.at(i); - if (clazz == prevClass) - break; // reached current class - - for (const Symbol *symbol = prevClass->find(funcName->identifier()); - symbol; symbol = symbol->next()) { - if (!symbol->name() - || !funcName->identifier()->isEqualTo(symbol->identifier())) { - continue; - } - if (symbol->type().isEqualTo(func->type())) { - funcReimplemented = true; - break; - } - } - } - if (funcReimplemented) - continue; - // Construct function item + const bool isReimplemented = (func != firstVirtual); const bool isPureVirtual = func->isPureVirtual(); QString itemName = printer.prettyType(func->type(), func->name()); if (isPureVirtual) itemName += QLatin1String(" = 0"); const QString itemReturnTypeString = printer.prettyType(func->returnType()); - QStandardItem *funcItem = new QStandardItem( - itemName + QLatin1String(" : ") + itemReturnTypeString); - if (!funcExistsInClass) { - funcItem->setCheckable(true); - funcItem->setCheckState(Qt::Checked); - } else { + itemName += QLatin1String(" : ") + itemReturnTypeString; + if (isReimplemented) + itemName += QLatin1String(" (redeclared)"); + QStandardItem *funcItem = new QStandardItem(itemName); + funcItem->setCheckable(true); + if (isReimplemented) { + factory->setHasReimplementedFunctions(true); funcItem->setEnabled(false); - funcItem->setData(formatReimpFunc.foreground(), Qt::ForegroundRole); - if (formatReimpFunc.background().isValid()) - funcItem->setData(formatReimpFunc.background(), Qt::BackgroundRole); + funcItem->setCheckState(Qt::Unchecked); + if (QStandardItem *first = virtualFunctions[firstVirtual]) { + if (!first->data(InsertVirtualMethodsDialog::Reimplemented).toBool()) { + first->setCheckState(isPureVirtual ? Qt::Checked : Qt::Unchecked); + factory->updateCheckBoxes(first); + } + } + } else { + if (!funcExistsInClass) { + funcItem->setCheckState(isPureVirtual ? Qt::Checked : Qt::Unchecked); + } else { + funcItem->setEnabled(false); + funcItem->setCheckState(Qt::Checked); + funcItem->setData(formatReimpFunc.foreground(), Qt::ForegroundRole); + if (formatReimpFunc.background().isValid()) + funcItem->setData(formatReimpFunc.background(), Qt::BackgroundRole); + } } funcItem->setData(qVariantFromValue((void *) func), InsertVirtualMethodsDialog::ClassOrFunction); funcItem->setData(isPureVirtual, InsertVirtualMethodsDialog::PureVirtual); funcItem->setData(acessSpec(*it), InsertVirtualMethodsDialog::AccessSpec); - funcItem->setData(funcExistsInClass, InsertVirtualMethodsDialog::Implemented); - funcItem->setCheckState(funcItemsCheckState); + funcItem->setData(funcExistsInClass || isReimplemented, + InsertVirtualMethodsDialog::Reimplemented); itemBase->appendRow(funcItem); + virtualFunctions[func] = funcItem; // update internal counters - if (funcExistsInClass) - ++m_implementedFunctionCount; - else + if (!funcExistsInClass) ++m_functionCount; } } if (itemBase->hasChildren()) { + itemBase->setData(false, InsertVirtualMethodsDialog::Reimplemented); + bool enabledFound = false; + Qt::CheckState state = Qt::Unchecked; for (int i = 0; i < itemBase->rowCount(); ++i) { - if (itemBase->child(i, 0)->isCheckable()) { + QStandardItem *childItem = itemBase->child(i, 0); + if (!childItem->isEnabled()) + continue; + if (!enabledFound) { + state = childItem->checkState(); + enabledFound = true; + } + if (childItem->isCheckable()) { if (!itemBase->isCheckable()) { itemBase->setCheckable(true); itemBase->setTristate(true); - itemBase->setData(false, InsertVirtualMethodsDialog::Implemented); + itemBase->setCheckState(state); } - if (itemBase->child(i, 0)->checkState() == Qt::Checked) { - itemBase->setCheckState(Qt::Checked); + if (state != childItem->checkState()) { + itemBase->setCheckState(Qt::PartiallyChecked); break; } } } + if (!enabledFound) { + itemBase->setCheckable(true); + itemBase->setEnabled(false); + } m_factory->classFunctionModel->invisibleRootItem()->appendRow(itemBase); } } @@ -4883,7 +4891,6 @@ public: bool isHeaderFile = false; m_cppFileName = correspondingHeaderOrSource(interface->fileName(), &isHeaderFile); m_factory->setHasImplementationFile(isHeaderFile && !m_cppFileName.isEmpty()); - m_factory->setHasReimplementedFunctions(m_implementedFunctionCount != 0); m_valid = true; } @@ -5087,7 +5094,6 @@ private: int m_insertPosDecl; int m_insertPosOutside; unsigned m_functionCount; - unsigned m_implementedFunctionCount; }; class InsertVirtualMethodsFilterModel : public QSortFilterProxyModel @@ -5113,14 +5119,14 @@ public: for (int i = 0; i < sourceModel()->rowCount(index); ++i) { const QModelIndex child = sourceModel()->index(i, 0, index); - if (!child.data(InsertVirtualMethodsDialog::Implemented).toBool()) + if (!child.data(InsertVirtualMethodsDialog::Reimplemented).toBool()) return true; } return false; } if (m_hideReimplemented) - return !index.data(InsertVirtualMethodsDialog::Implemented).toBool(); + return !index.data(InsertVirtualMethodsDialog::Reimplemented).toBool(); return true; } @@ -5174,7 +5180,7 @@ void InsertVirtualMethodsDialog::initGui() m_view->setHeaderHidden(true); groupBoxViewLayout->addWidget(m_view); m_hideReimplementedFunctions = - new QCheckBox(tr("&Hide already implemented functions of current class"), this); + new QCheckBox(tr("&Hide reimplemented functions"), this); groupBoxViewLayout->addWidget(m_hideReimplementedFunctions); // Insertion options @@ -5223,7 +5229,7 @@ void InsertVirtualMethodsDialog::initData() m_view->setModel(classFunctionFilterModel); m_expansionStateNormal.clear(); m_expansionStateReimp.clear(); - m_hideReimplementedFunctions->setVisible(m_hasReimplementedFunctions); + m_hideReimplementedFunctions->setEnabled(m_hasReimplementedFunctions); m_virtualKeyword->setChecked(m_insertKeywordVirtual); m_insertMode->setCurrentIndex(m_insertMode->findData(m_implementationMode)); @@ -5304,8 +5310,9 @@ void InsertVirtualMethodsDialog::updateCheckBoxes(QStandardItem *item) if (!item->isCheckable() || state == Qt::PartiallyChecked) return; for (int i = 0; i < item->rowCount(); ++i) { - if (item->child(i, 0)->isCheckable()) - item->child(i, 0)->setCheckState(state); + QStandardItem *childItem = item->child(i, 0); + if (childItem->isCheckable() && childItem->isEnabled()) + childItem->setCheckState(state); } } else { QStandardItem *parent = item->parent(); @@ -5313,7 +5320,8 @@ void InsertVirtualMethodsDialog::updateCheckBoxes(QStandardItem *item) return; const Qt::CheckState state = item->checkState(); for (int i = 0; i < parent->rowCount(); ++i) { - if (state != parent->child(i, 0)->checkState()) { + QStandardItem *childItem = parent->child(i, 0); + if (childItem->isEnabled() && state != childItem->checkState()) { parent->setCheckState(Qt::PartiallyChecked); return; } diff --git a/src/plugins/cppeditor/cppquickfixes.h b/src/plugins/cppeditor/cppquickfixes.h index 3e60341ce73..1ae1f20053d 100644 --- a/src/plugins/cppeditor/cppquickfixes.h +++ b/src/plugins/cppeditor/cppquickfixes.h @@ -532,7 +532,7 @@ class InsertVirtualMethodsDialog : public QDialog public: enum CustomItemRoles { ClassOrFunction = Qt::UserRole + 1, - Implemented = Qt::UserRole + 2, + Reimplemented = Qt::UserRole + 2, PureVirtual = Qt::UserRole + 3, AccessSpec = Qt::UserRole + 4 }; @@ -556,8 +556,10 @@ public: bool hideReimplementedFunctions() const; virtual bool gather(); -private slots: +public slots: void updateCheckBoxes(QStandardItem *item); + +private slots: void setHideReimplementedFunctions(bool hide); private: -- GitLab