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>

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

src/declarative/qml/qdeclarativecompiler_p.h

@@ -293,7 +293,7 @@ class Q_AUTOTEST_EXPORT QDeclarativeCompiler
     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;

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

src/declarative/qml/qdeclarativecustomparser_p.h

@@ -142,7 +142,7 @@ class Q_DECLARATIVE_EXPORT QDeclarativeCustomParser
     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;

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

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 @@ class RewriteBinding: protected AST::Visitor
     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 @@ class RewriteBinding: protected AST::Visitor
     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

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 {

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)
+    }
+    */
+}

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