qmllint: Store property's binding type separately

In the past the type of a property's bindings would override the base type.
This would lead to incompatible type warnings when trying to override that binding with another value that was compatible with the property type but not the binding one.

Change-Id: Ie0c0843e34f24e05aae7bbb429899fbe076e7262
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

10 files changed, 126 insertions, 19 deletions

diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp
index 61553405c2..f1a6d5abda 100644
--- a/src/qmlcompiler/qqmljsimportvisitor.cpp
+++ b/src/qmlcompiler/qqmljsimportvisitor.cpp
@@ -838,9 +838,10 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
     // just-visited one if the property exists and this type is valid
     QQmlJSMetaProperty property = m_currentScope->property(group->name.toString());
     if (property.isValid() && !property.type().isNull() && property.type()->canAssign(childScope)) {
-        property.setType(childScope);
-        property.setTypeName(getScopeName(childScope, QQmlJSScope::QMLScope));
-        m_currentScope->addOwnProperty(property);
+        QQmlJSMetaPropertyBinding binding(property);
+        binding.setType(childScope);
+        binding.setTypeName(getScopeName(childScope, QQmlJSScope::QMLScope));
+        m_currentScope->addOwnPropertyBinding(binding);
     } 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)
diff --git a/src/qmlcompiler/qqmljsmetatypes_p.h b/src/qmlcompiler/qqmljsmetatypes_p.h
index 4e914d37cc..43147d98a5 100644
--- a/src/qmlcompiler/qqmljsmetatypes_p.h
+++ b/src/qmlcompiler/qqmljsmetatypes_p.h
@@ -305,15 +305,10 @@ public:
 
     friend bool operator==(const QQmlJSMetaProperty &a, const QQmlJSMetaProperty &b)
     {
-        return a.m_propertyName == b.m_propertyName
-                && a.m_typeName == b.m_typeName
-                && a.m_bindable == b.m_bindable
-                && a.m_type == b.m_type
-                && a.m_isList == b.m_isList
-                && a.m_isWritable == b.m_isWritable
-                && a.m_isPointer == b.m_isPointer
-                && a.m_isAlias == b.m_isAlias
-                && a.m_revision == b.m_revision;
+        return a.m_propertyName == b.m_propertyName && a.m_typeName == b.m_typeName
+                && a.m_bindable == b.m_bindable && a.m_type == b.m_type && a.m_isList == b.m_isList
+                && a.m_isWritable == b.m_isWritable && a.m_isPointer == b.m_isPointer
+                && a.m_isAlias == b.m_isAlias && a.m_revision == b.m_revision;
     }
 
     friend bool operator!=(const QQmlJSMetaProperty &a, const QQmlJSMetaProperty &b)
@@ -324,9 +319,49 @@ public:
     friend size_t qHash(const QQmlJSMetaProperty &prop, size_t seed = 0)
     {
         return qHashMulti(seed, prop.m_propertyName, prop.m_typeName, prop.m_bindable,
-                          prop.m_type.toStrongRef().data(),
-                          prop.m_isList, prop.m_isWritable, prop.m_isPointer, prop.m_isAlias,
-                          prop.m_revision);
+                          prop.m_type.toStrongRef().data(), prop.m_isList, prop.m_isWritable,
+                          prop.m_isPointer, prop.m_isAlias, prop.m_revision);
+    }
+};
+
+class QQmlJSMetaPropertyBinding
+{
+    QString m_propertyName;
+    QString m_typeName;
+    QWeakPointer<const QQmlJSScope> m_type;
+
+public:
+    QQmlJSMetaPropertyBinding() = default;
+    QQmlJSMetaPropertyBinding(const QQmlJSMetaProperty &prop) : m_propertyName(prop.propertyName())
+    {
+    }
+
+    void setPropertyName(const QString &propertyName) { m_propertyName = propertyName; }
+    QString propertyName() const { return m_propertyName; }
+
+    void setTypeName(const QString &typeName) { m_typeName = typeName; }
+    QString typeName() const { return m_typeName; }
+
+    QSharedPointer<const QQmlJSScope> type() const { return m_type; }
+    void setType(const QSharedPointer<const QQmlJSScope> &type) { m_type = type; }
+
+    bool isValid() const { return !m_propertyName.isEmpty(); }
+
+    friend bool operator==(const QQmlJSMetaPropertyBinding &a, const QQmlJSMetaPropertyBinding &b)
+    {
+        return a.m_propertyName == b.m_propertyName && a.m_typeName == b.m_typeName
+                && a.m_type == b.m_type;
+    }
+
+    friend bool operator!=(const QQmlJSMetaPropertyBinding &a, const QQmlJSMetaPropertyBinding &b)
+    {
+        return !(a == b);
+    }
+
+    friend size_t qHash(const QQmlJSMetaPropertyBinding &binding, size_t seed = 0)
+    {
+        return qHashMulti(seed, binding.m_propertyName, binding.m_typeName,
+                          binding.m_type.toStrongRef().data());
     }
 };
 
diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp
index aea955e7c1..19f4fb9be8 100644
--- a/src/qmlcompiler/qqmljsscope.cpp
+++ b/src/qmlcompiler/qqmljsscope.cpp
@@ -373,6 +373,26 @@ bool QQmlJSScope::isPropertyLocallyRequired(const QString &name) const
     return m_requiredPropertyNames.contains(name);
 }
 
+bool QQmlJSScope::hasPropertyBinding(const QString &name) const
+{
+    return searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *scope) {
+        return scope->m_propertyBindings.contains(name);
+    });
+}
+
+QQmlJSMetaPropertyBinding QQmlJSScope::propertyBinding(const QString &name) const
+{
+    QQmlJSMetaPropertyBinding binding;
+    searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *scope) {
+        const auto it = scope->m_propertyBindings.find(name);
+        if (it == scope->m_propertyBindings.end())
+            return false;
+        binding = *it;
+        return true;
+    });
+    return binding;
+}
+
 bool QQmlJSScope::isFullyResolved() const
 {
     bool baseResolved = true;
diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h
index 3a8188a007..f895f5e746 100644
--- a/src/qmlcompiler/qqmljsscope_p.h
+++ b/src/qmlcompiler/qqmljsscope_p.h
@@ -222,6 +222,26 @@ public:
     bool isPropertyRequired(const QString &name) const;
     bool isPropertyLocallyRequired(const QString &name) const;
 
+    void addOwnPropertyBinding(const QQmlJSMetaPropertyBinding &binding)
+    {
+        m_propertyBindings.insert(binding.propertyName(), binding);
+    }
+    QHash<QString, QQmlJSMetaPropertyBinding> ownPropertyBindings() const
+    {
+        return m_propertyBindings;
+    }
+    QQmlJSMetaPropertyBinding ownPropertyBinding(const QString &name) const
+    {
+        return m_propertyBindings.value(name);
+    }
+    bool hasOwnPropertyBinding(const QString &name) const
+    {
+        return m_propertyBindings.contains(name);
+    }
+
+    bool hasPropertyBinding(const QString &name) const;
+    QQmlJSMetaPropertyBinding propertyBinding(const QString &name) const;
+
     bool isResolved() const { return m_baseTypeName.isEmpty() || !m_baseType.isNull(); }
     bool isFullyResolved() const;
 
@@ -327,6 +347,7 @@ private:
 
     QMultiHash<QString, QQmlJSMetaMethod> m_methods;
     QHash<QString, QQmlJSMetaProperty> m_properties;
+    QHash<QString, QQmlJSMetaPropertyBinding> m_propertyBindings;
     QHash<QString, QQmlJSMetaEnum> m_enumerations;
 
     QVector<QQmlJSAnnotation> m_annotations;
diff --git a/tests/auto/qml/qmllint/data/PropertyBase.qml b/tests/auto/qml/qmllint/data/PropertyBase.qml
new file mode 100644
index 0000000000..a7a3cb5bb4
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/PropertyBase.qml
@@ -0,0 +1,5 @@
+import QtQml 2.15
+
+QtObject {
+    property var foo
+}
diff --git a/tests/auto/qml/qmllint/data/PropertyBase2.qml b/tests/auto/qml/qmllint/data/PropertyBase2.qml
new file mode 100644
index 0000000000..cfb07da0f5
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/PropertyBase2.qml
@@ -0,0 +1,5 @@
+import QtQml 2.15
+
+PropertyBase {
+    foo: QtObject { property int bar: 5 }
+}
diff --git a/tests/auto/qml/qmllint/data/propertyBindingValue.qml b/tests/auto/qml/qmllint/data/propertyBindingValue.qml
new file mode 100644
index 0000000000..eb461e8fdd
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/propertyBindingValue.qml
@@ -0,0 +1,3 @@
+PropertyBase2 {
+    property int bar: foo.bar
+}
diff --git a/tests/auto/qml/qmllint/data/propertyOverride.qml b/tests/auto/qml/qmllint/data/propertyOverride.qml
new file mode 100644
index 0000000000..04c28485bb
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/propertyOverride.qml
@@ -0,0 +1,5 @@
+import QtQuick 2.15
+
+PropertyBase2 {
+    foo: Item {}
+}
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index 4647464a3b..c5e7181a92 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -707,6 +707,8 @@ void TestQmllint::cleanQmlCode_data()
     QTest::newRow("Unversioned change signal without arguments") << QStringLiteral("unversionChangedSignalSansArguments.qml");
     QTest::newRow("deprecatedFunctionOverride") << QStringLiteral("deprecatedFunctionOverride.qml");
     QTest::newRow("multilineStringEscaped") << QStringLiteral("multilineStringEscaped.qml");
+    QTest::newRow("propertyOverride") << QStringLiteral("propertyOverride.qml");
+    QTest::newRow("propertyBindingValue") << QStringLiteral("propertyBindingValue.qml");
 }
 
 void TestQmllint::cleanQmlCode()
diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp
index 1e889926a1..af56e0b2bf 100644
--- a/tools/qmllint/checkidentifiers.cpp
+++ b/tools/qmllint/checkidentifiers.cpp
@@ -115,8 +115,11 @@ void CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
 
         const auto property = scope->property(access.m_name);
         if (!property.propertyName().isEmpty()) {
-            const QString typeName = access.m_parentType.isEmpty() ? property.typeName()
-                                                                   : access.m_parentType;
+            const auto binding = scope->propertyBinding(access.m_name);
+            const QString typeName = access.m_parentType.isEmpty()
+                    ? (binding.isValid() ? binding.typeName() : property.typeName())
+                    : access.m_parentType;
+
             if (property.isList()) {
                 detectedRestrictiveKind = QLatin1String("list");
                 detectedRestrictiveName = access.m_name;
@@ -132,7 +135,11 @@ void CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
             }
 
             if (access.m_parentType.isEmpty()) {
-                scope = property.type();
+                if (binding.isValid())
+                    scope = binding.type();
+                else
+                    scope = property.type();
+
                 if (scope.isNull()) {
                     // Properties should always have a type. Otherwise something
                     // was missing from the import already.
@@ -342,7 +349,10 @@ void CheckIdentifiers::operator()(
                 if (memberAccessChain.isEmpty() || unknownBuiltins.contains(property.typeName()))
                     continue;
 
-                if (!property.type()) {
+                const auto binding = qmlScope->propertyBinding(memberAccessBase.m_name);
+                if (binding.isValid()) {
+                    checkMemberAccess(memberAccessChain, binding.type(), &property);
+                } else if (!property.type()) {
                     m_logger->log(QString::fromLatin1(
                                       "Type of property \"%2\" not found")
                                   .arg(memberAccessBase.m_name), Log_Type, memberAccessBase.m_location);