qmllint: Properly warn about calling properties

Previously whenever a property was called the warnings qmllint emitted
made it seem like the property was just not found instead of not being
callable.

Now we will warn if you try to call a property and warn about
properties shadowing methods, slots or signals. We also discourage
calling variant properties now that may or may not be a function.

This change also fixes an assert being hit in isMissingPropertyType
due to our previous, lackluster checks.

Fixes: QTBUG-101074
Pick-to: 6.3
Change-Id: I0790b4a4584f3430ee1d8ddf549611225a36cc5b
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qmlcompiler/qqmljstypepropagator.cpp

@@ -260,7 +260,7 @@ QQmlJS::SourceLocation QQmlJSTypePropagator::getCurrentBindingSourceLocation() c
     return combine(entries.constFirst().location, entries.constLast().location);
 }
 
-void QQmlJSTypePropagator::handleUnqualifiedAccess(const QString &name) const
+void QQmlJSTypePropagator::handleUnqualifiedAccess(const QString &name, bool isMethod) const
 {
     auto location = getCurrentSourceLocation();
 
@@ -271,8 +271,12 @@ void QQmlJSTypePropagator::handleUnqualifiedAccess(const QString &name) const
             return;
     }
 
-    if (isMissingPropertyType(m_function->qmlScope, name))
+    if (isMethod) {
+        if (isCallingProperty(m_function->qmlScope, name))
+            return;
+    } else if (isMissingPropertyType(m_function->qmlScope, name)) {
         return;
+    }
 
     std::optional<FixSuggestion> suggestion;
 
@@ -462,6 +466,46 @@ bool QQmlJSTypePropagator::isMissingPropertyType(QQmlJSScope::ConstPtr scope,
     return true;
 }
 
+bool QQmlJSTypePropagator::isCallingProperty(QQmlJSScope::ConstPtr scope, const QString &name) const
+{
+    auto property = scope->property(name);
+    if (!property.isValid())
+        return false;
+
+    QString propertyType = u"Property"_qs;
+
+    auto methods = scope->methods(name);
+
+    QString errorType;
+    if (!methods.isEmpty()) {
+        errorType = u"shadowed by a property."_qs;
+        switch (methods.first().methodType()) {
+        case QQmlJSMetaMethod::Signal:
+            propertyType = u"Signal"_qs;
+            break;
+        case QQmlJSMetaMethod::Slot:
+            propertyType = u"Slot"_qs;
+            break;
+        case QQmlJSMetaMethod::Method:
+            propertyType = u"Method"_qs;
+            break;
+        }
+    } else if (m_typeResolver->equals(property.type(), m_typeResolver->varType())) {
+        errorType =
+                u"a variant property. It may or may not be a method. Use a regular function instead."_qs;
+    } else if (m_typeResolver->equals(property.type(), m_typeResolver->jsValueType())) {
+        errorType =
+                u"a QJSValue property. It may or may not be a method. Use a regular Q_INVOKABLE instead."_qs;
+    } else {
+        errorType = u"not a method"_qs;
+    }
+
+    m_logger->log(u"%1 \"%2\" is %3"_qs.arg(propertyType, name, errorType), Log_Type,
+                  getCurrentSourceLocation(), true, true, {});
+
+    return true;
+}
+
 void QQmlJSTypePropagator::generate_LoadQmlContextPropertyLookup(int index)
 {
     // LoadQmlContextPropertyLookup does not use accumulatorIn. It always refers to the scope.
@@ -484,7 +528,7 @@ void QQmlJSTypePropagator::generate_LoadQmlContextPropertyLookup(int index)
 
     if (!m_state.accumulatorOut().isValid()) {
         setError(u"Cannot access value for name "_qs + name);
-        handleUnqualifiedAccess(name);
+        handleUnqualifiedAccess(name, false);
     } else if (m_typeResolver->genericType(m_state.accumulatorOut().storedType()).isNull()) {
         // It should really be valid.
         // We get the generic type from aotContext->loadQmlContextPropertyIdLookup().
@@ -799,6 +843,9 @@ void QQmlJSTypePropagator::generate_CallProperty(int nameIndex, int base, int ar
         setError(u"Type %1 does not have a property %2 for calling"_qs
                          .arg(callBase.descriptiveName(), propertyName));
 
+        if (callBase.isType() && isCallingProperty(callBase.type(), propertyName))
+            return;
+
         if (isRestricted(propertyName))
             return;
 
@@ -962,7 +1009,8 @@ void QQmlJSTypePropagator::propagateScopeLookupCall(const QString &functionName,
     setAccumulator(m_typeResolver->globalType(m_typeResolver->jsValueType()));
 
     setError(u"Cannot find function '%1'"_qs.arg(functionName));
-    handleUnqualifiedAccess(functionName);
+
+    handleUnqualifiedAccess(functionName, true);
 }
 
 void QQmlJSTypePropagator::generate_CallGlobalLookup(int index, int argc, int argv)

src/qmlcompiler/qqmljstypepropagator_p.h

@@ -208,9 +208,10 @@ private:
         bool needsMorePasses = false;
     };
 
-    void handleUnqualifiedAccess(const QString &name) const;
+    void handleUnqualifiedAccess(const QString &name, bool isMethod) const;
     void checkDeprecated(QQmlJSScope::ConstPtr scope, const QString &name, bool isMethod) const;
     bool isRestricted(const QString &propertyName) const;
+    bool isCallingProperty(QQmlJSScope::ConstPtr scope, const QString &name) const;
     bool isMissingPropertyType(QQmlJSScope::ConstPtr scope, const QString &type) const;
     QQmlJS::SourceLocation getCurrentSourceLocation() const;
     QQmlJS::SourceLocation getCurrentBindingSourceLocation() const;

tests/auto/qml/qmllint/data/callJSValueProp.qml

@@ -0,0 +1,6 @@
+import QtQml
+
+QtObject {
+    function foo() {}
+    Component.onCompleted: Qt.callLater(foo);
+}

tests/auto/qml/qmllint/data/callVarProp.qml

@@ -0,0 +1,7 @@
+import QtQml
+
+QtObject {
+    property var foo: () => {}
+
+    Component.onCompleted: foo()
+}

tests/auto/qml/qmllint/data/shadowedMethod.qml

@@ -0,0 +1,8 @@
+import QtQuick
+
+QtObject {
+    function foo() {}
+    property bool foo: false
+
+    Component.onCompleted: foo()
+}

tests/auto/qml/qmllint/data/shadowedSignal.qml

@@ -0,0 +1,6 @@
+import QtQuick
+
+MouseArea {
+    id: mouseArea
+    Component.onCompleted: pressed()
+}

tests/auto/qml/qmllint/data/shadowedSignalWithId.qml

@@ -0,0 +1,6 @@
+import QtQuick
+
+MouseArea {
+    id: mouseArea
+    Component.onCompleted: mouseArea.pressed()
+}

tests/auto/qml/qmllint/data/shadowedSlot.qml

@@ -0,0 +1,6 @@
+import QtQml
+
+ObjectModel {
+    property bool move: false
+    Component.onCompleted: move()
+}

tests/auto/qml/qmllint/tst_qmllint.cpp

@@ -862,6 +862,30 @@ void TestQmllint::dirtyQmlCode_data()
             << QStringLiteral("jsVarDeclarationsWriteConst.qml")
             << QStringLiteral("Cannot assign to read-only property constProp") << QString()
             << QString() << false;
+    QTest::newRow("shadowedSignal")
+            << QStringLiteral("shadowedSignal.qml")
+            << QStringLiteral("Signal \"pressed\" is shadowed by a property.") << QString()
+            << QString() << false;
+    QTest::newRow("shadowedSignalWithId")
+            << QStringLiteral("shadowedSignalWithId.qml")
+            << QStringLiteral("Signal \"pressed\" is shadowed by a property") << QString()
+            << QString() << false;
+    QTest::newRow("shadowedSlot") << QStringLiteral("shadowedSlot.qml")
+                                  << QStringLiteral("Slot \"move\" is shadowed by a property")
+                                  << QString() << QString() << false;
+    QTest::newRow("shadowedMethod") << QStringLiteral("shadowedMethod.qml")
+                                    << QStringLiteral("Method \"foo\" is shadowed by a property.")
+                                    << QString() << QString() << false;
+    QTest::newRow("callVarProp")
+            << QStringLiteral("callVarProp.qml")
+            << QStringLiteral("Property \"foo\" is a variant property. It may or may not be a "
+                              "method. Use a regular function instead.")
+            << QString() << QString() << false;
+    QTest::newRow("callJSValue")
+            << QStringLiteral("callJSValueProp.qml")
+            << QStringLiteral("Property \"callLater\" is a QJSValue property. It may or may not be "
+                              "a method. Use a regular Q_INVOKABLE instead.")
+            << QString() << QString() << false;
 }
 
 void TestQmllint::dirtyQmlCode()