QML: Let IDs in outer context override bound components' properties

This is necessary to make the usage of such IDs actually safe. If we let
local properties override outer IDs, then adding local properties in
later versions invalidates the ID lookups.

[ChangeLog][QtQml][Important Behavior Changes] In QML documents with
bound components, IDs defined in outer contexts override properties
defined in inner contexts now. This is how qmlcachegen has always
interpreted bound components when generating C++ code, and it is
required to make access to outer IDs actually safe. The interpreter and
JIT have previously preferred inner properties over outer IDs.

Pick-to: 6.6 6.5
Fixes: QTBUG-119162
Change-Id: Ic5d3cc3342b4518d3fde1b800efe1b95d8e8b210
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

9 files changed, 112 insertions, 18 deletions

diff --git a/src/qml/doc/src/qmllanguageref/documents/structure.qdoc b/src/qml/doc/src/qmllanguageref/documents/structure.qdoc
index c85dc53c1f..c2453e1cb5 100644
--- a/src/qml/doc/src/qmllanguageref/documents/structure.qdoc
+++ b/src/qml/doc/src/qmllanguageref/documents/structure.qdoc
@@ -64,13 +64,23 @@ class declaration.
 
 \section2 ComponentBehavior
 
-With this pragma you can restrict components defined in this file to only
-create objects within their original context. This holds for inline
-components as well as Component elements explicitly or implicitly created
-as properties. If a component is bound to its context, you can safely
-use IDs from the rest of the file within the component. Otherwise, the
-engine and the QML tooling cannot know in advance what type, if any, such
-IDs will resolve to at run time.
+You may have multiple components defined in the same QML file. The root
+scope of the QML file is a component, and you may additionally have
+elements of type \l QQmlComponent, explicitly or implicitly created
+as properties, or inline components. Those components are nested. Each
+of the inner components is within one specific outer component. Most of
+the time, IDs defined in an outer component are accessible within all
+its nested inner components. You can, however, create elements from a
+component in any a different context, with different IDs available.
+Doing so breaks the assumption that outer IDs are available. Therefore,
+the engine and the QML tooling cannot generally know in advance what
+type, if any, such IDs will resolve to at run time.
+
+With the ComponentBehavior pragma you can restrict all inner components
+defined in a file to only create objects within their original context.
+If a component is bound to its context, you can safely use IDs from
+outer components in the same file within the component. QML tooling will
+then assume the outer IDs with their specific types to be available.
 
 In order to bind the components to their context specify the \c{Bound}
 argument:
@@ -79,8 +89,34 @@ argument:
 pragma ComponentBehavior: Bound
 \endqml
 
-The default is \c{Unbound}. You can also specify it explicitly. In a
-future version of Qt the default will change to \c{Bound}.
+This implies that, in case of name clashes, IDs defined outside a bound
+component override local properties of objects created from the
+component. Otherwise it wouldn't actually be safe to use the IDs since
+later versions of a module might add more properties to the component.
+If the component is not bound, local properties override IDs defined
+outside the component, but not IDs defined inside the component.
+
+The example below prints the \e r property of the ListView object with
+the id \e color, not the \e r property of the rectangle's color.
+
+\qml
+pragma ComponentBehavior: Bound
+import QtQuick
+
+ListView {
+ id: color
+ property int r: 12
+ model: 1
+
+ delegate: Rectangle {
+  Component.onCompleted: console.log(color.r)
+ }
+}
+\endqml
+
+The default value of \c ComponentBehavior is \c{Unbound}. You can also
+specify it explicitly. In a future version of Qt the default will change
+to \c{Bound}.
 
 Delegate components bound to their context don't receive their own
 private contexts on instantiation. This means that model data can only
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 5ad3838658..747f776f4a 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -215,6 +215,11 @@ public:
         return data->flags & CompiledData::Unit::ValueTypesAddressable;
     }
 
+    bool componentsAreBound() const
+    {
+        return data->flags & CompiledData::Unit::ComponentsBound;
+    }
+
     int objectCount() const { return qmlData->nObjects; }
     const CompiledObject *objectAt(int index) const
     {
diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index f032e953b9..7b4e5e7387 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -269,9 +269,33 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r
         contextGetterFunction = QQmlContextWrapper::lookupScopeObjectProperty;
     }
 
+    QQmlRefPointer<QQmlContextData> outer = context;
     while (context) {
-        if (auto property = searchContextProperties(v4, context, name, hasProperty, base, lookup, originalLookup, ep))
-            return *property;
+        if (outer == context) {
+            if (auto property = searchContextProperties(
+                        v4, context, name, hasProperty, base, lookup, originalLookup, ep)) {
+                return *property;
+            }
+
+            outer = outer->parent();
+
+            if (const auto cu = context->typeCompilationUnit(); cu && cu->componentsAreBound()) {
+                // If components are bound in this CU, we can search the whole context hierarchy
+                // of the file. Bound components' contexts override their local properties.
+                // You also can't instantiate bound components outside of their creation
+                // context. Therefore this is safe.
+
+                for (;
+                     outer && outer->typeCompilationUnit() == cu;
+                     outer = outer->parent()) {
+                    if (auto property = searchContextProperties(
+                                v4, outer, name, hasProperty, base,
+                                nullptr, originalLookup, ep)) {
+                        return *property;
+                    }
+                }
+            }
+        }
 
         // Search scope object
         if (scopeObject) {
diff --git a/src/qml/qml/qqmlcomponent_p.h b/src/qml/qml/qqmlcomponent_p.h
index 8607cf7f3b..1d3f1d03aa 100644
--- a/src/qml/qml/qqmlcomponent_p.h
+++ b/src/qml/qml/qqmlcomponent_p.h
@@ -167,10 +167,7 @@ public:
     QObject *createWithProperties(QObject *parent, const QVariantMap &properties,
                                   QQmlContext *context, CreateBehavior behavior = CreateDefault);
 
-    bool isBound() const {
-        return compilationUnit
-                && (compilationUnit->unitData()->flags & QV4::CompiledData::Unit::ComponentsBound);
-    }
+    bool isBound() const { return compilationUnit && (compilationUnit->componentsAreBound()); }
 };
 
 QQmlComponentPrivate::ConstructionState::~ConstructionState()
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 3ecca952da..10255b5f8f 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -154,7 +154,7 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
     } else {
         Q_ASSERT(subComponentIndex >= 0);
         if (flags & CreationFlags::InlineComponent) {
-            if (compilationUnit->unitData()->flags & QV4::CompiledData::Unit::ComponentsBound
+            if (compilationUnit->componentsAreBound()
                     && compilationUnit != parentContext->typeCompilationUnit()) {
                 recordError({}, tr("Cannot instantiate bound inline component in different file"));
                 phase = ObjectsCreated;
@@ -164,7 +164,7 @@ QObject *QQmlObjectCreator::create(int subComponentIndex, QObject *parent, QQmlI
             isComponentRoot = true;
         } else {
             Q_ASSERT(flags & CreationFlags::NormalObject);
-            if (compilationUnit->unitData()->flags & QV4::CompiledData::Unit::ComponentsBound
+            if (compilationUnit->componentsAreBound()
                     && sharedState->creationContext != parentContext) {
                 recordError({}, tr("Cannot instantiate bound component "
                                    "outside its creation context"));
diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
index f8b2dd8dba..3ae107628c 100644
--- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
+++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
@@ -211,6 +211,7 @@ set(qml_files
     registerPropagation.qml
     registerelimination.qml
     revisions.qml
+    scopeIdLookup.qml
     scopeVsObject.qml
     script.js
     script.mjs
diff --git a/tests/auto/qml/qmlcppcodegen/data/scopeIdLookup.qml b/tests/auto/qml/qmlcppcodegen/data/scopeIdLookup.qml
new file mode 100644
index 0000000000..e23f180598
--- /dev/null
+++ b/tests/auto/qml/qmlcppcodegen/data/scopeIdLookup.qml
@@ -0,0 +1,20 @@
+pragma ComponentBehavior: Bound
+
+import QtQml
+
+QtObject {
+    id: root
+
+    property QtObject b: QtObject {
+        id: bar
+        objectName: "outer"
+    }
+
+    property Instantiator i: Instantiator {
+        model: 1
+        delegate: QtObject {
+            property QtObject bar: QtObject { objectName: "inner" }
+            Component.onCompleted: root.objectName = bar.objectName
+        }
+    }
+}
diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
index b341e1bb80..51f797b05b 100644
--- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
+++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
@@ -181,6 +181,7 @@ private slots:
     void registerElimination();
     void registerPropagation();
     void revisions();
+    void scopeIdLookup();
     void scopeObjectDestruction();
     void scopeVsObject();
     void sequenceToIterable();
@@ -3746,6 +3747,16 @@ void tst_QmlCppCodegen::revisions()
     QCOMPARE(o->property("gotten").toInt(), 5);
 }
 
+void tst_QmlCppCodegen::scopeIdLookup()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, QUrl(u"qrc:/qt/qml/TestTypes/scopeIdLookup.qml"_s));
+    QVERIFY2(!component.isError(), component.errorString().toUtf8());
+    QScopedPointer<QObject> object(component.create());
+    QVERIFY(!object.isNull());
+    QCOMPARE(object->property("objectName").toString(), u"outer"_s);
+}
+
 void tst_QmlCppCodegen::scopeObjectDestruction()
 {
     QQmlEngine engine;
diff --git a/tests/auto/quickcontrols/controls/data/tst_stackview.qml b/tests/auto/quickcontrols/controls/data/tst_stackview.qml
index 7dbe361cfc..098ee5903b 100644
--- a/tests/auto/quickcontrols/controls/data/tst_stackview.qml
+++ b/tests/auto/quickcontrols/controls/data/tst_stackview.qml
@@ -1801,7 +1801,7 @@ TestCase {
         StackView {
             id: stackView
             anchors.fill: parent
-            initialItem: cppComponent
+            initialItem: stackView.cppComponent
 
             property Component cppComponent: ComponentCreator.createComponent("import QtQuick; Rectangle { color: \"navajowhite\" }")
         }