Fix the evaluation of JS switch statements in QML bindings.

Task-number: QTBUG-17012 Change-Id: Ic132cf63ed08592fec9c759df1b8b4d5830acea6 Reviewed-by: Kent Hansen <kent.hansen@nokia.com>
sailfishos · Nov 29, 2011 · 7c83628 · 7c83628
1 parent 328d1d2
commit 7c83628
Show file tree

Hide file tree

Showing 8 changed files with 321 additions and 0 deletions.
diff --git a/src/declarative/qml/qdeclarativerewrite.cpp b/src/declarative/qml/qdeclarativerewrite.cpp
@@ -266,6 +266,60 @@ void RewriteBinding::endVisit(AST::LocalForEachStatement *)
     --_inLoop;
 }
 
+bool RewriteBinding::visit(AST::CaseBlock *ast)
+{
+    // Process the initial sequence of the case clauses.
+    for (AST::CaseClauses *it = ast->clauses; it; it = it->next) {
+        // Return the value of the last statement in the block, if this is the last `case clause'
+        // of the switch statement.
+        bool returnTheValueOfLastStatement = (it->next == 0) && (ast->defaultClause == 0) && (ast->moreClauses == 0);
+
+        if (AST::CaseClause *clause = it->clause) {
+            accept(clause->expression);
+            rewriteCaseStatements(clause->statements, returnTheValueOfLastStatement);
+        }
+    }
+
+    // Process the default case clause
+    if (ast->defaultClause) {
+        // Return the value of the last statement in the block, if this is the last `case clause'
+        // of the switch statement.
+        bool rewriteTheLastStatement = (ast->moreClauses == 0);
+
+        rewriteCaseStatements(ast->defaultClause->statements, rewriteTheLastStatement);
+    }
+
+    // Process trailing `case clauses'
+    for (AST::CaseClauses *it = ast->moreClauses; it; it = it->next) {
+        // Return the value of the last statement in the block, if this is the last `case clause'
+        // of the switch statement.
+        bool returnTheValueOfLastStatement = (it->next == 0);
+
+        if (AST::CaseClause *clause = it->clause) {
+            accept(clause->expression);
+            rewriteCaseStatements(clause->statements, returnTheValueOfLastStatement);
+        }
+    }
+
+    return false;
+}
+
+void RewriteBinding::rewriteCaseStatements(AST::StatementList *statements, bool rewriteTheLastStatement)
+{
+    for (AST::StatementList *it = statements; it; it = it->next) {
+        if (it->next && AST::cast<AST::BreakStatement *>(it->next->statement) != 0) {
+            // The value of the first statement followed by a `break'.
+            accept(it->statement);
+            break;
+        } else if (!it->next) {
+            if (rewriteTheLastStatement)
+                accept(it->statement);
+            else if (AST::Block *block = AST::cast<AST::Block *>(it->statement))
+                rewriteCaseStatements(block->statements, rewriteTheLastStatement);
+        }
+    }
+}
+
 QString RewriteSignalHandler::operator()(const QString &code, const QString &name)
 {
     return QStringLiteral("(function ") + name + QStringLiteral("() { ") + code + QStringLiteral(" })");

diff --git a/src/declarative/qml/qdeclarativerewrite_p.h b/src/declarative/qml/qdeclarativerewrite_p.h
@@ -93,6 +93,7 @@ class RewriteBinding: protected AST::Visitor
 
     void accept(AST::Node *node);
     QString rewrite(QString code, unsigned position, AST::Statement *node);
+    void rewriteCaseStatements(AST::StatementList *statements, bool rewriteTheLastStatement);
 
     virtual bool visit(AST::Block *ast);
     virtual bool visit(AST::ExpressionStatement *ast);
@@ -115,6 +116,8 @@ class RewriteBinding: protected AST::Visitor
     virtual bool visit(AST::LocalForEachStatement *ast);
     virtual void endVisit(AST::LocalForEachStatement *ast);
 
+    virtual bool visit(AST::CaseBlock *ast);
+
 private:
     int _inLoop;
 };

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.1.qml b/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.1.qml
@@ -0,0 +1,33 @@
+import Qt.test 1.0
+
+MyQmlObject {
+    value: {
+        var value = 0
+        switch (stringProperty) {
+        case "A":
+            value = value + 1
+            value = value + 1
+            /* should fall through */
+        case "S":
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        case "D": { // with curly braces
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        }
+        case "F": {
+            value = value + 1
+            value = value + 1
+            value = value + 1
+        }
+        /* should fall through */
+        default:
+            value = value + 1
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.2.qml b/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.2.qml
@@ -0,0 +1,33 @@
+import Qt.test 1.0
+
+MyQmlObject {
+    value: {
+        var value = 0
+        switch (stringProperty) {
+        case "A":
+            value = value + 1
+            value = value + 1
+            /* should fall through */
+        case "S":
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        default:
+            value = value + 1
+        case "D": { // with curly braces
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        }
+        case "F": {
+            value = value + 1
+            value = value + 1
+            value = value + 1
+        }
+        /* should fall through */
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.3.qml b/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.3.qml
@@ -0,0 +1,33 @@
+import Qt.test 1.0
+
+MyQmlObject {
+    value: {
+        var value = 0
+        switch (stringProperty) {
+        default:
+            value = value + 1
+        case "A":
+            value = value + 1
+            value = value + 1
+            /* should fall through */
+        case "S":
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        case "D": { // with curly braces
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        }
+        case "F": {
+            value = value + 1
+            value = value + 1
+            value = value + 1
+        }
+        /* should fall through */
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.4.qml b/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.4.qml
@@ -0,0 +1,31 @@
+import Qt.test 1.0
+
+MyQmlObject {
+    value: {
+        var value = 0
+        switch (stringProperty) {
+        case "A":
+            value = value + 1
+            value = value + 1
+            /* should fall through */
+        case "S":
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        case "D": { // with curly braces
+            value = value + 1
+            value = value + 1
+            value = value + 1
+            break;
+        }
+        case "F": {
+            value = value + 1
+            value = value + 1
+            value = value + 1
+        }
+        /* should fall through */
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.5.qml b/tests/auto/declarative/qdeclarativeecmascript/data/switchStatement.5.qml
@@ -0,0 +1,12 @@
+import Qt.test 1.0
+
+MyQmlObject {
+    value: {
+        var value = 0
+        switch (stringProperty) {
+        default:
+            value = value + 1
+        }
+    }
+}
+
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -232,6 +232,7 @@ private slots:
 
     void automaticSemicolon();
     void unaryExpression();
+    void switchStatement();
 
 private:
     static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
@@ -5219,6 +5220,127 @@ void tst_qdeclarativeecmascript::qtbug_22843()
     }
 }
 
+
+void tst_qdeclarativeecmascript::switchStatement()
+{
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("switchStatement.1.qml"));
+        MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
+        QVERIFY(object != 0);
+
+        // `object->value()' is the number of executed statements
+
+        object->setStringProperty("A");
+        QCOMPARE(object->value(), 5);
+
+        object->setStringProperty("S");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("D");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("F");
+        QCOMPARE(object->value(), 4);
+
+        object->setStringProperty("something else");
+        QCOMPARE(object->value(), 1);
+    }
+
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("switchStatement.2.qml"));
+        MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
+        QVERIFY(object != 0);
+
+        // `object->value()' is the number of executed statements
+
+        object->setStringProperty("A");
+        QCOMPARE(object->value(), 5);
+
+        object->setStringProperty("S");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("D");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("F");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("something else");
+        QCOMPARE(object->value(), 4);
+    }
+
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("switchStatement.3.qml"));
+        MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
+        QVERIFY(object != 0);
+
+        // `object->value()' is the number of executed statements
+
+        object->setStringProperty("A");
+        QCOMPARE(object->value(), 5);
+
+        object->setStringProperty("S");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("D");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("F");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("something else");
+        QCOMPARE(object->value(), 6);
+    }
+
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("switchStatement.4.qml"));
+        MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
+        QVERIFY(object != 0);
+
+        // `object->value()' is the number of executed statements
+
+        object->setStringProperty("A");
+        QCOMPARE(object->value(), 5);
+
+        object->setStringProperty("S");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("D");
+        QCOMPARE(object->value(), 3);
+
+        object->setStringProperty("F");
+        QCOMPARE(object->value(), 3);
+
+        QString warning = component.url().toString() + ":4: Unable to assign [undefined] to int";
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
+
+        object->setStringProperty("something else");
+    }
+
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("switchStatement.5.qml"));
+        MyQmlObject *object = qobject_cast<MyQmlObject *>(component.create());
+        QVERIFY(object != 0);
+
+        // `object->value()' is the number of executed statements
+
+        object->setStringProperty("A");
+        QCOMPARE(object->value(), 1);
+
+        object->setStringProperty("S");
+        QCOMPARE(object->value(), 1);
+
+        object->setStringProperty("D");
+        QCOMPARE(object->value(), 1);
+
+        object->setStringProperty("F");
+        QCOMPARE(object->value(), 1);
+
+        object->setStringProperty("something else");
+        QCOMPARE(object->value(), 1);
+    }
+}
+
 QTEST_MAIN(tst_qdeclarativeecmascript)
 
 #include "tst_qdeclarativeecmascript.moc"