qmllint: Properly process qmldir imports and dependencies

We don't want to pick the dependencies from the qmltypes files. Rather,
parse them directly from the qmldir. Do process versions, too.

Also, import explicitly given qmltypes files only once, and don't expose
QML types from dependencies as actual types. Hide them behind an
inaccessible prefix.

For the inaccessible prefix to work, we need to import the C++ class
names without the prefix. The prefix doesn't make sense for C++ names
anyway.

In addition, properly process version-less imports.

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

7 files changed, 112 insertions, 105 deletions

diff --git a/tests/auto/qml/qmllint/data/Things/plugins.qmltypes b/tests/auto/qml/qmllint/data/Things/plugins.qmltypes
index 448a966145..8b57cadb61 100644
--- a/tests/auto/qml/qmllint/data/Things/plugins.qmltypes
+++ b/tests/auto/qml/qmllint/data/Things/plugins.qmltypes
@@ -55,4 +55,11 @@ Module {
         isCreatable: false
         Property { name: "pixelDensity"; type: "double"; isReadonly: true }
     }
+    Component {
+        name: "ItemDerived"
+        prototype: "QQuickItem"
+        exports: [
+            "Things/ItemDerived 1.0"
+        ]
+    }
 }
diff --git a/tests/auto/qml/qmllint/data/Things/qmldir b/tests/auto/qml/qmllint/data/Things/qmldir
index c53af3a340..d91d4afc92 100644
--- a/tests/auto/qml/qmllint/data/Things/qmldir
+++ b/tests/auto/qml/qmllint/data/Things/qmldir
@@ -1,3 +1,5 @@
 module Things
 Something 1.0 SomethingElse.qml
 plugin doesNotExistPlugin
+depends QtQuick 2.0
+import QtQml
diff --git a/tests/auto/qml/qmllint/data/qmldirImportAndDepend/bad.qml b/tests/auto/qml/qmllint/data/qmldirImportAndDepend/bad.qml
new file mode 100644
index 0000000000..ef80aa54d6
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/qmldirImportAndDepend/bad.qml
@@ -0,0 +1,5 @@
+import Things
+
+Item {
+    objectName: "We cannot instantiate Item as it is not imported"
+}
diff --git a/tests/auto/qml/qmllint/data/qmldirImportAndDepend/good.qml b/tests/auto/qml/qmllint/data/qmldirImportAndDepend/good.qml
new file mode 100644
index 0000000000..275b531845
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/qmldirImportAndDepend/good.qml
@@ -0,0 +1,10 @@
+import Things
+
+QtObject {
+    objectName: "QtQml was imported from Things/qmldir"
+
+    ItemDerived {
+        objectName: "QQuickItem is depended upon and we know its properties"
+        x: 4
+    }
+}
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index d5912def6b..6d472842ca 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -183,6 +183,10 @@ void TestQmllint::dirtyQmlCode_data()
             << QStringLiteral("Cycle1.qml")
             << QString("Warning: Cycle2 is part of an inheritance cycle: Cycle2 -> Cycle3 -> Cycle1 -> Cycle2")
             << QString();
+    QTest::newRow("badQmldirImportAndDepend")
+            << QStringLiteral("qmldirImportAndDepend/bad.qml")
+            << QString("warning: Item was not found. Did you add all import paths?")
+            << QString();
 }
 
 void TestQmllint::dirtyQmlCode()
@@ -224,6 +228,7 @@ void TestQmllint::cleanQmlCode_data()
     QTest::newRow("EnumAccess2") << QStringLiteral("EnumAccess2.qml");
     QTest::newRow("ListProperty") << QStringLiteral("ListProperty.qml");
     QTest::newRow("AttachedType") << QStringLiteral("AttachedType.qml");
+    QTest::newRow("qmldirImportAndDepend") << QStringLiteral("qmldirImportAndDepend/good.qml");
 }
 
 void TestQmllint::cleanQmlCode()
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp
index 50123d8957..25dccfa046 100644
--- a/tools/qmllint/findwarnings.cpp
+++ b/tools/qmllint/findwarnings.cpp
@@ -36,7 +36,6 @@
 #include <QtQml/private/qqmljslexer_p.h>
 #include <QtQml/private/qqmljsparser_p.h>
 #include <QtQml/private/qv4codegen_p.h>
-#include <QtQml/private/qqmldirparser_p.h>
 #include <QtQml/private/qqmlimportresolver_p.h>
 
 #include <QtCore/qfile.h>
@@ -113,7 +112,7 @@ static const QLatin1String SlashQmldir             = QLatin1String("/qmldir");
 static const QLatin1String SlashPluginsDotQmltypes = QLatin1String("/plugins.qmltypes");
 
 void FindWarningVisitor::readQmltypes(const QString &filename,
-                                      QHash<QString, ScopeTree::ConstPtr> *objects, QStringList *dependencies)
+                                      QHash<QString, ScopeTree::ConstPtr> *objects)
 {
     const QFileInfo fileInfo(filename);
     if (!fileInfo.exists()) {
@@ -131,7 +130,8 @@ void FindWarningVisitor::readQmltypes(const QString &filename,
     QFile file(filename);
     file.open(QFile::ReadOnly);
     TypeDescriptionReader reader { filename, file.readAll() };
-    auto succ = reader(objects, dependencies);
+    QStringList dependencies;
+    auto succ = reader(objects, &dependencies);
     if (!succ)
         m_colorOut.writeUncolored(reader.errorMessage());
 }
@@ -140,9 +140,8 @@ FindWarningVisitor::Import FindWarningVisitor::readQmldir(const QString &path)
 {
     Import result;
     auto reader = createQmldirParserForFile(path + SlashQmldir);
-    const auto imports = reader.imports();
-    for (const auto &import : imports)
-        result.dependencies.append(import.module); // TODO: version
+    result.imports.append(reader.imports());
+    result.dependencies.append(reader.dependencies().values());
 
     QHash<QString, ScopeTree::Ptr> qmlComponents;
     const auto components = reader.components();
@@ -168,33 +167,29 @@ FindWarningVisitor::Import FindWarningVisitor::readQmldir(const QString &path)
         result.objects.insert( it.key(), ScopeTree::ConstPtr(it.value()));
 
     if (!reader.plugins().isEmpty() && QFile::exists(path + SlashPluginsDotQmltypes))
-        readQmltypes(path + SlashPluginsDotQmltypes, &result.objects, &result.dependencies);
+        readQmltypes(path + SlashPluginsDotQmltypes, &result.objects);
 
     return result;
 }
 
-void FindWarningVisitor::processImport(const QString &prefix, const FindWarningVisitor::Import &import)
+void FindWarningVisitor::processImport(
+        const QString &prefix, const FindWarningVisitor::Import &import, QTypeRevision version)
 {
-    for (auto const &dependency : qAsConst(import.dependencies)) {
-        auto const split = dependency.split(" ");
-        auto const &id = split.at(0);
-        if (split.length() > 1) {
-            const auto version = split.at(1).split('.');
-            importHelper(id, QString(), QTypeRevision::fromVersion(
-                             version.at(0).toInt(),
-                             version.length() > 1 ? version.at(1).toInt() : -1));
-        } else {
-            importHelper(id, QString(), QTypeRevision());
-        }
-
+    // Import the dependencies with an invalid prefix. The prefix will never be matched by actual
+    // QML code but the C++ types will be visible.
+    const QString invalidPrefix = QString::fromLatin1("$dependency$");
+    for (auto const &dependency : qAsConst(import.dependencies))
+        importHelper(dependency.typeName, invalidPrefix, dependency.version);
 
+    for (auto const &import : qAsConst(import.imports)) {
+        importHelper(import.module, prefix,
+                     import.isAutoImport ? version : import.version);
     }
 
     // add objects
     for (auto it = import.objects.begin(); it != import.objects.end(); ++it) {
         const auto &val = it.value();
-        m_types[it.key()] = val;
-        m_exportedName2Scope.insert(prefixedName(prefix, val->className()), val);
+        m_exportedName2Scope.insert(val->className(), val);
 
         const auto exports = val->exports();
         for (const auto &valExport : exports)
@@ -206,8 +201,45 @@ void FindWarningVisitor::processImport(const QString &prefix, const FindWarningV
     }
 }
 
+void FindWarningVisitor::importBareQmlTypes()
+{
+    for (auto const &dir : m_qmltypesDirs) {
+        Import result;
+        QDirIterator it { dir, QStringList() << QLatin1String("builtins.qmltypes"), QDir::NoFilter,
+                          QDirIterator::Subdirectories };
+        while (it.hasNext())
+            readQmltypes(it.next(), &result.objects);
+        processImport("", result, QTypeRevision());
+    }
+
+    if (m_qmltypesFiles.isEmpty()) {
+        for (auto const &qmltypesPath : m_qmltypesDirs) {
+            if (QFile::exists(qmltypesPath + SlashQmldir))
+                continue;
+            Import result;
+            QDirIterator it {
+                qmltypesPath, QStringList { QLatin1String("*.qmltypes") }, QDir::Files };
+
+            while (it.hasNext()) {
+                const QString name = it.next();
+                if (!name.endsWith(QLatin1String("/builtins.qmltypes")))
+                    readQmltypes(name, &result.objects);
+            }
+
+            processImport("", result, QTypeRevision());
+        }
+    } else {
+        Import result;
+
+        for (const auto &qmltypeFile : m_qmltypesFiles)
+            readQmltypes(qmltypeFile, &result.objects);
+
+        processImport("", result, QTypeRevision());
+    }
+}
+
 void FindWarningVisitor::importHelper(const QString &module, const QString &prefix,
-                                            QTypeRevision version)
+                                      QTypeRevision version)
 {
     const QString id = QString(module).replace(QLatin1Char('/'), QLatin1Char('.'));
     QPair<QString, QString> importId { id, prefix };
@@ -215,38 +247,12 @@ void FindWarningVisitor::importHelper(const QString &module, const QString &pref
         return;
     m_alreadySeenImports.insert(importId);
 
-    auto qmltypesPaths = qQmlResolveImportPaths(id, m_qmltypeDirs, version) + m_qmltypeDirs;
-
+    const auto qmltypesPaths = qQmlResolveImportPaths(id, m_qmltypesDirs, version);
     for (auto const &qmltypesPath : qmltypesPaths) {
         if (QFile::exists(qmltypesPath + SlashQmldir)) {
-            processImport(prefix, readQmldir(qmltypesPath));
-
-            // break so that we don't import unversioned qml components
-            // in addition to versioned ones
+            processImport(prefix, readQmldir(qmltypesPath), version);
             break;
         }
-
-        if (!m_qmltypeFiles.isEmpty())
-            continue;
-
-        Import result;
-
-        QDirIterator it { qmltypesPath, QStringList() << QLatin1String("*.qmltypes"), QDir::Files };
-
-        while (it.hasNext())
-            readQmltypes(it.next(), &result.objects, &result.dependencies);
-
-        processImport(prefix, result);
-    }
-
-    if (!m_qmltypeFiles.isEmpty())
-    {
-        Import result;
-
-        for (const auto &qmltypeFile : m_qmltypeFiles)
-            readQmltypes(qmltypeFile, &result.objects, &result.dependencies);
-
-        processImport("", result);
     }
 }
 
@@ -376,36 +382,8 @@ void FindWarningVisitor::throwRecursionDepthError()
 bool FindWarningVisitor::visit(QQmlJS::AST::UiProgram *)
 {
     enterEnvironment(ScopeType::QMLScope, "program");
-    QHash<QString, ScopeTree::ConstPtr> objects;
-    QStringList dependencies;
-    for (auto const &dir : m_qmltypeDirs) {
-        QDirIterator it { dir, QStringList() << QLatin1String("builtins.qmltypes"), QDir::NoFilter,
-                          QDirIterator::Subdirectories };
-        while (it.hasNext()) {
-            readQmltypes(it.next(), &objects, &dependencies);
-        }
-    }
+    importBareQmlTypes();
 
-    if (!m_qmltypeFiles.isEmpty())
-    {
-        for (const auto &qmltypeFile : m_qmltypeFiles) {
-            readQmltypes(qmltypeFile, &objects, &dependencies);
-        }
-    }
-
-    // add builtins
-    for (auto objectIt = objects.begin(); objectIt != objects.end(); ++objectIt) {
-        auto val = objectIt.value();
-        m_types[objectIt.key()] = val;
-
-        const auto exports = val->exports();
-        for (const auto &valExport : exports)
-            m_exportedName2Scope.insert(valExport.type(), val);
-
-        const auto enums = val->enums();
-        for (const auto &valEnum : enums)
-            m_currentScope->addEnum(valEnum);
-    }
     // add "self" (as we only ever check the first part of a qualified identifier, we get away with
     // using an empty ScopeTree
     m_exportedName2Scope.insert(QFileInfo { m_filePath }.baseName(), {});
@@ -612,12 +590,12 @@ bool FindWarningVisitor::visit(QQmlJS::AST::IdentifierExpression *idexp)
     return true;
 }
 
-FindWarningVisitor::FindWarningVisitor(QStringList qmltypeDirs, QStringList qmltypeFiles, QString code,
-                                                   QString fileName, bool silent, bool warnUnqualified,
-                                                   bool warnWithStatement, bool warnInheritanceCycle)
+FindWarningVisitor::FindWarningVisitor(
+        QStringList qmltypeDirs, QStringList qmltypesFiles, QString code, QString fileName,
+        bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle)
     : m_rootScope(ScopeTree::create(ScopeType::JSFunctionScope, "global")),
-      m_qmltypeDirs(std::move(qmltypeDirs)),
-      m_qmltypeFiles(std::move(qmltypeFiles)),
+      m_qmltypesDirs(std::move(qmltypeDirs)),
+      m_qmltypesFiles(std::move(qmltypesFiles)),
       m_code(std::move(code)),
       m_rootId(QLatin1String("<id>")),
       m_filePath(std::move(fileName)),
@@ -751,17 +729,15 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiImport *import)
         const QString importId = import->importId.toString();
         m_qmlid2scope.insert(importId, m_exportedName2Scope.value(importId));
     }
-    if (import->version) {
-        auto uri = import->importUri;
-        while (uri) {
-            path.append(uri->name);
-            path.append("/");
-            uri = uri->next;
-        }
-        path.chop(1);
-
-        importHelper(path, prefix, import->version->version);
+    auto uri = import->importUri;
+    while (uri) {
+        path.append(uri->name);
+        path.append("/");
+        uri = uri->next;
     }
+    path.chop(1);
+
+    importHelper(path, prefix, import->version ? import->version->version : QTypeRevision());
     return true;
 }
 
diff --git a/tools/qmllint/findwarnings.h b/tools/qmllint/findwarnings.h
index ee019b440d..87423db589 100644
--- a/tools/qmllint/findwarnings.h
+++ b/tools/qmllint/findwarnings.h
@@ -43,6 +43,7 @@
 #include "scopetree.h"
 #include "qcoloroutput.h"
 
+#include <QtQml/private/qqmldirparser_p.h>
 #include <QtQml/private/qqmljsastvisitor_p.h>
 #include <QtQml/private/qqmljsast_p.h>
 
@@ -52,25 +53,25 @@ class FindWarningVisitor : public QQmlJS::AST::Visitor
 {
     Q_DISABLE_COPY_MOVE(FindWarningVisitor)
 public:
-    explicit FindWarningVisitor(QStringList qmltypeDirs, QStringList qmltypeFiles, QString code,
-                                      QString fileName, bool silent, bool warnUnqualified,
-                                      bool warnWithStatement, bool warnInheritanceCycle);
+    explicit FindWarningVisitor(
+            QStringList qmltypeDirs, QStringList qmltypesFiles, QString code, QString fileName,
+            bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle);
     ~FindWarningVisitor() override = default;
     bool check();
 
 private:
     struct Import {
         QHash<QString, ScopeTree::ConstPtr> objects;
-        QStringList dependencies;
+        QList<QQmlDirParser::Import> imports;
+        QList<QQmlDirParser::Component> dependencies;
     };
 
     ScopeTree::Ptr m_rootScope;
     ScopeTree::Ptr m_currentScope;
     QQmlJS::AST::ExpressionNode *m_fieldMemberBase = nullptr;
-    QHash<QString, ScopeTree::ConstPtr> m_types;
     QHash<QString, ScopeTree::ConstPtr> m_exportedName2Scope;
-    QStringList m_qmltypeDirs;
-    QStringList m_qmltypeFiles;
+    QStringList m_qmltypesDirs;
+    QStringList m_qmltypesFiles;
     QString m_code;
     QHash<QString, ScopeTree::ConstPtr> m_qmlid2scope;
     QString m_rootId;
@@ -95,13 +96,14 @@ private:
 
     void enterEnvironment(ScopeType type, const QString &name);
     void leaveEnvironment();
+
+    void importBareQmlTypes();
     void importHelper(const QString &module, const QString &prefix = QString(),
                       QTypeRevision version = QTypeRevision());
 
-    void readQmltypes(const QString &filename, QHash<QString, ScopeTree::ConstPtr> *objects,
-                      QStringList *dependencies);
+    void readQmltypes(const QString &filename, QHash<QString, ScopeTree::ConstPtr> *objects);
     Import readQmldir(const QString &dirname);
-    void processImport(const QString &prefix, const Import &import);
+    void processImport(const QString &prefix, const Import &import, QTypeRevision version);
 
     ScopeTree::Ptr localFile2ScopeTree(const QString &filePath);