From 120eb155ef1f0da51c473b080880169a85e5ceb2 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 18 Feb 2021 11:40:31 +0100 Subject: qmllint: Check for existence of property types For each binding there should be a property and that property should have a type we recognize. Enums can be property types in C++. We support this by adding child scopes for such enums. The child scopes are then referenced by the QQmlJSMetaEnums and derive from int. The test then reveals that we were missing a few properties in QtQuick.tooling. Add those. Change-Id: I1deef94393ee0e17d34c2dc5980ebfbf25417f36 Reviewed-by: Fabian Kosmale (cherry picked from commit 08c8e8ac3ba8eb23ae5c158990f5d029ac9988ed) --- src/imports/tooling/Component.qml | 1 + src/imports/tooling/Parameter.qml | 1 + src/qmlcompiler/qqmljsimporter.cpp | 2 +- src/qmlcompiler/qqmljsimportvisitor.cpp | 12 +++--- src/qmlcompiler/qqmljsmetatypes_p.h | 9 ++++- src/qmlcompiler/qqmljsscope.cpp | 54 +++++++++++++++++-------- src/qmlcompiler/qqmljsscope_p.h | 6 ++- tests/auto/qml/qmllint/data/badBinding.qml | 5 +++ tests/auto/qml/qmllint/data/badPropertyType.qml | 6 +++ tests/auto/qml/qmllint/data/enumProperty.qml | 5 +++ tests/auto/qml/qmllint/tst_qmllint.cpp | 11 +++++ tools/qmllint/findwarnings.cpp | 35 +++++++++++++++- 12 files changed, 119 insertions(+), 28 deletions(-) create mode 100644 tests/auto/qml/qmllint/data/badBinding.qml create mode 100644 tests/auto/qml/qmllint/data/badPropertyType.qml create mode 100644 tests/auto/qml/qmllint/data/enumProperty.qml diff --git a/src/imports/tooling/Component.qml b/src/imports/tooling/Component.qml index 786e4a99df..942d838900 100644 --- a/src/imports/tooling/Component.qml +++ b/src/imports/tooling/Component.qml @@ -55,4 +55,5 @@ QtObject { property bool isCreatable: name.length > 0 property bool isComposite: false property string accessSemantics: "reference" + property string defaultProperty } diff --git a/src/imports/tooling/Parameter.qml b/src/imports/tooling/Parameter.qml index 108a80098f..a45f9d33ff 100644 --- a/src/imports/tooling/Parameter.qml +++ b/src/imports/tooling/Parameter.qml @@ -42,4 +42,5 @@ import QML QtObject { required property string name required property string type + property bool isPointer: false } diff --git a/src/qmlcompiler/qqmljsimporter.cpp b/src/qmlcompiler/qqmljsimporter.cpp index 823c0e6f2c..f03c32e2cf 100644 --- a/src/qmlcompiler/qqmljsimporter.cpp +++ b/src/qmlcompiler/qqmljsimporter.cpp @@ -235,7 +235,7 @@ void QQmlJSImporter::processImport( for (auto it = import.objects.begin(); it != import.objects.end(); ++it) { const auto &val = it.value(); if (val->baseType().isNull()) // Otherwise we have already done it in localFile2ScopeTree() - val->resolveTypes(types->cppNames); + QQmlJSScope::resolveTypes(val, types->cppNames); } } diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp index 11ab47e455..08cd9d254c 100644 --- a/src/qmlcompiler/qqmljsimportvisitor.cpp +++ b/src/qmlcompiler/qqmljsimportvisitor.cpp @@ -129,13 +129,13 @@ bool QQmlJSImportVisitor::visit(UiObjectDefinition *definition) if (!m_exportedRootScope) m_exportedRootScope = m_currentScope; - m_currentScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); return true; } void QQmlJSImportVisitor::endVisit(UiObjectDefinition *) { - m_currentScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); leaveEnvironment(); } @@ -494,13 +494,13 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob) enterEnvironment(QQmlJSScope::QMLScope, name, uiob->qualifiedTypeNameId->identifierToken); - m_currentScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); return true; } void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob) { - m_currentScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); const QQmlJSScope::ConstPtr childScope = m_currentScope; leaveEnvironment(); @@ -541,7 +541,7 @@ bool QQmlJSImportVisitor::visit(ESModule *module) void QQmlJSImportVisitor::endVisit(ESModule *) { - m_exportedRootScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports); } bool QQmlJSImportVisitor::visit(Program *) @@ -556,7 +556,7 @@ bool QQmlJSImportVisitor::visit(Program *) void QQmlJSImportVisitor::endVisit(Program *) { - m_exportedRootScope->resolveTypes(m_rootScopeImports); + QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports); } QT_END_NAMESPACE diff --git a/src/qmlcompiler/qqmljsmetatypes_p.h b/src/qmlcompiler/qqmljsmetatypes_p.h index 0ab56e9503..57328af836 100644 --- a/src/qmlcompiler/qqmljsmetatypes_p.h +++ b/src/qmlcompiler/qqmljsmetatypes_p.h @@ -62,6 +62,7 @@ class QQmlJSMetaEnum QList m_values; // empty if values unknown. QString m_name; QString m_alias; + QSharedPointer m_type; bool m_isFlag = false; public: @@ -89,13 +90,17 @@ public: int value(const QString &key) const { return m_values.value(m_keys.indexOf(key)); } bool hasKey(const QString &key) const { return m_keys.indexOf(key) != -1; } + QSharedPointer type() const { return m_type; } + void setType(const QSharedPointer &type) { m_type = type; } + friend bool operator==(const QQmlJSMetaEnum &a, const QQmlJSMetaEnum &b) { return a.m_keys == b.m_keys && a.m_values == b.m_values && a.m_name == b.m_name && a.m_alias == b.m_alias - && a.m_isFlag == b.m_isFlag; + && a.m_isFlag == b.m_isFlag + && a.m_type == b.m_type; } friend bool operator!=(const QQmlJSMetaEnum &a, const QQmlJSMetaEnum &b) @@ -105,7 +110,7 @@ public: friend size_t qHash(const QQmlJSMetaEnum &e, size_t seed = 0) { - return qHashMulti(seed, e.m_keys, e.m_values, e.m_name, e.m_alias, e.m_isFlag); + return qHashMulti(seed, e.m_keys, e.m_values, e.m_name, e.m_alias, e.m_isFlag, e.m_type); } }; diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp index 64fa44ef82..5c3ce8d139 100644 --- a/src/qmlcompiler/qqmljsscope.cpp +++ b/src/qmlcompiler/qqmljsscope.cpp @@ -190,7 +190,8 @@ QQmlJSScope::findJSIdentifier(const QString &id) const return std::optional{}; } -void QQmlJSScope::resolveTypes(const QHash &contextualTypes) +void QQmlJSScope::resolveTypes(const QQmlJSScope::Ptr &self, + const QHash &contextualTypes) { auto findType = [&](const QString &name) { auto type = contextualTypes.constFind(name); @@ -200,25 +201,46 @@ void QQmlJSScope::resolveTypes(const QHash &cont return QQmlJSScope::ConstPtr(); }; - if (!m_baseType && !m_baseTypeName.isEmpty()) - m_baseType = findType(m_baseTypeName); + if (!self->m_baseType && !self->m_baseTypeName.isEmpty()) + self->m_baseType = findType(self->m_baseTypeName); - if (!m_attachedType && !m_attachedTypeName.isEmpty()) - m_attachedType = findType(m_attachedTypeName); + if (!self->m_attachedType && !self->m_attachedTypeName.isEmpty()) + self->m_attachedType = findType(self->m_attachedTypeName); - if (!m_valueType && !m_valueTypeName.isEmpty()) - m_valueType = findType(m_valueTypeName); + if (!self->m_valueType && !self->m_valueTypeName.isEmpty()) + self->m_valueType = findType(self->m_valueTypeName); - if (!m_extensionType && !m_extensionTypeName.isEmpty()) - m_extensionType = findType(m_extensionTypeName); + if (!self->m_extensionType && !self->m_extensionTypeName.isEmpty()) + self->m_extensionType = findType(self->m_extensionTypeName); - for (auto it = m_properties.begin(), end = m_properties.end(); it != end; ++it) { + const auto intType = findType(QStringLiteral("int")); + Q_ASSERT(intType); // There always has to be a builtin "int" type + for (auto it = self->m_enumerations.begin(), end = self->m_enumerations.end(); + it != end; ++it) { + auto enumScope = QQmlJSScope::create(EnumScope, self); + enumScope->m_baseTypeName = QStringLiteral("int"); + enumScope->m_baseType = intType; + enumScope->m_semantics = AccessSemantics::Value; + enumScope->m_internalName = self->internalName() + QStringLiteral("::") + it->name(); + it->setType(ConstPtr(enumScope)); + } + + for (auto it = self->m_properties.begin(), end = self->m_properties.end(); it != end; ++it) { const QString typeName = it->typeName(); - if (!it->type() && !typeName.isEmpty()) - it->setType(findType(typeName)); + if (it->type() || typeName.isEmpty()) + continue; + + if (const auto type = findType(typeName)) { + it->setType(type); + continue; + } + + const auto enumeration = self->m_enumerations.find(typeName); + if (enumeration != self->m_enumerations.end()) + it->setType(enumeration->type()); } - for (auto it = m_methods.begin(), end = m_methods.end(); it != end; ++it) { + for (auto it = self->m_methods.begin(), end = self->m_methods.end(); it != end; ++it) { const QString returnTypeName = it->returnTypeName(); if (!it->returnType() && !returnTypeName.isEmpty()) it->setReturnType(findType(returnTypeName)); @@ -238,11 +260,11 @@ void QQmlJSScope::resolveTypes(const QHash &cont it->setParameterTypes(paramTypes); } - for (auto it = m_childScopes.begin(), end = m_childScopes.end(); it != end; ++it) { + for (auto it = self->m_childScopes.begin(), end = self->m_childScopes.end(); it != end; ++it) { QQmlJSScope::Ptr childScope = *it; switch (childScope->scopeType()) { case QQmlJSScope::GroupedPropertyScope: - searchBaseAndExtensionTypes(this, [&](const QQmlJSScope *type) { + searchBaseAndExtensionTypes(self.data(), [&](const QQmlJSScope *type) { const auto propertyIt = type->m_properties.find(childScope->internalName()); if (propertyIt != type->m_properties.end()) { childScope->m_baseType = QQmlJSScope::ConstPtr(propertyIt->type()); @@ -261,7 +283,7 @@ void QQmlJSScope::resolveTypes(const QHash &cont default: break; } - childScope->resolveTypes(contextualTypes); + resolveTypes(childScope, contextualTypes); } } diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h index 8f095cffbf..cbfc109154 100644 --- a/src/qmlcompiler/qqmljsscope_p.h +++ b/src/qmlcompiler/qqmljsscope_p.h @@ -96,7 +96,8 @@ public: JSLexicalScope, QMLScope, GroupedPropertyScope, - AttachedPropertyScope + AttachedPropertyScope, + EnumScope }; enum class AccessSemantics { @@ -260,7 +261,8 @@ public: return result; } - void resolveTypes(const QHash &contextualTypes); + static void resolveTypes(const QQmlJSScope::Ptr &self, + const QHash &contextualTypes); void setSourceLocation(const QQmlJS::SourceLocation &sourceLocation) { diff --git a/tests/auto/qml/qmllint/data/badBinding.qml b/tests/auto/qml/qmllint/data/badBinding.qml new file mode 100644 index 0000000000..8e781710a2 --- /dev/null +++ b/tests/auto/qml/qmllint/data/badBinding.qml @@ -0,0 +1,5 @@ +import QtQml + +QtObject { + doesNotExist: 12 +} diff --git a/tests/auto/qml/qmllint/data/badPropertyType.qml b/tests/auto/qml/qmllint/data/badPropertyType.qml new file mode 100644 index 0000000000..97484ea0bb --- /dev/null +++ b/tests/auto/qml/qmllint/data/badPropertyType.qml @@ -0,0 +1,6 @@ +import QtQml + +QtObject { + property badtype bad + bad: "abc" +} diff --git a/tests/auto/qml/qmllint/data/enumProperty.qml b/tests/auto/qml/qmllint/data/enumProperty.qml new file mode 100644 index 0000000000..d385346c87 --- /dev/null +++ b/tests/auto/qml/qmllint/data/enumProperty.qml @@ -0,0 +1,5 @@ +import QtQuick.Layouts + +GridLayout { + flow: GridLayout.TopToBottom +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index a86c202790..db44193871 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -306,6 +306,16 @@ void TestQmllint::dirtyQmlCode_data() << QStringLiteral("badAttached.qml") << QStringLiteral("unknown attached property scope WrongAttached.") << QString(); + QTest::newRow("BadBinding") + << QStringLiteral("badBinding.qml") + << QStringLiteral("Binding assigned to \"doesNotExist\", but no property " + "\"doesNotExist\" exists in the current element.") + << QString(); + QTest::newRow("BadPropertyType") + << QStringLiteral("badPropertyType.qml") + << QStringLiteral("No type found for property \"bad\". This may be due to a missing " + "import statement or incomplete qmltypes files.") + << QString(); } void TestQmllint::dirtyQmlCode() @@ -371,6 +381,7 @@ void TestQmllint::cleanQmlCode_data() QTest::newRow("grouped scope failure") << QStringLiteral("groupedScope.qml"); QTest::newRow("layouts depends quick") << QStringLiteral("layouts.qml"); QTest::newRow("attached") << QStringLiteral("attached.qml"); + QTest::newRow("enumProperty") << QStringLiteral("enumProperty.qml"); } void TestQmllint::cleanQmlCode() diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp index c238f874e2..b23d517d41 100644 --- a/tools/qmllint/findwarnings.cpp +++ b/tools/qmllint/findwarnings.cpp @@ -218,8 +218,41 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) } const QString signal = signalName(name); - if (signal.isEmpty()) + if (signal.isEmpty()) { + for (const auto &childScope : qmlScope->childScopes()) { + if ((childScope->scopeType() == QQmlJSScope::AttachedPropertyScope + || childScope->scopeType() == QQmlJSScope::GroupedPropertyScope) + && childScope->internalName() == name) { + return true; + } + } + + if (!qmlScope->hasProperty(name.toString())) { + m_errors.append({ + QStringLiteral("Binding assigned to \"%1\", but no property \"%1\" " + "exists in the current element.\n").arg(name), + QtWarningMsg, + uisb->firstSourceLocation() + }); + m_visitFailed = true; + return true; + } + + const auto property = qmlScope->property(name.toString()); + if (!property.type()) { + m_errors.append({ + QStringLiteral("No type found for property \"%1\". This may be due " + "to a missing import statement or incomplete " + "qmltypes files.\n").arg(name), + QtWarningMsg, + uisb->firstSourceLocation() + }); + m_visitFailed = true; + + } + return true; + } if (!qmlScope->hasMethod(signal) && m_warnUnqualified) { -- cgit v1.2.3