From 5060b58fdde00ca3fa4dc29bd58b80138a271b10 Mon Sep 17 00:00:00 2001 From: Alan Alpert Date: Wed, 1 Feb 2012 08:07:39 +1000 Subject: [PATCH] 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 --- src/declarative/qml/qdeclarativecompiler.cpp | 7 +- src/declarative/qml/qdeclarativecompiler_p.h | 2 +- src/declarative/qml/qdeclarativecustomparser.cpp | 4 +- src/declarative/qml/qdeclarativecustomparser_p.h | 2 +- src/declarative/qml/qdeclarativerewrite.cpp | 115 ++++++++++++++++++++- src/declarative/qml/qdeclarativerewrite_p.h | 17 ++- src/quick/util/qdeclarativeconnections.cpp | 2 +- .../data/rewriteMultiLineStrings.qml | 35 +++++++ .../tst_qdeclarativeecmascript.cpp | 11 ++ 9 files changed, 182 insertions(+), 13 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings.qml diff --git a/src/declarative/qml/qdeclarativecompiler.cpp b/src/declarative/qml/qdeclarativecompiler.cpp index 6c018c7..74dbd67 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 d28541c..67d925e 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 7b0946b..a746700 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 863e853..92d1ca7 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 exceptions; diff --git a/src/declarative/qml/qdeclarativerewrite.cpp b/src/declarative/qml/qdeclarativerewrite.cpp index 3d73c09..77da943 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 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 ba3f838..74c408c 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 _strStarts; + QList _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 314d914..e128e2e 100644 --- a/src/quick/util/qdeclarativeconnections.cpp +++ b/src/quick/util/qdeclarativeconnections.cpp @@ -226,7 +226,7 @@ QDeclarativeConnectionsParser::compile(const QList(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 0000000..1ae1b16 --- /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 fe9fd0e..1c382de 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 -- 2.7.4