diff options
Diffstat (limited to 'src/plugins/qmllint/quick/quicklintplugin.cpp')
| -rw-r--r-- | src/plugins/qmllint/quick/quicklintplugin.cpp | 392 |
1 files changed, 275 insertions, 117 deletions
diff --git a/src/plugins/qmllint/quick/quicklintplugin.cpp b/src/plugins/qmllint/quick/quicklintplugin.cpp index 3b89635748..52d4d759e3 100644 --- a/src/plugins/qmllint/quick/quicklintplugin.cpp +++ b/src/plugins/qmllint/quick/quicklintplugin.cpp @@ -3,17 +3,18 @@ #include "quicklintplugin.h" -#include <QtQmlCompiler/private/qqmljslogger_p.h> - QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; -static constexpr LoggerWarningId quickLayoutPositioning { "Quick.layout-positioning" }; -static constexpr LoggerWarningId quickAttachedPropertyType { "Quick.attached-property-type" }; -static constexpr LoggerWarningId quickControlsNativeCustomize { "Quick.controls-native-customize" }; -static constexpr LoggerWarningId quickAnchorCombinations { "Quick.anchor-combinations" }; -static constexpr LoggerWarningId quickUnexpectedVarType { "Quick.unexpected-var-type" }; +static constexpr QQmlSA::LoggerWarningId quickLayoutPositioning { "Quick.layout-positioning" }; +static constexpr QQmlSA::LoggerWarningId quickAttachedPropertyType { "Quick.attached-property-type" }; +static constexpr QQmlSA::LoggerWarningId quickControlsNativeCustomize { "Quick.controls-native-customize" }; +static constexpr QQmlSA::LoggerWarningId quickAnchorCombinations { "Quick.anchor-combinations" }; +static constexpr QQmlSA::LoggerWarningId quickUnexpectedVarType { "Quick.unexpected-var-type" }; +static constexpr QQmlSA::LoggerWarningId quickPropertyChangesParsed { "Quick.property-changes-parsed" }; +static constexpr QQmlSA::LoggerWarningId quickControlsAttachedPropertyReuse { "Quick.controls-attached-property-reuse" }; +static constexpr QQmlSA::LoggerWarningId quickAttachedPropertyReuse { "Quick.attached-property-reuse" }; ForbiddenChildrenPropertyValidatorPass::ForbiddenChildrenPropertyValidatorPass( QQmlSA::PassManager *manager) @@ -33,11 +34,11 @@ void ForbiddenChildrenPropertyValidatorPass::addWarning(QAnyStringView moduleNam bool ForbiddenChildrenPropertyValidatorPass::shouldRun(const QQmlSA::Element &element) { - if (!element->parentScope()) + if (!element.parentScope()) return false; - for (const auto pair : m_types.asKeyValueRange()) { - if (element->parentScope()->inherits(pair.first)) + for (const auto &pair : std::as_const(m_types).asKeyValueRange()) { + if (element.parentScope().inherits(pair.first)) return true; } @@ -46,18 +47,18 @@ bool ForbiddenChildrenPropertyValidatorPass::shouldRun(const QQmlSA::Element &el void ForbiddenChildrenPropertyValidatorPass::run(const QQmlSA::Element &element) { - for (const auto elementPair : m_types.asKeyValueRange()) { + for (const auto &elementPair : std::as_const(m_types).asKeyValueRange()) { const QQmlSA::Element &type = elementPair.first; - if (!element->parentScope()->inherits(type)) + if (!element.parentScope().inherits(type)) continue; for (const auto &warning : elementPair.second) { - if (!element->hasOwnPropertyBindings(warning.propertyName)) + if (!element.hasOwnPropertyBindings(warning.propertyName)) continue; - auto bindings = element->ownPropertyBindings(warning.propertyName); - - emitWarning(warning.message, quickLayoutPositioning, bindings.first->sourceLocation()); + const auto bindings = element.ownPropertyBindings(warning.propertyName); + const auto firstBinding = bindings.constBegin().value(); + emitWarning(warning.message, quickLayoutPositioning, firstBinding.sourceLocation()); } break; } @@ -75,8 +76,7 @@ QString AttachedPropertyTypeValidatorPass::addWarning(TypeDescription attachType QVarLengthArray<QQmlSA::Element, 4> elements; const QQmlSA::Element baseType = resolveType(attachType.module, attachType.name); - - QString typeName = baseType->attachedTypeName(); + const QQmlSA::Element attachedType = resolveAttached(attachType.module, attachType.name); for (const TypeDescription &desc : allowedTypes) { const QQmlSA::Element type = resolveType(desc.module, desc.name); @@ -85,36 +85,39 @@ QString AttachedPropertyTypeValidatorPass::addWarning(TypeDescription attachType elements.push_back(type); } - m_attachedTypes.insert({ std::make_pair<>( - typeName, Warning { elements, allowInDelegate, warning.toString() }) }); + m_attachedTypes.insert( + { std::make_pair<>(attachedType.internalId(), + Warning{ elements, allowInDelegate, warning.toString() }) }); - return typeName; + return attachedType.internalId(); } void AttachedPropertyTypeValidatorPass::checkWarnings(const QQmlSA::Element &element, const QQmlSA::Element &scopeUsedIn, - const QQmlJS::SourceLocation &location) + const QQmlSA::SourceLocation &location) { - auto warning = m_attachedTypes.constFind(element->internalName()); + auto warning = m_attachedTypes.constFind(element.internalId()); if (warning == m_attachedTypes.cend()) return; for (const QQmlSA::Element &type : warning->allowedTypes) { - if (scopeUsedIn->inherits(type)) + if (scopeUsedIn.inherits(type)) return; } if (warning->allowInDelegate) { - if (scopeUsedIn->isPropertyRequired(u"index"_s) - || scopeUsedIn->isPropertyRequired(u"model"_s)) + if (scopeUsedIn.isPropertyRequired(u"index"_s) + || scopeUsedIn.isPropertyRequired(u"model"_s)) return; // If the scope is at the root level, we cannot know whether it will be used // as a delegate or not. - if (!scopeUsedIn->parentScope() || scopeUsedIn->parentScope()->internalName() == u"global"_s) - return; + // ### TODO: add a method to check whether a scope is the global scope + // so that we do not need to use internalId + if (!scopeUsedIn.parentScope() || scopeUsedIn.parentScope().internalId() == u"global"_s) + return; - for (const QQmlJSMetaPropertyBinding &binding : - scopeUsedIn->parentScope()->propertyBindings(u"delegate"_s)) { + for (const QQmlSA::Binding &binding : + scopeUsedIn.parentScope().propertyBindings(u"delegate"_s)) { if (!binding.hasObject()) continue; if (binding.objectType() == scopeUsedIn) @@ -127,26 +130,31 @@ void AttachedPropertyTypeValidatorPass::checkWarnings(const QQmlSA::Element &ele void AttachedPropertyTypeValidatorPass::onBinding(const QQmlSA::Element &element, const QString &propertyName, - const QQmlJSMetaPropertyBinding &binding, + const QQmlSA::Binding &binding, const QQmlSA::Element &bindingScope, const QQmlSA::Element &value) { - Q_UNUSED(element) - Q_UNUSED(propertyName) - Q_UNUSED(bindingScope) Q_UNUSED(value) - checkWarnings(bindingScope->baseType(), element, binding.sourceLocation()); + // We can only analyze simple attached bindings since we don't see + // the grouped and attached properties that lead up to this here. + // + // TODO: This is very crude. + // We should add API for grouped and attached properties. + if (propertyName.count(QLatin1Char('.')) > 1) + return; + + checkWarnings(bindingScope.baseType(), element, binding.sourceLocation()); } void AttachedPropertyTypeValidatorPass::onRead(const QQmlSA::Element &element, const QString &propertyName, const QQmlSA::Element &readScope, - QQmlJS::SourceLocation location) + QQmlSA::SourceLocation location) { // If the attachment does not have such a property or method then // it's either a more general error or an enum. Enums are fine. - if (element->hasProperty(propertyName) || element->hasMethod(propertyName)) + if (element.hasProperty(propertyName) || element.hasMethod(propertyName)) checkWarnings(element, readScope, location); } @@ -154,7 +162,7 @@ void AttachedPropertyTypeValidatorPass::onWrite(const QQmlSA::Element &element, const QString &propertyName, const QQmlSA::Element &value, const QQmlSA::Element &writeScope, - QQmlJS::SourceLocation location) + QQmlSA::SourceLocation location) { Q_UNUSED(propertyName) Q_UNUSED(value) @@ -177,7 +185,6 @@ ControlsNativeValidatorPass::ControlsNativeValidatorPass(QQmlSA::PassManager *ma QStringList { "background", "contentItem", "header", "footer", "menuBar" } }, ControlElement { "ComboBox", QStringList { "indicator" } }, ControlElement { "Dial", QStringList { "handle" } }, - ControlElement { "Dialog", QStringList { "header", "footer" } }, ControlElement { "GroupBox", QStringList { "label" } }, ControlElement { "$internal$.QQuickIndicatorButton", QStringList { "indicator" }, false }, ControlElement { "Label", QStringList { "background" } }, @@ -205,7 +212,7 @@ ControlsNativeValidatorPass::ControlsNativeValidatorPass(QQmlSA::PassManager *ma if (type.isNull()) continue; - element.inheritsControl = !element.isControl && type->inherits(control); + element.inheritsControl = !element.isControl && type.inherits(control); element.element = type; } @@ -221,7 +228,7 @@ bool ControlsNativeValidatorPass::shouldRun(const QQmlSA::Element &element) // If our element inherits control, we don't have to individually check for them here. if (controlElement.inheritsControl) continue; - if (element->inherits(controlElement.element)) + if (element.inherits(controlElement.element)) return true; } return false; @@ -230,16 +237,16 @@ bool ControlsNativeValidatorPass::shouldRun(const QQmlSA::Element &element) void ControlsNativeValidatorPass::run(const QQmlSA::Element &element) { for (const ControlElement &controlElement : m_elements) { - if (element->inherits(controlElement.element)) { + if (element.inherits(controlElement.element)) { for (const QString &propertyName : controlElement.restrictedProperties) { - if (element->hasOwnPropertyBindings(propertyName)) { + if (element.hasOwnPropertyBindings(propertyName)) { emitWarning(QStringLiteral("Not allowed to override \"%1\" because native " "styles cannot be customized: See " "https://doc-snapshots.qt.io/qt6-dev/" "qtquickcontrols-customize.html#customization-" "reference for more information.") .arg(propertyName), - quickControlsNativeCustomize, element->sourceLocation()); + quickControlsNativeCustomize, element.sourceLocation()); } } // Since all the different types we have rules for don't inherit from each other (except @@ -253,14 +260,14 @@ void ControlsNativeValidatorPass::run(const QQmlSA::Element &element) AnchorsValidatorPass::AnchorsValidatorPass(QQmlSA::PassManager *manager) : QQmlSA::ElementPass(manager) + , m_item(resolveType("QtQuick", "Item")) { - m_item = resolveType("QtQuick", "Item"); } bool AnchorsValidatorPass::shouldRun(const QQmlSA::Element &element) { - return !m_item.isNull() && element->inherits(m_item) - && element->hasOwnPropertyBindings(u"anchors"_s); + return !m_item.isNull() && element.inherits(m_item) + && element.hasOwnPropertyBindings(u"anchors"_s); } void AnchorsValidatorPass::run(const QQmlSA::Element &element) @@ -272,21 +279,22 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) u"top"_s, u"bottom"_s, u"verticalCenter"_s, u"baseline"_s }; - QList<QQmlJSMetaPropertyBinding> anchorBindings = element->propertyBindings(u"anchors"_s); + QList<QQmlSA::Binding> anchorBindings = element.propertyBindings(u"anchors"_s); for (qsizetype i = anchorBindings.size() - 1; i >= 0; i--) { auto groupType = anchorBindings[i].groupType(); - if (groupType == nullptr) + if (groupType.isNull()) continue; for (const QString &name : properties) { - auto pair = groupType->ownPropertyBindings(name); - if (pair.first == pair.second) + + const auto &propertyBindings = groupType.ownPropertyBindings(name); + if (propertyBindings.begin() == propertyBindings.end()) continue; + bool isUndefined = false; - for (auto it = pair.first; it != pair.second; it++) { - if (it->bindingType() == QQmlJSMetaPropertyBinding::Script - && it->scriptValueType() == QQmlJSMetaPropertyBinding::ScriptValue_Undefined) { + for (const auto &propertyBinding : propertyBindings) { + if (propertyBinding.hasUndefinedScriptValue()) { isUndefined = true; break; } @@ -299,14 +307,15 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) } } - auto ownSourceLocation = [&](QStringList properties) { - QQmlJS::SourceLocation warnLoc; + auto ownSourceLocation = [&](QStringList properties) -> QQmlSA::SourceLocation { + QQmlSA::SourceLocation warnLoc; + for (const QString &name : properties) { if (bindings[name] & Own) { - QQmlSA::Element groupType = anchorBindings[0].groupType(); - auto bindingRange = groupType->ownPropertyBindings(name); - Q_ASSERT(bindingRange.first != bindingRange.second); - warnLoc = bindingRange.first->sourceLocation(); + QQmlSA::Element groupType = QQmlSA::Element{ anchorBindings[0].groupType() }; + auto bindings = groupType.ownPropertyBindings(name); + Q_ASSERT(bindings.begin() != bindings.end()); + warnLoc = bindings.begin().value().sourceLocation(); break; } } @@ -314,7 +323,7 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) }; if ((bindings[u"left"_s] & bindings[u"right"_s] & bindings[u"horizontalCenter"_s]) & Exists) { - QQmlJS::SourceLocation warnLoc = + QQmlSA::SourceLocation warnLoc = ownSourceLocation({ u"left"_s, u"right"_s, u"horizontalCenter"_s }); if (warnLoc.isValid()) { @@ -325,7 +334,7 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) } if ((bindings[u"top"_s] & bindings[u"bottom"_s] & bindings[u"verticalCenter"_s]) & Exists) { - QQmlJS::SourceLocation warnLoc = + QQmlSA::SourceLocation warnLoc = ownSourceLocation({ u"top"_s, u"bottom"_s, u"verticalCenter"_s }); if (warnLoc.isValid()) { emitWarning("Cannot specify top, bottom, and verticalCenter anchors at the same time.", @@ -335,7 +344,7 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) if ((bindings[u"baseline"_s] & (bindings[u"bottom"_s] | bindings[u"verticalCenter"_s])) & Exists) { - QQmlJS::SourceLocation warnLoc = + QQmlSA::SourceLocation warnLoc = ownSourceLocation({ u"baseline"_s, u"bottom"_s, u"verticalCenter"_s }); if (warnLoc.isValid()) { emitWarning("Baseline anchor cannot be used in conjunction with top, bottom, or " @@ -347,37 +356,36 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element) ControlsSwipeDelegateValidatorPass::ControlsSwipeDelegateValidatorPass(QQmlSA::PassManager *manager) : QQmlSA::ElementPass(manager) + , m_swipeDelegate(resolveType("QtQuick.Controls", "SwipeDelegate")) { - m_swipeDelegate = resolveType("QtQuick.Controls", "SwipeDelegate"); } bool ControlsSwipeDelegateValidatorPass::shouldRun(const QQmlSA::Element &element) { - return !m_swipeDelegate.isNull() && element->inherits(m_swipeDelegate); + return !m_swipeDelegate.isNull() && element.inherits(m_swipeDelegate); } void ControlsSwipeDelegateValidatorPass::run(const QQmlSA::Element &element) { for (const auto &property : { u"background"_s, u"contentItem"_s }) { - auto bindings = element->ownPropertyBindings(property); - for (auto it = bindings.first; it != bindings.second; it++) { - if (!it->hasObject()) + for (const auto &binding : element.ownPropertyBindings(property)) { + if (!binding.hasObject()) continue; - const QQmlSA::Element element = it->objectType(); - const auto bindings = element->propertyBindings(u"anchors"_s); + const QQmlSA::Element element = QQmlSA::Element{ binding.objectType() }; + const auto &bindings = element.propertyBindings(u"anchors"_s); if (bindings.isEmpty()) continue; - if (bindings.first().bindingType() != QQmlJSMetaPropertyBinding::GroupProperty) + if (bindings.first().bindingType() != QQmlSA::BindingType::GroupProperty) continue; auto anchors = bindings.first().groupType(); for (const auto &disallowed : { u"fill"_s, u"centerIn"_s, u"left"_s, u"right"_s }) { - if (anchors->hasPropertyBindings(disallowed)) { - QQmlJS::SourceLocation location; - auto ownBindings = anchors->ownPropertyBindings(disallowed); - if (ownBindings.first != ownBindings.second) { - location = ownBindings.first->sourceLocation(); + if (anchors.hasPropertyBindings(disallowed)) { + QQmlSA::SourceLocation location; + const auto &ownBindings = anchors.ownPropertyBindings(disallowed); + if (ownBindings.begin() != ownBindings.end()) { + location = ownBindings.begin().value().sourceLocation(); } emitWarning( @@ -391,30 +399,31 @@ void ControlsSwipeDelegateValidatorPass::run(const QQmlSA::Element &element) } } - auto swipe = element->ownPropertyBindings(u"swipe"_s); - if (swipe.first == swipe.second) + const auto &swipe = element.ownPropertyBindings(u"swipe"_s); + if (swipe.begin() == swipe.end()) return; - if (swipe.first->bindingType() != QQmlJSMetaPropertyBinding::GroupProperty) + const auto firstSwipe = swipe.begin().value(); + if (firstSwipe.bindingType() != QQmlSA::BindingType::GroupProperty) return; - auto group = swipe.first->groupType(); + auto group = firstSwipe.groupType(); - const std::array ownDirBindings = { group->ownPropertyBindings(u"right"_s), - group->ownPropertyBindings(u"left"_s), - group->ownPropertyBindings(u"behind"_s) }; + const std::array ownDirBindings = { group.ownPropertyBindings(u"right"_s), + group.ownPropertyBindings(u"left"_s), + group.ownPropertyBindings(u"behind"_s) }; auto ownBindingIterator = std::find_if(ownDirBindings.begin(), ownDirBindings.end(), - [](const auto &pair) { return pair.first != pair.second; }); + [](const auto &bindings) { return bindings.begin() != bindings.end(); }); if (ownBindingIterator == ownDirBindings.end()) return; - if (group->hasPropertyBindings(u"behind"_s) - && (group->hasPropertyBindings(u"right"_s) || group->hasPropertyBindings(u"left"_s))) { + if (group.hasPropertyBindings(u"behind"_s) + && (group.hasPropertyBindings(u"right"_s) || group.hasPropertyBindings(u"left"_s))) { emitWarning("SwipeDelegate: Cannot set both behind and left/right properties", - quickAnchorCombinations, ownBindingIterator->first->sourceLocation()); + quickAnchorCombinations, ownBindingIterator->begin().value().sourceLocation()); } } @@ -423,22 +432,14 @@ VarBindingTypeValidatorPass::VarBindingTypeValidatorPass( const QMultiHash<QString, TypeDescription> &expectedPropertyTypes) : QQmlSA::PropertyPass(manager) { - QMultiHash<QString, QQmlJSScope::ConstPtr> propertyTypes; - - for (const auto pair : expectedPropertyTypes.asKeyValueRange()) { - QQmlSA::Element propType; - - if (!pair.second.module.isEmpty()) { - propType = resolveType(pair.second.module, pair.second.name); - if (propType.isNull()) - continue; - } else { - auto scope = QQmlJSScope::create(); - scope->setInternalName(pair.second.name); - propType = scope; - } - - propertyTypes.insert(pair.first, propType); + QMultiHash<QString, QQmlSA::Element> propertyTypes; + + for (const auto &pair : expectedPropertyTypes.asKeyValueRange()) { + const QQmlSA::Element propType = pair.second.module.isEmpty() + ? resolveBuiltinType(pair.second.name) + : resolveType(pair.second.module, pair.second.name); + if (!propType.isNull()) + propertyTypes.insert(pair.first, propType); } m_expectedPropertyTypes = propertyTypes; @@ -446,7 +447,7 @@ VarBindingTypeValidatorPass::VarBindingTypeValidatorPass( void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element, const QString &propertyName, - const QQmlJSMetaPropertyBinding &binding, + const QQmlSA::Binding &binding, const QQmlSA::Element &bindingScope, const QQmlSA::Element &value) { @@ -463,14 +464,14 @@ void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element, if (!value.isNull()) { bindingType = value; } else { - if (QQmlJSMetaPropertyBinding::isLiteralBinding(binding.bindingType())) { + if (QQmlSA::Binding::isLiteralBinding(binding.bindingType())) { bindingType = resolveLiteralType(binding); } else { switch (binding.bindingType()) { - case QQmlJSMetaPropertyBinding::Object: - bindingType = binding.objectType(); + case QQmlSA::BindingType::Object: + bindingType = QQmlSA::Element{ binding.objectType() }; break; - case QQmlJSMetaPropertyBinding::Script: + case QQmlSA::BindingType::Script: break; default: return; @@ -479,11 +480,11 @@ void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element, } if (std::find_if(range.first, range.second, - [&](const QQmlSA::Element &scope) { return bindingType->inherits(scope); }) + [&](const QQmlSA::Element &scope) { return bindingType.inherits(scope); }) == range.second) { - const bool bindingTypeIsComposite = bindingType->isComposite(); - if (bindingTypeIsComposite && !bindingType->baseType()) { + const bool bindingTypeIsComposite = bindingType.isComposite(); + if (bindingTypeIsComposite && !bindingType.baseType()) { /* broken module or missing import, there is nothing we can really check here, as something is amiss. We simply skip this binding, and assume that whatever @@ -492,14 +493,13 @@ void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element, */ return; } - const QString bindingTypeName = QQmlJSScope::prettyName( - bindingTypeIsComposite - ? bindingType->baseType()->internalName() - : bindingType->internalName()); + const QString bindingTypeName = + bindingTypeIsComposite ? bindingType.baseType().name() + : bindingType.name(); QStringList expectedTypeNames; for (auto it = range.first; it != range.second; it++) - expectedTypeNames << QQmlJSScope::prettyName(it.value()->internalName()); + expectedTypeNames << it.value().name(); emitWarning(u"Unexpected type for property \"%1\" expected %2 got %3"_s.arg( propertyName, expectedTypeNames.join(u", "_s), bindingTypeName), @@ -507,9 +507,98 @@ void VarBindingTypeValidatorPass::onBinding(const QQmlSA::Element &element, } } +void AttachedPropertyReuse::onRead(const QQmlSA::Element &element, const QString &propertyName, + const QQmlSA::Element &readScope, + QQmlSA::SourceLocation location) +{ + const auto range = usedAttachedTypes.equal_range(readScope); + const auto attachedTypeAndLocation = std::find_if( + range.first, range.second, [&](const ElementAndLocation &elementAndLocation) { + return elementAndLocation.element == element; + }); + if (attachedTypeAndLocation != range.second) { + const QQmlSA::SourceLocation attachedLocation = attachedTypeAndLocation->location; + + // Ignore enum accesses, as these will not cause the attached object to be created. + // Also ignore anything we cannot determine. + if (!element.hasProperty(propertyName) && !element.hasMethod(propertyName)) + return; + + for (QQmlSA::Element scope = readScope.parentScope(); !scope.isNull(); + scope = scope.parentScope()) { + const auto range = usedAttachedTypes.equal_range(scope); + bool found = false; + for (auto it = range.first; it != range.second; ++it) { + if (it->element == element) { + found = true; + break; + } + } + if (!found) + continue; + + const QString id = resolveElementToId(scope, readScope); + const QQmlSA::SourceLocation idInsertLocation{ attachedLocation.offset(), 0, + attachedLocation.startLine(), + attachedLocation.startColumn() }; + QQmlSA::FixSuggestion suggestion{ "Reference it by id instead:"_L1, idInsertLocation, + id.isEmpty() ? u"<id>."_s : (id + '.'_L1) }; + + if (id.isEmpty()) + suggestion.setHint("You first have to give the element an id"_L1); + else + suggestion.setAutoApplicable(); + + emitWarning("Using attached type %1 already initialized in a parent scope."_L1.arg( + element.name()), + category, attachedLocation, suggestion); + } + + return; + } + + if (element.hasProperty(propertyName)) + return; // an actual property + + QQmlSA::Element type = resolveTypeInFileScope(propertyName); + QQmlSA::Element attached = resolveAttachedInFileScope(propertyName); + if (!type || !attached) + return; + + if (category == quickControlsAttachedPropertyReuse) { + for (QQmlSA::Element parent = attached; parent; parent = parent.baseType()) { + // ### TODO: Make it possible to resolve QQuickAttachedPropertyPropagator + // so that we don't have to compare the internal id + if (parent.internalId() == "QQuickAttachedPropertyPropagator"_L1) { + usedAttachedTypes.insert(readScope, {attached, location}); + break; + } + } + + } else { + usedAttachedTypes.insert(readScope, {attached, location}); + } +} + +void AttachedPropertyReuse::onWrite(const QQmlSA::Element &element, const QString &propertyName, + const QQmlSA::Element &value, const QQmlSA::Element &writeScope, + QQmlSA::SourceLocation location) +{ + Q_UNUSED(value); + onRead(element, propertyName, writeScope, location); +} + void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, const QQmlSA::Element &rootElement) { + const QQmlSA::LoggerWarningId attachedReuseCategory = [manager]() { + if (manager->isCategoryEnabled(quickAttachedPropertyReuse)) + return quickAttachedPropertyReuse; + if (manager->isCategoryEnabled(qmlAttachedPropertyReuse)) + return qmlAttachedPropertyReuse; + return quickControlsAttachedPropertyReuse; + }(); + const bool hasQuick = manager->hasImportedModule("QtQuick"); const bool hasQuickLayouts = manager->hasImportedModule("QtQuick.Layouts"); const bool hasQuickControls = manager->hasImportedModule("QtQuick.Templates") @@ -520,6 +609,7 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, if (hasQuick) { manager->registerElementPass(std::make_unique<AnchorsValidatorPass>(manager)); + manager->registerElementPass(std::make_unique<PropertyChangesValidatorPass>(manager)); auto forbiddenChildProperty = std::make_unique<ForbiddenChildrenPropertyValidatorPass>(manager); @@ -585,7 +675,7 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, { { "columnWidthProvider", { "", "function" } }, { "rowHeightProvider", { "", "function" } } }); addAttachedWarning({ "QtQuick", "Accessible" }, { { "QtQuick", "Item" } }, - "Accessible must be attached to an Item"); + "Accessible must be attached to an Item or an Action"); addAttachedWarning({ "QtQuick", "LayoutMirroring" }, { { "QtQuick", "Item" }, { "QtQuick", "Window" } }, "LayoutDirection attached property only works with Items and Windows"); @@ -598,8 +688,12 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, addAttachedWarning({ "QtQuick.Layouts", "StackLayout" }, { { "QtQuick", "Item" } }, "StackLayout must be attached to an Item"); } + + if (hasQuickControls) { manager->registerElementPass(std::make_unique<ControlsSwipeDelegateValidatorPass>(manager)); + manager->registerPropertyPass(std::make_unique<AttachedPropertyReuse>( + manager, attachedReuseCategory), "", ""); addAttachedWarning({ "QtQuick.Templates", "ScrollBar" }, { { "QtQuick", "Flickable" }, { "QtQuick.Templates", "ScrollView" } }, @@ -629,6 +723,9 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, addVarBindingWarning("QtQuick.Templates", "SpinBox", { { "textFromValue", { "", "function" } }, { "valueFromText", { "", "function" } } }); + } else if (attachedReuseCategory != quickControlsAttachedPropertyReuse) { + manager->registerPropertyPass(std::make_unique<AttachedPropertyReuse>( + manager, attachedReuseCategory), "", ""); } if (manager->hasImportedModule(u"QtQuick.Controls.macOS"_s) @@ -636,6 +733,67 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager, manager->registerElementPass(std::make_unique<ControlsNativeValidatorPass>(manager)); } +PropertyChangesValidatorPass::PropertyChangesValidatorPass(QQmlSA::PassManager *manager) + : QQmlSA::ElementPass(manager) + , m_propertyChanges(resolveType("QtQuick", "PropertyChanges")) +{ +} + +bool PropertyChangesValidatorPass::shouldRun(const QQmlSA::Element &element) +{ + return !m_propertyChanges.isNull() && element.inherits(m_propertyChanges); +} + +void PropertyChangesValidatorPass::run(const QQmlSA::Element &element) +{ + const QQmlSA::Binding::Bindings bindings = element.ownPropertyBindings(); + + const auto target = + std::find_if(bindings.constBegin(), bindings.constEnd(), + [](const auto binding) { return binding.propertyName() == u"target"_s; }); + if (target == bindings.constEnd()) + return; + + QString targetId = u"<id>"_s; + const auto targetLocation = target.value().sourceLocation(); + const QString targetBinding = sourceCode(targetLocation); + const QQmlSA::Element targetElement = resolveIdToElement(targetBinding, element); + if (!targetElement.isNull()) + targetId = targetBinding; + + bool hadCustomParsedBindings = false; + for (auto it = bindings.constBegin(); it != bindings.constEnd(); ++it) { + const auto &propertyName = it.key(); + const auto &propertyBinding = it.value(); + if (element.hasProperty(propertyName)) + continue; + + const QQmlSA::SourceLocation bindingLocation = propertyBinding.sourceLocation(); + if (!targetElement.isNull() && !targetElement.hasProperty(propertyName)) { + emitWarning( + "Unknown property \"%1\" in PropertyChanges."_L1.arg(propertyName), + quickPropertyChangesParsed, bindingLocation); + continue; + } + + QString binding = sourceCode(bindingLocation); + if (binding.length() > 16) + binding = binding.left(13) + "..."_L1; + + hadCustomParsedBindings = true; + emitWarning("Property \"%1\" is custom-parsed in PropertyChanges. " + "You should phrase this binding as \"%2.%1: %3\""_L1.arg(propertyName, targetId, + binding), + quickPropertyChangesParsed, bindingLocation); + } + + if (hadCustomParsedBindings && !targetElement.isNull()) { + emitWarning("You should remove any bindings on the \"target\" property and avoid " + "custom-parsed bindings in PropertyChanges.", + quickPropertyChangesParsed, targetLocation); + } +} + QT_END_NAMESPACE #include "moc_quicklintplugin.cpp" |