QQmlJSImportVisitor: Handle on binding inside grouped properties

The visitation logic for UiObjectBinding was entering nested scopes for
attached and grouped properties to check if their scopes need to be
resolved. However, the logic to leave those scopes was faulty: It left
the current scope as long as the current scope was a attached or grouped
property scope. That would however break if an on-binding (triggering
this code path) were placed in a grouped property. In that case, we
would leave the grouped property scope, even though it should have still
been the current scope.
Fix the issue by explicitly counting how many scopes we were entering,
and leave the corresponding number of scopes afterwards.

As a drive-by, turn a "verbal assertion" in a Q_ASSERT in endVisit.

Pick-to: 6.2
Fixes: QTBUG-98125
Change-Id: I270b1bf3fc5b38ad9d437df1ea6c55684d143378
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Fabian Kosmale 2021-11-09 16:52:43 +01:00
parent 5e8e12b342
commit 332099038d
3 changed files with 19 additions and 6 deletions

View File

@ -1668,6 +1668,7 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob)
name.chop(1);
bool needsResolution = false;
int scopesEnteredCounter = 0;
for (auto group = uiob->qualifiedId; group->next; group = group->next) {
const QString idName = group->name.toString();
@ -1677,11 +1678,11 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob)
const auto scopeKind = idName.front().isUpper() ? QQmlJSScope::AttachedPropertyScope
: QQmlJSScope::GroupedPropertyScope;
bool exists = enterEnvironmentNonUnique(scopeKind, idName, group->firstSourceLocation());
++scopesEnteredCounter;
needsResolution = needsResolution || !exists;
}
while (m_currentScope->scopeType() == QQmlJSScope::GroupedPropertyScope
|| m_currentScope->scopeType() == QQmlJSScope::AttachedPropertyScope) {
for (int i=0; i < scopesEnteredCounter; ++i) { // leave the scopes we entered again
leaveEnvironment();
}
@ -1705,6 +1706,7 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
leaveEnvironment();
auto group = uiob->qualifiedId;
int scopesEnteredCounter = 0;
for (; group->next; group = group->next) {
const QString idName = group->name.toString();
@ -1714,7 +1716,9 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
const auto scopeKind = idName.front().isUpper() ? QQmlJSScope::AttachedPropertyScope
: QQmlJSScope::GroupedPropertyScope;
// definitely exists
enterEnvironmentNonUnique(scopeKind, idName, group->firstSourceLocation());
[[maybe_unused]] bool exists = enterEnvironmentNonUnique(scopeKind, idName, group->firstSourceLocation());
Q_ASSERT(exists);
scopesEnteredCounter++;
}
// on ending the visit to UiObjectBinding, set the property type to the
@ -1739,10 +1743,8 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
uiob->firstSourceLocation(), uiob->hasOnToken };
}
while (m_currentScope->scopeType() == QQmlJSScope::GroupedPropertyScope
|| m_currentScope->scopeType() == QQmlJSScope::AttachedPropertyScope) {
for (int i = 0; i < scopesEnteredCounter; ++i)
leaveEnvironment();
}
}
bool QQmlJSImportVisitor::visit(ExportDeclaration *)

View File

@ -0,0 +1,10 @@
import QtQuick
Rectangle
{
id: root
border
{
ColorAnimation on color { }
}
}

View File

@ -889,6 +889,7 @@ void TestQmllint::cleanQmlCode_data()
QTest::newRow("initReadonly") << QStringLiteral("initReadonly.qml");
QTest::newRow("connectionNoParent") << QStringLiteral("connectionNoParent.qml"); // QTBUG-97600
QTest::newRow("goodGeneralizedGroup") << QStringLiteral("goodGeneralizedGroup.qml");
QTest::newRow("on binding in grouped property") << QStringLiteral("onBindingInGroupedProperty.qml");
}
void TestQmllint::cleanQmlCode()