qmllint: Fix several type checking issues

- Fixes the issue of QQmlComponent properties showing a type mismatch that isn't there
- Fixes duplicate scopes with the same internal name not being recognized as type matches
- Does not warn about incomplete / unresolvable types being incompatible anymore

Task-number: QTBUG-92571
Change-Id: Ia4399c44b87fee6614f2b8c95cb28fcaeec780b4
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

9 files changed, 125 insertions, 36 deletions

diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp
index c63ff89cc2..61553405c2 100644
--- a/src/qmlcompiler/qqmljsimportvisitor.cpp
+++ b/src/qmlcompiler/qqmljsimportvisitor.cpp
@@ -65,25 +65,6 @@ inline QString getScopeName(const QQmlJSScope::ConstPtr &scope, QQmlJSScope::Sco
     return scope->baseTypeName();
 }
 
-/*!
-  \internal
-  Checks whether \a derived type can be assigned to \a base type. Returns \c
-  true if the type hierarchy of \a derived contains a type equal to \a base.
-
-  \note Assigning \a derived to "QVariant" or "QJSValue" is always possible and
-  the function returns \c true in this case.
-*/
-static bool canAssign(const QQmlJSScope *derived, const QQmlJSScope *base)
-{
-    if (!base)
-        return false;
-    for (; derived; derived = derived->baseType().get()) {
-        if (derived == base)
-            return true;
-    }
-    return base->internalName() == u"QVariant"_qs || base->internalName() == u"QJSValue"_qs;
-}
-
 QQmlJSImportVisitor::QQmlJSImportVisitor(
         QQmlJSImporter *importer, const QString &implicitImportDirectory,
         const QStringList &qmltypesFiles, const QString &fileName, const QString &code, bool silent)
@@ -856,14 +837,28 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
     // on ending the visit to UiObjectBinding, set the property type to the
     // just-visited one if the property exists and this type is valid
     QQmlJSMetaProperty property = m_currentScope->property(group->name.toString());
-    if (property.isValid() && canAssign(childScope.get(), property.type().get())) {
+    if (property.isValid() && !property.type().isNull() && property.type()->canAssign(childScope)) {
         property.setType(childScope);
         property.setTypeName(getScopeName(childScope, QQmlJSScope::QMLScope));
         m_currentScope->addOwnProperty(property);
+    } else if (!m_currentScope->isFullyResolved()) {
+        // If the current scope is not fully resolved we cannot tell whether the property exists or
+        // not (we already warn elsewhere)
     } else if (!property.isValid()) {
         m_logger.log(QStringLiteral("Property \"%1\" is invalid or does not exist")
                              .arg(group->name.toString()),
                      Log_Property, group->firstSourceLocation());
+    } else if (property.type().isNull() || !property.type()->isFullyResolved()) {
+        // Property type is not fully resolved we cannot tell any more than this
+        m_logger.log(
+                QStringLiteral(
+                        "Property \"%1\" has incomplete type \"%2\". You may be missing an import.")
+                        .arg(group->name.toString())
+                        .arg(property.typeName()),
+                Log_Property, group->firstSourceLocation());
+    } else if (!childScope->isFullyResolved()) {
+        // If the childScope type is not fully resolved we cannot tell whether the type is
+        // incompatible (we already warn elsewhere)
     } else {
         // the type is incompatible
         m_logger.log(
diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp
index 15c3109b61..aea955e7c1 100644
--- a/src/qmlcompiler/qqmljsscope.cpp
+++ b/src/qmlcompiler/qqmljsscope.cpp
@@ -373,6 +373,20 @@ bool QQmlJSScope::isPropertyLocallyRequired(const QString &name) const
     return m_requiredPropertyNames.contains(name);
 }
 
+bool QQmlJSScope::isFullyResolved() const
+{
+    bool baseResolved = true;
+    searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *scope) {
+        if (!scope->isResolved()) {
+            baseResolved = false;
+            return true;
+        }
+        return false;
+    });
+
+    return baseResolved;
+}
+
 QQmlJSScope::Export::Export(QString package, QString type, const QTypeRevision &version) :
     m_package(std::move(package)),
     m_type(std::move(type)),
@@ -394,4 +408,27 @@ QQmlJSScope QDeferredFactory<QQmlJSScope>::create() const
     return std::move(*result);
 }
 
+bool QQmlJSScope::canAssign(const QQmlJSScope::ConstPtr &derived) const
+{
+    if (!derived)
+        return false;
+
+    bool isBaseComponent = false;
+    for (auto scope = this; scope; scope = scope->baseType().get()) {
+        if (internalName() == u"QQmlComponent"_qs) {
+            isBaseComponent = true;
+            break;
+        }
+    }
+
+    for (auto scope = derived; !scope.isNull(); scope = scope->baseType()) {
+        if (isSameType(scope))
+            return true;
+        if (isBaseComponent && scope->internalName() == u"QObject"_qs)
+            return true;
+    }
+
+    return internalName() == u"QVariant"_qs || internalName() == u"QJSValue"_qs;
+}
+
 QT_END_NAMESPACE
diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h
index 80129296fe..3a8188a007 100644
--- a/src/qmlcompiler/qqmljsscope_p.h
+++ b/src/qmlcompiler/qqmljsscope_p.h
@@ -222,6 +222,9 @@ public:
     bool isPropertyRequired(const QString &name) const;
     bool isPropertyLocallyRequired(const QString &name) const;
 
+    bool isResolved() const { return m_baseTypeName.isEmpty() || !m_baseType.isNull(); }
+    bool isFullyResolved() const;
+
     QString defaultPropertyName() const { return m_defaultPropertyName; }
     void setDefaultPropertyName(const QString &name) { m_defaultPropertyName = name; }
 
@@ -292,6 +295,31 @@ public:
         return {};
     }
 
+    /*!
+      \internal
+      Checks whether \a otherScope is the same type as this.
+
+      In addition to checking whether the scopes are identical, we also cover duplicate scopes with
+      the same internal name.
+    */
+    bool isSameType(const QQmlJSScope::ConstPtr &otherScope) const
+    {
+        return this == otherScope.get()
+                || (!this->internalName().isEmpty()
+                    && this->internalName() == otherScope->internalName());
+    }
+
+    /*!
+      \internal
+      Checks whether \a derived type can be assigned to this type. Returns \c
+      true if the type hierarchy of \a derived contains a type equal to this.
+
+      \note Assigning \a derived to "QVariant" or "QJSValue" is always possible and
+      the function returns \c true in this case. In addition any "QObject" based \a derived type
+      can be assigned to a this type if that type is derived from "QQmlComponent".
+    */
+    bool canAssign(const QQmlJSScope::ConstPtr &derived) const;
+
 private:
     QQmlJSScope(ScopeType type, const QQmlJSScope::Ptr &parentScope = QQmlJSScope::Ptr());
 
diff --git a/tests/auto/qml/qmllint/data/defaultPropertyListModel.qml b/tests/auto/qml/qmllint/data/defaultPropertyListModel.qml
new file mode 100644
index 0000000000..f0bac0fdf7
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/defaultPropertyListModel.qml
@@ -0,0 +1,6 @@
+import QtQuick 2.15
+import QtQml.Models 2.15
+
+Item {
+        ListModel {}
+}
diff --git a/tests/auto/qml/qmllint/data/defaultPropertyWithWrongType2.qml b/tests/auto/qml/qmllint/data/defaultPropertyVar.qml
index 53850ce066..7aac1a9a75 100644
--- a/tests/auto/qml/qmllint/data/defaultPropertyWithWrongType2.qml
+++ b/tests/auto/qml/qmllint/data/defaultPropertyVar.qml
@@ -2,5 +2,6 @@ import QtQml 2.0
 
 QtObject {
     default property var child
+
     QtObject {}
 }
diff --git a/tests/auto/qml/qmllint/data/propertyDelegate.qml b/tests/auto/qml/qmllint/data/propertyDelegate.qml
new file mode 100644
index 0000000000..aa4d2c35c8
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/propertyDelegate.qml
@@ -0,0 +1,5 @@
+import QtQuick 2.15
+
+Repeater {
+    delegate: Item {}
+}
diff --git a/tests/auto/qml/qmllint/data/unresolvedType.qml b/tests/auto/qml/qmllint/data/unresolvedType.qml
new file mode 100644
index 0000000000..63282bc262
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/unresolvedType.qml
@@ -0,0 +1,14 @@
+import QtQuick 2.0
+
+Item {
+    UnresolvedType {
+        unresolvedProperty: 5
+        Item {}
+        Item {}
+    }
+    Repeater {
+        model: 10
+        delegate: UnresolvedType {}
+    }
+    property UnresolvedType foo: Item {}
+}
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index 45cf03ff86..4647464a3b 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -518,11 +518,6 @@ void TestQmllint::dirtyQmlCode_data()
             << QStringLiteral("Cannot assign to default property of incompatible type")
             << QStringLiteral("Cannot assign to non-existent default property")
             << false;
-    QTest::newRow("DefaultPropertyWithWrongType(var)")
-            << QStringLiteral("defaultPropertyWithWrongType2.qml")
-            << QStringLiteral("Cannot assign to default property of incompatible type")
-            << QStringLiteral("Cannot assign to non-existent default property")
-            << false;
     QTest::newRow("InvalidImport")
             << QStringLiteral("invalidImport.qml")
             << QStringLiteral("Failed to import FooBar. Are your include paths set up properly?")
@@ -592,6 +587,10 @@ void TestQmllint::dirtyQmlCode_data()
             << QStringLiteral("String contains unescaped line terminator which is deprecated. Use "
                               "a template literal instead.")
             << QString() << true;
+    QTest::newRow("unresolvedType")
+            << QStringLiteral("unresolvedType.qml")
+            << QStringLiteral("UnresolvedType was not found. Did you add all import paths?")
+            << QStringLiteral("incompatible type") << false;
 }
 
 void TestQmllint::dirtyQmlCode()
@@ -690,6 +689,9 @@ void TestQmllint::cleanQmlCode_data()
     QTest::newRow("defaultPropertyList") << QStringLiteral("defaultPropertyList.qml");
     QTest::newRow("defaultPropertyComponent") << QStringLiteral("defaultPropertyComponent.qml");
     QTest::newRow("defaultPropertyComponent2") << QStringLiteral("defaultPropertyComponent.2.qml");
+    QTest::newRow("defaultPropertyListModel") << QStringLiteral("defaultPropertyListModel.qml");
+    QTest::newRow("defaultPropertyVar") << QStringLiteral("defaultPropertyVar.qml");
+    QTest::newRow("propertyDelegate") << QStringLiteral("propertyDelegate.qml");
     QTest::newRow("duplicateQmldirImport") << QStringLiteral("qmldirImport/duplicate.qml");
     QTest::newRow("Used imports") << QStringLiteral("used.qml");
     QTest::newRow("Unused imports (multi)") << QStringLiteral("unused_multi.qml");
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp
index 9e9301b104..51db82cbde 100644
--- a/tools/qmllint/findwarnings.cpp
+++ b/tools/qmllint/findwarnings.cpp
@@ -157,8 +157,10 @@ void FindWarningVisitor::checkDefaultProperty(const QQmlJSScope::ConstPtr &scope
             return;
     }
     if (defaultPropertyName.isEmpty()) {
-        m_logger.log(QStringLiteral("Cannot assign to non-existent default property"),
-                     Log_Property, scope->sourceLocation() );
+        if (scope->parentScope()->isFullyResolved()) {
+            m_logger.log(QStringLiteral("Cannot assign to non-existent default property"),
+                         Log_Property, scope->sourceLocation());
+        }
         return;
     }
 
@@ -176,20 +178,16 @@ void FindWarningVisitor::checkDefaultProperty(const QQmlJSScope::ConstPtr &scope
     }
     m_scopeHasDefaultPropertyAssignment[scopeOfDefaultProperty] = true;
 
-    auto propType = defaultProp.type().data();
-    if (!propType) // should be an error somewhere else
+    auto propType = defaultProp.type();
+    if (propType.isNull() || !propType->isFullyResolved()
+        || !scope->isFullyResolved()) // should be an error somewhere else
         return;
 
     // Assigning any element to a QQmlComponent property implicitly wraps it into a Component
-    if (defaultProp.typeName() == QStringLiteral("QQmlComponent"))
+    // Check whether the property can be assigned the scope
+    if (propType->canAssign(scope))
         return;
 
-    // scope's type hierarchy has to have property type
-    for (const QQmlJSScope *type = scope.data(); type; type = type->baseType().data()) {
-        if (type == propType)
-            return;
-    }
-
     m_logger.log(QStringLiteral("Cannot assign to default property of incompatible type"),
                  Log_Property, scope->sourceLocation());
 }
@@ -275,6 +273,9 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
         return true;
     }
 
+    if (!qmlScope->isFullyResolved())
+        return true;
+
     const QString signal = signalName(name);
     if (signal.isEmpty()) {
         for (const auto &childScope : qmlScope->childScopes()) {