qmllint: Fix attached object reuse false positives

Previously the warning would be triggered even when no property is
accessed or only an enum access occurred. Since neither of these actually
cause the attached object to be created, do not warn in these cases.

Change-Id: I53d2886268e2364f2d9bc088e0ba02dc1cfe4ee3
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Maximilian Goldstein 2021-11-24 12:02:55 +01:00
parent 2a04222cfc
commit 650e6739c4
3 changed files with 62 additions and 47 deletions

View File

@ -428,50 +428,6 @@ void QQmlJSTypePropagator::generate_LoadQmlContextPropertyLookup(int index)
+ u'.' + name
: name);
if (m_typeInfo != nullptr
&& m_state.accumulatorOut.variant() == QQmlJSRegisterContent::ScopeAttached) {
QQmlJSScope::ConstPtr attachedType = m_state.accumulatorOut.scopeType();
for (QQmlJSScope::ConstPtr scope = m_function->qmlScope->parentScope(); !scope.isNull();
scope = scope->parentScope()) {
if (m_typeInfo->usedAttachedTypes.values(scope).contains(attachedType)) {
auto it = std::find(m_function->addressableScopes.constBegin(),
m_function->addressableScopes.constEnd(), scope);
m_logger->logWarning(
u"Using attached type %1 already initialized in a parent scope."_qs
.arg(name),
Log_AttachedPropertyReuse, getCurrentSourceLocation());
FixSuggestion suggestion { Log_AttachedPropertyReuse, {} };
QQmlJS::SourceLocation fixLocation = getCurrentSourceLocation();
fixLocation.length = 0;
QString id = it == m_function->addressableScopes.constEnd() ? u"<id>"_qs : it.key();
suggestion.fixes << FixSuggestion::Fix {
u"Reference it by id instead:"_qs,
fixLocation, id + u"."_qs
};
fixLocation = scope->sourceLocation();
fixLocation.length = 0;
if (it == m_function->addressableScopes.constEnd()) {
suggestion.fixes
<< FixSuggestion::Fix { u"You first have to give the element an id"_qs,
QQmlJS::SourceLocation {},
{} };
}
m_logger->suggestFix(suggestion);
}
}
m_typeInfo->usedAttachedTypes.insert(m_function->qmlScope, attachedType);
}
if (!m_state.accumulatorOut.isValid() && m_typeResolver->isPrefix(name)) {
const QQmlJSRegisterContent inType = m_state.accumulatorIn.isValid()
? m_state.accumulatorIn
@ -562,6 +518,52 @@ void QQmlJSTypePropagator::propagatePropertyLookup(const QString &propertyName)
+ u'.' + propertyName
: propertyName);
if (m_typeInfo != nullptr
&& m_state.accumulatorIn.variant() == QQmlJSRegisterContent::ScopeAttached) {
QQmlJSScope::ConstPtr attachedType = m_state.accumulatorIn.scopeType();
for (QQmlJSScope::ConstPtr scope = m_function->qmlScope->parentScope(); !scope.isNull();
scope = scope->parentScope()) {
if (m_typeInfo->usedAttachedTypes.values(scope).contains(attachedType)) {
// Ignore enum accesses, as these will not cause the attached object to be created
if (m_state.accumulatorOut.isValid() && m_state.accumulatorOut.isEnumeration())
continue;
auto it = std::find(m_function->addressableScopes.constBegin(),
m_function->addressableScopes.constEnd(), scope);
m_logger->logWarning(
u"Using attached type %1 already initialized in a parent scope."_qs.arg(
m_state.accumulatorIn.scopeType()->internalName()),
Log_AttachedPropertyReuse, getCurrentSourceLocation());
FixSuggestion suggestion { Log_AttachedPropertyReuse, {} };
QQmlJS::SourceLocation fixLocation = getCurrentSourceLocation();
fixLocation.length = 0;
QString id = it == m_function->addressableScopes.constEnd() ? u"<id>"_qs : it.key();
suggestion.fixes << FixSuggestion::Fix { u"Reference it by id instead:"_qs,
fixLocation, id + u"."_qs };
fixLocation = scope->sourceLocation();
fixLocation.length = 0;
if (it == m_function->addressableScopes.constEnd()) {
suggestion.fixes
<< FixSuggestion::Fix { u"You first have to give the element an id"_qs,
QQmlJS::SourceLocation {},
{} };
}
m_logger->suggestFix(suggestion);
}
}
m_typeInfo->usedAttachedTypes.insert(m_function->qmlScope, attachedType);
}
if (!m_state.accumulatorOut.isValid()) {
if (m_typeResolver->isPrefix(propertyName)) {
Q_ASSERT(m_state.accumulatorIn.isValid());

View File

@ -0,0 +1,9 @@
import QtQuick
Text {
id: root
text: KeyNavigation.priority == KeyNavigation.BeforeItem ? "before" : "after"
Text {
text: root.KeyNavigation.priority == KeyNavigation.BeforeItem ? "before" : "after"
}
}

View File

@ -1104,9 +1104,13 @@ void TestQmllint::attachedPropertyReuse()
QStringList() << "--multiple-attached-objects"
<< "warning",
false)
.contains(QStringLiteral(
"Using attached type KeyNavigation already initialized in a parent "
"scope")));
.contains(QStringLiteral("Using attached type QQuickKeyNavigationAttached "
"already initialized in a parent "
"scope")));
QVERIFY(runQmllint("attachedPropEnum.qml", true,
QStringList() << "--multiple-attached-objects"
<< "warning")
.isEmpty());
}
void TestQmllint::shadowable()