QML: Improve the JS-to-JS type check when enforcing signatures

We do not have to coerce via the C++ type. Rather, we match the
JavaScript representations of the types and coerce as needed.

Task-number: QTBUG-113527
Change-Id: Id5c30cd46293f2d7aedd699f141a9fe19511b622
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 266 insertions, 76 deletions

diff --git a/src/qml/jsruntime/qv4function.cpp b/src/qml/jsruntime/qv4function.cpp
index 860309a4d2..c1725f8818 100644
--- a/src/qml/jsruntime/qv4function.cpp
+++ b/src/qml/jsruntime/qv4function.cpp
@@ -68,7 +68,7 @@ ReturnedValue Function::call(
         });
     case JsTyped:
         return QV4::coerceAndCall(
-                    context->engine(), aotCompiledFunction, thisObject, argv, argc,
+                    context->engine(), jsTypedFunction, compiledFunction, thisObject, argv, argc,
                     [this, context](const Value *thisObject, const Value *argv, int argc) {
             return doCall(this, thisObject, argv, argc, context);
         });
@@ -91,6 +91,12 @@ void Function::destroy()
     delete this;
 }
 
+static bool isSpecificType(const CompiledData::ParameterType &type)
+{
+    return type.typeNameIndexOrCommonType()
+           != (type.indexIsCommonType() ? quint32(CompiledData::CommonType::Invalid) : 0);
+}
+
 Function::Function(ExecutionEngine *engine, ExecutableCompilationUnit *unit,
                    const CompiledData::Function *function,
                    const QQmlPrivate::AOTCompiledFunction *aotFunction)
@@ -111,72 +117,55 @@ Function::Function(ExecutionEngine *engine, ExecutableCompilationUnit *unit,
         ic = ic->addMember(engine->identifierTable->asPropertyKey(compilationUnit->runtimeStrings[localsIndices[i]]), Attr_NotConfigurable);
 
     const CompiledData::Parameter *formalsIndices = compiledFunction->formalsTable();
-    const bool enforcesSignature = !aotFunction && unit->enforcesFunctionSignature();
-    bool hasTypes = false;
+    bool enforceJsTypes = !aotFunction && unit->enforcesFunctionSignature();
+
     for (quint32 i = 0; i < compiledFunction->nFormals; ++i) {
         ic = ic->addMember(engine->identifierTable->asPropertyKey(compilationUnit->runtimeStrings[formalsIndices[i].nameIndex]), Attr_NotConfigurable);
-        if (enforcesSignature
-                && !hasTypes
-                && formalsIndices[i].type.typeNameIndexOrCommonType()
-                   != quint32(QV4::CompiledData::CommonType::Invalid)) {
-            hasTypes = true;
-        }
+        if (enforceJsTypes && !isSpecificType(formalsIndices[i].type))
+            enforceJsTypes = false;
     }
     internalClass = ic->d();
 
     nFormals = compiledFunction->nFormals;
 
-    if (!enforcesSignature)
+    // If a function has any typed arguments, but an untyped return value, the return value is void.
+    // If it doesn't have any arguments at all and the return value is untyped, the function is
+    // untyped. Users can specifically set the return type to "void" to have it enforced.
+    if (!enforceJsTypes || (nFormals == 0 && !isSpecificType(compiledFunction->returnType)))
         return;
 
-    if (!hasTypes
-            && compiledFunction->returnType.typeNameIndexOrCommonType()
-               == quint32(QV4::CompiledData::CommonType::Invalid)) {
-        return;
-    }
+    JSTypedFunction *synthesized = new JSTypedFunction;
 
-    QQmlPrivate::AOTCompiledFunction *synthesized = new QQmlPrivate::AOTCompiledFunction;
     QQmlEnginePrivate *enginePrivate = QQmlEnginePrivate::get(engine->qmlEngine());
-
-    auto findMetaType = [&](const CompiledData::ParameterType &param) {
+    auto findQmlType = [&](const CompiledData::ParameterType &param) {
         const quint32 type = param.typeNameIndexOrCommonType();
         if (param.indexIsCommonType()) {
-            if (param.isList()) {
-                return QQmlPropertyCacheCreatorBase::listTypeForPropertyType(
-                    QV4::CompiledData::CommonType(type));
-            }
-            return QQmlPropertyCacheCreatorBase::metaTypeForPropertyType(
-                QV4::CompiledData::CommonType(type));
+            return QQmlMetaType::qmlType(QQmlPropertyCacheCreatorBase::metaTypeForPropertyType(
+                QV4::CompiledData::CommonType(type)));
         }
 
         if (type == 0)
-            return QMetaType();
-
-        const QQmlType qmltype = unit->typeNameCache->query(unit->stringAt(type)).type;
-        if (!qmltype.isValid())
-            return QMetaType();
+            return QQmlType();
 
-        const QMetaType metaType = param.isList() ? qmltype.qListTypeId() : qmltype.typeId();
-        if (metaType.isValid())
-            return metaType;
+        const QQmlType qmltype = unit->typeNameCache->query<QQmlImport::AllowRecursion>(
+                                                            unit->stringAt(type)).type;
+        if (!qmltype.isValid() || qmltype.typeId().isValid())
+            return qmltype;
 
         if (!qmltype.isComposite()) {
-            if (!qmltype.isInlineComponentType())
-                return QMetaType();
-            const QQmlType qmlType = unit->qmlTypeForComponent(qmltype.elementName());
-            return param.isList() ? qmlType.qListTypeId() : qmlType.typeId();
+            return qmltype.isInlineComponentType()
+                ? unit->qmlTypeForComponent(qmltype.elementName())
+                : QQmlType();
         }
 
-        const QQmlType qmlType = enginePrivate->typeLoader.getType(
-                    qmltype.sourceUrl())->compilationUnit()->qmlType;
-        return param.isList() ? qmlType.qListTypeId() : qmlType.typeId();
+        return enginePrivate->typeLoader.getType(qmltype.sourceUrl())->compilationUnit()->qmlType;
     };
 
     for (quint16 i = 0; i < nFormals; ++i)
-        synthesized->argumentTypes.append(findMetaType(formalsIndices[i].type));
+        synthesized->argumentTypes.append(findQmlType(formalsIndices[i].type));
 
-    synthesized->returnType = findMetaType(compiledFunction->returnType);
-    aotCompiledFunction = synthesized;
+    synthesized->returnType = findQmlType(compiledFunction->returnType);
+    jsTypedFunction = synthesized;
     kind = JsTyped;
 }
 
@@ -187,7 +176,7 @@ Function::~Function()
         delete codeRef;
     }
     if (kind == JsTyped)
-        delete aotCompiledFunction;
+        delete jsTypedFunction;
 }
 
 void Function::updateInternalClass(ExecutionEngine *engine, const QList<QByteArray> &parameters)
diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h
index 20356b9192..57d0deb734 100644
--- a/src/qml/jsruntime/qv4function_p.h
+++ b/src/qml/jsruntime/qv4function_p.h
@@ -51,6 +51,11 @@ protected:
     ~Function();
 
 public:
+    struct JSTypedFunction {
+        QList<QQmlType> argumentTypes;
+        QQmlType returnType;
+    };
+
     const CompiledData::Function *compiledFunction;
 
     QV4::ExecutableCompilationUnit *executableCompilationUnit() const
@@ -74,7 +79,10 @@ public:
     typedef ReturnedValue (*JittedCode)(CppStackFrame *, ExecutionEngine *);
     JittedCode jittedCode;
     JSC::MacroAssemblerCodeRef *codeRef;
-    const QQmlPrivate::AOTCompiledFunction *aotCompiledFunction = nullptr;
+    union {
+        const QQmlPrivate::AOTCompiledFunction *aotCompiledFunction = nullptr;
+        const JSTypedFunction *jsTypedFunction;
+    };
 
     // first nArguments names in internalClass are the actual arguments
     Heap::InternalClass *internalClass;
diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index a9cd25d569..45ca75008c 100644
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -545,8 +545,9 @@ ReturnedValue ArrowFunction::virtualCall(const QV4::FunctionObject *fo, const Va
         });
     case Function::JsTyped:
         return QV4::coerceAndCall(
-                    fo->engine(), function->aotCompiledFunction, thisObject, argv, argc,
-                    [fo](const Value *thisObject, const Value *argv, int argc) {
+                fo->engine(), function->jsTypedFunction, function->compiledFunction,
+                thisObject, argv, argc,
+                [fo](const Value *thisObject, const Value *argv, int argc) {
             return qfoDoCall(fo, thisObject, argv, argc);
         });
     default:
diff --git a/src/qml/jsruntime/qv4jscall.cpp b/src/qml/jsruntime/qv4jscall.cpp
index 350e0fe542..513ae59145 100644
--- a/src/qml/jsruntime/qv4jscall.cpp
+++ b/src/qml/jsruntime/qv4jscall.cpp
@@ -3,6 +3,8 @@
 
 #include "qv4jscall_p.h"
 
+#include <QtQml/qqmlinfo.h>
+
 #include <private/qqmlengine_p.h>
 #include <private/qv4qobjectwrapper_p.h>
 
@@ -20,4 +22,25 @@ void QV4::populateJSCallArguments(ExecutionEngine *v4, JSCallArguments &jsCall,
         jsCall.args[ii] = v4->metaTypeToJS(types[ii], args[ii + 1]);
 }
 
+void QV4::warnAboutCoercionToVoid(
+        ExecutionEngine *engine, const Value &value, CoercionProblem problem)
+{
+    auto log = qCritical().nospace().noquote();
+    if (const CppStackFrame *frame = engine->currentStackFrame)
+        log << frame->source() << ':' << frame->lineNumber() << ": ";
+    log << value.toQStringNoThrow()
+        << " should be coerced to void because";
+    switch (problem) {
+    case InsufficientAnnotation:
+        log << " the function called is insufficiently annotated.";
+        break;
+    case InvalidListType:
+        log << " the target type, a list of unknown elements, cannot be resolved.";
+        break;
+    }
+
+    log << " The original value is retained. This will change in a future version of Qt.";
+}
+
+
 QT_END_NAMESPACE
diff --git a/src/qml/jsruntime/qv4jscall_p.h b/src/qml/jsruntime/qv4jscall_p.h
index 2f6af8dd8b..e824863b29 100644
--- a/src/qml/jsruntime/qv4jscall_p.h
+++ b/src/qml/jsruntime/qv4jscall_p.h
@@ -14,11 +14,26 @@
 // We mean it.
 //
 
+#include <private/qqmlengine_p.h>
+#include <private/qqmllistwrapper_p.h>
+#include <private/qqmlvaluetype_p.h>
+#include <private/qqmlvaluetypewrapper_p.h>
 #include <private/qv4alloca_p.h>
+#include <private/qv4context_p.h>
+#include <private/qv4dateobject_p.h>
+#include <private/qv4function_p.h>
 #include <private/qv4functionobject_p.h>
 #include <private/qv4object_p.h>
 #include <private/qv4qobjectwrapper_p.h>
+#include <private/qv4regexpobject_p.h>
 #include <private/qv4scopedvalue_p.h>
+#include <private/qv4stackframe_p.h>
+#include <private/qv4urlobject_p.h>
+#include <private/qv4variantobject_p.h>
+
+#if QT_CONFIG(regularexpression)
+#include <QtCore/qregularexpression.h>
+#endif
 
 QT_BEGIN_NAMESPACE
 
@@ -189,43 +204,197 @@ bool convertAndCall(ExecutionEngine *engine, QObject *thisObject,
     return !jsResult->isUndefined();
 }
 
-template<typename Callable>
-ReturnedValue coerceAndCall(
-        ExecutionEngine *engine, const QQmlPrivate::AOTCompiledFunction *typedFunction,
-        const Value *thisObject, const Value *argv, int argc, Callable call)
+inline ReturnedValue coerce(
+    ExecutionEngine *engine, const Value &value, const QQmlType &qmlType, bool isList);
+
+inline QObject *coerceQObject(const Value &value, const QQmlType &qmlType)
 {
-    Scope scope(engine);
-    QV4::JSCallArguments jsCallData(scope, argc);
+    QObject *o;
+    if (const QV4::QObjectWrapper *wrapper = value.as<QV4::QObjectWrapper>())
+        o = wrapper->object();
+    else if (const QV4::QQmlTypeWrapper *wrapper = value.as<QQmlTypeWrapper>())
+        o = wrapper->object();
+    else
+        return nullptr;
 
-    const qsizetype numFunctionArguments = typedFunction->argumentTypes.size();
-    for (qsizetype i = 0; i < numFunctionArguments; ++i) {
-        const QMetaType argumentType = typedFunction->argumentTypes[i];
-        if (const qsizetype argumentSize = argumentType.sizeOf()) {
-            Q_ALLOCA_VAR(void, argument, argumentSize);
-            argumentType.construct(argument);
-            if (i < argc)
-                ExecutionEngine::metaTypeFromJS(argv[i], argumentType, argument);
-            jsCallData.args[i] = engine->metaTypeToJS(argumentType, argument);
-        } else {
-            jsCallData.args[i] = argv[i];
+    return (o && qmlobject_can_qml_cast(o, qmlType)) ? o : nullptr;
+}
+
+enum CoercionProblem
+{
+    InsufficientAnnotation,
+    InvalidListType
+};
+
+Q_QML_PRIVATE_EXPORT void warnAboutCoercionToVoid(
+        ExecutionEngine *engine, const Value &value, CoercionProblem problem);
+
+inline ReturnedValue coerceListType(
+    ExecutionEngine *engine, const Value &value, const QQmlType &qmlType)
+{
+    QMetaType type = qmlType.qListTypeId();
+    if (const QV4::Sequence *sequence = value.as<QV4::Sequence>()) {
+        const QQmlTypePrivate *typePrivate = sequence->d()->typePrivate();
+        if (typePrivate->listId == type)
+            return value.asReturnedValue();
+    }
+
+    if (const QmlListWrapper *list = value.as<QmlListWrapper>()) {
+        if (list->d()->propertyType() == type)
+            return value.asReturnedValue();
+    }
+
+    QMetaType listValueType = qmlType.typeId();
+    if (!listValueType.isValid()) {
+        warnAboutCoercionToVoid(engine, value, InvalidListType);
+        return value.asReturnedValue();
+    }
+
+    QV4::Scope scope(engine);
+
+    const ArrayObject *array = value.as<ArrayObject>();
+    if (!array) {
+        return (listValueType.flags() & QMetaType::PointerToQObject)
+                   ? QmlListWrapper::create(engine, listValueType)
+                   : SequencePrototype::fromData(engine, type, nullptr);
+    }
+
+    if (listValueType.flags() & QMetaType::PointerToQObject) {
+        QV4::Scoped<QmlListWrapper> newList(scope, QmlListWrapper::create(engine, listValueType));
+        QQmlListProperty<QObject> *listProperty = newList->d()->property();
+
+        const qsizetype length = array->getLength();
+        qsizetype i = 0;
+        for (; i < length; ++i) {
+            ScopedValue v(scope, array->get(i));
+            listProperty->append(listProperty, coerceQObject(v, qmlType));
         }
+
+        return newList->asReturnedValue();
     }
 
-    ScopedValue result(scope, call(thisObject, jsCallData.args, argc));
-    const QMetaType returnType = typedFunction->returnType;
-    if (const qsizetype returnSize = returnType.sizeOf()) {
-        Q_ALLOCA_VAR(void, returnValue, returnSize);
-        if (scope.hasException()) {
-            returnType.construct(returnValue);
-        } else if (returnType == QMetaType::fromType<QVariant>()) {
-            new (returnValue) QVariant(ExecutionEngine::toVariant(result, QMetaType {}));
-        } else {
-            returnType.construct(returnValue);
-            ExecutionEngine::metaTypeFromJS(result, returnType, returnValue);
+    QV4::Scoped<Sequence> sequence(scope, SequencePrototype::fromData(engine, type, nullptr));
+    const qsizetype length = array->getLength();
+    for (qsizetype i = 0; i < length; ++i)
+        sequence->containerPutIndexed(i, array->get(i));
+    return sequence->asReturnedValue();
+}
+
+inline ReturnedValue coerce(
+    ExecutionEngine *engine, const Value &value, const QQmlType &qmlType, bool isList)
+{
+    // These are all the named non-list, non-QObject builtins. Only those need special handling.
+    // Some of them may be wrapped in VariantObject because that is how they are stored in VME
+    // properties.
+    if (isList)
+        return coerceListType(engine, value, qmlType);
+
+    const QMetaType metaType = qmlType.typeId();
+    if (!metaType.isValid()) {
+        if (!value.isUndefined())
+            warnAboutCoercionToVoid(engine, value, InsufficientAnnotation);
+        return value.asReturnedValue();
+    }
+
+    switch (metaType.id()) {
+    case QMetaType::Void:
+        return Encode::undefined();
+    case QMetaType::QVariant:
+        return value.asReturnedValue();
+    case QMetaType::Int:
+        return Encode(value.toInt32());
+    case QMetaType::Double:
+        return value.convertedToNumber();
+    case QMetaType::QString:
+        return value.toString(engine)->asReturnedValue();
+    case QMetaType::Bool:
+        return Encode(value.toBoolean());
+    case QMetaType::QDateTime:
+        if (value.as<DateObject>())
+            return value.asReturnedValue();
+        if (const VariantObject *varObject = value.as<VariantObject>()) {
+            const QVariant &var = varObject->d()->data();
+            switch (var.metaType().id()) {
+            case QMetaType::QDateTime:
+                return engine->newDateObject(var.value<QDateTime>())->asReturnedValue();
+            case QMetaType::QTime:
+                return engine->newDateObject(var.value<QTime>(), nullptr, -1, 0)->asReturnedValue();
+            case QMetaType::QDate:
+                return engine->newDateObject(var.value<QDate>(), nullptr, -1, 0)->asReturnedValue();
+            default:
+                break;
+            }
+        }
+        return engine->newDateObject(QDateTime())->asReturnedValue();
+    case QMetaType::QUrl:
+        if (value.as<UrlObject>())
+            return value.asReturnedValue();
+        if (const VariantObject *varObject = value.as<VariantObject>()) {
+            const QVariant &var = varObject->d()->data();
+            return var.metaType() == QMetaType::fromType<QUrl>()
+                       ? engine->newUrlObject(var.value<QUrl>())->asReturnedValue()
+                       : engine->newUrlObject()->asReturnedValue();
         }
-        return engine->metaTypeToJS(returnType, returnValue);
+        // Since URL properties are stored as string, we need to support the string conversion here.
+        return value.isString()
+            ? engine->newUrlObject(QUrl(value.stringValue()->toQString()))->asReturnedValue()
+            : engine->newUrlObject()->asReturnedValue();
+#if QT_CONFIG(regularexpression)
+    case QMetaType::QRegularExpression:
+        if (value.as<RegExpObject>())
+            return value.asReturnedValue();
+        if (const VariantObject *varObject = value.as<VariantObject>()) {
+            const QVariant &var = varObject->d()->data();
+            if (var.metaType() == QMetaType::fromType<QRegularExpression>())
+                return engine->newRegExpObject(var.value<QRegularExpression>())->asReturnedValue();
+        }
+        return engine->newRegExpObject(QString(), 0)->asReturnedValue();
+#endif
+    default:
+        break;
+    }
+
+    if (metaType.flags() & QMetaType::PointerToQObject) {
+        return coerceQObject(value, qmlType)
+                ? value.asReturnedValue()
+                : Encode::null();
     }
-    return result->asReturnedValue();
+
+    if (const QQmlValueTypeWrapper *wrapper = value.as<QQmlValueTypeWrapper>()) {
+        if (wrapper->type() == metaType)
+            return value.asReturnedValue();
+    }
+
+    if (void *target = QQmlValueTypeProvider::heapCreateValueType(qmlType, value)) {
+        Heap::QQmlValueTypeWrapper *wrapper = engine->memoryManager->allocate<QQmlValueTypeWrapper>(
+                nullptr, metaType, QQmlMetaType::metaObjectForValueType(qmlType),
+                nullptr, -1, Heap::ReferenceObject::NoFlag);
+        Q_ASSERT(!wrapper->gadgetPtr());
+        wrapper->setGadgetPtr(target);
+        return wrapper->asReturnedValue();
+    }
+
+    return Encode::undefined();
+}
+
+template<typename Callable>
+ReturnedValue coerceAndCall(
+    ExecutionEngine *engine,
+    const Function::JSTypedFunction *typedFunction, const CompiledData::Function *compiledFunction,
+    const Value *thisObject, const Value *argv, int argc, Callable call)
+{
+    Scope scope(engine);
+
+    QV4::JSCallArguments jsCallData(scope, typedFunction->argumentTypes.size());
+    const CompiledData::Parameter *formals = compiledFunction->formalsTable();
+    for (qsizetype i = 0; i < jsCallData.argc; ++i) {
+        jsCallData.args[i] = coerce(
+            engine, i < argc ? argv[i] : Encode::undefined(),
+            typedFunction->argumentTypes[i], formals[i].type.isList());
+    }
+
+    ScopedValue result(scope, call(thisObject, jsCallData.args, jsCallData.argc));
+    return coerce(engine, result, typedFunction->returnType, compiledFunction->returnType.isList());
 }
 
 } // namespace QV4