qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not store instantiation errors in QQmlComponent 

QQmlComponent::createWithInitialProperties() allows to pass non-existent
properties to create the object with. In this case, we still return a
valid pointer (pre-existing), however, the state of the component is
affected: there are errors internally that fail repeated calls even with
valid properties specified. Fix this, aligning to the behavior of
QQmlComponent::createObject() which is a QML counterpart of the same
function (to a degree)

Additionally, make required properties warnings also not break the
component state: if a required property is not set, we return nullptr
instead of object. The error state is affected but it gets cleaned
on the next creation call, allowing one to set the missing required
properties

Unify the logic (with slight deviations) between create() (the
non-incubator-accepting version), createWithInitialProperties() and
createObject() as all they all should do roughly equivalent things

[ChangeLog][QQmlComponent][Important Behavior Changes] Setting properties
via createWithInitialProperties() and setInitialProperties() no longer
affects the QQmlComponent's error state unless there are unset required
properties. A warning is issued when a property could not be set.

Fixes: QTBUG-101439
Change-Id: I814c00bc3715d7a54004c3fcd0af6ee6d50b0726
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Andrei Golubev 2022-06-14 10:35:42 +02:00
parent d968b23671
commit 79f9e22ef7
7 changed files with 171 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

158
src/qml/qml/qqmlcomponent.cpp
View File

@ -303,7 +303,8 @@ void QQmlComponentPrivate::fromTypeData(const QQmlRefPointer<QQmlTypeData> &data
if (!compilationUnit) {
Q_ASSERT(data->isError());
state.errors = data->errors();
state.errors.clear();
state.appendErrors(data->errors());
}
}
@ -359,7 +360,7 @@ bool QQmlComponentPrivate::setInitialProperty(
segment = scope.engine->newString(properties.last());
object->put(segment, scope.engine->metaTypeToJS(value.metaType(), value.constData()));
if (scope.engine->hasException) {
state.errors.push_back(scope.engine->catchExceptionAsQmlError());
qmlWarning(base, scope.engine->catchExceptionAsQmlError());
scope.engine->hasException = false;
return false;
}
@ -380,7 +381,7 @@ bool QQmlComponentPrivate::setInitialProperty(
"%2 does not have a property called %1")
.arg(name, QQmlMetaType::prettyTypeName(base)));
}
state.errors.push_back(error);
qmlWarning(base, error);
return false;
}
@ -408,8 +409,8 @@ QQmlComponent::~QQmlComponent()
if (isError()) {
qWarning() << "This may have been caused by one of the following errors:";
for (const QQmlError &error : qAsConst(d->state.errors))
qWarning().nospace().noquote() << QLatin1String(" ") << error;
for (const QQmlComponentPrivate::AnnotatedQmlError &e : qAsConst(d->state.errors))
qWarning().nospace().noquote() << QLatin1String(" ") << e.error;
}
// we might not have the creator anymore if the engine is gone
@ -710,7 +711,7 @@ void QQmlComponentPrivate::loadUrl(const QUrl &newUrl, QQmlComponent::Compilatio
if (newUrl.isEmpty()) {
QQmlError error;
error.setDescription(QQmlComponent::tr("Invalid empty URL"));
state.errors << error;
state.errors.emplaceBack(error);
return;
}
@ -746,10 +747,11 @@ void QQmlComponentPrivate::loadUrl(const QUrl &newUrl, QQmlComponent::Compilatio
QList<QQmlError> QQmlComponent::errors() const
{
Q_D(const QQmlComponent);
if (isError())
return d->state.errors;
else
return QList<QQmlError>();
QList<QQmlError> errors;
errors.reserve(d->state.errors.size());
for (const QQmlComponentPrivate::AnnotatedQmlError &annotated : d->state.errors)
errors.emplaceBack(annotated.error);
return errors;
}
/*!
@ -773,7 +775,8 @@ QString QQmlComponent::errorString() const
QString ret;
if(!isError())
return ret;
for (const QQmlError &e : d->state.errors) {
for (const QQmlComponentPrivate::AnnotatedQmlError &annotated : d->state.errors) {
const QQmlError &e = annotated.error;
ret += e.url().toString() + QLatin1Char(':') +
QString::number(e.line()) + QLatin1Char(' ') +
e.description() + QLatin1Char('\n');
@ -824,23 +827,7 @@ QQmlComponent::QQmlComponent(QQmlComponentPrivate &dd, QObject *parent)
QObject *QQmlComponent::create(QQmlContext *context)
{
Q_D(QQmlComponent);
QObject *rv = d->doBeginCreate(this, context);
if (rv) {
completeCreate();
} else if (d->state.completePending) {
// overridden completCreate might assume that
// the object has actually been created
++creationDepth;
QQmlEnginePrivate *ep = QQmlEnginePrivate::get(d->engine);
d->complete(ep, &d->state);
--creationDepth;
}
if (rv && !d->requiredProperties().empty()) {
delete rv;
return nullptr;
}
return rv;
return d->createWithProperties(nullptr, QVariantMap {}, context);
}
/*!
@ -851,9 +838,9 @@ QObject *QQmlComponent::create(QQmlContext *context)
TODO: also mention errorString() when QTBUG-93239 is fixed
\endomit
If any of the \c initialProperties cannot be set, \l isError() will return
\c true, and the \l errors() function can be used to
get detailed information about the error(s).
If any of the \a initialProperties cannot be set, a warning is issued. If
there are unset required properties, the object creation fails and returns
\c nullptr, in which case \l isError() will return \c true.
\sa QQmlComponent::create
\since 5.14
@ -861,16 +848,47 @@ QObject *QQmlComponent::create(QQmlContext *context)
QObject *QQmlComponent::createWithInitialProperties(const QVariantMap& initialProperties, QQmlContext *context)
{
Q_D(QQmlComponent);
return d->createWithProperties(nullptr, initialProperties, context);
}
QObject *rv = d->doBeginCreate(this, context);
if (rv) {
setInitialProperties(rv, initialProperties);
completeCreate();
}
if (!d->requiredProperties().empty()) {
d->requiredProperties().clear();
static void QQmlComponent_setQmlParent(QObject *me, QObject *parent); // forward declaration
/*! \internal
*/
QObject *QQmlComponentPrivate::createWithProperties(QObject *parent, const QVariantMap &properties,
QQmlContext *context, CreateBehavior behavior)
{
Q_Q(QQmlComponent);
QObject *rv = doBeginCreate(q, context);
if (!rv) {
if (state.completePending) {
// overridden completCreate might assume that
// the object has actually been created
++creationDepth;
QQmlEnginePrivate *ep = QQmlEnginePrivate::get(engine);
complete(ep, &state);
--creationDepth;
}
return nullptr;
}
QQmlComponent_setQmlParent(rv, parent); // internally checks if parent is nullptr
q->setInitialProperties(rv, properties);
q->completeCreate();
if (!requiredProperties().empty()) {
if (behavior == CreateWarnAboutRequiredProperties) {
const RequiredProperties &unsetRequiredProperties = requiredProperties();
for (const auto &unsetRequiredProperty : unsetRequiredProperties) {
const QQmlError error = unsetRequiredPropertyToQQmlError(unsetRequiredProperty);
qmlWarning(rv, error);
}
}
delete rv;
rv = nullptr;
}
return rv;
}
@ -915,7 +933,7 @@ QObject *QQmlComponentPrivate::beginCreate(QQmlRefPointer<QQmlContextData> conte
auto cleanup = qScopeGuard([this] {
if (!state.errors.isEmpty() && lcQmlComponentGeneral().isDebugEnabled()) {
for (const auto &e : qAsConst(state.errors)) {
qCDebug(lcQmlComponentGeneral) << "QQmlComponent: " << e.toString();
qCDebug(lcQmlComponentGeneral) << "QQmlComponent: " << e.error.toString();
}
}
});
@ -939,6 +957,16 @@ QObject *QQmlComponentPrivate::beginCreate(QQmlRefPointer<QQmlContextData> conte
return nullptr;
}
// filter out temporary errors as they do not really affect component's
// state (they are not part of the document compilation)
state.errors.erase(std::remove_if(state.errors.begin(), state.errors.end(),
[](const QQmlComponentPrivate::AnnotatedQmlError &e) {
return e.isTransient;
}),
state.errors.end());
if (state.creator)
requiredProperties().clear();
if (!q->isReady()) {
qWarning("QQmlComponent: Component is not ready");
return nullptr;
@ -962,7 +990,7 @@ QObject *QQmlComponentPrivate::beginCreate(QQmlRefPointer<QQmlContextData> conte
state.creator.reset(new QQmlObjectCreator(std::move(context), compilationUnit, creationContext));
rv = state.creator->create(start);
if (!rv)
state.errors = state.creator->errors;
state.appendErrors(state.creator->errors);
enginePriv->dereferenceScarceResources();
if (rv) {
@ -997,7 +1025,7 @@ void QQmlComponentPrivate::beginDeferred(QQmlEnginePrivate *enginePriv,
QQmlRefPointer<QQmlContextData>()));
if (!state.creator->populateDeferredProperties(object, deferredData))
state.errors << state.creator->errors;
state.appendErrors(state.creator->errors);
deferredData->bindings.clear();
deferredState->push_back(std::move(state));
@ -1103,7 +1131,7 @@ void QQmlComponentPrivate::completeCreate()
const RequiredProperties& unsetRequiredProperties = requiredProperties();
for (const auto& unsetRequiredProperty: unsetRequiredProperties) {
QQmlError error = unsetRequiredPropertyToQQmlError(unsetRequiredProperty);
state.errors.push_back(error);
state.errors.push_back(QQmlComponentPrivate::AnnotatedQmlError { error, true });
}
if (state.completePending) {
++creationDepth;
@ -1545,46 +1573,14 @@ QObject *QQmlComponent::createObject(QObject *parent, const QVariantMap &propert
{
Q_D(QQmlComponent);
Q_ASSERT(d->engine);
QQmlContext *ctxt = creationContext();
if (!ctxt)
ctxt = d->engine->rootContext();
QObject *rv = beginCreate(ctxt);
if (!rv)
return nullptr;
Q_ASSERT(d->state.errors.isEmpty()); // otherwise beginCreate() would return nullptr
QQmlComponent_setQmlParent(rv, parent);
if (!properties.isEmpty()) {
setInitialProperties(rv, properties);
if (!d->state.errors.isEmpty()) {
qmlWarning(rv, d->state.errors);
d->state.errors.clear();
}
QObject *rv = d->createWithProperties(parent, properties, creationContext(),
QQmlComponentPrivate::CreateWarnAboutRequiredProperties);
if (rv) {
QQmlData *qmlData = QQmlData::get(rv);
Q_ASSERT(qmlData);
qmlData->explicitIndestructibleSet = false;
qmlData->indestructible = false;
}
if (!d->requiredProperties().empty()) {
QList<QQmlError> errors;
for (const auto &requiredProperty: qAsConst(d->requiredProperties())) {
errors.push_back(QQmlComponentPrivate::unsetRequiredPropertyToQQmlError(
requiredProperty));
}
if (!errors.isEmpty())
qmlWarning(rv, errors);
delete rv;
return nullptr;
}
d->completeCreate();
QQmlData *qmlData = QQmlData::get(rv);
Q_ASSERT(qmlData);
qmlData->explicitIndestructibleSet = false;
qmlData->indestructible = false;
return rv;
}

30
src/qml/qml/qqmlcomponent_p.h
View File

@ -75,10 +75,31 @@ public:
bool hadTopLevelRequiredProperties() const;
QQmlRefPointer<QV4::ExecutableCompilationUnit> compilationUnit;
struct AnnotatedQmlError
{
AnnotatedQmlError() = default;
AnnotatedQmlError(const QQmlError &error) // convenience ctor
: error(error)
{
}
AnnotatedQmlError(const QQmlError &error, bool transient)
: error(error), isTransient(transient)
{
}
QQmlError error;
bool isTransient = false; // tells if the error is temporary (e.g. unset required property)
};
struct ConstructionState {
std::unique_ptr<QQmlObjectCreator> creator;
QList<QQmlError> errors;
QList<AnnotatedQmlError> errors;
bool completePending = false;
void appendErrors(const QList<QQmlError> &qmlErrors)
{
for (const QQmlError &e : qmlErrors)
errors.emplaceBack(e);
}
};
ConstructionState state;
@ -103,6 +124,13 @@ public:
QObject *doBeginCreate(QQmlComponent *q, QQmlContext *context);
bool setInitialProperty(QObject *component, const QString &name, const QVariant& value);
enum CreateBehavior {
CreateDefault,
CreateWarnAboutRequiredProperties,
};
QObject *createWithProperties(QObject *parent, const QVariantMap &properties,
QQmlContext *context, CreateBehavior behavior = CreateDefault);
bool isBound() const {
return compilationUnit->unitData()->flags & QV4::CompiledData::Unit::ComponentsBound;
}

2
src/qml/types/qqmlbind.cpp
View File

@ -885,7 +885,7 @@ void QQmlBindPrivate::buildBindEntries(QQmlBind *q, QQmlComponentPrivate::Deferr
if (constructionState.creator.get()) {
++ep->inProgressCreations;
constructionState.creator->finalizePopulateDeferred();
constructionState.errors << constructionState.creator->errors;
constructionState.appendErrors(constructionState.creator->errors);
deferredState->push_back(std::move(constructionState));
}
} else {

2
src/quicktemplates2/qquickdeferredexecute.cpp
View File

@ -55,7 +55,7 @@ static bool beginDeferred(QQmlEnginePrivate *enginePriv, const QQmlProperty &pro
for (const QV4::CompiledData::Binding *binding : reversedBindings)
state.creator->populateDeferredBinding(property, deferData->deferredIdx, binding);
state.creator->finalizePopulateDeferred();
state.errors << state.creator->errors;
state.appendErrors(state.creator->errors);
deferredState->push_back(std::move(state));

13
tests/auto/qml/qqmlcomponent/data/createObjectWithScript.qml
View File

@ -8,6 +8,9 @@ Item{
property QtObject bindingTestObject : null
property QtObject bindingThisTestObject : null
property QtObject badRequired: null
property QtObject goodRequired: null
Component{
id: a
Rectangle {
@ -33,11 +36,21 @@ Item{
}
}
Component {
id: d
Item {
required property int i
}
}
Component.onCompleted: {
root.declarativerectangle = a.createObject(root, {"x":17,"y":17, "color":"white", "border.width":3, "innerRect.border.width": 20});
root.declarativeitem = b.createObject(root, {"x":17,"y":17,"testBool":true,"testInt":17,"testObject":root});
root.bindingTestObject = c.createObject(root, {'testValue': Qt.binding(function(){return width * 3}) }) // use root.width
root.bindingThisTestObject = c.createObject(root, {'testValue': Qt.binding(function(){return this.width * 3}) }) // use width of Item within 'c'
root.badRequired = d.createObject(root, { "not_i": 42 });
root.goodRequired = d.createObject(root, { "i": 42 });
}
}

49
tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
View File

@ -265,6 +265,16 @@ void tst_qqmlcomponent::qmlCreateObjectAutoParent()
void tst_qqmlcomponent::qmlCreateObjectWithProperties()
{
QQmlEngine engine;
QTest::ignoreMessage(
QtMsgType::QtWarningMsg,
QRegularExpression(".*createObjectWithScript.qml: Setting initial properties failed: "
"Item does not have a property called not_i"));
QTest::ignoreMessage(
QtMsgType::QtWarningMsg,
QRegularExpression(
".*createObjectWithScript.qml:42:13: Required property i was not initialized"));
QQmlComponent component(&engine, testFileUrl("createObjectWithScript.qml"));
QVERIFY2(component.errorString().isEmpty(), component.errorString().toUtf8());
QScopedPointer<QObject> object(component.create());
@ -313,6 +323,16 @@ void tst_qqmlcomponent::qmlCreateObjectWithProperties()
testBindingThisObj->setProperty("width", 200);
QCOMPARE(testBindingThisObj->property("testValue").value<int>(), 200 * 3);
}
{
QScopedPointer<QObject> badRequired(object->property("badRequired").value<QObject *>());
QVERIFY(!badRequired);
QScopedPointer<QObject> goodRequired(object->property("goodRequired").value<QObject *>());
QVERIFY(goodRequired);
QCOMPARE(goodRequired->parent(), object.data());
QCOMPARE(goodRequired->property("i").value<int>(), 42);
}
}
void tst_qqmlcomponent::qmlCreateObjectClean()
@ -1025,11 +1045,38 @@ void tst_qqmlcomponent::testSetInitialProperties()
// createWithInitialProperties: setting a nonexistent property
QQmlComponent comp(&eng);
comp.loadUrl(testFileUrl("allJSONTypes.qml"));
const QRegularExpression errorMessage { QStringLiteral(
".*allJSONTypes.qml: Setting initial properties failed: Item does not have a "
"property called notThePropertiesYoureLookingFor") };
QTest::ignoreMessage(QtMsgType::QtWarningMsg, errorMessage);
QScopedPointer<QObject> obj {
comp.createWithInitialProperties(QVariantMap { {"notThePropertiesYoureLookingFor", 42} })
};
QVERIFY(obj);
QVERIFY(comp.errorString().contains("Setting initial properties failed: Item does not have a property called notThePropertiesYoureLookingFor"));
QVERIFY(comp.isReady()); // despite the error, the component is still ready
// QTBUG-101439: repeated creation succeeds as well
QScopedPointer<QObject> objEmpty { comp.create() };
QVERIFY(objEmpty);
}
{
QQmlComponent comp(&eng);
comp.loadUrl(testFileUrl("requiredNotSet.qml"));
QTest::ignoreMessage(
QtMsgType::QtWarningMsg,
QRegularExpression(".*requiredNotSet.qml: Setting initial properties failed: Item "
"does not have a property called not_i"));
QScopedPointer<QObject> obj { comp.createWithInitialProperties(
QVariantMap { { QLatin1String("not_i"), QJsonValue { 42 } } }) };
QVERIFY(!obj);
QVERIFY(comp.isError());
QVERIFY(comp.errorString().contains("Required property i was not initialized"));
QScopedPointer<QObject> objGood { comp.createWithInitialProperties(
QVariantMap { { QLatin1String("i"), QJsonValue { 42 } } }) };
QVERIFY2(objGood, qPrintable(comp.errorString()));
QCOMPARE(objGood->property("i"), 42);
}
// QJSValue unpacking - QTBUG-101440

3
tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
View File

@ -4418,6 +4418,7 @@ void tst_qqmllanguage::deepProperty()
void tst_qqmllanguage::groupAssignmentFailure()
{
auto ep = std::make_unique<QQmlEngine>();
QTest::failOnWarning("QQmlComponent: Component destroyed while completion pending");
QTest::ignoreMessage(QtMsgType::QtWarningMsg, QRegularExpression(".*Invalid property assignment: url expected - Assigning null to incompatible properties in QML is deprecated. This will become a compile error in future versions of Qt..*"));
QQmlComponent component(ep.get(), testFileUrl("groupFailure.qml"));
QScopedPointer<QObject> o(component.create());
@ -5038,7 +5039,7 @@ static void beginDeferredOnce(QQmlEnginePrivate *enginePriv,
for (const QV4::CompiledData::Binding *binding: reversedBindings)
state.creator->populateDeferredBinding(property, deferData->deferredIdx, binding);
state.creator->finalizePopulateDeferred();
state.errors << state.creator->errors;
state.appendErrors(state.creator->errors);
deferredState->push_back(std::move(state));