diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp index e82c4c4ef7..855b104326 100644 --- a/src/qmlcompiler/qqmljsimportvisitor.cpp +++ b/src/qmlcompiler/qqmljsimportvisitor.cpp @@ -811,6 +811,21 @@ void QQmlJSImportVisitor::processPropertyBindings() } } +static std::optional +propertyForChangeHandler(const QQmlJSScope::ConstPtr &scope, QString name) +{ + if (!name.endsWith(QLatin1String("Changed"))) + return {}; + constexpr int length = int(sizeof("Changed") / sizeof(char)) - 1; + name.chop(length); + auto p = scope->property(name); + const bool isBindable = !p.bindable().isEmpty(); + const bool canNotify = !p.notify().isEmpty(); + if (p.isValid() && (isBindable || canNotify)) + return p; + return {}; +} + void QQmlJSImportVisitor::checkSignals() { for (auto it = m_signals.constBegin(); it != m_signals.constEnd(); ++it) { @@ -820,7 +835,32 @@ void QQmlJSImportVisitor::checkSignals() const auto signal = QQmlJSUtils::signalName(pair.first); const QQmlJSScope::ConstPtr signalScope = it.key(); - if (!signal.has_value() || !signalScope->hasMethod(*signal)) { + std::optional signalMethod; + const auto setSignalMethod = [&](const QQmlJSScope::ConstPtr &scope, + const QString &name) { + const auto methods = scope->methods(name, QQmlJSMetaMethod::Signal); + if (!methods.isEmpty()) + signalMethod = methods[0]; + }; + + if (signal.has_value()) { + if (signalScope->hasMethod(*signal)) { + setSignalMethod(signalScope, *signal); + } else if (auto p = propertyForChangeHandler(signalScope, *signal); p.has_value()) { + // we have a change handler of the form "onXChanged" where 'X' + // is a property name + + // NB: qqmltypecompiler prefers signal to bindable + if (auto notify = p->notify(); !notify.isEmpty()) { + setSignalMethod(signalScope, notify); + } else { + Q_ASSERT(!p->bindable().isEmpty()); + signalMethod = QQmlJSMetaMethod {}; // use dummy in this case + } + } + } + + if (!signalMethod.has_value()) { // haven't found anything std::optional fix; // There is a small chance of suggesting this fix for things that are not actually @@ -851,19 +891,7 @@ void QQmlJSImportVisitor::checkSignals() continue; } - QQmlJSMetaMethod scopeSignal; - for (QQmlJSScope::ConstPtr scope = it.key(); scope; scope = scope->baseType()) { - const auto methods = scope->ownMethods(); - const auto methodsRange = methods.equal_range(*signal); - for (auto method = methodsRange.first; method != methodsRange.second; ++method) { - if (method->methodType() != QQmlJSMetaMethod::Signal) - continue; - scopeSignal = *method; - break; - } - } - - const QStringList signalParameters = scopeSignal.parameterNames(); + const QStringList signalParameters = signalMethod->parameterNames(); if (pair.second.length() > signalParameters.length()) { m_logger->logWarning(QStringLiteral("Signal handler for \"%2\" has more formal" @@ -1528,17 +1556,9 @@ bool QQmlJSImportVisitor::visit(UiScriptBinding *scriptBinding) qMakePair(name.toString(), signalParameters) }); QQmlJSMetaMethod scopeSignal; - for (QQmlJSScope::ConstPtr qmlScope = m_savedBindingOuterScope; - qmlScope; qmlScope = qmlScope->baseType()) { - const auto methods = qmlScope->ownMethods(); - const auto methodsRange = methods.equal_range(*signal); - for (auto method = methodsRange.first; method != methodsRange.second; ++method) { - if (method->methodType() != QQmlJSMetaMethod::Signal) - continue; - scopeSignal = *method; - break; - } - } + const auto methods = m_savedBindingOuterScope->methods(*signal, QQmlJSMetaMethod::Signal); + if (!methods.isEmpty()) + scopeSignal = methods[0]; const auto firstSourceLocation = statement->firstSourceLocation(); bool hasMultilineStatementBody = diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp index 39dcf37a54..c60bad1ca5 100644 --- a/src/qmlcompiler/qqmljsscope.cpp +++ b/src/qmlcompiler/qqmljsscope.cpp @@ -149,6 +149,21 @@ QList QQmlJSScope::methods(const QString &name) const return results; } +QList QQmlJSScope::methods(const QString &name, QQmlJSMetaMethod::Type type) const +{ + QList results; + + searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *scope) { + const auto ownMethods = scope->ownMethods(name); + for (const auto &method : ownMethods) { + if (method.methodType() == type) + results.append(method); + } + return false; + }); + return results; +} + bool QQmlJSScope::hasEnumeration(const QString &name) const { return searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *scope) { diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h index 3db743c1e8..242f5671f4 100644 --- a/src/qmlcompiler/qqmljsscope_p.h +++ b/src/qmlcompiler/qqmljsscope_p.h @@ -193,6 +193,7 @@ public: bool hasMethod(const QString &name) const; QList methods(const QString &name) const; + QList methods(const QString &name, QQmlJSMetaMethod::Type type) const; void addOwnEnumeration(const QQmlJSMetaEnum &enumeration) { m_enumerations.insert(enumeration.name(), enumeration); } QHash ownEnumerations() const { return m_enumerations; } diff --git a/tests/auto/qml/qmllint/data/PropertyChangeHandlers/propertychangehandlers.qmltypes b/tests/auto/qml/qmllint/data/PropertyChangeHandlers/propertychangehandlers.qmltypes new file mode 100644 index 0000000000..d16b97dc6c --- /dev/null +++ b/tests/auto/qml/qmllint/data/PropertyChangeHandlers/propertychangehandlers.qmltypes @@ -0,0 +1,65 @@ +import QtQuick.tooling 1.2 +Module { + Component { + file: "typewithproperties.h" + name: "TypeWithProperties" + accessSemantics: "reference" + prototype: "QObject" + exports: ["QmltcTests/TypeWithProperties 1.0"] + exportMetaObjectRevisions: [256] + Property { + name: "a" + type: "double" + bindable: "bindableA" + read: "a" + write: "setA" + index: 0 + } + Property { + name: "b" + type: "QString" + read: "b" + write: "setB" + notify: "bChanged" + index: 1 + } + Property { + name: "c" + type: "QVariant" + read: "c" + write: "setC" + notify: "cWeirdSignal" + index: 2 + } + Property { + name: "d" + type: "QVariant" + bindable: "bindableD" + read: "d" + write: "setD" + notify: "dSignal" + index: 3 + } + Property { + name: "e" + type: "QString" + bindable: "bindableE" + read: "e" + write: "setE" + notify: "eChanged" + index: 3 + } + Signal { name: "bChanged" } + Signal { + name: "cWeirdSignal" + Parameter { type: "QVariant" } + } + Signal { + name: "dSignal" + Parameter { type: "QVariant" } + Parameter { type: "QString" } + } + Signal { name: "eChanged" } + Method { name: "wannabeSignal" } + } +} diff --git a/tests/auto/qml/qmllint/data/PropertyChangeHandlers/qmldir b/tests/auto/qml/qmllint/data/PropertyChangeHandlers/qmldir new file mode 100644 index 0000000000..6adb690e9c --- /dev/null +++ b/tests/auto/qml/qmllint/data/PropertyChangeHandlers/qmldir @@ -0,0 +1,3 @@ +module PropertyChangeHandlers +typeinfo propertychangehandlers.qmltypes +import QtQml diff --git a/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers1.qml b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers1.qml new file mode 100644 index 0000000000..a2d1c14aaa --- /dev/null +++ b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers1.qml @@ -0,0 +1,4 @@ +import PropertyChangeHandlers 1.0 +TypeWithProperties { + onAChanged: function(value) { console.log("error!", value); } +} diff --git a/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers2.qml b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers2.qml new file mode 100644 index 0000000000..8e6ed938b0 --- /dev/null +++ b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers2.qml @@ -0,0 +1,4 @@ +import PropertyChangeHandlers 1.0 +TypeWithProperties { + onBChanged: function(value) { console.log("error!", value); } +} diff --git a/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers3.qml b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers3.qml new file mode 100644 index 0000000000..58b56128d2 --- /dev/null +++ b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers3.qml @@ -0,0 +1,4 @@ +import PropertyChangeHandlers 1.0 +TypeWithProperties { + onXChanged: { console.log("error!"); } +} diff --git a/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers4.qml b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers4.qml new file mode 100644 index 0000000000..6526a46a8c --- /dev/null +++ b/tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers4.qml @@ -0,0 +1,4 @@ +import PropertyChangeHandlers 1.0 +TypeWithProperties { + onWannabeSignal: { console.log("error!"); } +} diff --git a/tests/auto/qml/qmllint/data/goodCppPropertyChangeHandlers.qml b/tests/auto/qml/qmllint/data/goodCppPropertyChangeHandlers.qml new file mode 100644 index 0000000000..1ec2284c7d --- /dev/null +++ b/tests/auto/qml/qmllint/data/goodCppPropertyChangeHandlers.qml @@ -0,0 +1,26 @@ +import PropertyChangeHandlers 1.0 +import QtQuick +Item { + TypeWithProperties { + onAChanged: { console.log("OK"); } + onBChanged: { console.log("OK"); } + onCChanged: { console.log("OK"); } + onDChanged: { console.log("OK"); } + onEChanged: { console.log("OK"); } + } + + TypeWithProperties { + onCWeirdSignal: { console.log("OK"); } + onDSignal: { console.log("OK"); } + } + + TypeWithProperties { + onCWeirdSignal: function(value) { console.log("OK?", value); } + onDSignal: function(value, str) { console.log("OK?", value, str); } + } + + TypeWithProperties { + onCChanged: function(value) { console.log("OK?", value); } + onDChanged: function(value, str) { console.log("OK?", value, str); } + } +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index b5c1d02f80..37b9004517 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -788,6 +788,24 @@ void TestQmllint::dirtyQmlCode_data() << QStringLiteral("Property \"objectName\" not found on type \"Foozle\"") << QStringLiteral("Unqualified access") << QString() << false; + QTest::newRow("cppPropertyChangeHandlers-wrong-parameters-size-bindable") + << QStringLiteral("badCppPropertyChangeHandlers1.qml") + << QStringLiteral("Signal handler for \"onAChanged\" has more formal parameters than " + "the signal it handles") + << QString() << QString() << false; + QTest::newRow("cppPropertyChangeHandlers-wrong-parameters-size-notify") + << QStringLiteral("badCppPropertyChangeHandlers2.qml") + << QStringLiteral("Signal handler for \"onBChanged\" has more formal parameters than " + "the signal it handles") + << QString() << QString() << false; + QTest::newRow("cppPropertyChangeHandlers-no-property") + << QStringLiteral("badCppPropertyChangeHandlers3.qml") + << QStringLiteral("no matching signal found for handler \"onXChanged\"") << QString() + << QString() << false; + QTest::newRow("cppPropertyChangeHandlers-not-a-signal") + << QStringLiteral("badCppPropertyChangeHandlers4.qml") + << QStringLiteral("no matching signal found for handler \"onWannabeSignal\"") + << QString() << QString() << false; } void TestQmllint::dirtyQmlCode() @@ -947,6 +965,8 @@ void TestQmllint::cleanQmlCode_data() QTest::newRow("ID overrides property") << QStringLiteral("accessibleId.qml"); QTest::newRow("matchByName") << QStringLiteral("matchByName.qml"); QTest::newRow("QObject.hasOwnProperty") << QStringLiteral("qobjectHasOwnProperty.qml"); + QTest::newRow("cppPropertyChangeHandlers") + << QStringLiteral("goodCppPropertyChangeHandlers.qml"); } void TestQmllint::cleanQmlCode()