DesignSystem: Detect and reject cyclic design system bindings

Task-number: QDS-14670
Change-Id: I35a40037e18983b200004a12aeb788ae51c7d764
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/components/designsystemview/collectionmodel.cpp

@@ -237,13 +237,17 @@ bool CollectionModel::setData(const QModelIndex &index, const QVariant &value, i
     ThemeProperty p = value.value<ThemeProperty>();
     switch (role) {
     case Qt::EditRole: {
+        const auto [groupType, propName] = m_propertyInfoList[index.row()];
+        p.name = propName;
         if (p.isBinding) {
-            if (!m_store->resolvedDSBinding(p.value.toString()))
+            // Check if binding is valid design system binding.
+            const QString collectionName = m_store->typeName(m_collection).value_or("");
+            const QString propName = QString::fromLatin1(p.name);
+            CollectionBinding currentPropBinding{collectionName, propName};
+            if (!m_store->resolvedDSBinding(p.value.toString(), currentPropBinding))
                 return false; // Invalid binding, it must resolved to a valid property.
         }
 
-        const auto [groupType, propName] = m_propertyInfoList[index.row()];
-        p.name = propName;
         const ThemeId id = m_themeIdList[index.column()];
         if (m_collection->updateProperty(id, groupType, p)) {
             updateCache();
@@ -283,10 +287,8 @@ bool CollectionModel::setHeaderData(int section,
         if (auto propInfo = findPropertyName(section)) {
             auto [groupType, propName] = *propInfo;
             success = m_collection->renameProperty(groupType, propName, newName);
-            if (success) {
+            if (success)
-                const auto collectionName = m_store->typeName(m_collection);
                 m_store->refactorBindings(m_collection, propName, newName);
-            }
         }
     } else {
         // Theme

src/plugins/qmldesigner/libs/designsystem/dsconstants.h

@@ -37,5 +37,20 @@ constexpr const char *GroupId(const GroupType type) {
     return "unknown";
 }
 
+constexpr std::optional<QmlDesigner::GroupType> groupIdToGroupType(const char *type)
+{
+    const std::string_view typeStr(type);
+    if (typeStr == "colors")
+        return QmlDesigner::GroupType::Colors;
+    if (typeStr == "flags")
+        return QmlDesigner::GroupType::Flags;
+    if (typeStr == "numbers")
+        return QmlDesigner::GroupType::Numbers;
+    if (typeStr == "strings")
+        return QmlDesigner::GroupType::Strings;
+
+    return {};
+}
+
 using DSBindingInfo = std::tuple<PropertyName, ThemeId, GroupType, QStringView>;
 }

src/plugins/qmldesigner/libs/designsystem/dsstore.cpp

@@ -284,36 +284,18 @@ QStringList DSStore::collectionNames() const
     return names;
 }
 
-std::optional<ThemeProperty> DSStore::resolvedDSBinding(QStringView binding) const
+std::optional<ThemeProperty> DSStore::resolvedDSBinding(QStringView binding,
+                                                        std::optional<CollectionBinding> avoidValue) const
 {
     if (auto parts = unpackDSBinding(binding)) {
-        auto &[collectionName, _, propertyName] = *parts;
+        auto &[collectionName, groupId, propertyName] = *parts;
-        return resolvedDSBinding(collectionName, propertyName);
+        return resolvedDSBinding({collectionName, propertyName}, groupId, avoidValue);
     }
 
     qCDebug(dsLog) << "Resolving binding failed. Unexpected binding" << binding;
     return {};
 }
 
-std::optional<ThemeProperty> DSStore::resolvedDSBinding(QStringView collectionName,
-                                                        QStringView propertyName) const
-{
-    auto itr = m_collections.find(collectionName.toString());
-    if (itr == m_collections.end())
-        return {};
-
-    const DSThemeManager &boundCollection = itr->second;
-    if (const auto group = boundCollection.groupType(propertyName.toLatin1())) {
-        auto property = boundCollection.property(boundCollection.activeTheme(),
-                                                 *group,
-                                                 propertyName.toLatin1());
-        if (property)
-            return property->isBinding ? resolvedDSBinding(property->value.toString()) : *property;
-    }
-
-    return {};
-}
-
 void DSStore::refactorBindings(QStringView oldCollectionName, QStringView newCollectionName)
 {
     for (auto &[_, currentCollection] : m_collections) {
@@ -372,11 +354,11 @@ void DSStore::breakBindings(DSThemeManager *collection, PropertyName propertyNam
                 qCDebug(dsLog) << "Error breaking binding. Unexpected binding" << expression;
                 continue;
             }
-            const auto &[boundCollection, _, boundProp] = *bindingParts;
+            const auto &[boundCollection, groupId, boundProp] = *bindingParts;
             if (boundCollection != collectionName || propertyName != boundProp.toLatin1())
                 continue;
 
-            if (auto value = resolvedDSBinding(*collectionName, boundProp))
+            if (auto value = resolvedDSBinding({*collectionName, boundProp}, groupId))
                 currentCollection.updateProperty(themeId, gt, {propName, value->value});
         }
     }
@@ -391,11 +373,11 @@ void DSStore::breakBindings(DSThemeManager *collection, QStringView removeCollec
             continue;
         }
 
-        const auto &[boundCollection, _, boundProp] = *bindingParts;
+        const auto &[boundCollection, groupId, boundProp] = *bindingParts;
         if (boundCollection != removeCollection)
             continue;
 
-        if (auto value = resolvedDSBinding(boundCollection, boundProp))
+        if (auto value = resolvedDSBinding({boundCollection, boundProp}, groupId))
             collection->updateProperty(themeId, gt, {propName, value->value});
     }
 }
@@ -407,6 +389,67 @@ QString DSStore::uniqueCollectionName(const QString &hint) const
     });
 }
 
+std::optional<ThemeProperty> DSStore::boundProperty(const CollectionBinding &binding,
+                                                    QStringView groupId) const
+{
+    auto bindingGroupType = groupIdToGroupType(groupId.toLatin1());
+    if (!bindingGroupType)
+        return {};
+
+    auto itr = m_collections.find(binding.collection.toString());
+    if (itr != m_collections.end()) {
+        const DSThemeManager &boundCollection = itr->second;
+        const auto propertyName = binding.propName.toLatin1();
+        if (const auto group = boundCollection.groupType(propertyName)) {
+            if (group != *bindingGroupType)
+                return {}; // Found property has a different group.
+
+            return boundCollection.property(boundCollection.activeTheme(), *group, propertyName);
+        }
+    }
+    return {};
+}
+
+std::optional<ThemeProperty> DSStore::resolvedDSBinding(CollectionBinding binding,
+                                                        QStringView groupId,
+                                                        std::optional<CollectionBinding> avoidValue) const
+{
+    std::unordered_set<QStringView> visited;
+    const auto hasCycle = [&visited](const CollectionBinding &binding) {
+        // Return true if the dsBinding token exists, insert otherwise.
+        const auto token = QString("%1.%2").arg(binding.collection, binding.propName);
+        return !visited.emplace(token).second;
+    };
+
+    if (avoidValue) // Insert the extra binding for cycle detection
+        hasCycle(*avoidValue);
+
+    std::optional<ThemeProperty> resolvedProperty;
+    do {
+        if (hasCycle(binding)) {
+            qCDebug(dsLog) << "Cyclic binding";
+            resolvedProperty = {};
+        } else {
+            resolvedProperty = boundProperty(binding, groupId);
+        }
+
+        if (resolvedProperty && resolvedProperty->isBinding) {
+            // The value is again a binding.
+            if (auto bindingParts = unpackDSBinding(resolvedProperty->value.toString())) {
+                std::tie(binding.collection, std::ignore, binding.propName) = *bindingParts;
+            } else {
+                qCDebug(dsLog) << "Invalid binding" << resolvedProperty->value.toString();
+                resolvedProperty = {};
+            }
+        }
+    } while (resolvedProperty && resolvedProperty->isBinding);
+
+    if (!resolvedProperty)
+        qCDebug(dsLog) << "Can not resolve binding." << binding.collection << binding.propName;
+
+    return resolvedProperty;
+}
+
 DSThemeManager *DSStore::collection(const QString &typeName)
 {
     auto itr = m_collections.find(typeName);

src/plugins/qmldesigner/libs/designsystem/dsstore.h

@@ -11,6 +11,12 @@
 namespace QmlDesigner {
 class ExternalDependenciesInterface;
 
+struct CollectionBinding
+{
+    QStringView collection;
+    QStringView propName;
+};
+
 class DESIGNSYSTEM_EXPORT DSStore
 {
     Q_DECLARE_TR_FUNCTIONS(DSStore)
@@ -38,9 +44,8 @@ public:
     std::optional<Utils::FilePath> moduleDirPath() const;
     QStringList collectionNames() const;
 
-    std::optional<ThemeProperty> resolvedDSBinding(QStringView binding) const;
+    std::optional<ThemeProperty> resolvedDSBinding(QStringView binding,
-    std::optional<ThemeProperty> resolvedDSBinding(QStringView collectionName,
+                                                   std::optional<CollectionBinding> avoidValue = {}) const;
-                                                   QStringView propertyName) const;
 
     void refactorBindings(QStringView oldCollectionName, QStringView newCollectionName);
     void refactorBindings(DSThemeManager *srcCollection, PropertyName from, PropertyName to);
@@ -51,6 +56,11 @@ public:
     QString uniqueCollectionName(const QString &hint) const;
 
 private:
+    std::optional<ThemeProperty> boundProperty(const CollectionBinding &binding,
+                                               QStringView groupId) const;
+    std::optional<ThemeProperty> resolvedDSBinding(CollectionBinding binding,
+                                                   QStringView groupId,
+                                                   std::optional<CollectionBinding> avoidValue = {}) const;
     std::optional<QString> loadCollection(const QString &typeName, const Utils::FilePath &qmlFilePath);
     std::optional<QString> writeQml(const DSThemeManager &mgr,
                                     const QString &typeName,