QQmlProperty: Add flexibility to initProperty()

Using private API, you can now construct a QQmlProperty from an unknown
object by passing a dot-separated string that starts with an ID. You can
also have a plain signal name (without "on") as the last part of the
name. Both variants are optional and have to be enabled via flags.

In Qt7, we should clean up the "on" vs. plain signal name affair. For
now we can't because everything expects the "on".

Constructing properties from ID'd objects can only be done in specific
places. Therefore this has to be manually enabled.

Furthermore, you can now create properties, not only signals, from plain
QMetaObjects. This is enabled unconditionally because there was no
reason for this to not work before.

Change-Id: I9ff764130b70f9d023ab63d492f83290e8e87ef3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

4 files changed, 251 insertions, 49 deletions

diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 707b43cfd4..7dff37e3a6 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -58,6 +58,7 @@
 #include <private/qv4functionobject_p.h>
 #include <private/qv4qobjectwrapper_p.h>
 #include <private/qqmlbuiltinfunctions_p.h>
+#include <private/qqmlirbuilder_p.h>
 
 #include <QStringList>
 #include <QVector>
@@ -234,14 +235,15 @@ QQmlProperty::QQmlProperty(QObject *obj, const QString &name, QQmlEngine *engine
 }
 
 QQmlProperty QQmlPropertyPrivate::create(QObject *target, const QString &propertyName,
-                                         const QQmlRefPointer<QQmlContextData> &context)
+                                         const QQmlRefPointer<QQmlContextData> &context,
+                                         QQmlPropertyPrivate::InitFlags flags)
 {
     QQmlProperty result;
     auto d = new QQmlPropertyPrivate;
     result.d = d;
     d->context = context;
     d->engine = context ? context->engine() : nullptr;
-    d->initProperty(target, propertyName);
+    d->initProperty(target, propertyName, flags);
     if (!result.isValid()) {
         d->object = nullptr;
         d->context = nullptr;
@@ -265,10 +267,10 @@ QQmlRefPointer<QQmlContextData> QQmlPropertyPrivate::effectiveContext() const
         return nullptr;
 }
 
-void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name)
+// ### Qt7: Do not accept the "onFoo" syntax for signals anymore, and change the flags accordingly.
+void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name,
+                                       QQmlPropertyPrivate::InitFlags flags)
 {
-    if (!obj) return;
-
     QQmlRefPointer<QQmlTypeNameCache> typeNameCache = context ? context->imports() : nullptr;
 
     QObject *currentObject = obj;
@@ -319,12 +321,30 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name)
             }
 
             QQmlPropertyData local;
-            QQmlPropertyData *property =
-                    QQmlPropertyCache::property(engine, currentObject, pathName, context, &local);
+            QQmlPropertyData *property = currentObject
+                    ? QQmlPropertyCache::property(engine, currentObject, pathName, context, &local)
+                    : nullptr;
+
+            if (!property) {
+                // Not a property; Might be an ID
+                if (!(flags & InitFlag::AllowId))
+                    return;
+
+                for (auto idContext = context; idContext; idContext = idContext->parent()) {
+                    const int objectId = idContext->propertyIndex(pathName.toString());
+                    if (objectId != -1 && objectId < idContext->numIdValues()) {
+                        currentObject = context->idValue(objectId);
+                        break;
+                    }
+                }
+
+                if (!currentObject)
+                    return;
 
-            if (!property) return; // Not a property
-            if (property->isFunction())
+                continue;
+            } else if (property->isFunction()) {
                 return; // Not an object property
+            }
 
             if (ii == (path.count() - 2) && QQmlMetaType::isValueType(property->propType())) {
                 // We're now at a value type property
@@ -362,12 +382,41 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name)
         }
 
         terminal = path.last();
+    } else if (!currentObject) {
+        return;
     }
 
-    if (terminal.size() >= 3 && terminal.at(0) == u'o' && terminal.at(1) == u'n'
-            && (terminal.at(2).isUpper() || terminal.at(2) == u'_')) {
+    auto findSignalInMetaObject = [&](const QByteArray &signalName) {
+        const QMetaMethod method = findSignalByName(currentObject->metaObject(), signalName);
+        if (!method.isValid())
+            return false;
+
+        object = currentObject;
+        core.load(method);
+        return true;
+    };
+
+    QQmlData *ddata = QQmlData::get(currentObject, false);
+    auto findChangeSignal = [&](QStringView signalName) {
+        const QString changed = QStringLiteral("Changed");
+        if (signalName.endsWith(changed)) {
+            const QStringView propName = signalName.first(signalName.length() - changed.length());
+            QQmlPropertyData *d = ddata->propertyCache->property(propName, currentObject, context);
+            while (d && d->isFunction())
+                d = ddata->propertyCache->overrideData(d);
+
+            if (d && d->notifyIndex() != -1) {
+                object = currentObject;
+                core = *ddata->propertyCache->signal(d->notifyIndex());
+                return true;
+            }
+        }
+        return false;
+    };
 
-        QString signalName = terminal.mid(2).toString();
+    const QString terminalString = terminal.toString();
+    if (QmlIR::IRBuilder::isSignalPropertyName(terminalString)) {
+        QString signalName = terminalString.mid(2);
         int firstNon_;
         int length = signalName.length();
         for (firstNon_ = 0; firstNon_ < length; ++firstNon_)
@@ -375,13 +424,14 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name)
                 break;
         signalName[firstNon_] = signalName.at(firstNon_).toLower();
 
-        // XXX - this code treats methods as signals
-
-        QQmlData *ddata = QQmlData::get(currentObject, false);
         if (ddata && ddata->propertyCache) {
-
             // Try method
-            QQmlPropertyData *d = ddata->propertyCache->property(signalName, currentObject, context);
+            QQmlPropertyData *d = ddata->propertyCache->property(
+                        signalName, currentObject, context);
+
+            // ### Qt7: This code treats methods as signals. It should use d->isSignal().
+            //          That would be a change in behavior, though. Right now you can construct a
+            //          QQmlProperty from such a thing.
             while (d && !d->isFunction())
                 d = ddata->propertyCache->overrideData(d);
 
@@ -391,39 +441,53 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name)
                 return;
             }
 
-            // Try property
-            if (signalName.endsWith(QLatin1String("Changed"))) {
-                const QStringView propName = QStringView{signalName}.mid(0, signalName.length() - 7);
-                QQmlPropertyData *d = ddata->propertyCache->property(propName, currentObject, context);
-                while (d && d->isFunction())
-                    d = ddata->propertyCache->overrideData(d);
+            if (findChangeSignal(signalName))
+                return;
+        } else if (findSignalInMetaObject(signalName.toUtf8())) {
+            return;
+        }
+    }
 
-                if (d && d->notifyIndex() != -1) {
-                    object = currentObject;
-                    core = *ddata->propertyCache->signal(d->notifyIndex());
-                    return;
-                }
-            }
+    if (ddata && ddata->propertyCache) {
+        QQmlPropertyData *property = ddata->propertyCache->property(
+                    terminal, currentObject, context);
 
-        } else {
-            QMetaMethod method = findSignalByName(currentObject->metaObject(),
-                                                  signalName.toLatin1());
-            if (method.isValid()) {
+        // Technically, we might find an override that is not a function.
+        while (property && !property->isSignal()) {
+            if (!property->isFunction()) {
                 object = currentObject;
-                core.load(method);
+                core = *property;
+                nameCache = terminalString;
                 return;
             }
+            property = ddata->propertyCache->overrideData(property);
         }
-    }
 
-    // Property
-    QQmlPropertyData local;
-    QQmlPropertyData *property =
-        QQmlPropertyCache::property(engine, currentObject, terminal, context, &local);
-    if (property && !property->isFunction()) {
-        object = currentObject;
-        core = *property;
-        nameCache = terminal.toString();
+        if (!(flags & InitFlag::AllowSignal))
+            return;
+
+        if (property) {
+            Q_ASSERT(property->isSignal());
+            object = currentObject;
+            core = *property;
+            return;
+        }
+
+        // At last: Try the change signal.
+        findChangeSignal(terminal);
+    } else {
+        // We might still find the property in the metaobject, even without property cache.
+        const QByteArray propertyName = terminal.toUtf8();
+        const QMetaProperty prop = findPropertyByName(currentObject->metaObject(), propertyName);
+
+        if (prop.isValid()) {
+            object = currentObject;
+            core.load(prop);
+            return;
+        }
+
+        if (flags & InitFlag::AllowSignal)
+            findSignalInMetaObject(terminal.toUtf8());
     }
 }
 
@@ -709,7 +773,6 @@ QString QQmlProperty::name() const
     if (!d)
         return QString();
     if (d->nameCache.isEmpty()) {
-        // ###
         if (!d->object) {
         } else if (d->isValueType()) {
             const QMetaObject *valueTypeMetaObject = QQmlMetaType::metaObjectForValueType(d->core.propType());
@@ -718,8 +781,15 @@ QString QQmlProperty::name() const
             const char *vtName = valueTypeMetaObject->property(d->valueTypeData.coreIndex()).name();
             d->nameCache = d->core.name(d->object) + QLatin1Char('.') + QString::fromUtf8(vtName);
         } else if (type() & SignalProperty) {
-            QString name = QLatin1String("on") + d->core.name(d->object);
-            name[2] = name.at(2).toUpper();
+            // ### Qt7: Return the original signal name here. Do not prepend "on"
+            QString name = QStringLiteral("on") + d->core.name(d->object);
+            for (int i = 2, end = name.length(); i != end; ++i) {
+                const QChar c = name.at(i);
+                if (c != u'_') {
+                    name[i] = c.toUpper();
+                    break;
+                }
+            }
             d->nameCache = name;
         } else {
             d->nameCache = d->core.name(d->object);
@@ -1767,6 +1837,16 @@ QMetaMethod QQmlPropertyPrivate::findSignalByName(const QMetaObject *mo, const Q
     return QMetaMethod();
 }
 
+/*!
+    Return the property corresponding to \a name
+*/
+QMetaProperty QQmlPropertyPrivate::findPropertyByName(const QMetaObject *mo, const QByteArray &name)
+{
+    Q_ASSERT(mo);
+    const int i = mo->indexOfProperty(name);
+    return i < 0 ? QMetaProperty() : mo->property(i);
+}
+
 /*! \internal
     If \a indexInSignalRange is true, \a index is treated as a signal index
     (see QObjectPrivate::signalIndex()), otherwise it is treated as a
diff --git a/src/qml/qml/qqmlproperty_p.h b/src/qml/qml/qqmlproperty_p.h
index 8800d9949a..700a299369 100644
--- a/src/qml/qml/qqmlproperty_p.h
+++ b/src/qml/qml/qqmlproperty_p.h
@@ -73,6 +73,13 @@ class QQmlBoundSignalExpression;
 class Q_QML_PRIVATE_EXPORT QQmlPropertyPrivate : public QQmlRefCount
 {
 public:
+    enum class InitFlag {
+        None        = 0x0,
+        AllowId     = 0x1,
+        AllowSignal = 0x2
+    };
+    Q_DECLARE_FLAGS(InitFlags, InitFlag);
+
     QQmlRefPointer<QQmlContextData> context;
     QPointer<QQmlEngine> engine;
     QPointer<QObject> object;
@@ -94,7 +101,7 @@ public:
 
     QQmlRefPointer<QQmlContextData> effectiveContext() const;
 
-    void initProperty(QObject *obj, const QString &name);
+    void initProperty(QObject *obj, const QString &name, InitFlags flags = InitFlag::None);
     void initDefault(QObject *obj);
 
     bool isValueType() const;
@@ -159,6 +166,7 @@ public:
     static bool write(const QQmlProperty &that, const QVariant &, QQmlPropertyData::WriteFlags);
     static QQmlPropertyIndex propertyIndex(const QQmlProperty &that);
     static QMetaMethod findSignalByName(const QMetaObject *mo, const QByteArray &);
+    static QMetaProperty findPropertyByName(const QMetaObject *mo, const QByteArray &);
     static bool connect(const QObject *sender, int signal_index,
                         const QObject *receiver, int method_index,
                         int type = 0, int *types = nullptr);
@@ -169,11 +177,13 @@ public:
             const QVariant &value, const QQmlRefPointer<QQmlContextData> &ctxt);
     static QQmlProperty create(
             QObject *target, const QString &propertyName,
-            const QQmlRefPointer<QQmlContextData> &context);
+            const QQmlRefPointer<QQmlContextData> &context,
+            QQmlPropertyPrivate::InitFlags flags);
 
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QQmlPropertyPrivate::BindingFlags)
+Q_DECLARE_OPERATORS_FOR_FLAGS(QQmlPropertyPrivate::InitFlags);
 
 QT_END_NAMESPACE
 
diff --git a/src/quick/util/qquickpropertychanges.cpp b/src/quick/util/qquickpropertychanges.cpp
index b7e02f730d..639ff88cc1 100644
--- a/src/quick/util/qquickpropertychanges.cpp
+++ b/src/quick/util/qquickpropertychanges.cpp
@@ -417,7 +417,8 @@ QQuickPropertyChangesPrivate::property(const QString &property)
     Q_Q(QQuickPropertyChanges);
     QQmlData *ddata = QQmlData::get(q);
     QQmlProperty prop = QQmlPropertyPrivate::create(
-                object, property, ddata ? ddata->outerContext : QQmlRefPointer<QQmlContextData>());
+                object, property, ddata ? ddata->outerContext : QQmlRefPointer<QQmlContextData>(),
+                QQmlPropertyPrivate::InitFlag::None);
     if (!prop.isValid()) {
         qmlWarning(q) << QQuickPropertyChanges::tr("Cannot assign to non-existent property \"%1\"").arg(property);
         return QQmlProperty();
diff --git a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
index 9cf622c2d7..7012153c20 100644
--- a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
+++ b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
@@ -203,6 +203,11 @@ private slots:
 
     void dontRemoveQPropertyBinding();
     void compatResolveUrls();
+
+    void initFlags_data();
+    void initFlags();
+
+    void constructFromPlainMetaObject();
 private:
     QQmlEngine engine;
 };
@@ -2391,6 +2396,112 @@ void tst_qqmlproperty::compatResolveUrls()
 #endif
 }
 
+void tst_qqmlproperty::initFlags_data()
+{
+    QTest::addColumn<bool>("passObject");
+    QTest::addColumn<QString>("name");
+    QTest::addColumn<QQmlPropertyPrivate::InitFlags>("flags");
+
+    const QString names[] = {
+        QStringLiteral("foo"),
+        QStringLiteral("self.foo"),
+        QStringLiteral("onFoo"),
+        QStringLiteral("self.onFoo"),
+        QStringLiteral("bar"),
+        QStringLiteral("self.bar"),
+        QStringLiteral("abar"),
+        QStringLiteral("self.abar"),
+    };
+
+    const QQmlPropertyPrivate::InitFlags flagSets[] = {
+        QQmlPropertyPrivate::InitFlag::None,
+        QQmlPropertyPrivate::InitFlag::AllowId,
+        QQmlPropertyPrivate::InitFlag::AllowSignal,
+        QQmlPropertyPrivate::InitFlag::AllowId | QQmlPropertyPrivate::InitFlag::AllowSignal,
+    };
+
+    for (int i = 0; i < 2; ++i) {
+        const bool passObject = (i != 0);
+        for (const QString &name : names) {
+            for (const QQmlPropertyPrivate::InitFlags flagSet : flagSets) {
+                const QString rowName = QStringLiteral("%1,%2,%3")
+                        .arg(passObject).arg(name).arg(flagSet.toInt());
+                QTest::addRow("%s", qPrintable(rowName)) << passObject << name << flagSet;
+            }
+        }
+    }
+}
+
+void tst_qqmlproperty::initFlags()
+{
+    QFETCH(bool, passObject);
+    QFETCH(QString, name);
+    QFETCH(QQmlPropertyPrivate::InitFlags, flags);
+
+    QQmlEngine engine;
+    QQmlComponent c(&engine);
+    c.setData(R"(
+        import QtQml
+        QtObject {
+            id: self
+            signal foo()
+            property int bar: 12
+            property alias abar: self.bar
+        }
+    )", QUrl());
+    QVERIFY(c.isReady());
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+
+    QQmlRefPointer<QQmlContextData> context = QQmlContextData::get(qmlContext(o.data()));
+
+    const QQmlProperty property = QQmlPropertyPrivate::create(
+                    passObject ? o.data() : nullptr, name, context, flags);
+
+    const bool usesId = name.startsWith(QStringLiteral("self."));
+    const bool hasSignal = name.endsWith(QStringLiteral("foo"));
+    if (!passObject && !usesId) {
+        QVERIFY(!property.isValid());
+    } else if (usesId && !(flags & QQmlPropertyPrivate::InitFlag::AllowId)) {
+        QVERIFY(!property.isValid());
+    } else if (hasSignal && !(flags & QQmlPropertyPrivate::InitFlag::AllowSignal)) {
+        QVERIFY(!property.isValid());
+    } else {
+        QVERIFY(property.isValid());
+        if (name.endsWith(QStringLiteral("bar"))) {
+            QVERIFY(property.isProperty());
+            QCOMPARE(property.name(), usesId ? name.mid(strlen("self.")) : name);
+            QCOMPARE(property.propertyMetaType(), QMetaType::fromType<int>());
+        } else {
+            QVERIFY(property.isSignalProperty());
+            QCOMPARE(property.name(), QStringLiteral("onFoo"));
+            QVERIFY(!property.propertyMetaType().isValid());
+        }
+    }
+
+}
+
+void tst_qqmlproperty::constructFromPlainMetaObject()
+{
+    QScopedPointer<PropertyObject> obj(new PropertyObject);
+
+    QQmlData *data = QQmlData::get(obj.data());
+    QVERIFY(data == nullptr);
+
+    QQmlProperty prop(obj.data(), "rectProperty");
+    QVERIFY(prop.isValid());
+    QVERIFY(prop.isProperty());
+    QCOMPARE(prop.propertyMetaType(), QMetaType::fromType<QRect>());
+
+    QQmlProperty sig(obj.data(), "onOddlyNamedNotifySignal");
+    QVERIFY(sig.isValid());
+    QVERIFY(sig.isSignalProperty());
+    QVERIFY(!sig.propertyMetaType().isValid());
+
+    data = QQmlData::get(obj.data());
+    QVERIFY(data == nullptr);
+}
+
 QTEST_MAIN(tst_qqmlproperty)
 
 #include "tst_qqmlproperty.moc"