qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

QQmlJSImportVisitor: Fix property change handler detection logic 

We can have weird cases like `onXChanged: {}` with x's NOTIFY named
xNotReallyChanged and those should still function correctly; bindable
properties without notify can also use property change handlers

Picking to 6.3 as this is pretty much a bug + qmltc needs this as well
to support the weird cases (as otherwise it'll just fail in the visitor)

Pick-to: 6.3
Change-Id: Ib9a1ce8b7d76133a89bcf0dab16f25659ce69c2b
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@qt.io>
This commit is contained in:
Andrei Golubev 2021-12-17 12:57:01 +01:00
parent ee05e35ad3
commit 72efa7f981
11 changed files with 191 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/qmlcompiler/qqmljsimportvisitor.cpp
View File

@ -811,6 +811,21 @@ void QQmlJSImportVisitor::processPropertyBindings()
}
}
static std::optional<QQmlJSMetaProperty>
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<QQmlJSMetaMethod> 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<FixSuggestion> 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 =

15
src/qmlcompiler/qqmljsscope.cpp
View File

@ -149,6 +149,21 @@ QList<QQmlJSMetaMethod> QQmlJSScope::methods(const QString &name) const
return results;
}
QList<QQmlJSMetaMethod> QQmlJSScope::methods(const QString &name, QQmlJSMetaMethod::Type type) const
{
QList<QQmlJSMetaMethod> 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) {

1
src/qmlcompiler/qqmljsscope_p.h
View File

@ -193,6 +193,7 @@ public:
bool hasMethod(const QString &name) const;
QList<QQmlJSMetaMethod> methods(const QString &name) const;
QList<QQmlJSMetaMethod> methods(const QString &name, QQmlJSMetaMethod::Type type) const;
void addOwnEnumeration(const QQmlJSMetaEnum &enumeration) { m_enumerations.insert(enumeration.name(), enumeration); }
QHash<QString, QQmlJSMetaEnum> ownEnumerations() const { return m_enumerations; }

65
tests/auto/qml/qmllint/data/PropertyChangeHandlers/propertychangehandlers.qmltypes Normal file
View File

@ -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" }
}
}

3
tests/auto/qml/qmllint/data/PropertyChangeHandlers/qmldir Normal file
View File

@ -0,0 +1,3 @@
module PropertyChangeHandlers
typeinfo propertychangehandlers.qmltypes
import QtQml

4
tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers1.qml Normal file
View File

@ -0,0 +1,4 @@
import PropertyChangeHandlers 1.0
TypeWithProperties {
onAChanged: function(value) { console.log("error!", value); }
}

4
tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers2.qml Normal file
View File

@ -0,0 +1,4 @@
import PropertyChangeHandlers 1.0
TypeWithProperties {
onBChanged: function(value) { console.log("error!", value); }
}

4
tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers3.qml Normal file
View File

@ -0,0 +1,4 @@
import PropertyChangeHandlers 1.0
TypeWithProperties {
onXChanged: { console.log("error!"); }
}

4
tests/auto/qml/qmllint/data/badCppPropertyChangeHandlers4.qml Normal file
View File

@ -0,0 +1,4 @@
import PropertyChangeHandlers 1.0
TypeWithProperties {
onWannabeSignal: { console.log("error!"); }
}

26
tests/auto/qml/qmllint/data/goodCppPropertyChangeHandlers.qml Normal file
View File

@ -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); }
}
}

20
tests/auto/qml/qmllint/tst_qmllint.cpp
View File

@ -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()