Skip to content

Commit

Permalink
QJS{Engine,Value}: Remove QRegExp-specific functions
Browse files Browse the repository at this point in the history 
Rationale: QRegExp regular expressions have different semantics than
JavaScript RegExp. This can cause data loss and unexpected behavior.

qjsvalue_cast() and fromScriptValue() can still be used to convert
between QRegExp and JS RegExp.

Task-number: QTBUG-23604
Change-Id: Iacf4aaea232aff9e4cecf4afa40753229bc5d643
Reviewed-by: Simon Hausmann <simon.hausmann@nokia.com>
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@nokia.com>
  • Loading branch information
Kent Hansen authored and Qt by Nokia committed Feb 1, 2012
1 parent 422e299 commit 3a017cc
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 147 deletions.
36 changes: 1 addition & 35 deletions src/declarative/qml/v8/qjsengine.cpp
View file
Expand Up @@ -127,8 +127,7 @@ Q_DECLARE_METATYPE(QList<int>)
the object-specific functionality in QJSValue to manipulate the
script object (e.g. QJSValue::setProperty()). Similarly, use
newArray() to create a JavaScript array object. Use newDate() to
create a \c{Date} object, and newRegExp() to create a \c{RegExp}
object.
create a \c{Date} object.
\section1 QObject Integration
Expand Down Expand Up @@ -477,39 +476,6 @@ QJSValue QJSEngine::newDate(double date)
return d->scriptValueFromInternal(v8::Handle<v8::Value>(v8::Date::New(date)));
}

/*!
\obsolete
Creates a JavaScript object of class RegExp with the given
\a regexp.
\sa QJSValue::toRegExp()
*/
QJSValue QJSEngine::newRegExp(const QRegExp &regexp)
{
Q_D(QJSEngine);
QScriptIsolate api(d, QScriptIsolate::NotNullEngine);
v8::HandleScope handleScope;
return QJSValuePrivate::get(d->newRegExp(regexp));
}

/*!
\obsolete
Creates a JavaScript object of class RegExp with the given
\a pattern and \a flags.
The legal flags are 'g' (global), 'i' (ignore case), and 'm'
(multiline).
*/
QJSValue QJSEngine::newRegExp(const QString &pattern, const QString &flags)
{
Q_D(QJSEngine);
QScriptIsolate api(d, QScriptIsolate::NotNullEngine);
v8::HandleScope handleScope;
return QJSValuePrivate::get(d->newRegExp(pattern, flags));
}

#endif // QT_DEPRECATED

/*!
Expand Down
5 changes: 0 additions & 5 deletions src/declarative/qml/v8/qjsengine.h
View file
Expand Up @@ -39,8 +39,6 @@ QT_BEGIN_NAMESPACE
class QDateTime;
class QV8Engine;

class QRegExp;

template <typename T>
inline T qjsvalue_cast(const QJSValue &);

Expand Down Expand Up @@ -96,9 +94,6 @@ class Q_DECLARATIVE_EXPORT QJSEngine

QT_DEPRECATED QJSValue newVariant(const QVariant &value);

QT_DEPRECATED QJSValue newRegExp(const QRegExp &regexp);

QT_DEPRECATED QJSValue newRegExp(const QString &pattern, const QString &flags);
QT_DEPRECATED QJSValue newDate(double value);
QT_DEPRECATED QJSValue newDate(const QDateTime &value);
#endif
Expand Down
25 changes: 1 addition & 24 deletions src/declarative/qml/v8/qjsvalue.cpp
View file
Expand Up @@ -28,7 +28,6 @@
#include "qjsvalue_p.h"
#include "qscript_impl_p.h"
#include "qscriptshareddata_p.h"
#include <QtCore/qregexp.h>
#include <QtCore/qstring.h>

/*!
Expand Down Expand Up @@ -650,7 +649,7 @@ quint32 QJSValue::toUInt32() const
\row \o QVariant Object \o The result is the QVariant value of the object (no conversion).
\row \o QObject Object \o A QVariant containing a pointer to the QObject.
\row \o Date Object \o A QVariant containing the date value (toDateTime()).
\row \o RegExp Object \o A QVariant containing the regular expression value (toRegExp()).
\row \o RegExp Object \o A QVariant containing the regular expression value.
\row \o Array Object \o The array is converted to a QVariantList. Each element is converted to a QVariant, recursively; cyclic references are not followed.
\row \o Object \o The object is converted to a QVariantMap. Each property is converted to a QVariant, recursively; cyclic references are not followed.
\endtable
Expand Down Expand Up @@ -1104,26 +1103,6 @@ QDateTime QJSValue::toDateTime() const
return d->toDataTime();
}

#ifdef QT_DEPRECATED

/*!
\obsolete
Returns the QRegExp representation of this value.
If this QJSValue is not a regular expression, an empty
QRegExp is returned.
\sa isRegExp()
*/
QRegExp QJSValue::toRegExp() const
{
Q_D(const QJSValue);
QScriptIsolate api(d->engine());
return d->toRegExp();
}

#endif // QT_DEPRECATED

/*!
Returns true if this QJSValue is an object of the Date class;
otherwise returns false.
Expand All @@ -1140,8 +1119,6 @@ bool QJSValue::isDate() const
/*!
Returns true if this QJSValue is an object of the RegExp class;
otherwise returns false.
\sa QJSEngine::newRegExp()
*/
bool QJSValue::isRegExp() const
{
Expand Down
2 changes: 0 additions & 2 deletions src/declarative/qml/v8/qjsvalue.h
View file
Expand Up @@ -41,7 +41,6 @@ class QVariant;
class QObject;
struct QMetaObject;
class QDateTime;
class QRegExp;

typedef QList<QJSValue> QJSValueList;

Expand Down Expand Up @@ -142,7 +141,6 @@ class Q_DECLARATIVE_EXPORT QJSValue
QT_DEPRECATED bool isFunction() const;
QT_DEPRECATED qint32 toInt32() const;
QT_DEPRECATED quint32 toUInt32() const;
QT_DEPRECATED QRegExp toRegExp() const;

QT_DEPRECATED bool instanceOf(const QJSValue &other) const;

Expand Down
9 changes: 0 additions & 9 deletions src/declarative/qml/v8/qjsvalue_impl_p.h
View file
Expand Up @@ -324,15 +324,6 @@ inline QDateTime QJSValuePrivate::toDataTime() const

}

inline QRegExp QJSValuePrivate::toRegExp() const
{
if (!isRegExp())
return QRegExp();

v8::HandleScope handleScope;
return QJSConverter::toRegExp(v8::Handle<v8::RegExp>::Cast(m_value));
}

QObject* QJSValuePrivate::toQObject() const
{
if (!isJSBased())
Expand Down
1 change: 0 additions & 1 deletion src/declarative/qml/v8/qjsvalue_p.h
View file
Expand Up @@ -90,7 +90,6 @@ class QJSValuePrivate
inline quint32 toUInt32() const;
inline quint16 toUInt16() const;
inline QDateTime toDataTime() const;
inline QRegExp toRegExp() const;
inline QObject *toQObject() const;
inline QVariant toVariant() const;

Expand Down
32 changes: 0 additions & 32 deletions src/declarative/qml/v8/qv8engine.cpp
View file
Expand Up @@ -1007,38 +1007,6 @@ void QV8Engine::Exception::pop()
m_message = pair.second;
}

QScriptPassPointer<QJSValuePrivate> QV8Engine::newRegExp(const QString &pattern, const QString &flags)
{
int f = v8::RegExp::kNone;

QString::const_iterator i = flags.constBegin();
for (; i != flags.constEnd(); ++i) {
switch (i->unicode()) {
case 'i':
f |= v8::RegExp::kIgnoreCase;
break;
case 'm':
f |= v8::RegExp::kMultiline;
break;
case 'g':
f |= v8::RegExp::kGlobal;
break;
default:
{
// ignore a Syntax Error.
}
}
}

v8::Handle<v8::RegExp> regexp = v8::RegExp::New(QJSConverter::toString(pattern), static_cast<v8::RegExp::Flags>(f));
return new QJSValuePrivate(this, regexp);
}

QScriptPassPointer<QJSValuePrivate> QV8Engine::newRegExp(const QRegExp &regexp)
{
return new QJSValuePrivate(this, QJSConverter::toRegExp(regexp));
}


// Converts a QVariantList to JS.
// The result is a new Array object with length equal to the length
Expand Down
3 changes: 0 additions & 3 deletions src/declarative/qml/v8/qv8engine_p.h
View file
Expand Up @@ -336,9 +336,6 @@ class Q_DECLARATIVE_EXPORT QV8Engine
// Return the QML global "scope" object for the \a ctxt context and \a scope object.
inline v8::Local<v8::Object> qmlScope(QDeclarativeContextData *ctxt, QObject *scope);

QScriptPassPointer<QJSValuePrivate> newRegExp(const QRegExp &regexp);
QScriptPassPointer<QJSValuePrivate> newRegExp(const QString &pattern, const QString &flags);

// Return a JS wrapper for the given QObject \a object
inline v8::Handle<v8::Value> newQObject(QObject *object);
inline v8::Handle<v8::Value> newQObject(QObject *object, const ObjectOwnership ownership);
Expand Down
44 changes: 19 additions & 25 deletions tests/auto/declarative/qjsengine/tst_qjsengine.cpp
View file
Expand Up @@ -792,24 +792,18 @@ void tst_QJSEngine::newRegExp()
{
QSKIP("Test failing - QTBUG-22238");
QJSEngine eng;
for (int x = 0; x < 2; ++x) {
QJSValue rexp;
if (x == 0)
rexp = eng.newRegExp("foo", "bar");
else
rexp = eng.newRegExp(QRegExp("foo"));
QCOMPARE(rexp.isValid(), true);
QCOMPARE(rexp.isRegExp(), true);
QCOMPARE(rexp.isObject(), true);
QVERIFY(rexp.isCallable()); // in JSC, RegExp objects are callable
// prototype should be RegExp.prototype
QCOMPARE(rexp.prototype().isValid(), true);
QCOMPARE(rexp.prototype().isObject(), true);
QCOMPARE(rexp.prototype().isRegExp(), false);
QCOMPARE(rexp.prototype().strictlyEquals(eng.evaluate("RegExp.prototype")), true);
QJSValue rexp = eng.toScriptValue(QRegExp("foo"));
QCOMPARE(rexp.isValid(), true);
QCOMPARE(rexp.isRegExp(), true);
QCOMPARE(rexp.isObject(), true);
QVERIFY(rexp.isCallable()); // in JSC, RegExp objects are callable
// prototype should be RegExp.prototype
QCOMPARE(rexp.prototype().isValid(), true);
QCOMPARE(rexp.prototype().isObject(), true);
QCOMPARE(rexp.prototype().isRegExp(), false);
QCOMPARE(rexp.prototype().strictlyEquals(eng.evaluate("RegExp.prototype")), true);

QCOMPARE(rexp.toRegExp().pattern(), QRegExp("foo").pattern());
}
QCOMPARE(qjsvalue_cast<QRegExp>(rexp).pattern(), QRegExp("foo").pattern());
}

void tst_QJSEngine::jsRegExp()
Expand Down Expand Up @@ -2746,7 +2740,7 @@ void tst_QJSEngine::valueConversion_regExp()
QRegExp in = QRegExp("foo");
QJSValue val = eng.toScriptValue(in);
QVERIFY(val.isRegExp());
QRegExp out = val.toRegExp();
QRegExp out = qjsvalue_cast<QRegExp>(val);
QEXPECT_FAIL("", "QTBUG-6136: JSC-based back-end doesn't preserve QRegExp::patternSyntax (always uses RegExp2)", Continue);
QCOMPARE(out.patternSyntax(), in.patternSyntax());
QCOMPARE(out.pattern(), in.pattern());
Expand All @@ -2757,15 +2751,15 @@ void tst_QJSEngine::valueConversion_regExp()
QRegExp in = QRegExp("foo", Qt::CaseSensitive, QRegExp::RegExp2);
QJSValue val = eng.toScriptValue(in);
QVERIFY(val.isRegExp());
QCOMPARE(val.toRegExp(), in);
QCOMPARE(qjsvalue_cast<QRegExp>(val), in);
}
{
QRegExp in = QRegExp("foo");
in.setMinimal(true);
QJSValue val = eng.toScriptValue(in);
QVERIFY(val.isRegExp());
QEXPECT_FAIL("", "QTBUG-6136: JSC-based back-end doesn't preserve QRegExp::minimal (always false)", Continue);
QCOMPARE(val.toRegExp().isMinimal(), in.isMinimal());
QCOMPARE(qjsvalue_cast<QRegExp>(val).isMinimal(), in.isMinimal());
}
}

Expand Down Expand Up @@ -5032,10 +5026,10 @@ void tst_QJSEngine::reentrancy_objectCreation()
QCOMPARE(d2.toDateTime(), d1.toDateTime());
}
{
QJSValue r1 = eng1.newRegExp("foo", "gim");
QJSValue r2 = eng2.newRegExp("foo", "gim");
QCOMPARE(r1.toRegExp(), r2.toRegExp());
QCOMPARE(r2.toRegExp(), r1.toRegExp());
QJSValue r1 = eng1.evaluate("new RegExp('foo', 'gim')");
QJSValue r2 = eng2.evaluate("new RegExp('foo', 'gim')");
QCOMPARE(qjsvalue_cast<QRegExp>(r1), qjsvalue_cast<QRegExp>(r2));
QCOMPARE(qjsvalue_cast<QRegExp>(r2), qjsvalue_cast<QRegExp>(r1));
}
{
QJSValue o1 = eng1.newQObject(this);
Expand Down Expand Up @@ -6004,7 +5998,7 @@ void tst_QJSEngine::qRegExpInport()

QJSEngine eng;
QJSValue rexp;
rexp = eng.newRegExp(rx);
rexp = eng.toScriptValue(rx);

QCOMPARE(rexp.isValid(), true);
QCOMPARE(rexp.isRegExp(), true);
Expand Down
23 changes: 12 additions & 11 deletions tests/auto/declarative/qjsvalue/tst_qjsvalue.cpp
View file
Expand Up @@ -1061,7 +1061,8 @@ void tst_QJSValue::toVariant()

{
QRegExp rx = QRegExp("[0-9a-z]+", Qt::CaseSensitive, QRegExp::RegExp2);
QJSValue rxObject = eng.newRegExp(rx);
QJSValue rxObject = eng.toScriptValue(rx);
QVERIFY(rxObject.isRegExp());
QVariant var = rxObject.toVariant();
QCOMPARE(var, QVariant(rx));
}
Expand Down Expand Up @@ -1215,30 +1216,30 @@ void tst_QJSValue::toRegExp()
{
QJSEngine eng;
{
QRegExp rx = eng.evaluate("/foo/").toRegExp();
QRegExp rx = qjsvalue_cast<QRegExp>(eng.evaluate("/foo/"));
QVERIFY(rx.isValid());
QCOMPARE(rx.patternSyntax(), QRegExp::RegExp2);
QCOMPARE(rx.pattern(), QString::fromLatin1("foo"));
QCOMPARE(rx.caseSensitivity(), Qt::CaseSensitive);
QVERIFY(!rx.isMinimal());
}
{
QRegExp rx = eng.evaluate("/bar/gi").toRegExp();
QRegExp rx = qjsvalue_cast<QRegExp>(eng.evaluate("/bar/gi"));
QVERIFY(rx.isValid());
QCOMPARE(rx.patternSyntax(), QRegExp::RegExp2);
QCOMPARE(rx.pattern(), QString::fromLatin1("bar"));
QCOMPARE(rx.caseSensitivity(), Qt::CaseInsensitive);
QVERIFY(!rx.isMinimal());
}

QVERIFY(eng.evaluate("[]").toRegExp().isEmpty());
QVERIFY(eng.evaluate("{}").toRegExp().isEmpty());
QVERIFY(eng.globalObject().toRegExp().isEmpty());
QVERIFY(QJSValue().toRegExp().isEmpty());
QVERIFY(QJSValue(123).toRegExp().isEmpty());
QVERIFY(QJSValue(false).toRegExp().isEmpty());
QVERIFY(eng.nullValue().toRegExp().isEmpty());
QVERIFY(eng.undefinedValue().toRegExp().isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(eng.evaluate("[]")).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(eng.evaluate("{}")).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(eng.globalObject()).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(QJSValue()).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(QJSValue(123)).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(QJSValue(false)).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(eng.nullValue()).isEmpty());
QVERIFY(qjsvalue_cast<QRegExp>(eng.undefinedValue()).isEmpty());
}

void tst_QJSValue::instanceOf_twoEngines()
Expand Down

0 comments on commit 3a017cc

Please sign in to comment.