QmlLintQuickPlugin: Warn about unexpected property binding types

Sometimes we use var or Item property types when we actually expect
some very specific types. We warn about these at runtime, let's also
warn in the linter.

Task-number: QTBUG-102859
Change-Id: I68c2bc8b2bf5097723a432d02d76f45651f4ef12
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

6 files changed, 161 insertions, 15 deletions

diff --git a/src/plugins/qmllint/quick/quicklintplugin.cpp b/src/plugins/qmllint/quick/quicklintplugin.cpp
index fca4fecc36..6759171600 100644
--- a/src/plugins/qmllint/quick/quicklintplugin.cpp
+++ b/src/plugins/qmllint/quick/quicklintplugin.cpp
@@ -406,13 +406,91 @@ void ControlsSwipeDelegateValidatorPass::run(const QQmlSA::Element &element)
     }
 }
 
+VarBindingTypeValidatorPass::VarBindingTypeValidatorPass(
+        QQmlSA::PassManager *manager,
+        const QMultiHash<QString, TypeDescription> &expectedPropertyTypes)
+    : QQmlSA::PropertyPass(manager)
+{
+    QMultiHash<QString, QQmlJSScope::ConstPtr> propertyTypes;
+
+    for (const auto &pair : expectedPropertyTypes.asKeyValueRange()) {
+        QQmlSA::Element propType;
+
+        if (!pair.second.module.isEmpty()) {
+            propType = resolveType(pair.second.module, pair.second.name);
+            if (propType.isNull())
+                continue;
+        } else {
+            auto scope = QQmlJSScope::create();
+            scope->setInternalName(pair.second.name);
+            propType = scope;
+        }
+
+        propertyTypes.insert(pair.first, propType);
+    }
+
+    m_expectedPropertyTypes = propertyTypes;
+}
+
+void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element,
+                                            const QString &propertyName,
+                                            const QQmlJSMetaPropertyBinding &binding,
+                                            const QQmlSA::Element &bindingScope,
+                                            const QQmlSA::Element &value)
+{
+    Q_UNUSED(bindingScope);
+
+    const auto range = m_expectedPropertyTypes.equal_range(propertyName);
+
+    if (range.first == range.second)
+        return;
+
+    QQmlSA::Element bindingType;
+
+    if (!value.isNull()) {
+        bindingType = value;
+    } else {
+        if (QQmlJSMetaPropertyBinding::isLiteralBinding(binding.bindingType())) {
+            bindingType = resolveLiteralType(binding);
+        } else {
+            switch (binding.bindingType()) {
+            case QQmlJSMetaPropertyBinding::Object:
+                bindingType = binding.objectType();
+                break;
+            case QQmlJSMetaPropertyBinding::Script:
+                break;
+            default:
+                return;
+            }
+        }
+    }
+
+    if (std::find_if(range.first, range.second,
+                     [&](const QQmlSA::Element &scope) { return element->inherits(scope); })
+        == range.second) {
+
+        const QString bindingTypeName = QQmlJSScope::prettyName(
+                bindingType->isComposite() ? bindingType->baseType()->internalName()
+                                           : bindingType->internalName());
+        QStringList expectedTypeNames;
+
+        for (auto it = range.first; it != range.second; it++)
+            expectedTypeNames << QQmlJSScope::prettyName(it.value()->internalName());
+
+        emitWarning(u"Unexpected type for property \"%1\" expected %2 got %3"_s.arg(
+                            propertyName, expectedTypeNames.join(u", "_s), bindingTypeName),
+                    binding.sourceLocation());
+    }
+}
+
 void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager,
                                         const QQmlSA::Element &rootElement)
 {
     const bool hasQuick = manager->hasImportedModule("QtQuick");
     const bool hasQuickLayouts = manager->hasImportedModule("QtQuick.Layouts");
     const bool hasQuickControls = manager->hasImportedModule("QtQuick.Templates")
-            || manager->hasImportedModule("QtQuick.Controls");
+            || manager->hasImportedModule("QtQuick.Controls")
+            || manager->hasImportedModule("QtQuick.Controls.Basic");
 
     Q_UNUSED(rootElement);
 
@@ -459,17 +537,29 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager,
 
     auto attachedPropertyType = std::make_shared<AttachedPropertyTypeValidatorPass>(manager);
 
-    auto addAttachedWarning =
-            [&](AttachedPropertyTypeValidatorPass::TypeDescription attachedType,
-                QList<AttachedPropertyTypeValidatorPass::TypeDescription> allowedTypes,
-                QAnyStringView warning, bool allowInDelegate = false) {
-                QString attachedTypeName = attachedPropertyType->addWarning(
-                        attachedType, allowedTypes, allowInDelegate, warning);
-                manager->registerPropertyPass(attachedPropertyType, attachedType.module,
-                                              u"$internal$."_s + attachedTypeName);
+    auto addAttachedWarning = [&](TypeDescription attachedType, QList<TypeDescription> allowedTypes,
+                                  QAnyStringView warning, bool allowInDelegate = false) {
+        QString attachedTypeName = attachedPropertyType->addWarning(attachedType, allowedTypes,
+                                                                    allowInDelegate, warning);
+        manager->registerPropertyPass(attachedPropertyType, attachedType.module,
+                                      u"$internal$."_s + attachedTypeName, {}, false);
+    };
+
+    auto addVarBindingWarning =
+            [&](QAnyStringView moduleName, QAnyStringView typeName,
+                const QMultiHash<QString, TypeDescription> &expectedPropertyTypes) {
+                auto varBindingType = std::make_shared<VarBindingTypeValidatorPass>(
+                        manager, expectedPropertyTypes);
+                for (const auto &propertyName : expectedPropertyTypes.uniqueKeys()) {
+                    manager->registerPropertyPass(varBindingType, moduleName, typeName,
+                                                  propertyName);
+                }
             };
 
     if (hasQuick) {
+        addVarBindingWarning("QtQuick", "TableView",
+                             { { "columnWidthProvider", { "", "function" } },
+                               { "rowHeightProvider", { "", "function" } } });
         addAttachedWarning({ "QtQuick", "Accessible" }, { { "QtQuick", "Item" } },
                            "Accessible must be attached to an Item");
         addAttachedWarning({ "QtQuick", "LayoutMirroring" },
@@ -509,6 +599,12 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager,
                 { "QtQuick.Templates", "Tumbler" }, { { "QtQuick", "Tumbler" } },
                 "Tumbler: attached properties of Tumbler must be accessed through a delegate item",
                 true);
+        addVarBindingWarning("QtQuick.Templates", "Tumbler",
+                             { { "contentItem", { "QtQuick", "PathView" } },
+                               { "contentItem", { "QtQuick", "ListView" } } });
+        addVarBindingWarning("QtQuick.Templates", "SpinBox",
+                             { { "textFromValue", { "", "function" } },
+                               { "valueFromText", { "", "function" } } });
     }
 
     if (manager->hasImportedModule(u"QtQuick.Controls.macOS"_s)
diff --git a/src/plugins/qmllint/quick/quicklintplugin.h b/src/plugins/qmllint/quick/quicklintplugin.h
index b1c9430ba6..be368f0880 100644
--- a/src/plugins/qmllint/quick/quicklintplugin.h
+++ b/src/plugins/qmllint/quick/quicklintplugin.h
@@ -13,6 +13,12 @@
 
 QT_BEGIN_NAMESPACE
 
+struct TypeDescription
+{
+    QString module;
+    QString name;
+};
+
 class QmlLintQuickPlugin : public QObject, public QQmlSA::LintPlugin
 {
     Q_OBJECT
@@ -46,12 +52,6 @@ private:
 class AttachedPropertyTypeValidatorPass : public QQmlSA::PropertyPass
 {
 public:
-    struct TypeDescription
-    {
-        QString module;
-        QString name;
-    };
-
     AttachedPropertyTypeValidatorPass(QQmlSA::PassManager *manager);
 
     QString addWarning(TypeDescription attachType, QList<TypeDescription> allowedTypes,
@@ -125,6 +125,20 @@ private:
     QQmlSA::Element m_swipeDelegate;
 };
 
+class VarBindingTypeValidatorPass : public QQmlSA::PropertyPass
+{
+public:
+    VarBindingTypeValidatorPass(QQmlSA::PassManager *manager,
+                                const QMultiHash<QString, TypeDescription> &expectedPropertyTypes);
+
+    void onBinding(const QQmlSA::Element &element, const QString &propertyName,
+                   const QQmlJSMetaPropertyBinding &binding, const QQmlSA::Element &bindingScope,
+                   const QQmlSA::Element &value) override;
+
+private:
+    QMultiHash<QString, QQmlSA::Element> m_expectedPropertyTypes;
+};
+
 QT_END_NAMESPACE
 
 #endif // QUICKLINTPLUGIN_H
diff --git a/src/qmlcompiler/qqmlsa.cpp b/src/qmlcompiler/qqmlsa.cpp
index 28ea9b9135..e9acf3b90c 100644
--- a/src/qmlcompiler/qqmlsa.cpp
+++ b/src/qmlcompiler/qqmlsa.cpp
@@ -43,6 +43,11 @@ Element GenericPass::resolveType(QAnyStringView moduleName, QAnyStringView typeN
     return module[typeName.toString()].scope;
 }
 
+Element GenericPass::resolveLiteralType(const QQmlJSMetaPropertyBinding &binding)
+{
+    return binding.literalType(d->manager->m_typeResolver);
+}
+
 /*!
  * \brief PassManager::registerElementPass registers ElementPass
           with the pass manager.
diff --git a/src/qmlcompiler/qqmlsa_p.h b/src/qmlcompiler/qqmlsa_p.h
index 51c47dce85..30669aec2a 100644
--- a/src/qmlcompiler/qqmlsa_p.h
+++ b/src/qmlcompiler/qqmlsa_p.h
@@ -48,6 +48,8 @@ public:
     void emitWarning(QAnyStringView message,
                      QQmlJS::SourceLocation srcLocation = QQmlJS::SourceLocation());
     Element resolveType(QAnyStringView moduleName, QAnyStringView typeName); // #### TODO: revisions
+    Element resolveLiteralType(const QQmlJSMetaPropertyBinding &binding);
+
 private:
     std::unique_ptr<GenericPassPrivate> d; // PIMPL might be overkill
 };
diff --git a/tests/auto/qml/qmllint/data/pluginQuick_varProp.qml b/tests/auto/qml/qmllint/data/pluginQuick_varProp.qml
new file mode 100644
index 0000000000..17b5d99d6b
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/pluginQuick_varProp.qml
@@ -0,0 +1,16 @@
+import QtQuick
+import QtQuick.Controls
+
+Item {
+    SpinBox {
+        textFromValue: null
+        valueFromText: { return 6; }
+    }
+    TableView {
+        columnWidthProvider: null
+        rowHeightProvider: { return 3; }
+    }
+    Tumbler {
+        contentItem: Item {}
+    }
+}
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index 6a7431e4be..fd05d8594b 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -1759,6 +1759,19 @@ void TestQmllint::quickPlugin()
                                    16, 9 },
                      } });
 
+    runTest("pluginQuick_varProp.qml",
+            Result {
+                    { Message {
+                              u"Unexpected type for property \"contentItem\" expected QQuickPathView, QQuickListView got QQuickItem"_s },
+                      Message {
+                              u"Unexpected type for property \"columnWidthProvider\" expected function got null"_s },
+                      Message {
+                              u"Unexpected type for property \"textFromValue\" expected function got null"_s },
+                      Message {
+                              u"Unexpected type for property \"valueFromText\" expected function got int"_s },
+                      Message {
+                              u"Unexpected type for property \"rowHeightProvider\" expected function got int"_s } } });
+
     runTest("pluginQuick_attachedClean.qml", Result::clean());
 }
 #endif