qmllint: Add ability to warn about not reusing attached types

This is mostly useful as an replacement for Quick Controls' tst_sanity but might also be useful in some other instances.

Fixes: QTBUG-96572
Change-Id: I5cf414bfeb369cbc394563c5c5ed807599b09a2f
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qmlcompiler/qqmljslogger.cpp

@@ -81,7 +81,11 @@ const QMap<QString, QQmlJSLogger::Option> &QQmlJSLogger::options() {
         { QStringLiteral("controls-sanity"),
           QQmlJSLogger::Option(
                   Log_ControlsSanity, QStringLiteral("ControlsSanity"),
-                  QStringLiteral("Performance checks used for QuickControl's implementation"),
+                  QStringLiteral("Performance checks used for QuickControl's implementation"), QtCriticalMsg, false) },
+        { QStringLiteral("multiple-attached-objects"),
+          QQmlJSLogger::Option(
+                  Log_AttachedPropertyReuse, QStringLiteral("AttachedPropertyReuse"),
+                  QStringLiteral("Warn if attached types from parent components aren't reused"),
                   QtCriticalMsg, false) }
     };
 

src/qmlcompiler/qqmljslogger_p.h

@@ -104,7 +104,8 @@ enum QQmlJSLoggerCategory {
     Log_Syntax,
     Log_Compiler,
     Log_ControlsSanity,
-    QQmlJSLoggerCategory_Last = Log_ControlsSanity
+    Log_AttachedPropertyReuse,
+    QQmlJSLoggerCategory_Last = Log_AttachedPropertyReuse
 };
 
 struct FixSuggestion

src/qmlcompiler/qqmljsmetatypes_p.h

@@ -43,6 +43,7 @@
 #include <QtCore/qstringlist.h>
 #include <QtCore/qsharedpointer.h>
 #include <QtCore/qvariant.h>
+#include <QtCore/qhash.h>
 
 #include "qqmljsannotation_p.h"
 

src/qmlcompiler/qqmljsscope_p.h

@@ -450,6 +450,11 @@ private:
     QQmlJS::SourceLocation m_sourceLocation;
 };
 
+struct QQmlJSTypeInfo
+{
+    QMultiHash<QQmlJSScope::ConstPtr, QQmlJSScope::ConstPtr> usedAttachedTypes;
+};
+
 QT_END_NAMESPACE
 
 #endif // QQMLJSSCOPE_P_H

src/qmlcompiler/qqmljstypepropagator.cpp

@@ -32,8 +32,12 @@
 QT_BEGIN_NAMESPACE
 
 QQmlJSTypePropagator::QQmlJSTypePropagator(QV4::Compiler::JSUnitGenerator *unitGenerator,
-                                           QQmlJSTypeResolver *typeResolver, QQmlJSLogger *logger)
-    : m_typeResolver(typeResolver), m_jsUnitGenerator(unitGenerator), m_logger(logger)
+                                           QQmlJSTypeResolver *typeResolver, QQmlJSLogger *logger,
+                                           QQmlJSTypeInfo *typeInfo)
+    : m_typeResolver(typeResolver),
+      m_jsUnitGenerator(unitGenerator),
+      m_logger(logger),
+      m_typeInfo(typeInfo)
 {
 }
 
@@ -446,6 +450,47 @@ 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_currentScope->parentScope(); !scope.isNull();
+             scope = scope->parentScope()) {
+            if (m_typeInfo->usedAttachedTypes.values(scope).contains(attachedType)) {
+                auto it = std::find(m_addressableScopes.constBegin(),
+                                    m_addressableScopes.constEnd(), scope);
+
+                FixSuggestion suggestion { Log_AttachedPropertyReuse, QtInfoMsg, {} };
+
+                QQmlJS::SourceLocation fixLocation = getCurrentSourceLocation();
+                fixLocation.length = 0;
+
+                QString id = it == m_addressableScopes.constEnd() ? u"<id>"_qs : it.key();
+
+                suggestion.fixes << FixSuggestion::Fix {
+                    u"Using attached type %1 already initialized in a parent scope. Reference it by id instead:"_qs
+                            .arg(name),
+                    QtWarningMsg, fixLocation, id + u"."_qs
+                };
+
+                fixLocation = scope->sourceLocation();
+                fixLocation.length = 0;
+
+                if (it == m_addressableScopes.constEnd()) {
+                    suggestion.fixes
+                            << FixSuggestion::Fix { u"You first have to give the element an id"_qs,
+                                                    QtInfoMsg,
+                                                    QQmlJS::SourceLocation {},
+                                                    {} };
+                }
+
+                m_logger->suggestFix(suggestion);
+            }
+        }
+
+        m_typeInfo->usedAttachedTypes.insert(m_currentScope, attachedType);
+    }
+
     if (!m_state.accumulatorOut.isValid() && m_typeResolver->isPrefix(name)) {
         const QQmlJSRegisterContent inType = m_state.accumulatorIn.isValid()
                 ? m_state.accumulatorIn

src/qmlcompiler/qqmljstypepropagator_p.h

@@ -53,7 +53,8 @@ QT_BEGIN_NAMESPACE
 struct QQmlJSTypePropagator : public QV4::Moth::ByteCodeHandler
 {
     QQmlJSTypePropagator(QV4::Compiler::JSUnitGenerator *unitGenerator,
-                         QQmlJSTypeResolver *typeResolver, QQmlJSLogger *logger);
+                         QQmlJSTypeResolver *typeResolver, QQmlJSLogger *logger,
+                         QQmlJSTypeInfo *typeInfo = nullptr);
     ~QQmlJSTypePropagator();
     auto returnType() const { return m_returnType; }
 
@@ -294,6 +295,7 @@ private:
     const QV4::Compiler::Context *m_currentContext;
     QHash<QString, QQmlJSScope::ConstPtr> m_addressableScopes;
     QQmlJSLogger *m_logger;
+    QQmlJSTypeInfo *m_typeInfo;
 
     // Not part of the state, as the back jumps are the reason for running multiple passes
     QMultiHash<int, ExpectedRegisterState> m_jumpOriginRegisterStateByTargetInstructionOffset;

tests/auto/qml/qmllint/data/attachedPropNotReused.qml

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

tests/auto/qml/qmllint/tst_qmllint.cpp

@@ -80,6 +80,8 @@ private Q_SLOTS:
     void listIndices();
     void lazyAndDirect();
 
+    void attachedPropertyReuse();
+
 private:
     QString runQmllint(const QString &fileToLint, std::function<void(QProcess &)> handleResult,
                        const QStringList &extraArgs = QStringList(), bool ignoreSettings = true);
@@ -991,5 +993,16 @@ void TestQmllint::anchors()
                     .contains(u"Using anchors here"_qs));
 }
 
+void TestQmllint::attachedPropertyReuse()
+{
+    QVERIFY(runQmllint("attachedPropNotReused.qml", true,
+                       QStringList() << "--multiple-attached-objects"
+                                     << "warning",
+                       false)
+                    .contains(QStringLiteral(
+                            "Using attached type KeyNavigation already initialized in a parent "
+                            "scope. Reference it by id instead")));
+}
+
 QTEST_MAIN(TestQmllint)
 #include "tst_qmllint.moc"

tools/qmllint/codegen.cpp

@@ -41,6 +41,7 @@ Codegen::Codegen(QQmlJSImporter *importer, const QString &fileName,
       m_logger(logger),
       m_code(code)
 {
+    m_typeInfo = new QQmlJSTypeInfo;
 }
 
 void Codegen::setDocument(QmlIR::JSCodeGen *codegen, QmlIR::Document *document)
@@ -305,7 +306,7 @@ bool Codegen::generateFunction(const QV4::Compiler::Context *context, Function *
         }
     }
 
-    QQmlJSTypePropagator propagator(m_unitGenerator, m_typeResolver.get(), m_logger);
+    QQmlJSTypePropagator propagator(m_unitGenerator, m_typeResolver.get(), m_logger, m_typeInfo);
 
     if (!function->returnType) {
         if (function->ast->typeAnnotation) {

tools/qmllint/codegen.h

@@ -98,6 +98,7 @@ private:
     QV4::Compiler::JSUnitGenerator *m_unitGenerator = nullptr;
     QStringList m_entireSourceCodeLines;
     QQmlJSLogger *m_logger;
+    QQmlJSTypeInfo *m_typeInfo;
     const QString m_code;
     std::unique_ptr<QQmlJSTypeResolver> m_typeResolver;