Replace signal name manipulations with QQmlSignalNames

Remove custom implementations found in qqmljs* and use the
static helper methods from qqmlsignalnames_p.h instead. This sometimes
requires to move some code around to avoid bugs with property that do
not have letters in their name.
Add a warning in the JS implementation of the SignalSpy.qml that the
used heuristic might fail on certain signal names.
Add tests in in tst_qqmllanguage to see if the property change handlers
work correctly for weird names.

Change-Id: I4dc73c34df7f77f529511fa04ab5fcc5385b59fc
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

10 files changed, 126 insertions, 41 deletions

diff --git a/tests/auto/qml/common/tst_qml_common.cpp b/tests/auto/qml/common/tst_qml_common.cpp
index 67d72500e0..13d7ac0f72 100644
--- a/tests/auto/qml/common/tst_qml_common.cpp
+++ b/tests/auto/qml/common/tst_qml_common.cpp
@@ -37,6 +37,8 @@ void tst_qml_common::tst_propertyNameToChangedHandlerName_data()
     QTest::addRow("___123aProperty") << u"___123a"_s << u"on___123AChanged"_s;
     QTest::addRow("___123Property") << u"___123"_s << u"on___123Changed"_s;
     QTest::addRow("AProperty") << u"A"_s << u"onAChanged"_s;
+    QTest::addRow("_Property") << u"_"_s << u"on_Changed"_s;
+    QTest::addRow("$Property") << u"$"_s << u"on$Changed"_s;
 }
 void tst_qml_common::tst_propertyNameToChangedHandlerName()
 {
@@ -67,6 +69,8 @@ void tst_qml_common::tst_signalNameToHandlerName_data()
     QTest::addRow("___123aProperty") << u"___123a"_s << u"on___123A"_s;
     QTest::addRow("___123Property") << u"___123"_s << u"on___123"_s;
     QTest::addRow("AProperty") << u"A"_s << u"onA"_s;
+    QTest::addRow("_Property") << u"_"_s << u"on_"_s;
+    QTest::addRow("$Property") << u"$"_s << u"on$"_s;
 }
 
 void tst_qml_common::tst_signalNameToHandlerName()
@@ -150,6 +154,8 @@ void tst_qml_common::tst_isChangedHandlerName_data()
     QTest::addRow("empty") << u""_s << false;
     QTest::addRow("empty2") << u"onChanged"_s << false;
     QTest::addRow("on") << u"on"_s << false;
+    QTest::addRow("on_Changed") << u"on_Changed"_s << true;
+    QTest::addRow("on$Changed") << u"on$Changed"_s << true;
 }
 void tst_qml_common::tst_isChangedHandlerName()
 {
diff --git a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
index 97507f2512..586ed8ea8e 100644
--- a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
+++ b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
@@ -22,6 +22,7 @@
 #include <QtQml/qqmlproperty.h>
 #include <QtQml/qqmlincubator.h>
 #include <QtQml/qqmlapplicationengine.h>
+#include <QtQml/private/qqmlsignalnames_p.h>
 #include <QtQuick/qquickitem.h>
 
 #include <QtNetwork/qhostaddress.h>
@@ -244,7 +245,7 @@ void tst_QQmlEngineDebugService::recursiveObjectTest(
         QCOMPARE(p.objectDebugId, QQmlDebugService::idForObject(o));
 
         // signal properties are fake - they are generated from QQmlAbstractBoundSignal children
-        if (p.name.startsWith("on") && p.name.size() > 2 && p.name[2].isUpper()) {
+        if (QQmlSignalNames::isHandlerName(p.name)) {
             QString signal = p.value.toString();
             QQmlBoundSignalExpression *expr = QQmlPropertyPrivate::signalExpression(QQmlProperty(o, p.name));
             QVERIFY(expr && expr->expression() == signal);
diff --git a/tests/auto/qml/qmlcppcodegen/data/conversions2.qml b/tests/auto/qml/qmlcppcodegen/data/conversions2.qml
index c3a9414ae2..d0d1fc9b8f 100644
--- a/tests/auto/qml/qmlcppcodegen/data/conversions2.qml
+++ b/tests/auto/qml/qmlcppcodegen/data/conversions2.qml
@@ -94,6 +94,8 @@ Item {
     }
 
     function qtest_signalHandlerName(sn) {
+        // Warning: to not test for signal handlers like this in actual code.
+        // Use the helper methods in QQmlSignalNames instead.
         if (sn.substr(0, 2) === "on" && sn[2] === sn[2].toUpperCase())
             return sn
         return "on" + sn.substr(0, 1).toUpperCase() + sn.substr(1)
diff --git a/tests/auto/qml/qmlcppcodegen/data/methods.qml b/tests/auto/qml/qmlcppcodegen/data/methods.qml
index 3abd14c9c1..c045c2249b 100644
--- a/tests/auto/qml/qmlcppcodegen/data/methods.qml
+++ b/tests/auto/qml/qmlcppcodegen/data/methods.qml
@@ -35,6 +35,8 @@ BirthdayParty {
     }
 
     function stuff(sn) {
+        // Warning: to not test for signal handlers like this in actual code.
+        // Use the helper methods in QQmlSignalNames instead.
         if (sn.substr(0, 2) === "on" && sn[2] === sn[2].toUpperCase())
             return sn
         return "on" + sn.substr(0, 1).toUpperCase() + sn.substr(1)
diff --git a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.3.qml b/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.3.qml
deleted file mode 100644
index d611e0fe30..0000000000
--- a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.3.qml
+++ /dev/null
@@ -1,12 +0,0 @@
-import QtQuick 2.0
-
-Item {
-    property int changeCount: 0
-
-    // invalid property name - we don't allow $
-    property bool $nameWithDollarsign: false
-
-    on$NameWithDollarsignChanged: {
-        changeCount = changeCount + 4;
-    }
-}
diff --git a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.4.qml b/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.4.qml
deleted file mode 100644
index a6862517c6..0000000000
--- a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlotErrors.4.qml
+++ /dev/null
@@ -1,12 +0,0 @@
-import QtQuick 2.0
-
-Item {
-    property int changeCount: 0
-
-    property bool _6nameWithUnderscoreNumber: false
-
-    // invalid property name - the first character after an underscore must be a letter
-    on_6NameWithUnderscoreNumberChanged: {
-        changeCount = changeCount + 3;
-    }
-}
diff --git a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlots.qml b/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlots.qml
index f91fb71f1f..9a3141e15a 100644
--- a/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlots.qml
+++ b/tests/auto/qml/qqmlecmascript/data/changeslots/propertyChangeSlots.qml
@@ -6,6 +6,8 @@ Item {
     property bool normalName: false
     property bool _nameWithUnderscore: false
     property bool ____nameWithUnderscores: false
+    property bool _6nameWithUnderscoreNumber: false
+    property bool $nameWithDollarsign: false
 
     onNormalNameChanged: {
         changeCount = changeCount + 1;
@@ -19,9 +21,20 @@ Item {
         changeCount = changeCount + 3;
     }
 
+    on$NameWithDollarsignChanged: {
+        changeCount = changeCount + 4;
+    }
+
+    on_6NameWithUnderscoreNumberChanged: {
+        changeCount = changeCount + 5;
+    }
+
     Component.onCompleted: {
         normalName = true;
         _nameWithUnderscore = true;
         ____nameWithUnderscores = true;
+        $nameWithDollarsign = true;
+        _6nameWithUnderscoreNumber = true;
     }
+
 }
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 69a342bcb9..ca578e1f83 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -5300,6 +5300,7 @@ void tst_qqmlecmascript::propertyChangeSlots()
     QQmlComponent component(&engine, testFileUrl("changeslots/propertyChangeSlots.qml"));
     QScopedPointer<QObject> object(component.create());
     QVERIFY2(object, qPrintable(component.errorString()));
+    QCOMPARE(object->property("changeCount"), 15);
 
     // ensure that invalid property names fail properly.
     QTest::ignoreMessage(QtWarningMsg, "QQmlComponent: Component is not ready");
@@ -5315,20 +5316,6 @@ void tst_qqmlecmascript::propertyChangeSlots()
     QCOMPARE(e2.errors().at(0).toString(), expectedErrorString);
     object.reset(e2.create());
     QVERIFY(!object);
-
-    QTest::ignoreMessage(QtWarningMsg, "QQmlComponent: Component is not ready");
-    QQmlComponent e3(&engine, testFileUrl("changeslots/propertyChangeSlotErrors.3.qml"));
-    expectedErrorString = e3.url().toString() + QLatin1String(":9:5: Cannot assign to non-existent property \"on$NameWithDollarsignChanged\"");
-    QCOMPARE(e3.errors().at(0).toString(), expectedErrorString);
-    object.reset(e3.create());
-    QVERIFY(!object);
-
-    QTest::ignoreMessage(QtWarningMsg, "QQmlComponent: Component is not ready");
-    QQmlComponent e4(&engine, testFileUrl("changeslots/propertyChangeSlotErrors.4.qml"));
-    expectedErrorString = e4.url().toString() + QLatin1String(":9:5: Cannot assign to non-existent property \"on_6NameWithUnderscoreNumberChanged\"");
-    QCOMPARE(e4.errors().at(0).toString(), expectedErrorString);
-    object.reset(e4.create());
-    QVERIFY(!object);
 }
 
 void tst_qqmlecmascript::propertyVar_data()
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 462082ed03..fbc09f35e0 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -422,6 +422,10 @@ private slots:
 
     void attachedInCtor();
     void byteArrayConversion();
+    void propertySignalNames_data();
+    void propertySignalNames();
+    void signalNames_data();
+    void signalNames();
 
 private:
     QQmlEngine engine;
@@ -8140,6 +8144,99 @@ void tst_qqmllanguage::byteArrayConversion()
     QCOMPARE(receiver->byteArrays[0], QByteArray("\1\2\3"));
     QCOMPARE(receiver->byteArrays[1], QByteArray("\4\5\6"));
 }
+void tst_qqmllanguage::propertySignalNames_data()
+{
+    QTest::addColumn<QString>("propertyName");
+    QTest::addColumn<QString>("propertyChangedSignal");
+    QTest::addColumn<QString>("propertyChangedHandler");
+    QTest::addRow("helloWorld") << u"helloWorld"_s << u"helloWorldChanged"_s
+                                << u"onHelloWorldChanged"_s;
+    QTest::addRow("$helloWorld") << u"$helloWorld"_s << u"$helloWorldChanged"_s
+                                 << u"on$HelloWorldChanged"_s;
+    QTest::addRow("_helloWorld") << u"_helloWorld"_s << u"_helloWorldChanged"_s
+                                 << u"on_HelloWorldChanged"_s;
+    QTest::addRow("_") << u"_"_s << u"_Changed"_s << u"on_Changed"_s;
+    QTest::addRow("$") << u"$"_s << u"$Changed"_s << u"on$Changed"_s;
+    QTest::addRow("ä") << u"ä"_s << u"äChanged"_s << u"onÄChanged"_s;
+    QTest::addRow("___123a") << u"___123a"_s << u"___123aChanged"_s << u"on___123AChanged"_s;
+}
+void tst_qqmllanguage::propertySignalNames()
+{
+    QFETCH(QString, propertyName);
+    QFETCH(QString, propertyChangedSignal);
+    QFETCH(QString, propertyChangedHandler);
+    QQmlEngine e;
+    QQmlComponent c(&e);
+    c.setData(uR"(
+import QtQuick
+Item {
+    property int %1: 456
+    property bool success: false
+    function f() { %1 = 123; }
+    function g() { %2(); }
+    %3: success = true
+})"_s.arg(propertyName, propertyChangedSignal, propertyChangedHandler)
+                      .toUtf8(),
+              QUrl());
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(o != nullptr);
+    const QMetaObject *metaObject = o->metaObject();
+    auto signalIndex =
+            metaObject->indexOfSignal(propertyChangedSignal.append("()").toStdString().c_str());
+    QVERIFY(signalIndex > -1);
+    auto signal = metaObject->method(signalIndex);
+    QVERIFY(signal.isValid());
+    QSignalSpy changeSignal(o.data(), signal);
+    QMetaObject::invokeMethod(o.data(), "f");
+    QCOMPARE(o->property(propertyName.toStdString().c_str()), 123);
+    QVERIFY(changeSignal.size() == 1);
+    QCOMPARE(o->property("success"), true);
+    QMetaObject::invokeMethod(o.data(), "g");
+    QVERIFY(changeSignal.size() == 2);
+}
+void tst_qqmllanguage::signalNames_data()
+{
+    QTest::addColumn<QString>("signalName");
+    QTest::addColumn<QString>("handlerName");
+    QTest::addRow("helloWorld") << u"helloWorld"_s << u"onHelloWorld"_s;
+    QTest::addRow("$helloWorld") << u"$helloWorld"_s << u"on$HelloWorld"_s;
+    QTest::addRow("_helloWorld") << u"_helloWorld"_s << u"on_HelloWorld"_s;
+    QTest::addRow("_") << u"_"_s << u"on_"_s;
+    QTest::addRow("aUmlaut") << u"ä"_s << u"onÄ"_s;
+    QTest::addRow("___123a") << u"___123a"_s << u"on___123A"_s;
+}
+void tst_qqmllanguage::signalNames()
+{
+    QFETCH(QString, signalName);
+    QFETCH(QString, handlerName);
+    QQmlEngine e;
+    QQmlComponent c(&e);
+    c.setData(uR"(
+import QtQuick
+Item {
+    signal %1()
+    property bool success: false
+    function f() { %1(); }
+    %2: success = true
+})"_s.arg(signalName, handlerName)
+                      .toUtf8(),
+              QUrl());
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(o != nullptr);
+    const QMetaObject *metaObject = o->metaObject();
+    auto signalIndex = metaObject->indexOfSignal(signalName.append("()").toStdString().c_str());
+    QVERIFY(signalIndex > -1);
+    auto signal = metaObject->method(signalIndex);
+    QVERIFY(signal.isValid());
+    QSignalSpy changeSignal(o.data(), signal);
+    signal.invoke(o.data());
+    QVERIFY(changeSignal.size() == 1);
+    QCOMPARE(o->property("success"), true);
+    QMetaObject::invokeMethod(o.data(), "f");
+    QVERIFY(changeSignal.size() == 2);
+}
 
 QTEST_MAIN(tst_qqmllanguage)
 
diff --git a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
index a84ffb87b4..8df8e12dc7 100644
--- a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
+++ b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
@@ -8,6 +8,7 @@
 #include <QtQml/private/qqmlengine_p.h>
 #include <QtQmlModels/private/qqmllistmodel_p.h>
 #include <QtQml/private/qqmlexpression_p.h>
+#include <QtQml/private/qqmlsignalnames_p.h>
 #include <QQmlComponent>
 
 #include <QtCore/qtimer.h>
@@ -1098,12 +1099,12 @@ void tst_qqmllistmodel::property_changes()
     expr.evaluate();
     QVERIFY2(!expr.hasError(), qPrintable(expr.error().toString()));
 
-    QString signalHandler = "on" + QString(roleName[0].toUpper()) + roleName.mid(1, roleName.size()) + "Changed:";
+    QString signalHandler = QQmlSignalNames::propertyNameToChangedHandlerName(roleName);
     QString qml = "import QtQuick 2.0\n"
                   "Connections {\n"
                         "property bool gotSignal: false\n"
                         "target: model.get(" + QString::number(listIndex) + ")\n"
-                        + signalHandler + " gotSignal = true\n"
+                        + signalHandler + ": gotSignal = true\n"
                   "}\n";
 
     QQmlComponent component(&engine);