qmlcompiler: Process properties only once parsing is completed

Now we do most properties after the fact which eliminates false positives on unknown types that are not known at the time of traversing the AST. It also allows for chaining inline components.
This effectively introduces a two pass system but there are some exceptions (i.e. signals) where some deduction that should be run after the fact still runs during parsing.

Fixes: QTBUG-93652
Change-Id: Ic1ac0e8ce2d5d5dfbe80c16a341b10cf2b078d81
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit a991d3c1ec5b679f37ea19d7cbef576f3fc1029e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

5 files changed, 186 insertions, 95 deletions

diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp
index 0417898276..94af2e6666 100644
--- a/src/qmlcompiler/qqmljsimportvisitor.cpp
+++ b/src/qmlcompiler/qqmljsimportvisitor.cpp
@@ -297,8 +297,11 @@ void QQmlJSImportVisitor::endVisit(UiProgram *)
 {
     resolveAliases();
     processDefaultProperties();
+    processPropertyTypes();
     checkPropertyBindings();
     checkSignals();
+    processPropertyBindingObjects();
+    checkRequiredProperties();
 
     for (const auto &scope : m_objectBindingScopes)
         checkInheritanceCycle(scope);
@@ -461,6 +464,131 @@ void QQmlJSImportVisitor::processDefaultProperties()
     }
 }
 
+void QQmlJSImportVisitor::processPropertyTypes()
+{
+    for (const PendingPropertyType &type : m_pendingPropertyTypes) {
+        Q_ASSERT(type.scope->hasOwnProperty(type.name));
+
+        auto property = type.scope->ownProperty(type.name);
+
+        if (const auto propertyType = m_rootScopeImports.value(property.typeName())) {
+            property.setType(propertyType);
+            type.scope->addOwnProperty(property);
+        } else {
+            m_logger.log(property.typeName()
+                                 + QStringLiteral(" was not found. Did you add all import paths?"),
+                         Log_Import, type.location);
+        }
+    }
+}
+
+void QQmlJSImportVisitor::processPropertyBindingObjects()
+{
+    for (const PendingPropertyObjectBinding &objectBinding : m_pendingPropertyObjectBindings) {
+        const QString propertyName = objectBinding.name;
+        QQmlJSScope::ConstPtr childScope = objectBinding.childScope;
+
+        QQmlJSMetaProperty property = objectBinding.scope->property(propertyName);
+
+        if (property.isValid() && !property.type().isNull()
+            && (objectBinding.onToken || property.type()->canAssign(objectBinding.childScope))) {
+
+            QQmlJSMetaPropertyBinding binding =
+                    objectBinding.scope->hasOwnPropertyBinding(propertyName)
+                    ? objectBinding.scope->ownPropertyBinding(propertyName)
+                    : QQmlJSMetaPropertyBinding(property);
+
+            const QString typeName = getScopeName(childScope, QQmlJSScope::QMLScope);
+
+            if (objectBinding.onToken) {
+                if (childScope->hasInterface(QStringLiteral("QQmlPropertyValueInterceptor"))) {
+                    if (binding.hasInterceptor()) {
+                        m_logger.log(QStringLiteral("Duplicate interceptor on property \"%1\"")
+                                             .arg(propertyName),
+                                     Log_Property, objectBinding.location);
+                    } else {
+                        binding.setInterceptor(childScope);
+                        binding.setInterceptorTypeName(typeName);
+
+                        objectBinding.scope->addOwnPropertyBinding(binding);
+                    }
+                } else if (childScope->hasInterface(QStringLiteral("QQmlPropertyValueSource"))) {
+                    if (binding.hasValueSource()) {
+                        m_logger.log(QStringLiteral("Duplicate value source on property \"%1\"")
+                                             .arg(propertyName),
+                                     Log_Property, objectBinding.location);
+                    } else if (binding.hasValue()) {
+                        m_logger.log(QStringLiteral("Cannot combine value source and binding on "
+                                                    "property \"%1\"")
+                                             .arg(propertyName),
+                                     Log_Property, objectBinding.location);
+                    } else {
+                        binding.setValueSource(childScope);
+                        binding.setValueSourceTypeName(typeName);
+                        objectBinding.scope->addOwnPropertyBinding(binding);
+                    }
+                } else {
+                    m_logger.log(
+                            QStringLiteral("On-binding for property \"%1\" has wrong type \"%2\"")
+                                    .arg(propertyName)
+                                    .arg(typeName),
+                            Log_Property, objectBinding.location);
+                }
+            } else {
+                // TODO: Warn here if binding.hasValue() is true
+                if (binding.hasValueSource()) {
+                    m_logger.log(
+                            QStringLiteral(
+                                    "Cannot combine value source and binding on property \"%1\"")
+                                    .arg(propertyName),
+                            Log_Property, objectBinding.location);
+                } else {
+                    binding.setValue(childScope);
+                    binding.setValueTypeName(typeName);
+                    objectBinding.scope->addOwnPropertyBinding(binding);
+                }
+            }
+        } else if (!objectBinding.scope->isFullyResolved()) {
+            // If the current scope is not fully resolved we cannot tell whether the property exists
+            // or not (we already warn elsewhere)
+        } else if (!property.isValid()) {
+            m_logger.log(QStringLiteral("Property \"%1\" is invalid or does not exist")
+                                 .arg(propertyName),
+                         Log_Property, objectBinding.location);
+        } else if (property.type().isNull() || !property.type()->isFullyResolved()) {
+            // Property type is not fully resolved we cannot tell any more than this
+            m_logger.log(QStringLiteral("Property \"%1\" has incomplete type \"%2\". You may be "
+                                        "missing an import.")
+                                 .arg(propertyName)
+                                 .arg(property.typeName()),
+                         Log_Property, objectBinding.location);
+        } else if (!childScope->isFullyResolved()) {
+            // If the childScope type is not fully resolved we cannot tell whether the type is
+            // incompatible (we already warn elsewhere)
+        } else {
+            // the type is incompatible
+            m_logger.log(QStringLiteral("Property \"%1\" of type \"%2\" is assigned an "
+                                        "incompatible type \"%3\"")
+                                 .arg(propertyName)
+                                 .arg(property.typeName())
+                                 .arg(getScopeName(childScope, QQmlJSScope::QMLScope)),
+                         Log_Property, objectBinding.location);
+        }
+    }
+}
+
+void QQmlJSImportVisitor::checkRequiredProperties()
+{
+    for (const auto &required : m_requiredProperties) {
+        if (!required.scope->hasProperty(required.name)) {
+            m_logger.log(
+                    QStringLiteral("Property \"%1\" was marked as required but does not exist.")
+                            .arg(required.name),
+                    Log_Required, required.location);
+        }
+    }
+}
+
 void QQmlJSImportVisitor::checkPropertyBindings()
 {
     for (auto it = m_propertyBindings.constBegin(); it != m_propertyBindings.constEnd(); ++it) {
@@ -806,9 +934,6 @@ bool QQmlJSImportVisitor::visit(UiPublicMember *publicMember)
             if (m_rootScopeImports.contains(name) && !m_rootScopeImports[name].isNull()) {
                 if (m_importTypeLocationMap.contains(name))
                     m_usedTypes.insert(name);
-            } else {
-                m_logger.log(name + QStringLiteral(" was not found. Did you add all import paths?"),
-                             Log_Import);
             }
         }
         QQmlJSMetaProperty prop;
@@ -821,6 +946,10 @@ bool QQmlJSImportVisitor::visit(UiPublicMember *publicMember)
             const QString internalName = type->internalName();
             prop.setTypeName(internalName.isEmpty() ? typeName : internalName);
         } else {
+            if (!isAlias)
+                m_pendingPropertyTypes
+                        << PendingPropertyType { m_currentScope, prop.propertyName(),
+                                                 publicMember->firstSourceLocation() };
             prop.setTypeName(typeName);
         }
         prop.setAnnotations(parseAnnotations(publicMember->annotations));
@@ -841,13 +970,8 @@ bool QQmlJSImportVisitor::visit(UiRequired *required)
 {
     const QString name = required->name.toString();
 
-    // The required property must be defined in some scope
-    if (!m_currentScope->hasProperty(name)) {
-        m_logger.log(QStringLiteral("Property \"%1\" was marked as required but does not exist.").arg(name),
-                     Log_Required,
-                     required->firstSourceLocation());
-        return true;
-    }
+    m_requiredProperties << RequiredProperty { m_currentScope, name,
+                                               required->firstSourceLocation() };
 
     m_currentScope->setPropertyLocallyRequired(name, true);
     return true;
@@ -1354,94 +1478,13 @@ void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob)
 
     const QString propertyName = group->name.toString();
 
-    QQmlJSMetaProperty property = m_currentScope->property(propertyName);
-
     if (m_currentScope->isInCustomParserParent()) {
         // These warnings do not apply for custom parsers and their children and need to be handled
         // on a case by case basis
-    } else if (property.isValid() && !property.type().isNull()
-               && (uiob->hasOnToken || property.type()->canAssign(childScope))) {
-
-        QQmlJSMetaPropertyBinding binding = m_currentScope->hasOwnPropertyBinding(propertyName)
-                ? m_currentScope->ownPropertyBinding(propertyName)
-                : QQmlJSMetaPropertyBinding(property);
-
-        const QString typeName = getScopeName(childScope, QQmlJSScope::QMLScope);
-
-        if (uiob->hasOnToken) {
-            if (childScope->hasInterface(QStringLiteral("QQmlPropertyValueInterceptor"))) {
-                if (binding.hasInterceptor()) {
-                    m_logger.log(QStringLiteral("Duplicate interceptor on property \"%1\"")
-                                         .arg(propertyName),
-                                 Log_Property, uiob->firstSourceLocation());
-                } else {
-                    binding.setInterceptor(childScope);
-                    binding.setInterceptorTypeName(typeName);
-
-                    m_currentScope->addOwnPropertyBinding(binding);
-                }
-            } else if (childScope->hasInterface(QStringLiteral("QQmlPropertyValueSource"))) {
-                if (binding.hasValueSource()) {
-                    m_logger.log(QStringLiteral("Duplicate value source on property \"%1\"")
-                                         .arg(propertyName),
-                                 Log_Property, uiob->firstSourceLocation());
-                } else if (binding.hasValue()) {
-                    m_logger.log(
-                            QStringLiteral(
-                                    "Cannot combine value source and binding on property \"%1\"")
-                                    .arg(propertyName),
-                            Log_Property, uiob->firstSourceLocation());
-                } else {
-                    binding.setValueSource(childScope);
-                    binding.setValueSourceTypeName(typeName);
-                    m_currentScope->addOwnPropertyBinding(binding);
-                }
-            } else {
-                m_logger.log(QStringLiteral("On-binding for property \"%1\" has wrong type \"%2\"")
-                                     .arg(propertyName)
-                                     .arg(typeName),
-                             Log_Property, uiob->firstSourceLocation());
-            }
-        } else {
-            // TODO: Warn here if binding.hasValue() is true
-            if (binding.hasValueSource()) {
-                m_logger.log(
-                        QStringLiteral("Cannot combine value source and binding on property \"%1\"")
-                                .arg(propertyName),
-                        Log_Property, uiob->firstSourceLocation());
-            } else {
-                binding.setValue(childScope);
-                binding.setValueTypeName(typeName);
-                m_currentScope->addOwnPropertyBinding(binding);
-            }
-        }
-    } else if (!m_currentScope->isFullyResolved()) {
-        // If the current scope is not fully resolved we cannot tell whether the property exists or
-        // not (we already warn elsewhere)
-    } else if (!property.isValid()) {
-        m_logger.log(
-                QStringLiteral("Property \"%1\" is invalid or does not exist").arg(propertyName),
-                Log_Property, group->firstSourceLocation());
-    } else if (property.type().isNull() || !property.type()->isFullyResolved()) {
-        // Property type is not fully resolved we cannot tell any more than this
-        m_logger.log(
-                QStringLiteral(
-                        "Property \"%1\" has incomplete type \"%2\". You may be missing an import.")
-                        .arg(propertyName)
-                        .arg(property.typeName()),
-                Log_Property, group->firstSourceLocation());
-    } else if (!childScope->isFullyResolved()) {
-        // If the childScope type is not fully resolved we cannot tell whether the type is
-        // incompatible (we already warn elsewhere)
     } else {
-        // the type is incompatible
-        m_logger.log(
-                QStringLiteral(
-                        "Property \"%1\" of type \"%2\" is assigned an incompatible type \"%3\"")
-                        .arg(propertyName)
-                        .arg(property.typeName())
-                        .arg(getScopeName(childScope, QQmlJSScope::QMLScope)),
-                Log_Property, group->firstSourceLocation());
+        m_pendingPropertyObjectBindings
+                << PendingPropertyObjectBinding { m_currentScope, childScope, propertyName,
+                                                  uiob->firstSourceLocation(), uiob->hasOnToken };
     }
 
     while (m_currentScope->scopeType() == QQmlJSScope::GroupedPropertyScope
diff --git a/src/qmlcompiler/qqmljsimportvisitor_p.h b/src/qmlcompiler/qqmljsimportvisitor_p.h
index 5a91de50ac..1b7278c2a4 100644
--- a/src/qmlcompiler/qqmljsimportvisitor_p.h
+++ b/src/qmlcompiler/qqmljsimportvisitor_p.h
@@ -164,6 +164,9 @@ protected:
     void addDefaultProperties();
     void processDefaultProperties();
     void checkPropertyBindings();
+    void checkRequiredProperties();
+    void processPropertyTypes();
+    void processPropertyBindingObjects();
     void checkSignals();
     void flushPendingSignalParameters();
 
@@ -175,7 +178,33 @@ protected:
     // Used to temporarily store annotations for functions and generators wrapped in UiSourceElements
     QVector<QQmlJSAnnotation> m_pendingMethodAnnotations;
 
+    struct PendingPropertyType
+    {
+        QQmlJSScope::Ptr scope;
+        QString name;
+        QQmlJS::SourceLocation location;
+    };
+
+    struct PendingPropertyObjectBinding
+    {
+        QQmlJSScope::Ptr scope;
+        QQmlJSScope::ConstPtr childScope;
+        QString name;
+        QQmlJS::SourceLocation location;
+        bool onToken;
+    };
+
+    struct RequiredProperty
+    {
+        QQmlJSScope::Ptr scope;
+        QString name;
+        QQmlJS::SourceLocation location;
+    };
+
     QHash<QQmlJSScope::Ptr, QVector<QQmlJSScope::Ptr>> m_pendingDefaultProperties;
+    QVector<PendingPropertyType> m_pendingPropertyTypes;
+    QVector<PendingPropertyObjectBinding> m_pendingPropertyObjectBindings;
+    QVector<RequiredProperty> m_requiredProperties;
     QVector<QQmlJSScope::Ptr> m_objectBindingScopes;
     QVector<QQmlJSScope::Ptr> m_objectDefinitionScopes;
 
diff --git a/tests/auto/qml/qmllint/data/bindingBeforeDeclaration.qml b/tests/auto/qml/qmllint/data/bindingBeforeDeclaration.qml
new file mode 100644
index 0000000000..7fc592d7f0
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/bindingBeforeDeclaration.qml
@@ -0,0 +1,6 @@
+import QtQuick
+
+Item {
+    binding: QtObject {}
+    property QtObject binding
+}
diff --git a/tests/auto/qml/qmllint/data/inlineComponentsChained.qml b/tests/auto/qml/qmllint/data/inlineComponentsChained.qml
new file mode 100644
index 0000000000..cc063b1bc2
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/inlineComponentsChained.qml
@@ -0,0 +1,10 @@
+import QtQuick
+
+Item {
+  component A : B { required foo }
+  component B : C { property QtObject foo }
+  component C : Item {
+  }
+  A { foo: QtObject {} }
+}
+
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index ce518171e6..5c307d9fb8 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -158,7 +158,8 @@ void TestQmllint::testUnknownCausesFail()
                 QStringLiteral("Warning: %1: Unknown was not found. Did you add all import paths?").arg(testFile("unknownElement.qml"))));
     unknownNotFound = runQmllint("TypeWithUnknownPropertyType.qml", false);
     QVERIFY(unknownNotFound.contains(
-            QStringLiteral("Warning: %1: Something was not found. Did you add all import paths?")
+            QStringLiteral(
+                    "Warning: %1:4:5: Something was not found. Did you add all import paths?")
                     .arg(testFile("TypeWithUnknownPropertyType.qml"))));
 }
 
@@ -754,7 +755,9 @@ void TestQmllint::cleanQmlCode_data()
     QTest::newRow("qjsroot") << QStringLiteral("qjsroot.qml");
     QTest::newRow("InlineComponent") << QStringLiteral("inlineComponent.qml");
     QTest::newRow("InlineComponentWithComponents") << QStringLiteral("inlineComponentWithComponents.qml");
+    QTest::newRow("InlineComponentsChained") << QStringLiteral("inlineComponentsChained.qml");
     QTest::newRow("ignoreWarnings") << QStringLiteral("ignoreWarnings.qml");
+    QTest::newRow("BindingBeforeDeclaration") << QStringLiteral("bindingBeforeDeclaration.qml");
 }
 
 void TestQmllint::cleanQmlCode()