QQmlPropertyBinding: improve error reporting

This change ensures that bindings created in QML between new-style
properties contain information about which property caused the loop. To
do this, we store additional information about the property involved to
retrieve its name and position at a later point. We print the warning in
case we detect a binding loop in evaluate, and also set the error
reporting callback correctly, so that the condition can be reported when
the loop is detected in another part of the binding evaluation.

In addition, we do not only set the QPropertyBinding's error member when
JS evaluation results in an error, but also print the warning with
qmlWarning.

Fixes: QTBUG-87733
Change-Id: Idb25237d1f57355ca31189e6bf2a918430b3a810
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

6 files changed, 118 insertions, 0 deletions

diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml
new file mode 100644
index 0000000000..a320ba56e6
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerEager.qml
@@ -0,0 +1,6 @@
+import test
+
+BindingLoop {
+    eager1: eager2+1
+    eager2: eager1+1
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml
new file mode 100644
index 0000000000..aabd41e61c
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerLazy.qml
@@ -0,0 +1,13 @@
+import test
+import QtQml
+
+QtObject {
+    property BindingLoop a: BindingLoop {
+        value: b.eager1+1
+    }
+
+    property BindingLoop b: BindingLoop {
+        eager1: a.value+1
+    }
+}
+
diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml
new file mode 100644
index 0000000000..88553ed4a7
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopEagerOld.qml
@@ -0,0 +1,6 @@
+import test
+
+BindingLoop {
+    eager1: oldprop+1
+    oldprop: eager1+1
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml
new file mode 100644
index 0000000000..98fef4cbda
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyEager.qml
@@ -0,0 +1,6 @@
+import test
+
+BindingLoop {
+    eager1: value+1
+    value: eager1+1
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml
new file mode 100644
index 0000000000..c054282f93
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/bindingLoopLazyLazy.qml
@@ -0,0 +1,12 @@
+import test
+import QtQml
+
+BindingLoop {
+    id: loop
+    value: value2 + 1
+    value2: value + 1
+
+    Component.onCompleted: {
+        let x = loop.value2 // if we do not read the value, we don't detect the loop...
+    }
+}
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 02d044aa63..86bd32ccbe 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -53,6 +53,7 @@
 #include <private/qv4objectiterator_p.h>
 #include <private/qqmlabstractbinding_p.h>
 #include <private/qqmlvaluetypeproxybinding_p.h>
+#include <QtCore/private/qproperty_p.h>
 
 #ifdef Q_CC_MSVC
 #define NO_INLINE __declspec(noinline)
@@ -92,6 +93,8 @@ private slots:
     void signalAssignment();
     void signalArguments();
     void bindingLoop();
+    void cppPropertyBindingLoop_data();
+    void cppPropertyBindingLoop();
     void basicExpressions();
     void basicExpressions_data();
     void arrayExpressions();
@@ -883,6 +886,78 @@ void tst_qqmlecmascript::bindingLoop()
     delete object;
 }
 
+
+struct QPropertyBindingLoop : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(float value MEMBER value BINDABLE bindableValue)
+    Q_PROPERTY(float value2 MEMBER value2 BINDABLE bindableValue2)
+    Q_PROPERTY(float eager1 READ eager1 WRITE setEager1 BINDABLE bindableEager1)
+    Q_PROPERTY(float eager2 READ eager2 WRITE setEager2 BINDABLE bindableEager2)
+    Q_PROPERTY(float oldprop READ oldprop WRITE setOldprop NOTIFY oldpropChanged)
+signals:
+    void oldpropChanged();
+public:
+
+    float eager1() {return eager1Data;}
+    void setEager1(float f) {eager1Data = f;}
+    float eager2() {return eager2Data;}
+    void setEager2(float f) {eager2Data = f;}
+    QProperty<float> value;
+    QProperty<float> value2;
+    QBindable<float> bindableValue() { return QBindable<float>(&value); }
+    QBindable<float> bindableValue2() { return QBindable<float>(&value2); }
+    QBindable<float> bindableEager1() {return QBindable<float>(&eager1Data);}
+    QBindable<float> bindableEager2() {return QBindable<float>(&eager2Data);}
+    float m_oldprop;
+
+    float oldprop() const;
+    void setOldprop(float oldprop);
+
+private:
+    Q_OBJECT_COMPAT_PROPERTY(QPropertyBindingLoop, float, eager1Data, &QPropertyBindingLoop::setEager1);
+    Q_OBJECT_COMPAT_PROPERTY(QPropertyBindingLoop, float, eager2Data, &QPropertyBindingLoop::setEager2);
+
+};
+
+
+float QPropertyBindingLoop::oldprop() const
+{
+    return m_oldprop;
+}
+
+void QPropertyBindingLoop::setOldprop(float oldprop)
+{
+    m_oldprop = oldprop;
+    emit oldpropChanged();
+}
+
+void tst_qqmlecmascript::cppPropertyBindingLoop_data()
+{
+    QTest::addColumn<QString>("file");
+    QTest::addColumn<QString>("warningMsg");
+
+    QTest::newRow("eager eager") << "bindingLoopEagerEager.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")";
+    QTest::newRow("lazy lazy") << "bindingLoopLazyLazy.qml" << R"(:7:5: QML BindingLoop: Binding loop detected for property "value2")";
+    QTest::newRow("lazy eager") << "bindingLoopLazyEager.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")";
+    QTest::newRow("eager lazy") << "bindingLoopEagerLazy.qml" << R"(:10:9: QML BindingLoop: Binding loop detected for property "eager1")";
+    QTest::newRow("eager old") << "bindingLoopEagerOld.qml" << R"(:4:5: QML BindingLoop: Binding loop detected for property "eager1")";
+
+    qmlRegisterType<QPropertyBindingLoop>("test", 1, 0, "BindingLoop");
+}
+
+void tst_qqmlecmascript::cppPropertyBindingLoop()
+{
+    QFETCH(QString, file);
+    QFETCH(QString, warningMsg);
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl(file));
+    QString warning = component.url().toString() + warningMsg;
+    QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData());
+    QScopedPointer<QObject> object { component.create() };
+    QVERIFY2(object, qPrintable(component.errorString()));
+}
+
 void tst_qqmlecmascript::basicExpressions_data()
 {
     QTest::addColumn<QString>("expression");