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.5
Fixes: QTBUG-119162
Change-Id: Ic5d3cc3342b4518d3fde1b800efe1b95d8e8b210
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 84d950bc32)
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>

src/qml/doc/src/qmllanguageref/documents/definetypes.qdoc

@@ -254,13 +254,23 @@ The same declaration can also be given for C++-defined types. See
 
 \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:
@@ -269,8 +279,33 @@ 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

src/qml/jsruntime/qv4executablecompilationunit_p.h

@@ -213,6 +213,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
     {

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) {

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()

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"));

tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt

@@ -198,6 +198,7 @@ set(qml_files
     registerPropagation.qml
     registerelimination.qml
     revisions.qml
+    scopeIdLookup.qml
     scopeVsObject.qml
     script.js
     script.mjs

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
+        }
+    }
+}

tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp

@@ -163,6 +163,7 @@ private slots:
     void registerElimination();
     void registerPropagation();
     void revisions();
+    void scopeIdLookup();
     void scopeObjectDestruction();
     void scopeVsObject();
     void sequenceToIterable();
@@ -3445,6 +3446,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;

tests/auto/quickcontrols/controls/data/tst_stackview.qml

@@ -1604,7 +1604,7 @@ TestCase {
         StackView {
             id: stackView
             anchors.fill: parent
-            initialItem: cppComponent
+            initialItem: stackView.cppComponent
 
             property Component cppComponent: ComponentCreator.createComponent("import QtQuick; Rectangle { color: \"navajowhite\" }")
         }