QmlLintQuickPlugin: Warn about various SwipeDelegate issues

Implements anchor related and back/left/behind combination warnings.

Also adds a quality-of-life improvement in tst_qmllint where we print the
line when an expected message is missing.

Task-number: QTBUG-102277
Task-number: QTBUG-102859
Change-Id: I56068c75e3c6187845b079a6689debefa363a5e4
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Maximilian Goldstein 2022-05-02 15:58:50 +02:00
parent 5cd59bff0c
commit 78483744b4
4 changed files with 136 additions and 6 deletions

View File

@ -317,6 +317,79 @@ void AnchorsValidatorPass::run(const QQmlSA::Element &element)
}
}
ControlsSwipeDelegateValidatorPass::ControlsSwipeDelegateValidatorPass(QQmlSA::PassManager *manager)
: QQmlSA::ElementPass(manager)
{
m_swipeDelegate = resolveType("QtQuick.Controls", "SwipeDelegate");
}
bool ControlsSwipeDelegateValidatorPass::shouldRun(const QQmlSA::Element &element)
{
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())
continue;
const QQmlSA::Element element = it->objectType();
const auto bindings = element->propertyBindings(u"anchors"_s);
if (bindings.isEmpty())
continue;
if (bindings.first().bindingType() != QQmlJSMetaPropertyBinding::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();
}
emitWarning(
u"SwipeDelegate: Cannot use horizontal anchors with %1; unable to layout the item."_s
.arg(property),
location);
break;
}
}
break;
}
}
auto swipe = element->ownPropertyBindings(u"swipe"_s);
if (swipe.first == swipe.second)
return;
if (swipe.first->bindingType() != QQmlJSMetaPropertyBinding::GroupProperty)
return;
auto group = swipe.first->groupType();
const auto 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; });
if (ownBindingIterator == ownDirBindings.end())
return;
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",
ownBindingIterator->first->sourceLocation());
}
}
void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager,
const QQmlSA::Element &rootElement)
{
@ -397,6 +470,8 @@ void QmlLintQuickPlugin::registerPasses(QQmlSA::PassManager *manager,
"StackView attached property only works with Items");
attachedPropertyType->addWarning("ToolTip", { { "QtQuick", "Item" } },
"ToolTip must be attached to an Item");
manager->registerElementPass(std::make_unique<ControlsSwipeDelegateValidatorPass>(manager));
}
manager->registerElementPass(std::move(attachedPropertyType));

View File

@ -128,6 +128,18 @@ private:
QQmlSA::Element m_item;
};
class ControlsSwipeDelegateValidatorPass : public QQmlSA::ElementPass
{
public:
ControlsSwipeDelegateValidatorPass(QQmlSA::PassManager *manager);
bool shouldRun(const QQmlSA::Element &element) override;
void run(const QQmlSA::Element &element) override;
private:
QQmlSA::Element m_swipeDelegate;
};
QT_END_NAMESPACE
#endif // QUICKLINTPLUGIN_H

View File

@ -0,0 +1,19 @@
import QtQuick.Controls
import QtQuick
Item {
SwipeDelegate {
contentItem: Item { anchors.left: parent.left }
background: Item { anchors.right: parent.right }
swipe.left: Item {}
swipe.behind: Item {}
}
SwipeDelegate {
contentItem: Item { anchors.centerIn: parent }
background: Item { anchors.fill: parent }
swipe.right: Item {}
swipe.behind: Item {}
}
}

View File

@ -1514,23 +1514,28 @@ void TestQmllint::searchWarnings(const QJsonArray &warnings, const QString &subs
}
const auto toDescription = [](const QJsonArray &warnings, const QString &substring,
bool must = true) {
quint32 line, quint32 column, bool must = true) {
// Note: this actually produces a very poorly formatted multi-line
// description, but this is how we also do it in cleanQmlCode test case,
// so this should suffice. in any case this mainly aids the debugging
// and CI stays (or should stay) clean.
return QStringLiteral("qmllint output '%1' %2 contain '%3'")
.arg(QString::fromUtf8(QJsonDocument(warnings).toJson(QJsonDocument::Compact)),
QString msg = QStringLiteral("qmllint output '%1' %2 contain '%3'")
.arg(QString::fromUtf8(
QJsonDocument(warnings).toJson(QJsonDocument::Compact)),
must ? u"must" : u"must NOT", substring);
if (line != 0 || column != 0)
msg += u" (%1:%2)"_s.arg(line).arg(column);
return msg;
};
if (shouldContain == StringContained) {
if (!contains)
qWarning() << toDescription(warnings, substring);
qWarning() << toDescription(warnings, substring, line, column);
QVERIFY(contains);
} else {
if (contains)
qWarning() << toDescription(warnings, substring, false);
qWarning() << toDescription(warnings, substring, line, column, false);
QVERIFY(!contains);
}
}
@ -1715,6 +1720,25 @@ void TestQmllint::quickPlugin()
u"LayoutDirection attached property only works with Items and Windows"_s },
Message { u"Layout must be attached to Item elements"_s },
Message { u"StackView attached property only works with Items"_s } } });
runTest("pluginQuick_swipeDelegate.qml",
Result { {
Message {
u"SwipeDelegate: Cannot use horizontal anchors with contentItem; unable to layout the item."_s,
6, 43 },
Message {
u"SwipeDelegate: Cannot use horizontal anchors with background; unable to layout the item."_s,
7, 43 },
Message { u"SwipeDelegate: Cannot set both behind and left/right properties"_s,
9, 9 },
Message {
u"SwipeDelegate: Cannot use horizontal anchors with contentItem; unable to layout the item."_s,
13, 47 },
Message {
u"SwipeDelegate: Cannot use horizontal anchors with background; unable to layout the item."_s,
14, 42 },
Message { u"SwipeDelegate: Cannot set both behind and left/right properties"_s,
16, 9 },
} });
}
#endif