Error out when compiling signal handlers with arguments in qml files

Ahead of time we cannot tell whether the use of "arguments" in a signal
hander refers to the JS arguments object or a potential arguments signal
parameter. Resolving that requires access to information we currently
don't have. The QML engine has it at run-time (in
SignalHandlerConverter) and that's why it works there accordingly.
However when generating caches ahead of time, let's rather produce an
error message with a hint how to work around it instead of producing
differing behavior at run-time.

Task-number: QTBUG-60011
Change-Id: I9e460bd467dbb5998f12a44c439223ea44e7bbad
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

2 files changed, 69 insertions, 2 deletions

diff --git a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp
index b7e616a050..b69071dd59 100644
--- a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp
+++ b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp
@@ -43,6 +43,7 @@ private slots:
 
     void loadGeneratedFile();
     void translationExpressionSupport();
+    void errorOnArgumentsInSignalHandler();
 };
 
 // A wrapper around QQmlComponent to ensure the temporary reference counts
@@ -67,15 +68,20 @@ public:
     }
 };
 
-static bool generateCache(const QString &qmlFileName)
+static bool generateCache(const QString &qmlFileName, QByteArray *capturedStderr = nullptr)
 {
     QProcess proc;
-    proc.setProcessChannelMode(QProcess::ForwardedChannels);
+    if (capturedStderr == nullptr)
+        proc.setProcessChannelMode(QProcess::ForwardedChannels);
     proc.setProgram(QLibraryInfo::location(QLibraryInfo::BinariesPath) + QDir::separator() + QLatin1String("qmlcachegen"));
     proc.setArguments(QStringList() << (QLatin1String("--target-architecture=") + QSysInfo::buildCpuArchitecture()) << (QLatin1String("--target-abi=") + QSysInfo::buildAbi()) << qmlFileName);
     proc.start();
     if (!proc.waitForFinished())
         return false;
+
+    if (capturedStderr)
+        *capturedStderr = proc.readAllStandardError();
+
     if (proc.exitStatus() != QProcess::NormalExit)
         return false;
     return proc.exitCode() == 0;
@@ -158,6 +164,31 @@ void tst_qmlcachegen::translationExpressionSupport()
     QCOMPARE(obj->property("text").toString(), QString("All Ok"));
 }
 
+void tst_qmlcachegen::errorOnArgumentsInSignalHandler()
+{
+    QTemporaryDir tempDir;
+    QVERIFY(tempDir.isValid());
+
+    const auto writeTempFile = [&tempDir](const QString &fileName, const char *contents) {
+        QFile f(tempDir.path() + '/' + fileName);
+        const bool ok = f.open(QIODevice::WriteOnly | QIODevice::Truncate);
+        Q_ASSERT(ok);
+        f.write(contents);
+        return f.fileName();
+    };
+
+    const QString testFilePath = writeTempFile("test.qml", "import QtQml 2.2\n"
+                                                           "QtObject {\n"
+                                                           "    signal mySignal(var arguments);\n"
+                                                           "    onMySignal: console.log(arguments);\n"
+                                                           "}");
+
+
+    QByteArray errorOutput;
+    QVERIFY(!generateCache(testFilePath, &errorOutput));
+    QVERIFY2(errorOutput.contains("error: The use of the arguments object in signal handlers is"), errorOutput);
+}
+
 QTEST_GUILESS_MAIN(tst_qmlcachegen)
 
 #include "tst_qmlcachegen.moc"
diff --git a/tools/qmlcachegen/qmlcachegen.cpp b/tools/qmlcachegen/qmlcachegen.cpp
index b201176d5e..a62adc82f4 100644
--- a/tools/qmlcachegen/qmlcachegen.cpp
+++ b/tools/qmlcachegen/qmlcachegen.cpp
@@ -114,6 +114,37 @@ static void annotateListElements(QmlIR::Document *document)
     }
 }
 
+static bool checkArgumentsObjectUseInSignalHandlers(const QmlIR::Document &doc, Error *error)
+{
+    for (QmlIR::Object *object: qAsConst(doc.objects)) {
+        for (auto binding = object->bindingsBegin(); binding != object->bindingsEnd(); ++binding) {
+            if (binding->type != QV4::CompiledData::Binding::Type_Script)
+                continue;
+            const QString propName =  doc.stringAt(binding->propertyNameIndex);
+            if (!propName.startsWith(QLatin1String("on"))
+                || propName.length() < 3
+                || !propName.at(2).isUpper())
+                continue;
+            auto compiledFunction = doc.jsModule.functions.value(object->runtimeFunctionIndices.at(binding->value.compiledScriptIndex));
+            if (!compiledFunction)
+                continue;
+            if (compiledFunction->usesArgumentsObject) {
+                error->message = QLatin1Char(':') + QString::number(compiledFunction->line) + QLatin1Char(':');
+                if (compiledFunction->column > 0)
+                    error->message += QString::number(compiledFunction->column) + QLatin1Char(':');
+
+                error->message += QLatin1String(" error: The use of the arguments object in signal handlers is\n"
+                                                "not supported when compiling qml files ahead of time, because it may be ambiguous if\n"
+                                                "any signal parameter is called \"arguments\". Unfortunately we cannot distinguish\n"
+                                                "between it being a parameter or the JavaScript arguments object at this point.\n"
+                                                "Consider renaming the parameter of the signal if applicable.");
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 static bool compileQmlFile(const QString &inputFileName, const QString &outputFileName, QV4::EvalISelFactory *iselFactory, const QString &targetABI, Error *error)
 {
     QmlIR::Document irDocument(/*debugMode*/false);
@@ -173,6 +204,11 @@ static bool compileQmlFile(const QString &inputFileName, const QString &outputFi
             object->runtimeFunctionIndices.allocate(pool, runtimeFunctionIndices);
         }
 
+        if (!checkArgumentsObjectUseInSignalHandlers(irDocument, error)) {
+            *error = error->augment(inputFileName);
+            return false;
+        }
+
         QmlIR::QmlUnitGenerator generator;
 
         {