From e735e95f06c2cd0b4b22fdd1da9d72a42252e754 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Fri, 15 Jan 2021 16:55:26 +0100 Subject: [PATCH] "New Class" wizard: Check custom base class for QObject parent That is, if the user specifies a custom base class, we check whether its constructor takes a "QObject *parent" parameter, and if it does, we give the derived class one as well. This is technically a heuristic, but the pattern is pretty stable in the Qt world. Fixes: QTCREATORBUG-25156 Change-Id: Ie64440929df61cca7258d6d692c5de62970f9a65 Reviewed-by: Christian Stenger --- .../templates/wizards/classes/cpp/file.cpp | 2 + .../templates/wizards/classes/cpp/file.h | 2 +- src/plugins/cpptools/cppmodelmanager.cpp | 9 ++ src/plugins/cpptools/cppmodelmanager.h | 2 + src/plugins/cpptools/cpptoolsjsextension.cpp | 90 +++++++++++++++++++ src/plugins/cpptools/cpptoolsjsextension.h | 9 +- src/plugins/cpptools/cpptoolsplugin.cpp | 4 +- 7 files changed, 113 insertions(+), 5 deletions(-) diff --git a/share/qtcreator/templates/wizards/classes/cpp/file.cpp b/share/qtcreator/templates/wizards/classes/cpp/file.cpp index 6bf6885858a..1676ef0e7f8 100644 --- a/share/qtcreator/templates/wizards/classes/cpp/file.cpp +++ b/share/qtcreator/templates/wizards/classes/cpp/file.cpp @@ -13,6 +13,8 @@ public: %{CN}::%{CN}(QObject *parent) : QObject(parent)%{JS: ('%{SharedDataInit}') ? ', %{SharedDataInit}' : ''} @elsif '%{Base}' === 'QWidget' || '%{Base}' === 'QMainWindow' %{CN}::%{CN}(QWidget *parent) : %{Base}(parent)%{JS: ('%{SharedDataInit}') ? ', %{SharedDataInit}' : ''} +@elsif %{JS: Cpp.hasQObjectParent('%{Base}')} +%{CN}::%{CN}(QObject *parent) : %{Base}(parent)%{JS: ('%{SharedDataInit}') ? ', %{SharedDataInit}' : ''} @else %{CN}::%{CN}()%{JS: ('%{SharedDataInit}') ? ' : %{SharedDataInit}' : ''} @endif diff --git a/share/qtcreator/templates/wizards/classes/cpp/file.h b/share/qtcreator/templates/wizards/classes/cpp/file.h index 67e4dfd4d1c..df35bd553c2 100644 --- a/share/qtcreator/templates/wizards/classes/cpp/file.h +++ b/share/qtcreator/templates/wizards/classes/cpp/file.h @@ -33,7 +33,7 @@ class %{CN} Q_OBJECT @endif public: -@if '%{Base}' === 'QObject' +@if '%{Base}' === 'QObject' || %{JS: Cpp.hasQObjectParent('%{Base}')} explicit %{CN}(QObject *parent = nullptr); @elsif '%{Base}' === 'QWidget' || '%{Base}' === 'QMainWindow' explicit %{CN}(QWidget *parent = nullptr); diff --git a/src/plugins/cpptools/cppmodelmanager.cpp b/src/plugins/cpptools/cppmodelmanager.cpp index 7315b303df8..5502c9fa58f 100644 --- a/src/plugins/cpptools/cppmodelmanager.cpp +++ b/src/plugins/cpptools/cppmodelmanager.cpp @@ -42,6 +42,7 @@ #include "cpprefactoringchanges.h" #include "cpprefactoringengine.h" #include "cppsourceprocessor.h" +#include "cpptoolsjsextension.h" #include "cpptoolsplugin.h" #include "cpptoolsconstants.h" #include "cpptoolsreuse.h" @@ -54,6 +55,7 @@ #include #include #include +#include #include #include #include @@ -575,6 +577,13 @@ CppModelManager *CppModelManager::instance() return m_instance; } +void CppModelManager::registerJsExtension() +{ + Core::JsExpander::registerGlobalObject("Cpp", [this] { + return new CppToolsJsExtension(&d->m_locatorData); + }); +} + void CppModelManager::initCppTools() { // Objects diff --git a/src/plugins/cpptools/cppmodelmanager.h b/src/plugins/cpptools/cppmodelmanager.h index eeae9180c66..253e4856cc5 100644 --- a/src/plugins/cpptools/cppmodelmanager.h +++ b/src/plugins/cpptools/cppmodelmanager.h @@ -98,6 +98,8 @@ public: static CppModelManager *instance(); + void registerJsExtension(); + // Documented in source file. enum ProgressNotificationMode { ForcedProgressNotification, diff --git a/src/plugins/cpptools/cpptoolsjsextension.cpp b/src/plugins/cpptools/cpptoolsjsextension.cpp index 8c4778b3a91..64df64c1013 100644 --- a/src/plugins/cpptools/cpptoolsjsextension.cpp +++ b/src/plugins/cpptools/cpptoolsjsextension.cpp @@ -26,6 +26,8 @@ #include "cpptoolsjsextension.h" #include "cppfilesettingspage.h" +#include "cpplocatordata.h" +#include "cppworkingcopy.h" #include @@ -33,9 +35,13 @@ #include #include +#include +#include +#include #include #include +#include #include #include #include @@ -118,6 +124,90 @@ QString CppToolsJsExtension::closeNamespaces(const QString &klass) const return result; } +bool CppToolsJsExtension::hasQObjectParent(const QString &klassName) const +{ + // This is a synchronous function, but the look-up is potentially expensive. + // Since it's not crucial information, we just abort if retrieving it takes too long, + // in order not to freeze the UI. + // TODO: Any chance to at least cache between successive invocations for the same dialog? + // I don't see it atm... + QElapsedTimer timer; + timer.start(); + static const int timeout = 5000; + + // Find symbol. + QList candidates; + m_locatorData->filterAllFiles([&](const IndexItem::Ptr &item) { + if (timer.elapsed() > timeout) + return IndexItem::VisitorResult::Break; + if (item->scopedSymbolName() == klassName) { + candidates = {item}; + return IndexItem::VisitorResult::Break; + } + if (item->symbolName() == klassName) + candidates << item; + return IndexItem::VisitorResult::Recurse; + }); + if (timer.elapsed() > timeout) + return false; + if (candidates.isEmpty()) + return false; + const IndexItem::Ptr item = candidates.first(); + + // Find class in AST. + const CPlusPlus::Snapshot snapshot = CppModelManager::instance()->snapshot(); + const WorkingCopy workingCopy = CppModelManager::instance()->workingCopy(); + QByteArray source = workingCopy.source(item->fileName()); + if (source.isEmpty()) { + QFile file(item->fileName()); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + return false; + source = file.readAll(); + } + const auto doc = snapshot.preprocessedDocument(source, item->fileName()); + if (!doc) + return false; + doc->check(); + if (!doc->translationUnit()) + return false; + if (timer.elapsed() > timeout) + return false; + CPlusPlus::ClassSpecifierAST *classSpec = nullptr; + const QList astPath = CPlusPlus::ASTPath(doc)(item->line(), item->column()); + for (auto it = astPath.rbegin(); it != astPath.rend(); ++it) { + if ((classSpec = (*it)->asClassSpecifier())) + break; + } + if (!classSpec) + return false; + + // Check whether constructor has a QObject parent parameter. + CPlusPlus::Overview overview; + const CPlusPlus::Class * const klass = classSpec->symbol; + if (!klass) + return false; + for (auto it = klass->memberBegin(); it != klass->memberEnd(); ++it) { + const CPlusPlus::Symbol * const member = *it; + if (overview.prettyName(member->name()) != item->symbolName()) + continue; + const CPlusPlus::Function *function = (*it)->asFunction(); + if (!function) + function = member->type().type()->asFunctionType(); + if (!function) + continue; + for (int i = 0; i < function->argumentCount(); ++i) { + const CPlusPlus::Symbol * const arg = function->argumentAt(i); + const QString argName = overview.prettyName(arg->name()); + const QString argType = overview.prettyType(arg->type()) + .split("::", Qt::SkipEmptyParts).last(); + if (argName == "parent" && argType == "QObject *") + return true; + } + } + + return false; +} + QString CppToolsJsExtension::includeStatement( const QString &fullyQualifiedClassName, const QString &suffix, diff --git a/src/plugins/cpptools/cpptoolsjsextension.h b/src/plugins/cpptools/cpptoolsjsextension.h index b026465b0c4..ce844f0a7e1 100644 --- a/src/plugins/cpptools/cpptoolsjsextension.h +++ b/src/plugins/cpptools/cpptoolsjsextension.h @@ -30,6 +30,8 @@ #include namespace CppTools { +class CppLocatorData; // FIXME: Belongs in namespace Internal + namespace Internal { /** @@ -40,7 +42,8 @@ class CppToolsJsExtension : public QObject Q_OBJECT public: - explicit CppToolsJsExtension(QObject *parent = nullptr) : QObject(parent) { } + explicit CppToolsJsExtension(CppLocatorData *locatorData, QObject *parent = nullptr) + : QObject(parent), m_locatorData(locatorData) { } // Generate header guard: Q_INVOKABLE QString headerGuard(const QString &in) const; @@ -55,6 +58,7 @@ public: Q_INVOKABLE QString classToHeaderGuard(const QString &klass, const QString &extension) const; Q_INVOKABLE QString openNamespaces(const QString &klass) const; Q_INVOKABLE QString closeNamespaces(const QString &klass) const; + Q_INVOKABLE bool hasQObjectParent(const QString &klassName) const; // Find header file for class. Q_INVOKABLE QString includeStatement( @@ -63,6 +67,9 @@ public: const QStringList &specialClasses, const QString &pathOfIncludingFile ); + +private: + CppLocatorData * const m_locatorData; }; } // namespace Internal diff --git a/src/plugins/cpptools/cpptoolsplugin.cpp b/src/plugins/cpptools/cpptoolsplugin.cpp index 47a604ae0cc..fa13b359a30 100644 --- a/src/plugins/cpptools/cpptoolsplugin.cpp +++ b/src/plugins/cpptools/cpptoolsplugin.cpp @@ -33,7 +33,6 @@ #include "cpptoolsbridge.h" #include "cpptoolsbridgeqtcreatorimplementation.h" #include "cpptoolsconstants.h" -#include "cpptoolsjsextension.h" #include "cpptoolsreuse.h" #include "cpptoolssettings.h" #include "projectinfo.h" @@ -46,7 +45,6 @@ #include #include #include -#include #include #include #include @@ -167,7 +165,7 @@ bool CppToolsPlugin::initialize(const QStringList &arguments, QString *error) d = new CppToolsPluginPrivate; d->initialize(); - JsExpander::registerGlobalObject("Cpp"); + CppModelManager::instance()->registerJsExtension(); ExtensionSystem::PluginManager::addObject(&d->m_cppProjectUpdaterFactory); // Menus