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
Pick-to: 6.7
Change-Id: I6b1860f594a3060653e732b9570cc3bbff0c34a9
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 51 insertions, 13 deletions

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index 8574c96bc1..c6b2434146 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;
 }
@@ -1355,12 +1358,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()) {
@@ -1368,25 +1386,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 31dcea25b7..ea06a11006 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -1413,6 +1413,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()
@@ -1424,7 +1428,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);