QQmlComponent::loadFromModule: Improve error handling

Most importantly, fix the (attempted) loading of a non-existent inline
component: As soon as we have the compilation unit, we can simply check
whether the inline component exists via its name.
This avoids an assert down the line in beginCreate which assumes that at
this point the inlineComponentName has been properly vetted.
This also allows us to create a better error message, and to catch the
easy mistake of passing a file name instead of a type name to
createComponent.

Moreover, ensure that we do not claim that the inline component is ready
when only the outer component has been loaded – a direct connection to
the changed signals followed by a call to create would otherwise yield
surprising results.

Fixes: QTBUG-124225
Change-Id: I6b1860f594a3060653e732b9570cc3bbff0c34a9
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 59bdfc6c5240632c1778c1937e08e6c9be282746)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 51 insertions, 13 deletions

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index b895af085d..08365e8ec0 100644
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -29,6 +29,9 @@
 #include <QtCore/qloggingcategory.h>
 #include <qqmlinfo.h>
 
+
+using namespace Qt::Literals::StringLiterals;
+
 namespace {
     Q_CONSTINIT thread_local int creationDepth = 0;
 }
@@ -1354,12 +1357,27 @@ void QQmlComponentPrivate::completeLoadFromModule(QAnyStringView uri, QAnyString
 {
     Q_Q(QQmlComponent);
 
+    // we always mimic the progressChanged behavior from loadUrl
     auto reportError = [&](QString msg) {
         QQmlError error;
         error.setDescription(msg);
         state.errors.push_back(std::move(error));
+        progress = 1;
+        emit q->progressChanged(1);
         emit q->statusChanged(q->Error);
     };
+    auto emitProgressReset = [&](){
+        if (progress != 0) {
+            progress = 0;
+            emit q->progressChanged(0);
+        }
+    };
+    auto emitComplete = [&]() {
+        progress = 1;
+        emit q->progressChanged(1);
+        emit q->statusChanged(q->status());
+    };
+    emitProgressReset();
     if (moduleStatus == LoadHelper::ResolveTypeResult::NoSuchModule) {
         reportError(QLatin1String(R"(No module named "%1" found)").arg(uri.toString()));
     } else if (!type.isValid()) {
@@ -1367,25 +1385,37 @@ void QQmlComponentPrivate::completeLoadFromModule(QAnyStringView uri, QAnyString
                     .arg(uri.toString(), typeName.toString()));
     } else if (type.isCreatable()) {
         clear();
-        // mimic the progressChanged behavior from loadUrl
-        if (progress != 0) {
-            progress = 0;
-            emit q->progressChanged(0);
-        }
         loadedType = type;
-        progress = 1;
-        emit q->progressChanged(1);
-        emit q->statusChanged(q->status());
-
+        emitComplete();
     } else if (type.isComposite()) {
+        // loadUrl takes care of signal emission
         loadUrl(type.sourceUrl(), mode);
     } else if (type.isInlineComponentType()) {
         auto baseUrl = type.sourceUrl();
         baseUrl.setFragment(QString());
-        loadUrl(baseUrl, mode);
-        if (!q->isError()) {
-            inlineComponentName = std::make_unique<QString>(type.elementName());
-            Q_ASSERT(!inlineComponentName->isEmpty());
+        {
+            // we don't want to emit status changes from the "helper" loadUrl below
+            // because it would signal success to early
+            QSignalBlocker blockSignals(q);
+            // we really need to continue in a synchronous way, otherwise we can't check the CU
+            loadUrl(baseUrl, QQmlComponent::PreferSynchronous);
+        }
+        if (q->isError()) {
+            emitComplete();
+            return;
+        }
+        QString elementName = type.elementName();
+        if (compilationUnit->inlineComponentId(elementName) == -1) {
+            QString realTypeName = typeName.toString();
+            realTypeName.truncate(realTypeName.indexOf(u'.'));
+            QString errorMessage = R"(Type "%1" from module "%2" contains no inline component named "%3".)"_L1.arg(
+                    realTypeName, uri.toString(), elementName);
+            if (elementName == u"qml")
+                errorMessage += " To load the type \"%1\", drop the \".qml\" extension."_L1.arg(realTypeName);
+            reportError(std::move(errorMessage));
+        } else {
+            inlineComponentName = std::make_unique<QString>(std::move(elementName));
+            emitComplete();
         }
     } else if (type.isSingleton() || type.isCompositeSingleton()) {
         reportError(QLatin1String(R"(%1 is a singleton, and cannot be loaded)").arg(typeName.toString()));
diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
index b186e3190a..e1c8a6e6d7 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -1412,6 +1412,10 @@ void tst_qqmlcomponent::loadFromModuleFailures_data()
     QTest::addRow("CppSingleton") << u"QtQuick"_s
                                   << u"Application"_s
                                   << u"Application is a singleton, and cannot be loaded"_s;
+    QTest::addRow("passedFileName") << "plainqml"
+                                    << "Plain.qml"
+                                    << R"(Type "Plain" from module "plainqml" contains no inline component named "qml". )"
+                                       R"(To load the type "Plain", drop the ".qml" extension.)";
 }
 
 void tst_qqmlcomponent::loadFromModuleFailures()
@@ -1423,7 +1427,11 @@ void tst_qqmlcomponent::loadFromModuleFailures()
     QQmlEngine engine;
     QQmlComponent component(&engine);
     QSignalSpy errorSpy(&component, &QQmlComponent::statusChanged);
+    QSignalSpy progressSpy(&component, &QQmlComponent::progressChanged);
     component.loadFromModule(uri, typeName);
+    // verify that we changed the progress correctly to 1
+    QTRY_VERIFY(!progressSpy.isEmpty());
+    QTRY_COMPARE(progressSpy.last().at(0).toDouble(), 1.0);
     QVERIFY(!errorSpy.isEmpty());
     QCOMPARE(errorSpy.first().first().value<QQmlComponent::Status>(),
              QQmlComponent::Error);