qmllint: Properly warn about missing required properties in delegates

Previously we would often suggest to add an id to the view containing
the delegate so you could access the model that way which is very
misleading and might be especially bad as an automatic suggestion.

This change now recommends that the user adds a required property,
although we currently lack the logic to properly insert one for the user.

Fixes: QTBUG-103101
Change-Id: I4cd858ff6617d12b9abf070f524dc626d699e4f1
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qmlcompiler/qqmljstypepropagator.cpp

@@ -364,26 +364,53 @@ void QQmlJSTypePropagator::handleUnqualifiedAccess(const QString &name, bool isM
         }
     }
 
-    for (QQmlJSScope::ConstPtr scope = m_function->qmlScope; !scope.isNull();
-         scope = scope->parentScope()) {
-        if (scope->hasProperty(name)) {
-            const QString id = m_function->addressableScopes.id(scope);
+    // Might be a delegate just missing a required property.
+    // This heuristic does not recognize all instances of this occurring but should be sufficient
+    // protection against wrongly suggesting to add an id to the view to access the model that way
+    // which is very misleading
+    if (name == u"model" || name == u"index") {
+        if (QQmlJSScope::ConstPtr parent = m_function->qmlScope->parentScope(); !parent.isNull()) {
+            const auto bindings = parent->ownPropertyBindings(u"delegate"_s);
 
-            suggestion = FixSuggestion {};
+            for (auto it = bindings.first; it != bindings.second; it++) {
+                if (!it->hasObject())
+                    continue;
+                if (it->objectType() == m_function->qmlScope) {
+                    suggestion = FixSuggestion {};
 
-            QQmlJS::SourceLocation fixLocation = location;
-            fixLocation.length = 0;
-            suggestion->fixes << FixSuggestion::Fix {
-                name + QLatin1String(" is a member of a parent element\n")
-                        + QLatin1String("      You can qualify the access with its id "
-                                        "to avoid this warning:\n"),
-                fixLocation, (id.isEmpty() ? u"<id>."_s : (id + u'.')), QString(), id.isEmpty()
-            };
-
-            if (id.isEmpty()) {
-                suggestion->fixes << FixSuggestion::Fix {
-                    u"You first have to give the element an id"_s, QQmlJS::SourceLocation {}, {}
+                    suggestion->fixes << FixSuggestion::Fix {
+                        name + u" is implicitly injected into this delegate. Add a required property instead."_s,
+                        m_function->qmlScope->sourceLocation(), QString(), QString(), true
+                    };
                 };
+
+                break;
+            }
+        }
+    }
+
+    if (!suggestion.has_value()) {
+        for (QQmlJSScope::ConstPtr scope = m_function->qmlScope; !scope.isNull();
+             scope = scope->parentScope()) {
+            if (scope->hasProperty(name)) {
+                const QString id = m_function->addressableScopes.id(scope);
+
+                suggestion = FixSuggestion {};
+
+                QQmlJS::SourceLocation fixLocation = location;
+                fixLocation.length = 0;
+                suggestion->fixes << FixSuggestion::Fix {
+                    name + QLatin1String(" is a member of a parent element\n")
+                            + QLatin1String("      You can qualify the access with its id "
+                                            "to avoid this warning:\n"),
+                    fixLocation, (id.isEmpty() ? u"<id>."_s : (id + u'.')), QString(), id.isEmpty()
+                };
+
+                if (id.isEmpty()) {
+                    suggestion->fixes << FixSuggestion::Fix {
+                        u"You first have to give the element an id"_s, QQmlJS::SourceLocation {}, {}
+                    };
+                }
             }
         }
     }

tests/auto/qml/qmllint/data/delegateContextProperties.qml

@@ -0,0 +1,9 @@
+import QtQuick
+
+ListView {
+   model: ListModel {}
+   delegate: Text {
+       text: model.text
+       color: index % 2 ? "red" : "blue"
+   }
+}

tests/auto/qml/qmllint/tst_qmllint.cpp

@@ -269,6 +269,16 @@ void TestQmllint::testUnqualified_data()
     QTest::newRow("crashConnections")
             << QStringLiteral("crashConnections.qml")
             << Result { { Message { QStringLiteral("Unqualified access"), 4, 13 } } };
+
+    QTest::newRow("delegateContextProperties")
+            << QStringLiteral("delegateContextProperties.qml")
+            << Result { { Message { QStringLiteral("Unqualified access"), 6, 14 },
+                          Message { QStringLiteral("Unqualified access"), 7, 15 },
+                          Message { QStringLiteral("model is implicitly injected into this "
+                                                   "delegate. Add a required property instead.") },
+                          Message {
+                                  QStringLiteral("index is implicitly injected into this delegate. "
+                                                 "Add a required property instead.") } } };
 }
 
 void TestQmllint::testUnknownCausesFail()