diff options
author | Maximilian Goldstein <max.goldstein@qt.io> | 2021-06-11 14:32:31 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2021-06-17 12:35:02 +0000 |
commit | 0700f03e0163fe2e86c02fda3a7c173cb88faa0e (patch) | |
tree | 5dc94a7d7777f18515b4e11192262bef9514fdee | |
parent | 94063d0c01b1471554773c00c2f6dd4a916fe0ac (diff) |
qmlcompiler: Fully support required properties
Previously support of required properties was limited to detecting whether a property that was required actually exists. Now it also enables us to determine whether or not the required property was ever bound to.
Still limited by the fact we do not fully support script bindings yet.
Fixes: QTBUG-86755
Change-Id: I1abb921d3b4f86a7929f0f829b541088e0c2bf60
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 803a5fda05832b139cc1d76b666777491c708a96)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/imports/tooling/Parameter.qml | 4 | ||||
-rw-r--r-- | src/imports/tooling/Property.qml | 2 | ||||
-rw-r--r-- | src/qmlcompiler/qqmljsimportvisitor.cpp | 59 | ||||
-rw-r--r-- | src/qmlcompiler/qqmljsimportvisitor_p.h | 2 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/requiredProperty.qml | 1 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/requiredPropertyBindings.qml | 24 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/requiredPropertyBindingsLater.qml | 18 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/requiredPropertyBindingsNow.qml | 17 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/tst_qmllint.cpp | 27 |
9 files changed, 146 insertions, 8 deletions
diff --git a/src/imports/tooling/Parameter.qml b/src/imports/tooling/Parameter.qml index a45f9d33ff..073f83c51a 100644 --- a/src/imports/tooling/Parameter.qml +++ b/src/imports/tooling/Parameter.qml @@ -40,7 +40,7 @@ import QML QtObject { - required property string name - required property string type + property string name + property string type property bool isPointer: false } diff --git a/src/imports/tooling/Property.qml b/src/imports/tooling/Property.qml index dd5be3661d..0f1da3928c 100644 --- a/src/imports/tooling/Property.qml +++ b/src/imports/tooling/Property.qml @@ -40,7 +40,7 @@ import QML Member { - required property string type + property string type property bool isPointer: false property bool isReadonly: false property bool isRequired: false diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp index 94af2e6666..4aaf294835 100644 --- a/src/qmlcompiler/qqmljsimportvisitor.cpp +++ b/src/qmlcompiler/qqmljsimportvisitor.cpp @@ -298,7 +298,7 @@ void QQmlJSImportVisitor::endVisit(UiProgram *) resolveAliases(); processDefaultProperties(); processPropertyTypes(); - checkPropertyBindings(); + processPropertyBindings(); checkSignals(); processPropertyBindingObjects(); checkRequiredProperties(); @@ -587,9 +587,58 @@ void QQmlJSImportVisitor::checkRequiredProperties() Log_Required, required.location); } } + + for (const auto &defScope : m_objectDefinitionScopes) { + if (defScope->parentScope() == m_globalScope || defScope->isInlineComponent()) + continue; + + QVector<QQmlJSScope::ConstPtr> scopesToSearch; + for (QQmlJSScope::ConstPtr scope = defScope; scope; scope = scope->baseType()) { + scopesToSearch << scope; + const auto ownProperties = scope->ownProperties(); + for (auto propertyIt = ownProperties.constBegin(); + propertyIt != ownProperties.constEnd(); ++propertyIt) { + const QString propName = propertyIt.key(); + + QQmlJSScope::ConstPtr prevRequiredScope; + for (QQmlJSScope::ConstPtr requiredScope : scopesToSearch) { + if (requiredScope->isPropertyLocallyRequired(propName)) { + bool found = + std::find_if(scopesToSearch.constBegin(), scopesToSearch.constEnd(), + [&](QQmlJSScope::ConstPtr scope) { + return scope->hasPropertyBinding(propName); + }) + != scopesToSearch.constEnd(); + + if (!found) { + const QString propertyScopeName = scopesToSearch.length() > 1 + ? getScopeName(scopesToSearch.at(scopesToSearch.length() - 2), + QQmlJSScope::QMLScope) + : u"here"_qs; + const QString requiredScopeName = prevRequiredScope + ? getScopeName(prevRequiredScope, QQmlJSScope::QMLScope) + : u"here"_qs; + + QString message = + QStringLiteral( + "Component is missing required property %1 from %2") + .arg(propName) + .arg(propertyScopeName); + if (requiredScope != scope) + message += QStringLiteral(" (marked as required by %3)") + .arg(requiredScopeName); + + m_logger.log(message, Log_Required, defScope->sourceLocation()); + } + } + prevRequiredScope = requiredScope; + } + } + } + } } -void QQmlJSImportVisitor::checkPropertyBindings() +void QQmlJSImportVisitor::processPropertyBindings() { for (auto it = m_propertyBindings.constBegin(); it != m_propertyBindings.constEnd(); ++it) { QQmlJSScope::Ptr propertyScope = it.key(); @@ -637,6 +686,12 @@ void QQmlJSImportVisitor::checkPropertyBindings() m_logger.log(message, Log_Deprecation, propertyScope->sourceLocation()); } + + QQmlJSMetaPropertyBinding binding(property); + + // TODO: Actually store the value + + scope->addOwnPropertyBinding(binding); } } } diff --git a/src/qmlcompiler/qqmljsimportvisitor_p.h b/src/qmlcompiler/qqmljsimportvisitor_p.h index 1b7278c2a4..66107f0921 100644 --- a/src/qmlcompiler/qqmljsimportvisitor_p.h +++ b/src/qmlcompiler/qqmljsimportvisitor_p.h @@ -163,7 +163,7 @@ protected: QVector<QQmlJSAnnotation> parseAnnotations(QQmlJS::AST::UiAnnotationList *list); void addDefaultProperties(); void processDefaultProperties(); - void checkPropertyBindings(); + void processPropertyBindings(); void checkRequiredProperties(); void processPropertyTypes(); void processPropertyBindingObjects(); diff --git a/tests/auto/qml/qmllint/data/requiredProperty.qml b/tests/auto/qml/qmllint/data/requiredProperty.qml index b10f0f67cd..6e0eec54e7 100644 --- a/tests/auto/qml/qmllint/data/requiredProperty.qml +++ b/tests/auto/qml/qmllint/data/requiredProperty.qml @@ -3,4 +3,5 @@ import QtQml 2.15 QtObject { property int x required x + x: 5 } diff --git a/tests/auto/qml/qmllint/data/requiredPropertyBindings.qml b/tests/auto/qml/qmllint/data/requiredPropertyBindings.qml new file mode 100644 index 0000000000..3bba84f412 --- /dev/null +++ b/tests/auto/qml/qmllint/data/requiredPropertyBindings.qml @@ -0,0 +1,24 @@ +import QtQuick 2.0 + +Item { + component Base : Item { + required property string required_now_string + property string required_later_string + + required property QtObject required_now_object + property QtObject required_later_object + } + + component Derived : Base { + required required_later_string + required required_later_object + } + + Derived { + required_now_string: "" + required_later_string: "" + + required_now_object: QtObject {} + required_later_object: QtObject {} + } +} diff --git a/tests/auto/qml/qmllint/data/requiredPropertyBindingsLater.qml b/tests/auto/qml/qmllint/data/requiredPropertyBindingsLater.qml new file mode 100644 index 0000000000..65c59d7161 --- /dev/null +++ b/tests/auto/qml/qmllint/data/requiredPropertyBindingsLater.qml @@ -0,0 +1,18 @@ +import QtQuick 2.0 + +Item { + component Base : Item { + required property string required_now_string + property string required_later_string + property string required_even_later_string + } + + component Derived : Base { + required required_later_string + } + + Derived { + required required_even_later_string + required_now_string: "" + } +} diff --git a/tests/auto/qml/qmllint/data/requiredPropertyBindingsNow.qml b/tests/auto/qml/qmllint/data/requiredPropertyBindingsNow.qml new file mode 100644 index 0000000000..701760b4e7 --- /dev/null +++ b/tests/auto/qml/qmllint/data/requiredPropertyBindingsNow.qml @@ -0,0 +1,17 @@ +import QtQuick 2.0 + +Item { + component Base : Item { + required property string required_now_string + property string required_later_string + } + + component Derived : Base { + required required_later_string + } + + Derived { + required property string required_defined_here_string + required_later_string: "" + } +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index 5c307d9fb8..4c40062252 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -828,8 +828,31 @@ void TestQmllint::requiredProperty() { QVERIFY(runQmllint("requiredProperty.qml", true).isEmpty()); - const QString errors = runQmllint("requiredMissingProperty.qml", false); - QVERIFY(errors.contains(QStringLiteral("Property \"foo\" was marked as required but does not exist."))); + { + const QString errors = runQmllint("requiredMissingProperty.qml", false); + QVERIFY(errors.contains( + QStringLiteral("Property \"foo\" was marked as required but does not exist."))); + } + + QVERIFY(runQmllint("requiredPropertyBindings.qml", true).isEmpty()); + + { + const QString errors = runQmllint("requiredPropertyBindingsNow.qml", false); + QVERIFY(errors.contains(QStringLiteral( + "Component is missing required property required_now_string from Base"))); + QVERIFY(errors.contains(QStringLiteral( + "Component is missing required property required_defined_here_string from here"))); + } + + { + const QString errors = runQmllint("requiredPropertyBindingsLater.qml", false); + QVERIFY(errors.contains( + QStringLiteral("Component is missing required property required_later_string from " + "Base (marked as required by Derived)"))); + QVERIFY(errors.contains( + QStringLiteral("Component is missing required property required_even_later_string " + "from Base (marked as required by here)"))); + } } void TestQmllint::settingsFile() |