From 212204b4fff05aed43a931e71855948ec9910b59 Mon Sep 17 00:00:00 2001 From: Roberto Raggi Date: Mon, 20 Feb 2012 14:46:26 +0100 Subject: [PATCH] Fix rewrite of multiline string literals. This commits ensures that we don't rewrite `\'-terminated multiline string literals. Also, it fixes the processing of \r characters inside the string literals. Change-Id: If3d7c1b83c7306b9ccb1be31412b6f8e76434c41 Reviewed-by: Oswald Buddenhagen Reviewed-by: Aaron Kennedy --- .gitattributes | 1 + src/declarative/qml/qdeclarativerewrite.cpp | 123 ++++++++---------- src/declarative/qml/qdeclarativerewrite_p.h | 5 +- .../data/rewriteMultiLineStrings_crlf.1.qml | 13 ++ .../tst_qdeclarativeecmascript.cpp | 23 ++-- 5 files changed, 85 insertions(+), 80 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings_crlf.1.qml diff --git a/.gitattributes b/.gitattributes index a3c6108370..157176bdb8 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ .tag ident +*_crlf.* eol=crlf diff --git a/src/declarative/qml/qdeclarativerewrite.cpp b/src/declarative/qml/qdeclarativerewrite.cpp index 77da943704..688e75a325 100644 --- a/src/declarative/qml/qdeclarativerewrite.cpp +++ b/src/declarative/qml/qdeclarativerewrite.cpp @@ -51,6 +51,45 @@ DEFINE_BOOL_CONFIG_OPTION(rewriteDump, QML_REWRITE_DUMP); namespace QDeclarativeRewrite { +static void rewriteStringLiteral(AST::StringLiteral *ast, const QString *code, int startPosition, TextWriter *writer) +{ + const unsigned position = ast->firstSourceLocation().begin() - startPosition + 1; + const unsigned length = ast->literalToken.length - 2; + const QStringRef spell = code->midRef(position, length); + const int end = spell.size(); + int index = 0; + + while (index < end) { + const QChar ch = spell.at(index++); + + if (index < end && ch == QLatin1Char('\\')) { + int pos = index; + + // skip a possibly empty sequence of \r characters + while (pos < end && spell.at(pos) == QLatin1Char('\r')) + ++pos; + + if (pos < end && spell.at(pos) == QLatin1Char('\n')) { + // This is a `\' followed by a newline terminator. + // In this case there's nothing to replace. We keep the code + // as it is and we resume the searching. + index = pos + 1; // refresh the index + } + } else if (ch == QLatin1Char('\r') || ch == QLatin1Char('\n')) { + const QString sep = ch == QLatin1Char('\r') ? QLatin1String("\\r") : QLatin1String("\\n"); + const int pos = index - 1; + QString s = sep; + + while (index < end && spell.at(index) == ch) { + s += sep; + ++index; + } + + writer->replace(position + pos, index - pos, s); + } + } +} + bool SharedBindingTester::isSharable(const QString &code) { Engine engine; @@ -204,40 +243,7 @@ bool RewriteBinding::visit(AST::ExpressionStatement *ast) 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")); - } - } - + rewriteStringLiteral(ast, _code, _position, _writer); return false; } @@ -361,6 +367,13 @@ void RewriteBinding::rewriteCaseStatements(AST::StatementList *statements, bool } } +RewriteSignalHandler::RewriteSignalHandler() + : _writer(0) + , _code(0) + , _position(0) +{ +} + void RewriteSignalHandler::accept(AST::Node *node) { AST::Node::acceptChild(node, this); @@ -368,42 +381,10 @@ void RewriteSignalHandler::accept(AST::Node *node) bool RewriteSignalHandler::visit(AST::StringLiteral *ast) { - unsigned startOfExpressionStatement = ast->firstSourceLocation().begin() - _position; - _strStarts << startOfExpressionStatement + 1; - _strLens << ast->firstSourceLocation().length - 2; - + rewriteStringLiteral(ast, _code, _position, _writer); 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) { if (rewriteDump()) { @@ -417,13 +398,15 @@ QString RewriteSignalHandler::operator()(QDeclarativeJS::AST::Node *node, const if (!expression && !statement) return code; - _strStarts.clear(); - _strLens.clear(); + TextWriter w; + _writer = &w; + _code = &code; + _position = expression ? expression->firstSourceLocation().begin() : statement->firstSourceLocation().begin(); accept(node); QString rewritten = code; - rewriteMultilineStrings(rewritten); + w.write(&rewritten); rewritten = QStringLiteral("(function ") + name + QStringLiteral("() { ") + rewritten + QStringLiteral(" })"); diff --git a/src/declarative/qml/qdeclarativerewrite_p.h b/src/declarative/qml/qdeclarativerewrite_p.h index 74c408cd21..7d73e6a132 100644 --- a/src/declarative/qml/qdeclarativerewrite_p.h +++ b/src/declarative/qml/qdeclarativerewrite_p.h @@ -126,11 +126,12 @@ class RewriteBinding: protected AST::Visitor class RewriteSignalHandler: protected AST::Visitor { - QList _strStarts; - QList _strLens; + TextWriter *_writer; + const QString *_code; int _position; public: + RewriteSignalHandler(); QString operator()(QDeclarativeJS::AST::Node *node, const QString &code, const QString &name); protected: diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings_crlf.1.qml b/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings_crlf.1.qml new file mode 100644 index 0000000000..f84ba8c722 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/rewriteMultiLineStrings_crlf.1.qml @@ -0,0 +1,13 @@ +import QtQuick 2.0 + +Rectangle { + id: root + + Component.onCompleted: { + var o = Qt.createQmlObject("import QtQuick 2.0; \ + \ + Item { \ + property bool b: true; \ + }", root, "Instance") + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index d30766f982..cc94e6fb4a 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -1570,8 +1570,6 @@ void tst_qdeclarativeecmascript::compileInvalidBinding() { // QTBUG-23387: ensure that invalid bindings don't cause a crash. QDeclarativeComponent component(&engine, testFileUrl("v8bindingException.qml")); - QString warning = component.url().toString() + ":16: SyntaxError: Unexpected token ILLEGAL"; - QTest::ignoreMessage(QtWarningMsg, warning.toLatin1().constData()); QObject *object = component.create(); QVERIFY(object != 0); delete object; @@ -5287,12 +5285,21 @@ void tst_qdeclarativeecmascript::qtbug_21864() 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; + { + // QTBUG-23387 + QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings.qml")); + QObject *o = component.create(); + QVERIFY(o != 0); + QTRY_COMPARE(o->property("test").toBool(), true); + delete o; + } + + { + QDeclarativeComponent component(&engine, testFileUrl("rewriteMultiLineStrings_crlf.1.qml")); + QObject *o = component.create(); + QVERIFY(o != 0); + delete o; + } } void tst_qdeclarativeecmascript::qobjectConnectionListExceptionHandling()