Support change slots for properties starting with '_'

According to ECMA-262r3, property names may begin with a letter,
underscore ('_'), dollar sign ('$'), or unicode escape sequence.
We previously supported Change slots for properties only if the
property name began with a letter; this patch adds support for
properties which begin with one or more underscore.

Task-number: QTBUG-17950
Reviewed-by: Aaron Kennedy
Change-Id: I6f28bde18a38e32c2131e0990fe0f69bda36f90e

diff --git a/src/declarative/qml/qdeclarativecompiler.cpp b/src/declarative/qml/qdeclarativecompiler.cpp
index 31a9f66..3a686be 100644 (file)
--- a/src/declarative/qml/qdeclarativecompiler.cpp
+++ b/src/declarative/qml/qdeclarativecompiler.cpp
@@ -122,13 +122,26 @@ bool QDeclarativeCompiler::isAttachedPropertyName(const QByteArray &name)
 /*!
     Returns true if \a name refers to a signal property, false otherwise.
 
-    Signal property names are those that start with "on", followed by a capital
-    letter.
+    Signal property names are those that start with "on", followed by a first
+    character which is either a capital letter or one or more underscores followed
+    by a capital letter, which is then followed by other allowed characters.
+
+    Note that although ECMA-262r3 supports dollarsigns and escaped unicode
+    character codes in property names, for simplicity and performance reasons
+    QML only supports letters, numbers and underscores.
 */
 bool QDeclarativeCompiler::isSignalPropertyName(const QByteArray &name)
 {
-    return name.length() >= 3 && name.startsWith("on") &&
-           'A' <= name.at(2) && 'Z' >= name.at(2);
+    if (name.length() < 3) return false;
+    if (!name.startsWith("on")) return false;
+    int ns = name.size();
+    for (int i = 2; i < ns; ++i) {
+        char curr = name.at(i);
+        if (curr == '_') continue;
+        if (curr >= 'A' && curr <= 'Z') return true;
+        return false;
+    }
+    return false; // consists solely of underscores - invalid.
 }
 
 /*!
@@ -1340,8 +1353,15 @@ bool QDeclarativeCompiler::buildSignal(QDeclarativeParser::Property *prop, QDecl
     QByteArray name = prop->name;
     Q_ASSERT(name.startsWith("on"));
     name = name.mid(2);
-    if(name[0] >= 'A' && name[0] <= 'Z')
-        name[0] = name[0] - 'A' + 'a';
+
+    // Note that the property name could start with any alpha or '_' or '$' character,
+    // so we need to do the lower-casing of the first alpha character.
+    for (int firstAlphaIndex = 0; firstAlphaIndex < name.size(); ++firstAlphaIndex) {
+        if (name[firstAlphaIndex] >= 'A' && name[firstAlphaIndex] <= 'Z') {
+            name[firstAlphaIndex] = name[firstAlphaIndex] - 'A' + 'a';
+            break;
+        }
+    }
 
     bool notInRevision = false;
     int sigIdx = indexOfSignal(obj, name, &notInRevision);

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.1.qml b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.1.qml
new file mode 100644 (file)
index 0000000..3182d6b
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.1.qml
@@ -0,0 +1,12 @@
+import QtQuick 1.0
+
+Item {
+    property int changeCount: 0
+
+    property bool _nameWithUnderscore: false
+
+    // this should error, since the first alpha isn't capitalised.
+    on_nameWithUnderscoreChanged: {
+        changeCount = changeCount + 2;
+    }
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.2.qml b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.2.qml
new file mode 100644 (file)
index 0000000..50ef0b3
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.2.qml
@@ -0,0 +1,12 @@
+import QtQuick 1.0
+
+Item {
+    property int changeCount: 0
+
+    property bool ____nameWithUnderscores: false
+
+    // this should error, since the first alpha isn't capitalised
+    on____nameWithUnderscoresChanged: {
+        changeCount = changeCount + 3;
+    }
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.3.qml b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.3.qml
new file mode 100644 (file)
index 0000000..343cf91
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.3.qml
@@ -0,0 +1,12 @@
+import QtQuick 1.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/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.4.qml b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.4.qml
new file mode 100644 (file)
index 0000000..b845987
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlotErrors.4.qml
@@ -0,0 +1,12 @@
+import QtQuick 1.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/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlots.qml b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlots.qml
new file mode 100644 (file)
index 0000000..d31c893
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/changeslots/propertyChangeSlots.qml
@@ -0,0 +1,27 @@
+import QtQuick 1.0
+
+Item {
+    property int changeCount: 0
+
+    property bool normalName: false
+    property bool _nameWithUnderscore: false
+    property bool ____nameWithUnderscores: false
+
+    onNormalNameChanged: {
+        changeCount = changeCount + 1;
+    }
+
+    on_NameWithUnderscoreChanged: {
+        changeCount = changeCount + 2;
+    }
+
+    on____NameWithUnderscoresChanged: {
+        changeCount = changeCount + 3;
+    }
+
+    Component.onCompleted: {
+        normalName = true;
+        _nameWithUnderscore = true;
+        ____nameWithUnderscores = true;
+    }
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index a408000..b2859b5 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -147,6 +147,7 @@ private slots:
     void moduleApi();
     void importScripts();
     void scarceResources();
+    void propertyChangeSlots();
 
     void bug1();
     void bug2();
@@ -2777,6 +2778,48 @@ void tst_qdeclarativeecmascript::scarceResources()
     delete object;
 }
 
+void tst_qdeclarativeecmascript::propertyChangeSlots()
+{
+    // ensure that allowable property names are allowed and onPropertyNameChanged slots are generated correctly.
+    QDeclarativeComponent component(&engine, TEST_FILE("changeslots/propertyChangeSlots.qml"));
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+    delete object;
+
+    // ensure that invalid property names fail properly.
+    QTest::ignoreMessage(QtWarningMsg, "QDeclarativeComponent: Component is not ready");
+    QDeclarativeComponent e1(&engine, TEST_FILE("changeslots/propertyChangeSlotErrors.1.qml"));
+    QString expectedErrorString = e1.url().toString() + QLatin1String(":9:5: Cannot assign to non-existent property \"on_nameWithUnderscoreChanged\"");
+    QCOMPARE(e1.errors().at(0).toString(), expectedErrorString);
+    object = e1.create();
+    QVERIFY(object == 0);
+    delete object;
+
+    QTest::ignoreMessage(QtWarningMsg, "QDeclarativeComponent: Component is not ready");
+    QDeclarativeComponent e2(&engine, TEST_FILE("changeslots/propertyChangeSlotErrors.2.qml"));
+    expectedErrorString = e2.url().toString() + QLatin1String(":9:5: Cannot assign to non-existent property \"on____nameWithUnderscoresChanged\"");
+    QCOMPARE(e2.errors().at(0).toString(), expectedErrorString);
+    object = e2.create();
+    QVERIFY(object == 0);
+    delete object;
+
+    QTest::ignoreMessage(QtWarningMsg, "QDeclarativeComponent: Component is not ready");
+    QDeclarativeComponent e3(&engine, TEST_FILE("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 = e3.create();
+    QVERIFY(object == 0);
+    delete object;
+
+    QTest::ignoreMessage(QtWarningMsg, "QDeclarativeComponent: Component is not ready");
+    QDeclarativeComponent e4(&engine, TEST_FILE("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 = e4.create();
+    QVERIFY(object == 0);
+    delete object;
+}
+
 // Test that assigning a null object works 
 // Regressed with: df1788b4dbbb2826ae63f26bdf166342595343f4
 void tst_qdeclarativeecmascript::nullObjectBinding()