QML: Fix precedence of module imports

Types imported transitively via a qmldir import statement should not
shadow types available from the module itself.

Change-Id: Id34edc5c5e2fff4ba37009f4bab9039b7ed18dff
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

12 files changed, 114 insertions, 39 deletions

diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp
index b1d304924d..275c53cfda 100644
--- a/src/qml/qml/qqmlimport.cpp
+++ b/src/qml/qml/qqmlimport.cpp
@@ -242,12 +242,12 @@ public:
 
     bool addLibraryImport(const QString& uri, const QString &prefix,
                           QTypeRevision version, const QString &qmldirIdentifier,
-                          const QString &qmldirUrl, bool incomplete,
+                          const QString &qmldirUrl, uint flags,
                           QQmlImportDatabase *database,
                           QList<QQmlError> *errors);
 
     bool addFileImport(const QString &uri, const QString &prefix, QTypeRevision version,
-                       bool isImplicitImport, bool incomplete, QQmlImportDatabase *database,
+                       uint flags, QQmlImportDatabase *database,
                        QList<QQmlError> *errors);
 
     bool updateQmldirContent(const QString &uri, const QString &prefix,
@@ -294,7 +294,7 @@ public:
     QQmlImportInstance *addImportToNamespace(QQmlImportNamespace *nameSpace, const QString &uri,
                                              const QString &url, QTypeRevision version,
                                              QV4::CompiledData::Import::ImportType type,
-                                             QList<QQmlError> *errors, bool lowPrecedence = false);
+                                             QList<QQmlError> *errors, uint flags);
 
     bool populatePluginPairVector(QVector<StaticPluginPair> &result, const QString &uri, const QStringList &versionUris,
                                      const QString &qmldirPath, QList<QQmlError> *errors);
@@ -1513,7 +1513,7 @@ QQmlImportNamespace *QQmlImportsPrivate::importNamespace(const QString &prefix)
 
 QQmlImportInstance *QQmlImportsPrivate::addImportToNamespace(
         QQmlImportNamespace *nameSpace, const QString &uri, const QString &url, QTypeRevision version,
-        QV4::CompiledData::Import::ImportType type, QList<QQmlError> *errors, bool lowPrecedence)
+        QV4::CompiledData::Import::ImportType type, QList<QQmlError> *errors, uint flags)
 {
     Q_ASSERT(nameSpace);
     Q_ASSERT(errors);
@@ -1526,18 +1526,32 @@ QQmlImportInstance *QQmlImportsPrivate::addImportToNamespace(
     import->localDirectoryPath = QQmlFile::urlToLocalFileOrQrc(url);
     import->version = version;
     import->isLibrary = (type == QV4::CompiledData::Import::ImportLibrary);
-
-    if (lowPrecedence)
+    if (flags & QQmlImports::ImportImplicit) {
+        import->implicitlyImported = true;
         nameSpace->imports.append(import);
-    else
+    } else if (flags & QQmlImports::ImportLowPrecedence) {
+        if (nameSpace->imports.isEmpty()) {
+            nameSpace->imports.append(import);
+        } else {
+            for (auto it = nameSpace->imports.rbegin(), end = nameSpace->imports.rend();
+                 it != end; ++it) {
+
+                if (!(*it)->implicitlyImported) {
+                    nameSpace->imports.insert(it.base(), import);
+                    break;
+                }
+            }
+        }
+    } else {
         nameSpace->imports.prepend(import);
+    }
 
     return import;
 }
 
 bool QQmlImportsPrivate::addLibraryImport(
         const QString& uri, const QString &prefix, QTypeRevision version,
-        const QString &qmldirIdentifier, const QString &qmldirUrl, bool incomplete,
+        const QString &qmldirIdentifier, const QString &qmldirUrl, uint flags,
         QQmlImportDatabase *database, QList<QQmlError> *errors)
 {
     Q_ASSERT(database);
@@ -1548,10 +1562,11 @@ bool QQmlImportsPrivate::addLibraryImport(
 
     QQmlImportInstance *inserted = addImportToNamespace(
                 nameSpace, uri, qmldirUrl, version,
-                QV4::CompiledData::Import::ImportLibrary, errors);
+                QV4::CompiledData::Import::ImportLibrary, errors,
+                flags);
     Q_ASSERT(inserted);
 
-    if (!incomplete) {
+    if (!(flags & QQmlImports::ImportIncomplete)) {
         QQmlTypeLoaderQmldirContent qmldir;
 
         if (!qmldirIdentifier.isEmpty()) {
@@ -1596,8 +1611,8 @@ bool QQmlImportsPrivate::addLibraryImport(
     return true;
 }
 
-bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix, QTypeRevision version,
-                                       bool isImplicitImport, bool incomplete,
+bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix,
+                                       QTypeRevision version, uint flags,
                                        QQmlImportDatabase *database, QList<QQmlError> *errors)
 {
     Q_ASSERT(errors);
@@ -1622,7 +1637,7 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix
 
         const QString dir = localFileOrQrc.left(localFileOrQrc.lastIndexOf(Slash) + 1);
         if (!typeLoader->directoryExists(dir)) {
-            if (!isImplicitImport) {
+            if (!(flags & QQmlImports::ImportImplicit)) {
                 QQmlError error;
                 error.setDescription(QQmlImportDatabase::tr("\"%1\": no such directory").arg(uri));
                 error.setUrl(QUrl(qmldirUrl));
@@ -1640,9 +1655,9 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix
         if (!typeLoader->absoluteFilePath(localFileOrQrc).isEmpty())
             qmldirIdentifier = localFileOrQrc;
 
-    } else if (nameSpace->prefix.isEmpty() && !incomplete) {
+    } else if (nameSpace->prefix.isEmpty() && !(flags & QQmlImports::ImportIncomplete)) {
 
-        if (!isImplicitImport) {
+        if (!(flags & QQmlImports::ImportImplicit)) {
             QQmlError error;
             error.setDescription(QQmlImportDatabase::tr("import \"%1\" has no qmldir and no namespace").arg(importUri));
             error.setUrl(QUrl(qmldirUrl));
@@ -1662,7 +1677,7 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix
     //     if the implicit import has already been explicitly added, otherwise we can run into issues
     //     with duplicate imports. However remember that we attempted to add this as implicit import, to
     //     allow for the loading of internal types.
-    if (isImplicitImport) {
+    if (flags & QQmlImports::ImportImplicit) {
         for (QList<QQmlImportInstance *>::const_iterator it = nameSpace->imports.constBegin();
              it != nameSpace->imports.constEnd(); ++it) {
             if ((*it)->url == url) {
@@ -1674,10 +1689,12 @@ bool QQmlImportsPrivate::addFileImport(const QString& uri, const QString &prefix
 
     QQmlImportInstance *inserted = addImportToNamespace(
                 nameSpace, importUri, url, version, QV4::CompiledData::Import::ImportFile,
-                errors, isImplicitImport);
+                errors, flags);
     Q_ASSERT(inserted);
+    if (flags & QQmlImports::ImportImplicit)
+        inserted->implicitlyImported = true;
 
-    if (!incomplete && !qmldirIdentifier.isEmpty()) {
+    if (!(flags & QQmlImports::ImportIncomplete) && !qmldirIdentifier.isEmpty()) {
         QQmlTypeLoaderQmldirContent qmldir;
         if (!getQmldirContent(qmldirIdentifier, importUri, &qmldir, errors))
             return false;
@@ -1760,8 +1777,8 @@ bool QQmlImports::addImplicitImport(QQmlImportDatabase *importDb, QList<QQmlErro
         qDebug().nospace() << "QQmlImports(" << qPrintable(baseUrl().toString())
                            << ")::addImplicitImport";
 
-    bool incomplete = !isLocal(baseUrl());
-    return d->addFileImport(QLatin1String("."), QString(), QTypeRevision(), true, incomplete,
+    uint flags = ImportImplicit | (!isLocal(baseUrl()) ? ImportIncomplete : 0);
+    return d->addFileImport(QLatin1String("."), QString(), QTypeRevision(), flags,
                             importDb, errors);
 }
 
@@ -1803,7 +1820,7 @@ bool QQmlImports::addInlineComponentImport(QQmlImportInstance *const importInsta
 */
 bool QQmlImports::addFileImport(QQmlImportDatabase *importDb,
                                 const QString& uri, const QString& prefix, QTypeRevision version,
-                                bool incomplete, QList<QQmlError> *errors)
+                                uint flags, QList<QQmlError> *errors)
 {
     Q_ASSERT(importDb);
     Q_ASSERT(errors);
@@ -1812,12 +1829,13 @@ bool QQmlImports::addFileImport(QQmlImportDatabase *importDb,
         qDebug().nospace() << "QQmlImports(" << qPrintable(baseUrl().toString()) << ')' << "::addFileImport: "
                            << uri << ' ' << version << " as " << prefix;
 
-    return d->addFileImport(uri, prefix, version, false, incomplete, importDb, errors);
+    return d->addFileImport(uri, prefix, version, flags, importDb, errors);
 }
 
 bool QQmlImports::addLibraryImport(QQmlImportDatabase *importDb,
                                    const QString &uri, const QString &prefix, QTypeRevision version,
-                                   const QString &qmldirIdentifier, const QString& qmldirUrl, bool incomplete, QList<QQmlError> *errors)
+                                   const QString &qmldirIdentifier, const QString& qmldirUrl,
+                                   uint flags, QList<QQmlError> *errors)
 {
     Q_ASSERT(importDb);
     Q_ASSERT(errors);
@@ -1826,7 +1844,8 @@ bool QQmlImports::addLibraryImport(QQmlImportDatabase *importDb,
         qDebug().nospace() << "QQmlImports(" << qPrintable(baseUrl().toString()) << ')' << "::addLibraryImport: "
                            << uri << ' ' << version << " as " << prefix;
 
-    return d->addLibraryImport(uri, prefix, version, qmldirIdentifier, qmldirUrl, incomplete, importDb, errors);
+    return d->addLibraryImport(uri, prefix, version, qmldirIdentifier, qmldirUrl, flags,
+                               importDb, errors);
 }
 
 bool QQmlImports::updateQmldirContent(QQmlImportDatabase *importDb,
diff --git a/src/qml/qml/qqmlimport_p.h b/src/qml/qml/qqmlimport_p.h
index 5f16880199..b512ed2bae 100644
--- a/src/qml/qml/qqmlimport_p.h
+++ b/src/qml/qml/qqmlimport_p.h
@@ -134,6 +134,7 @@ class Q_QML_PRIVATE_EXPORT QQmlImports
 {
 public:
     enum ImportVersion { FullyVersioned, PartiallyVersioned, Unversioned };
+    enum ImportFlag { ImportIncomplete = 0x1, ImportLowPrecedence = 0x2, ImportImplicit = 0x4 };
 
     QQmlImports(QQmlTypeLoader *);
     QQmlImports(const QQmlImports &);
@@ -162,11 +163,12 @@ public:
 
     bool addFileImport(QQmlImportDatabase *,
                        const QString& uri, const QString& prefix, QTypeRevision version,
-                       bool incomplete, QList<QQmlError> *errors);
+                       uint flags, QList<QQmlError> *errors);
 
     bool addLibraryImport(QQmlImportDatabase *importDb,
                           const QString &uri, const QString &prefix, QTypeRevision version,
-                          const QString &qmldirIdentifier, const QString &qmldirUrl, bool incomplete, QList<QQmlError> *errors);
+                          const QString &qmldirIdentifier, const QString &qmldirUrl, uint flags,
+                          QList<QQmlError> *errors);
 
     bool updateQmldirContent(QQmlImportDatabase *importDb,
                              const QString &uri, const QString &prefix,
diff --git a/src/qml/qml/qqmlscriptblob.cpp b/src/qml/qml/qqmlscriptblob.cpp
index 4ca506a926..7b7a842d04 100644
--- a/src/qml/qml/qqmlscriptblob.cpp
+++ b/src/qml/qml/qqmlscriptblob.cpp
@@ -234,7 +234,7 @@ void QQmlScriptBlob::initializeFromCompilationUnit(const QQmlRefPointer<QV4::Exe
         QList<QQmlError> errors;
         for (quint32 i = 0, count = script->importCount(); i < count; ++i) {
             const QV4::CompiledData::Import *import = script->importAt(i);
-            if (!addImport(import, &errors)) {
+            if (!addImport(import, {}, &errors)) {
                 Q_ASSERT(errors.size());
                 QQmlError error(errors.takeFirst());
                 error.setUrl(m_importCache.baseUrl());
diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp
index 324d5a863f..32f5bca8d9 100644
--- a/src/qml/qml/qqmltypedata.cpp
+++ b/src/qml/qml/qqmltypedata.cpp
@@ -199,7 +199,7 @@ bool QQmlTypeData::tryLoadFromDiskCache()
     for (int i = 0, count = m_compiledData->importCount(); i < count; ++i) {
         const QV4::CompiledData::Import *import = m_compiledData->importAt(i);
         QList<QQmlError> errors;
-        if (!addImport(import, &errors)) {
+        if (!addImport(import, {}, &errors)) {
             Q_ASSERT(errors.size());
             QQmlError error(errors.takeFirst());
             error.setUrl(m_importCache.baseUrl());
@@ -699,7 +699,7 @@ void QQmlTypeData::continueLoadFromIR()
     QList<QQmlError> errors;
 
     for (const QV4::CompiledData::Import *import : qAsConst(m_document->imports)) {
-        if (!addImport(import, &errors)) {
+        if (!addImport(import, {}, &errors)) {
             Q_ASSERT(errors.size());
             QQmlError error(errors.takeFirst());
             error.setUrl(m_importCache.baseUrl());
diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 1b7993af61..ff0633a475 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -556,12 +556,14 @@ bool QQmlTypeLoader::Blob::updateQmldir(const QQmlRefPointer<QQmlQmldirData> &da
     return true;
 }
 
-bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QList<QQmlError> *errors)
+bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, uint flags,
+                                     QList<QQmlError> *errors)
 {
-    return addImport(std::make_shared<PendingImport>(this, import), errors);
+    return addImport(std::make_shared<PendingImport>(this, import), flags, errors);
 }
 
-bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr import, QList<QQmlError> *errors)
+bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr import, uint flags,
+                                     QList<QQmlError> *errors)
 {
     Q_ASSERT(errors);
 
@@ -584,7 +586,7 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
             // This is a local library import
             if (!m_importCache.addLibraryImport(
                         importDatabase, import->uri, import->qualifier,
-                        import->version, qmldirFilePath, qmldirUrl, false, errors))
+                        import->version, qmldirFilePath, qmldirUrl, flags, errors))
                 return false;
 
             if (!loadImportDependencies(import, qmldirFilePath, errors))
@@ -618,7 +620,7 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
 
             if (!m_importCache.addLibraryImport(
                         importDatabase, import->uri, import->qualifier, import->version,
-                        QString(), QString(), false, errors)) {
+                        QString(), QString(), flags, errors)) {
                 return false;
             }
         } else {
@@ -638,7 +640,7 @@ bool QQmlTypeLoader::Blob::addImport(QQmlTypeLoader::Blob::PendingImportPtr impo
                 // Add this library and request the possible locations for it
                 if (!m_importCache.addLibraryImport(
                             importDatabase, import->uri, import->qualifier, import->version,
-                            QString(), QString(), true, errors))
+                            QString(), QString(), flags | QQmlImports::ImportIncomplete, errors))
                     return false;
 
                 // Probe for all possible locations
@@ -721,11 +723,19 @@ bool QQmlTypeLoader::Blob::loadImportDependencies(PendingImportPtr currentImport
         dependencyImport->uri = implicitImport;
         dependencyImport->qualifier = currentImport->qualifier;
         dependencyImport->version = currentImport->version;
-        if (!addImport(dependencyImport, errors))
+        if (!addImport(dependencyImport, QQmlImports::ImportLowPrecedence, errors)) {
+            QQmlError error;
+            error.setDescription(
+                        QString::fromLatin1(
+                            "Failed to load dependencies for module \"%1\" version %2.%3")
+                        .arg(currentImport->uri)
+                        .arg(currentImport->version.majorVersion())
+                        .arg(currentImport->version.minorVersion()));
+            errors->append(error);
             return false;
+        }
     }
 
-
     return true;
 }
 
diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h
index 094360a51b..eb6e549911 100644
--- a/src/qml/qml/qqmltypeloader_p.h
+++ b/src/qml/qml/qqmltypeloader_p.h
@@ -109,8 +109,9 @@ public:
         using PendingImportPtr = std::shared_ptr<PendingImport>;
 
     protected:
-        bool addImport(const QV4::CompiledData::Import *import, QList<QQmlError> *errors);
-        bool addImport(PendingImportPtr import, QList<QQmlError> *errors);
+        bool addImport(const QV4::CompiledData::Import *import, uint flags,
+                       QList<QQmlError> *errors);
+        bool addImport(PendingImportPtr import, uint flags, QList<QQmlError> *errors);
 
         bool fetchQmldir(const QUrl &url, PendingImportPtr import, int priority, QList<QQmlError> *errors);
         bool updateQmldir(const QQmlRefPointer<QQmlQmldirData> &data, PendingImportPtr import, QList<QQmlError> *errors);
diff --git a/tests/auto/qml/qqmlimport/data/A/Child.qml b/tests/auto/qml/qqmlimport/data/A/Child.qml
new file mode 100644
index 0000000000..d1713ba804
--- /dev/null
+++ b/tests/auto/qml/qqmlimport/data/A/Child.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    objectName: "a"
+}
diff --git a/tests/auto/qml/qqmlimport/data/A/qmldir b/tests/auto/qml/qqmlimport/data/A/qmldir
new file mode 100644
index 0000000000..7488570377
--- /dev/null
+++ b/tests/auto/qml/qqmlimport/data/A/qmldir
@@ -0,0 +1,3 @@
+module A
+Child 1.0 Child.qml
+import B
diff --git a/tests/auto/qml/qqmlimport/data/B/Child.qml b/tests/auto/qml/qqmlimport/data/B/Child.qml
new file mode 100644
index 0000000000..587c996b7e
--- /dev/null
+++ b/tests/auto/qml/qqmlimport/data/B/Child.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    objectName: "b"
+}
diff --git a/tests/auto/qml/qqmlimport/data/B/qmldir b/tests/auto/qml/qqmlimport/data/B/qmldir
new file mode 100644
index 0000000000..595583a2a3
--- /dev/null
+++ b/tests/auto/qml/qqmlimport/data/B/qmldir
@@ -0,0 +1,3 @@
+module B
+Child 1.0 Child.qml
+ChildB 1.0 Child.qml
diff --git a/tests/auto/qml/qqmlimport/data/dependencies.qml b/tests/auto/qml/qqmlimport/data/dependencies.qml
new file mode 100644
index 0000000000..e6cf51e467
--- /dev/null
+++ b/tests/auto/qml/qqmlimport/data/dependencies.qml
@@ -0,0 +1,12 @@
+import QtQml
+import A
+
+QtObject {
+    // Type Child imported from A takes precedence over type Child indirectly imported from B.
+    property QtObject childA: Child {}
+    property string a: childA.objectName
+
+    // ChildB only exists in B.
+    property QtObject childB: ChildB {}
+    property string b: childB.objectName
+}
diff --git a/tests/auto/qml/qqmlimport/tst_qqmlimport.cpp b/tests/auto/qml/qqmlimport/tst_qqmlimport.cpp
index 316c4cb168..ac21a2fac6 100644
--- a/tests/auto/qml/qqmlimport/tst_qqmlimport.cpp
+++ b/tests/auto/qml/qqmlimport/tst_qqmlimport.cpp
@@ -52,6 +52,7 @@ private slots:
     void partialImportVersions_data();
     void partialImportVersions();
     void registerModuleImport();
+    void importDependenciesPrecedence();
     void cleanup();
     void envResourceImportPath();
 };
@@ -388,6 +389,20 @@ void tst_QQmlImport::registerModuleImport()
     }
 }
 
+void tst_QQmlImport::importDependenciesPrecedence()
+{
+    QQmlEngine engine;
+    engine.addImportPath(dataDirectory());
+
+    QQmlComponent component(&engine, testFile("dependencies.qml"));
+    QVERIFY(component.isReady());
+
+    QScopedPointer<QObject> instance(component.create());
+    QVERIFY(!instance.isNull());
+    QCOMPARE(instance->property("a").toString(), QString::fromLatin1("a"));
+    QCOMPARE(instance->property("b").toString(), QString::fromLatin1("b"));
+}
+
 
 QTEST_MAIN(tst_QQmlImport)