QV4Engine: Unify fromValue and metaTypeToJS

Somehow, we ended up with two codepaths doing essentially the same
thing: constructing a JS value from a QVariant. metaTypeToJS is invoked
from QJSEngine::toScriptValue, whereas fromVariant() is used in various
places internally.

metaTypeToJS lacks proper handling for a number of cases, such as
builtin types like QPointF, which lead to toScriptValue(QPointF)
(incorrectly, and uselessly) constructing a VariantObject which couldn't
then do anything useful.

[ChangeLog][QtQml] QJSEngine::toScriptValue will now return correct
JavaScript objects in more cases, for example, for gadget types like
QPointF.

[ChangeLog][QtQml] QJSEngine::toScriptValue now uses the same
behavior as the rest of the engine when building JavaScript values, which
will cause the types of some returned JavaScript objects to change. For
instance, string lists are now returned as sequence objects, not array
objects, and QChar now constructs a JavaScript string.

Change-Id: I0290eb7c9c46e7b508d497cc33cd61d9378f3872
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 93 insertions, 9 deletions

diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index d2e0035b57..01b9465f58 100644
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -66,6 +66,10 @@ private slots:
     void newArray();
     void newArray_HooliganTask218092();
     void newArray_HooliganTask233836();
+    void toScriptValue_data();
+    void toScriptValue();
+    void toScriptValuenotroundtripped_data();
+    void toScriptValuenotroundtripped();
     void newVariant();
     void newVariant_valueOfToString();
     void newVariant_valueOfEnum();
@@ -481,17 +485,97 @@ void tst_QJSEngine::newArray_HooliganTask233836()
     }
 }
 
+void tst_QJSEngine::toScriptValue_data()
+{
+    QTest::addColumn<QVariant>("input");
+
+    QTest::newRow("UnknownType") << QVariant(int(QMetaType::UnknownType), nullptr);
+    QTest::newRow("Nullptr") << QVariant(int(QMetaType::Nullptr), nullptr);
+    QTest::newRow("true") << QVariant(true);
+    QTest::newRow("false") << QVariant(false);
+    QTest::newRow("int") << QVariant(int(42));
+    QTest::newRow("uint") << QVariant(uint(42));
+    QTest::newRow("longlong") << QVariant(qlonglong(4242));
+    QTest::newRow("ulonglong") << QVariant(qulonglong(4242));
+    QTest::newRow("double") << QVariant(double(42.42));
+    QTest::newRow("float") << QVariant(float(42.42));
+    QTest::newRow("qstring") << QVariant(QString::fromLatin1("hello"));
+    QTest::newRow("qbytearray") << QVariant(QByteArray("hello"));
+    QTest::newRow("short") << QVariant(short('r'));
+    QTest::newRow("ushort") << QVariant(short('b'));
+    QTest::newRow("char") << QVariant(char('r'));
+    QTest::newRow("uchar") << QVariant(uchar('b'));
+    QTest::newRow("qchar") << QVariant(QString::fromUtf8("å").at(0));
+    QTest::newRow("qdate") << QVariant(QDate(1925, 5, 8));
+    QTest::newRow("qtime") << QVariant(QTime(4, 5, 6));
+    QTest::newRow("qregularexpression") << QVariant(QRegularExpression(".*"));
+    QTest::newRow("qpointf") << QVariant(QPointF(42, 24));
+    QTest::newRow("qvariantlist") << QVariant(QVariantList() << 42.24 << 5 << "hello");
+    QTest::newRow("qvariantlist_point") << QVariant(QVariantList() << 42.24 << QPointF(42.24, 24.42) << QPointF(24.42, 42.24));
+    QVariantMap vm; vm.insert("test", 55); vm.insert("abc", 42.42);;
+    QTest::newRow("qvariantmap") << QVariant(vm);
+    vm.clear(); vm.insert("point1", QPointF(42.24, 24.42)); vm.insert("point2", QPointF(42.24, 24.42));
+    QTest::newRow("qvariantmap_point") << QVariant(vm);
+    QTest::newRow("qvariant") << QVariant(QVariant(42));
+    QTest::newRow("QList<int>") << QVariant::fromValue(QList<int>() << 1 << 2 << 3 << 4);
+    QTest::newRow("QVector<int>") << QVariant::fromValue(QVector<int>() << 1 << 2 << 3 << 4);
+    QTest::newRow("QList<QString>") << QVariant::fromValue(QVector<QString>() << "1" << "2" << "3" << "4");
+    QTest::newRow("QStringList") << QVariant::fromValue(QStringList() << "1" << "2" << "3" << "4");
+    QTest::newRow("QMap<QString, QString>") << QVariant::fromValue(QMap<QString, QString>{{ "1", "2" }, { "3", "4" }});
+    QTest::newRow("QHash<QString, QString>") << QVariant::fromValue(QHash<QString, QString>{{ "1", "2" }, { "3", "4" }});
+    QTest::newRow("QMap<QString, QPointF>") << QVariant::fromValue(QMap<QString, QPointF>{{ "1", { 42.24, 24.42 } }, { "3", { 24.42, 42.24 } }});
+    QTest::newRow("QHash<QString, QPointF>") << QVariant::fromValue(QHash<QString, QPointF>{{ "1", { 42.24, 24.42 } }, { "3", { 24.42, 42.24 } }});
+}
+
+void tst_QJSEngine::toScriptValue()
+{
+    QFETCH(QVariant, input);
+
+    QJSEngine engine;
+    QJSValue outputJS = engine.toScriptValue(input);
+    QVariant output = engine.fromScriptValue<QVariant>(outputJS);
+
+    QCOMPARE(input, output);
+}
+
+void tst_QJSEngine::toScriptValuenotroundtripped_data()
+{
+    QTest::addColumn<QVariant>("input");
+    QTest::addColumn<QVariant>("output");
+
+    QTest::newRow("QList<QObject*>") << QVariant::fromValue(QList<QObject*>() << this) << QVariant(QVariantList() << QVariant::fromValue(this));
+    QTest::newRow("QObjectList") << QVariant::fromValue(QObjectList() << this) << QVariant(QVariantList() << QVariant::fromValue(this));
+    QTest::newRow("QList<QPoint>") << QVariant::fromValue(QList<QPointF>() << QPointF(42.24, 24.42) << QPointF(42.24, 24.42)) << QVariant(QVariantList() << QPointF(42.24, 24.42) << QPointF(42.24, 24.42));
+    QTest::newRow("QVector<QPoint>") << QVariant::fromValue(QVector<QPointF>() << QPointF(42.24, 24.42) << QPointF(42.24, 24.42)) << QVariant(QVariantList() << QPointF(42.24, 24.42) << QPointF(42.24, 24.42));
+    QTest::newRow("VoidStar") << QVariant(int(QMetaType::VoidStar), nullptr) << QVariant(int(QMetaType::Nullptr), nullptr);
+    QTest::newRow("qregex") << QVariant(QRegExp(".*", Qt::CaseSensitive, QRegExp::RegExp2)) << QVariant(QRegularExpression(".*"));
+}
+
+// This is almost the same as toScriptValue, but the inputs don't roundtrip to
+// exactly the same value.
+void tst_QJSEngine::toScriptValuenotroundtripped()
+{
+    QFETCH(QVariant, input);
+    QFETCH(QVariant, output);
+
+    QJSEngine engine;
+    QJSValue outputJS = engine.toScriptValue(input);
+    QVariant actualOutput = engine.fromScriptValue<QVariant>(outputJS);
+
+    QCOMPARE(actualOutput, output);
+}
+
 void tst_QJSEngine::newVariant()
 {
     QJSEngine eng;
     {
         QJSValue opaque = eng.toScriptValue(QVariant(QPoint(1, 2)));
         QVERIFY(!opaque.isUndefined());
-        QCOMPARE(opaque.isVariant(), true);
+        QCOMPARE(opaque.isVariant(), false);
         QVERIFY(!opaque.isCallable());
         QCOMPARE(opaque.isObject(), true);
         QVERIFY(!opaque.prototype().isUndefined());
-        QCOMPARE(opaque.prototype().isVariant(), true);
+        QCOMPARE(opaque.prototype().isVariant(), false);
         QVERIFY(opaque.property("valueOf").callWithInstance(opaque).equals(opaque));
     }
 }
@@ -505,7 +589,7 @@ void tst_QJSEngine::newVariant_valueOfToString()
         QJSValue value = object.property("valueOf").callWithInstance(object);
         QVERIFY(value.isObject());
         QVERIFY(value.strictlyEquals(object));
-        QCOMPARE(object.toString(), QString::fromLatin1("QVariant(QPoint, QPoint(10,20))"));
+        QCOMPARE(object.toString(), QString::fromLatin1("QPoint(10, 20)"));
     }
 }
 
@@ -1503,7 +1587,7 @@ void tst_QJSEngine::valueConversion_QVariant()
     {
         QVariant var = qVariantFromValue(QPoint(123, 456));
         QJSValue val = eng.toScriptValue(var);
-        QVERIFY(val.isVariant());
+        QVERIFY(!val.isVariant());
         QCOMPARE(val.toVariant(), var);
     }
 
diff --git a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
index 21ebf6b10b..2b80970559 100644
--- a/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
+++ b/tests/auto/qml/qjsvalue/tst_qjsvalue.cpp
@@ -404,8 +404,8 @@ void tst_QJSValue::toString()
     // variant should use internal valueOf(), then fall back to QVariant::toString(),
     // then fall back to "QVariant(typename)"
     QJSValue variant = eng.toScriptValue(QPoint(10, 20));
-    QVERIFY(variant.isVariant());
-    QCOMPARE(variant.toString(), QString::fromLatin1("QVariant(QPoint, QPoint(10,20))"));
+    QVERIFY(!variant.isVariant());
+    QCOMPARE(variant.toString(), QString::fromLatin1("QPoint(10, 20)"));
     variant = eng.toScriptValue(QUrl());
     QVERIFY(variant.isVariant());
     QVERIFY(variant.toString().isEmpty());
@@ -2288,8 +2288,8 @@ void tst_QJSValue::strictlyEquals()
     {
         QJSValue var1 = eng.toScriptValue(QVariant(QStringList() << "a"));
         QJSValue var2 = eng.toScriptValue(QVariant(QStringList() << "a"));
-        QVERIFY(var1.isArray());
-        QVERIFY(var2.isArray());
+        QVERIFY(!var1.isArray());
+        QVERIFY(!var2.isArray());
         QVERIFY(!var1.strictlyEquals(var2));
     }
     {
diff --git a/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp b/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
index 6ccfc77c25..8a01524b5b 100644
--- a/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
+++ b/tests/auto/qml/qqmlvaluetypes/tst_qqmlvaluetypes.cpp
@@ -1710,7 +1710,7 @@ void tst_qqmlvaluetypes::sequences()
         QJSValue value = engine.toScriptValue(qcharVector);
         QCOMPARE(value.property("length").toInt(), qcharVector.length());
         for (int i = 0; i < qcharVector.length(); ++i)
-            QCOMPARE(value.property(i).toInt(), qcharVector.at(i));
+            QCOMPARE(value.property(i).toString(), qcharVector.at(i));
     }
     {
         MyTypeObject a, b, c;