Allow Connections to handle signals using JavaScript functions

Requiring full function definitions as signal handlers has two
advantages:

1, We don't need a custom parser that magically recognizes properties
   which would otherwise be an error in other components.
2, The user is forced to specify the full signature of the handler,
   including any parameters. This helps when the functions will
   eventually be compiled to C++

The old behavior is retained, generating a warning if any of the magic
bindings are still set in a Connections element. Only if no magic
bindings are found, the functions are connected. This is because there
might be functions named onFoo in old-style Connections elements and
silently connecting those to any matching signals would be a change in
behavior.

Change-Id: I8c78d8994fdcddd355fe822cde9a0702dc8c75de
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 73 insertions, 3 deletions

diff --git a/src/qml/types/qqmlconnections.cpp b/src/qml/types/qqmlconnections.cpp
index 8ec754a9df..14aadb6c86 100644
--- a/src/qml/types/qqmlconnections.cpp
+++ b/src/qml/types/qqmlconnections.cpp
@@ -44,6 +44,7 @@
 #include <private/qqmlboundsignal_p.h>
 #include <qqmlcontext.h>
 #include <private/qqmlcontext_p.h>
+#include <private/qqmlvmemetaobject_p.h>
 #include <qqmlinfo.h>
 
 #include <QtCore/qdebug.h>
@@ -105,7 +106,7 @@ public:
     \qml
     MouseArea {
         Connections {
-            onClicked: foo(parameters)
+            function onClicked(mouse) { foo(mouse) }
         }
     }
     \endqml
@@ -122,7 +123,7 @@ public:
     \qml
     Connections {
         target: area
-        onClicked: foo(parameters)
+        function onClicked(mouse) { foo(mouse) }
     }
     \endqml
 
@@ -270,8 +271,73 @@ void QQmlConnections::connectSignals()
     if (!d->componentcomplete || (d->targetSet && !target()))
         return;
 
-    if (d->bindings.isEmpty())
+    if (d->bindings.isEmpty()) {
+        connectSignalsToMethods();
+    } else {
+        qmlWarning(this) << tr("Implicitly defined onFoo properties in Connections are deprecated. "
+                               "Use this syntax instead: function onFoo(<arguments>) { ... }");
+        connectSignalsToBindings();
+    }
+}
+
+void QQmlConnections::connectSignalsToMethods()
+{
+    Q_D(QQmlConnections);
+
+    QObject *target = this->target();
+    QQmlData *ddata = QQmlData::get(this);
+    if (!ddata)
         return;
+
+    QV4::ExecutionEngine *engine = ddata->context->engine->handle();
+
+    QQmlContextData *ctxtdata = ddata->outerContext;
+    for (int i = ddata->propertyCache->methodOffset(),
+             end = ddata->propertyCache->methodOffset() + ddata->propertyCache->methodCount();
+         i < end;
+         ++i) {
+
+        QQmlPropertyData *handler = ddata->propertyCache->method(i);
+        if (!handler || !handler->isVMEFunction())
+            continue;
+
+        const QString propName = handler->name(this);
+
+        QQmlProperty prop(target, propName);
+        if (prop.isValid() && (prop.type() & QQmlProperty::SignalProperty)) {
+            int signalIndex = QQmlPropertyPrivate::get(prop)->signalIndex();
+            auto *signal = new QQmlBoundSignal(target, signalIndex, this, qmlEngine(this));
+            signal->setEnabled(d->enabled);
+
+            QV4::Scope scope(engine);
+            QV4::ScopedContext global(scope, engine->rootContext());
+
+            QQmlVMEMetaObject *vmeMetaObject = QQmlVMEMetaObject::get(this);
+            Q_ASSERT(vmeMetaObject); // the fact we found the property above should guarentee this
+
+            QV4::ScopedFunctionObject method(scope, vmeMetaObject->vmeMethod(handler->coreIndex()));
+
+            QQmlBoundSignalExpression *expression =
+                    ctxtdata ? new QQmlBoundSignalExpression(
+                                       target, signalIndex, ctxtdata, this,
+                                       method->as<QV4::FunctionObject>()->function())
+                             : nullptr;
+
+            signal->takeExpression(expression);
+            d->boundsignals += signal;
+        } else if (!d->ignoreUnknownSignals && propName.startsWith("on") && propName.length() > 2
+                   && propName.at(2).isUpper()) {
+            qmlWarning(this) << tr("Detected function \"%1\" in Connections element. "
+                                   "This is probably intended to be a signal handler but no "
+                                   "signal of the target matches the name.").arg(propName);
+        }
+    }
+}
+
+// TODO: Drop this as soon as we can
+void QQmlConnections::connectSignalsToBindings()
+{
+    Q_D(QQmlConnections);
     QObject *target = this->target();
     QQmlData *ddata = QQmlData::get(this);
     QQmlContextData *ctxtdata = ddata ? ddata->outerContext : nullptr;
diff --git a/src/qml/types/qqmlconnections_p.h b/src/qml/types/qqmlconnections_p.h
index f6ad1eb46c..1acc86239f 100644
--- a/src/qml/types/qqmlconnections_p.h
+++ b/src/qml/types/qqmlconnections_p.h
@@ -91,10 +91,14 @@ Q_SIGNALS:
 
 private:
     void connectSignals();
+    void connectSignalsToMethods();
+    void connectSignalsToBindings();
+
     void classBegin() override;
     void componentComplete() override;
 };
 
+// TODO: Drop this class as soon as we can
 class QQmlConnectionsParser : public QQmlCustomParser
 {
 public: