Rewrite multiline strings properly

Because the bindings rewriter works on code strings, it would leave
multiline strings across multiple lines (which is illegal in ECMAScript.

It now manually breaks them up when it sees them, by replacing a \n
character with a literal \n.

Since RewriteSignalHandler now likes to have the AST passed in too, the
related method in QDeclarativeCompiler (and its customers) have been
altered to use the QDeclarativeScript::Value instead of just a string.

Task-number: QTBUG-23387
Change-Id: Id060de37e70590c9da2a902038ed02d948fdd70f
Reviewed-by: Alan Alpert <alan.alpert@nokia.com>

9 files changed, 182 insertions, 13 deletions

diff --git a/src/declarative/qml/qdeclarativecompiler.cpp b/src/declarative/qml/qdeclarativecompiler.cpp
index 6c018c7841..74dbd6767f 100644
--- a/src/declarative/qml/qdeclarativecompiler.cpp
+++ b/src/declarative/qml/qdeclarativecompiler.cpp
@@ -1346,9 +1346,8 @@ void QDeclarativeCompiler::genObjectBody(QDeclarativeScript::Object *obj)
 
             Instruction::StoreSignal store;
             store.signalIndex = prop->index;
-            QDeclarativeRewrite::RewriteSignalHandler rewriteSignalHandler;
             const QString &rewrite =
-                    rewriteSignalHandler(v->value.asScript().trimmed(), prop->name().toString());
+                    rewriteSignalHandler(v->value, prop->name().toString());
             store.value = output->indexForString(rewrite);
             store.context = v->signalExpressionContextStack;
             store.line = v->location.start.line;
@@ -2635,10 +2634,10 @@ int QDeclarativeCompiler::rewriteBinding(const QDeclarativeScript::Variant& valu
     return output->indexForString(rewrite);
 }
 
-QString QDeclarativeCompiler::rewriteSignalHandler(const QString &handler, const QString &name)
+QString QDeclarativeCompiler::rewriteSignalHandler(const QDeclarativeScript::Variant& value, const QString &name)
 {
     QDeclarativeRewrite::RewriteSignalHandler rewriteSignalHandler;
-    return rewriteSignalHandler(handler, name);
+    return rewriteSignalHandler(value.asAST(), value.asScript(), name);
 }
 
 // Ensures that the dynamic meta specification on obj is valid
diff --git a/src/declarative/qml/qdeclarativecompiler_p.h b/src/declarative/qml/qdeclarativecompiler_p.h
index d28541c6f2..67d925e74b 100644
--- a/src/declarative/qml/qdeclarativecompiler_p.h
+++ b/src/declarative/qml/qdeclarativecompiler_p.h
@@ -293,7 +293,7 @@ public:
     int evaluateEnum(const QByteArray& script) const; // for QDeclarativeCustomParser::evaluateEnum
     const QMetaObject *resolveType(const QString& name) const; // for QDeclarativeCustomParser::resolveType
     int rewriteBinding(const QDeclarativeScript::Variant& value, const QString& name); // for QDeclarativeCustomParser::rewriteBinding
-    QString rewriteSignalHandler(const QString &handler, const QString &name);  // for QDeclarativeCustomParser::rewriteSignalHandler
+    QString rewriteSignalHandler(const QDeclarativeScript::Variant& value, const QString &name);  // for QDeclarativeCustomParser::rewriteSignalHandler
 
 private:
     typedef QDeclarativeCompiledData::Instruction Instruction;
diff --git a/src/declarative/qml/qdeclarativecustomparser.cpp b/src/declarative/qml/qdeclarativecustomparser.cpp
index 7b0946ba71..a746700aa6 100644
--- a/src/declarative/qml/qdeclarativecustomparser.cpp
+++ b/src/declarative/qml/qdeclarativecustomparser.cpp
@@ -311,9 +311,9 @@ QDeclarativeBinding::Identifier QDeclarativeCustomParser::rewriteBinding(const Q
     Returns a rewritten \a handler. \a name
     is used as the name of the rewritten function.
 */
-QString QDeclarativeCustomParser::rewriteSignalHandler(const QString &handler, const QString &name)
+QString QDeclarativeCustomParser::rewriteSignalHandler(const QDeclarativeScript::Variant &value, const QString &name)
 {
-    return compiler->rewriteSignalHandler(handler, name);
+    return compiler->rewriteSignalHandler(value , name);
 }
 
 QT_END_NAMESPACE
diff --git a/src/declarative/qml/qdeclarativecustomparser_p.h b/src/declarative/qml/qdeclarativecustomparser_p.h
index 863e85314b..92d1ca7af2 100644
--- a/src/declarative/qml/qdeclarativecustomparser_p.h
+++ b/src/declarative/qml/qdeclarativecustomparser_p.h
@@ -142,7 +142,7 @@ protected:
     const QMetaObject *resolveType(const QString&) const;
 
     QDeclarativeBinding::Identifier rewriteBinding(const QDeclarativeScript::Variant&, const QString&);
-    QString rewriteSignalHandler(const QString&, const QString&);
+    QString rewriteSignalHandler(const QDeclarativeScript::Variant&, const QString&);
 
 private:
     QList<QDeclarativeError> exceptions;
diff --git a/src/declarative/qml/qdeclarativerewrite.cpp b/src/declarative/qml/qdeclarativerewrite.cpp
index 3d73c094b7..77da943704 100644
--- a/src/declarative/qml/qdeclarativerewrite.cpp
+++ b/src/declarative/qml/qdeclarativerewrite.cpp
@@ -110,6 +110,7 @@ QString RewriteBinding::operator()(QDeclarativeJS::AST::Node *node, const QStrin
     _writer = &w;
     _position = expression ? expression->firstSourceLocation().begin() : statement->firstSourceLocation().begin();
     _inLoop = 0;
+    _code = &code;
 
     accept(node);
 
@@ -152,6 +153,7 @@ QString RewriteBinding::rewrite(QString code, unsigned position,
     _writer = &w;
     _position = position;
     _inLoop = 0;
+    _code = &code;
 
     accept(node);
 
@@ -200,6 +202,45 @@ bool RewriteBinding::visit(AST::ExpressionStatement *ast)
     return false;
 }
 
+bool RewriteBinding::visit(AST::StringLiteral *ast)
+{
+    /* When rewriting the code for bindings, we have to remove ILLEGAL JS tokens like newlines.
+       They're still in multi-line strings, because the QML parser allows them, but we have to
+       rewrite them to be JS compliant.
+
+       For performance reasons, we don't go for total correctness. \r is only replaced if a
+       \n was found (since most line endings are \n or \r\n) and QChar::LineSeparator is not
+       even considered. QTBUG-24064.
+
+       Note that rewriteSignalHandler has a function just like this one, for the same reason.
+    */
+
+    unsigned startOfString = ast->firstSourceLocation().begin() + 1 - _position;
+    unsigned stringLength = ast->firstSourceLocation().length - 2;
+
+    int lastIndex = -1;
+    bool foundNewLine = false;
+    QStringRef subStr(_code, startOfString, stringLength);
+    while (true) {
+        lastIndex = subStr.indexOf(QLatin1Char('\n'), lastIndex + 1);
+        if (lastIndex == -1)
+            break;
+        foundNewLine = true;
+        _writer->replace(startOfString+lastIndex, 1, QLatin1String("\\n"));
+    }
+
+    if (foundNewLine) {
+        while (true) {
+            lastIndex = subStr.indexOf(QLatin1Char('\r'), lastIndex + 1);
+            if (lastIndex == -1)
+                break;
+            _writer->replace(startOfString+lastIndex, 1, QLatin1String("\\r"));
+        }
+    }
+
+    return false;
+}
+
 bool RewriteBinding::visit(AST::DoWhileStatement *)
 {
     ++_inLoop;
@@ -320,9 +361,79 @@ void RewriteBinding::rewriteCaseStatements(AST::StatementList *statements, bool
     }
 }
 
-QString RewriteSignalHandler::operator()(const QString &code, const QString &name)
+void RewriteSignalHandler::accept(AST::Node *node)
+{
+    AST::Node::acceptChild(node, this);
+}
+
+bool RewriteSignalHandler::visit(AST::StringLiteral *ast)
+{
+    unsigned startOfExpressionStatement = ast->firstSourceLocation().begin() - _position;
+    _strStarts << startOfExpressionStatement + 1;
+    _strLens << ast->firstSourceLocation().length - 2;
+
+    return false;
+}
+
+void RewriteSignalHandler::rewriteMultilineStrings(QString &code)
+{
+    QList<int> replaceR, replaceN;
+    for (int i=0; i < _strStarts.count(); i++) {
+        QStringRef curSubstr = QStringRef(&code, _strStarts[i], _strLens[i]);
+        int lastIndex = -1;
+        while (true) {
+            lastIndex = curSubstr.indexOf(QLatin1Char('\n'), lastIndex + 1);
+            if (lastIndex == -1)
+                break;
+            replaceN << _strStarts[i]+lastIndex;
+        }
+
+        if (replaceN.count()) {
+            while (true) {
+                lastIndex = curSubstr.indexOf(QLatin1Char('\r'), lastIndex + 1);
+                if (lastIndex == -1)
+                    break;
+                replaceR << _strStarts[i]+lastIndex;
+            }
+        }
+    }
+    for (int ii = replaceN.count() - 1; ii >= 0; ii--)
+        code.replace(replaceN[ii], 1, QLatin1String("\\n"));
+    if (replaceR.count())
+        for (int ii = replaceR.count() - 1; ii >= 0; ii--)
+            code.replace(replaceR[ii], 1, QLatin1String("\\r"));
+}
+
+QString RewriteSignalHandler::operator()(QDeclarativeJS::AST::Node *node, const QString &code, const QString &name)
 {
-    return QStringLiteral("(function ") + name + QStringLiteral("() { ") + code + QStringLiteral(" })");
+    if (rewriteDump()) {
+        qWarning() << "=============================================================";
+        qWarning() << "Rewrote:";
+        qWarning() << qPrintable(code);
+    }
+
+    QDeclarativeJS::AST::ExpressionNode *expression = node->expressionCast();
+    QDeclarativeJS::AST::Statement *statement = node->statementCast();
+    if (!expression && !statement)
+        return code;
+
+    _strStarts.clear();
+    _strLens.clear();
+    _position = expression ? expression->firstSourceLocation().begin() : statement->firstSourceLocation().begin();
+    accept(node);
+
+    QString rewritten = code;
+    rewriteMultilineStrings(rewritten);
+
+    rewritten = QStringLiteral("(function ") + name + QStringLiteral("() { ") + rewritten + QStringLiteral(" })");
+
+    if (rewriteDump()) {
+        qWarning() << "To:";
+        qWarning() << qPrintable(rewritten);
+        qWarning() << "=============================================================";
+    }
+
+    return rewritten;
 }
 
 } // namespace QDeclarativeRewrite
diff --git a/src/declarative/qml/qdeclarativerewrite_p.h b/src/declarative/qml/qdeclarativerewrite_p.h
index ba3f8385e7..74c408cd21 100644
--- a/src/declarative/qml/qdeclarativerewrite_p.h
+++ b/src/declarative/qml/qdeclarativerewrite_p.h
@@ -80,6 +80,7 @@ class RewriteBinding: protected AST::Visitor
     unsigned _position;
     TextWriter *_writer;
     QString _name;
+    const QString *_code;
 
 public:
     QString operator()(const QString &code, bool *ok = 0, bool *sharable = 0);
@@ -95,6 +96,7 @@ protected:
     QString rewrite(QString code, unsigned position, AST::Statement *node);
     void rewriteCaseStatements(AST::StatementList *statements, bool rewriteTheLastStatement);
 
+    virtual bool visit(AST::StringLiteral *ast);
     virtual bool visit(AST::Block *ast);
     virtual bool visit(AST::ExpressionStatement *ast);
 
@@ -122,10 +124,21 @@ private:
     int _inLoop;
 };
 
-class RewriteSignalHandler
+class RewriteSignalHandler: protected AST::Visitor
 {
+    QList<int> _strStarts;
+    QList<int> _strLens;
+    int _position;
+
 public:
-    QString operator()(const QString &code, const QString &name);
+    QString operator()(QDeclarativeJS::AST::Node *node, const QString &code, const QString &name);
+
+protected:
+    void rewriteMultilineStrings(QString &code);
+
+    using AST::Visitor::visit;
+    void accept(AST::Node *node);
+    virtual bool visit(AST::StringLiteral *ast);
 };
 
 } // namespace QDeclarativeRewrite
diff --git a/src/quick/util/qdeclarativeconnections.cpp b/src/quick/util/qdeclarativeconnections.cpp
index 314d914936..e128e2e1c1 100644
--- a/src/quick/util/qdeclarativeconnections.cpp
+++ b/src/quick/util/qdeclarativeconnections.cpp
@@ -226,7 +226,7 @@ QDeclarativeConnectionsParser::compile(const QList<QDeclarativeCustomParserPrope
                 QDeclarativeScript::Variant v = qvariant_cast<QDeclarativeScript::Variant>(value);
                 if (v.isScript()) {
                     ds << propName;
-                    ds << rewriteSignalHandler(v.asScript(), propName);
+                    ds << rewriteSignalHandler(v, propName);
                     ds << propLine;
                     ds << propColumn;
                 } else {
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings.qml b/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings.qml
new file mode 100644
index 0000000000..1ae1b162b2
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings.qml
@@ -0,0 +1,35 @@
+import QtQuick 2.0
+
+Item {
+    id: root;
+    property bool test: str == str2 && (txt != null && txt.str == root.str)
+    property Text txt: null
+    //Constant doesn't hit rewriter
+    property string str: 'same
+multiline
+string 5 !'
+    property string str2: '';
+    Component {
+        id: comp
+        Text {
+            property var value: 1
+            property string str: 'same
+multiline
+string ' + value + " !"
+            Component.onCompleted: { //Separate codepath for signal handers in rewriter
+                root.str2 = 'same
+multiline
+string ' + value + " !"
+            }
+        }
+    }
+    Component.onCompleted: txt = comp.createObject(root,{"value" : 5})
+    /*
+    Timer {
+        interval: 1000
+        running: true
+        repeat: true
+        onTriggered: console.debug( "Test: " + test + '\n' + str + '\n:\n' + str2 + "\n:\n" + txt.str)
+    }
+    */
+}
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index fe9fd0e8c4..1c382de228 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -229,6 +229,7 @@ private slots:
     void qtbug_22679();
     void qtbug_22843_data();
     void qtbug_22843();
+    void rewriteMultiLineStrings();
     void revisionErrors();
     void revision();
 
@@ -5235,6 +5236,16 @@ void tst_qdeclarativeecmascript::qtbug_21864()
     delete o;
 }
 
+void tst_qdeclarativeecmascript::rewriteMultiLineStrings()
+{
+    // QTBUG-23387
+    QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings.qml"));
+    QObject *o = component.create();
+    QVERIFY(o != 0);
+    QTRY_COMPARE(o->property("test").toBool(), true);
+    delete o;
+}
+
 void tst_qdeclarativeecmascript::qobjectConnectionListExceptionHandling()
 {
     // QTBUG-23375