Prevent creation of nonexistent binding expressions

Change-Id: Id6f9f35cd40667d694fcdf06d51c4642ae71afcd
Reviewed-by: Qt CI Patch Build Bot <ci_patchbuild_bot@qt.io>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/components/connectioneditor/bindingmodel.cpp

@@ -3,8 +3,8 @@
 
 #include "bindingmodel.h"
 #include "bindingmodelitem.h"
-#include "connectionview.h"
 #include "connectioneditorutils.h"
+#include "connectionview.h"
 #include "modelfwd.h"
 
 #include <bindingproperty.h>
@@ -16,6 +16,8 @@
 
 #include <utils/qtcassert.h>
 
+#include <QSignalBlocker>
+
 namespace QmlDesigner {
 
 BindingModel::BindingModel(ConnectionView *parent)
@@ -250,15 +252,15 @@ BindingModelBackendDelegate::BindingModelBackendDelegate(BindingModel *parent)
     , m_sourceNodeProperty()
 {
     connect(&m_sourceNode, &StudioQmlComboBoxBackend::activated, this, [this]() {
-        expressionChanged();
+        sourceNodeChanged();
     });
 
     connect(&m_sourceNodeProperty, &StudioQmlComboBoxBackend::activated, this, [this]() {
-        expressionChanged();
+        sourcePropertyNameChanged();
     });
 
     connect(&m_property, &StudioQmlComboBoxBackend::activated, this, [this]() {
-        propertyNameChanged();
+        targetPropertyNameChanged();
     });
 }
 
@@ -267,7 +269,7 @@ void BindingModelBackendDelegate::update(const BindingProperty &property, Abstra
     if (!property.isValid())
         return;
 
-    auto addName = [](QStringList&& list, const QString& name) {
+    auto addName = [](QStringList &&list, const QString &name) {
         if (!list.contains(name))
             list.prepend(name);
         return std::move(list);
@@ -282,7 +284,8 @@ void BindingModelBackendDelegate::update(const BindingProperty &property, Abstra
     m_sourceNode.setModel(sourceNodes);
     m_sourceNode.setCurrentText(sourceNodeName);
 
-    auto sourceproperties = addName(availableSourceProperties(property, view), sourcePropertyName);
+    auto availableProperties = availableSourceProperties(sourceNodeName, property, view);
+    auto sourceproperties = addName(std::move(availableProperties), sourcePropertyName);
     m_sourceNodeProperty.setModel(sourceproperties);
     m_sourceNodeProperty.setCurrentText(sourcePropertyName);
 
@@ -316,14 +319,40 @@ StudioQmlComboBoxBackend *BindingModelBackendDelegate::sourceProperty()
     return &m_sourceNodeProperty;
 }
 
-void BindingModelBackendDelegate::expressionChanged() const
+void BindingModelBackendDelegate::sourceNodeChanged()
 {
-    auto commit = [this]() {
+    BindingModel *model = qobject_cast<BindingModel *>(parent());
+    QTC_ASSERT(model, return);
+
+    ConnectionView *view = model->connectionView();
+    QTC_ASSERT(view, return);
+
+    const QString sourceNode = m_sourceNode.currentText();
+    const QString sourceProperty = m_sourceNodeProperty.currentText();
+
+    BindingProperty targetProperty = model->currentProperty();
+    QStringList properties = availableSourceProperties(sourceNode, targetProperty, view);
+
+    if (!properties.contains(sourceProperty)) {
+        QSignalBlocker blocker(this);
+        properties.prepend("---");
+        m_sourceNodeProperty.setModel(properties);
+        m_sourceNodeProperty.setCurrentText({"---"});
+    }
+    sourcePropertyNameChanged();
+}
+
+void BindingModelBackendDelegate::sourcePropertyNameChanged() const
+{
+    const QString sourceProperty = m_sourceNodeProperty.currentText();
+    if (sourceProperty.isEmpty() || sourceProperty == "---")
+        return;
+
+    auto commit = [this, sourceProperty]() {
         BindingModel *model = qobject_cast<BindingModel *>(parent());
         QTC_ASSERT(model, return);
 
         const QString sourceNode = m_sourceNode.currentText();
-        const QString sourceProperty = m_sourceNodeProperty.currentText();
         QString expression;
         if (sourceProperty.isEmpty())
             expression = sourceNode;
@@ -337,7 +366,7 @@ void BindingModelBackendDelegate::expressionChanged() const
     callLater(commit);
 }
 
-void BindingModelBackendDelegate::propertyNameChanged() const
+void BindingModelBackendDelegate::targetPropertyNameChanged() const
 {
     auto commit = [this]() {
         BindingModel *model = qobject_cast<BindingModel *>(parent());

src/plugins/qmldesigner/components/connectioneditor/bindingmodel.h

@@ -86,8 +86,9 @@ public:
 
 private:
     QString targetNode() const;
-    void expressionChanged() const;
+    void sourceNodeChanged();
-    void propertyNameChanged() const;
+    void sourcePropertyNameChanged() const;
+    void targetPropertyNameChanged() const;
 
     StudioQmlComboBoxBackend *property();
     StudioQmlComboBoxBackend *sourceNode();

src/plugins/qmldesigner/components/connectioneditor/connectioneditorutils.cpp

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 #include "connectioneditorutils.h"
 
-#include <QtCore/qvariant.h>
 #include <abstractproperty.h>
 #include <abstractview.h>
 #include <bindingproperty.h>
@@ -58,61 +57,66 @@ PropertyName uniquePropertyName(const PropertyName &suggestion, const ModelNode
     return {};
 }
 
-NodeMetaInfo dynamicTypeMetaInfo(const AbstractProperty& property)
+NodeMetaInfo dynamicTypeNameToNodeMetaInfo(const TypeName &typeName, Model *model)
 {
     // Note: Uses old mechanism to create the NodeMetaInfo and supports
     // only types we care about in the connection editor.
     // TODO: Support all possible AbstractProperty types and move to the
     // AbstractProperty class.
-    if (property.dynamicTypeName() == "bool")
+    if (typeName == "bool")
-        return property.model()->boolMetaInfo();
+        return model->boolMetaInfo();
-    else if (property.dynamicTypeName() == "int")
+    else if (typeName == "int")
-        return property.model()->metaInfo("QML.int");
+        return model->metaInfo("QML.int");
-    else if (property.dynamicTypeName() == "real")
+    else if (typeName == "real")
-        return property.model()->metaInfo("QML.real");
+        return model->metaInfo("QML.real");
-    else if (property.dynamicTypeName() == "color")
+    else if (typeName == "color")
-        return property.model()->metaInfo("QML.color");
+        return model->metaInfo("QML.color");
-    else if (property.dynamicTypeName() == "string")
+    else if (typeName == "string")
-        return property.model()->metaInfo("QML.string");
+        return model->metaInfo("QML.string");
-    else if (property.dynamicTypeName() == "url")
+    else if (typeName == "url")
-        return property.model()->metaInfo("QML.url");
+        return model->metaInfo("QML.url");
-    else if (property.dynamicTypeName() == "variant")
+    else if (typeName == "variant")
-        return property.model()->metaInfo("QML.variant");
+        return model->metaInfo("QML.variant");
     else
-        qWarning() << __FUNCTION__ << " type " << property.dynamicTypeName() << "not found";
+        qWarning() << __FUNCTION__ << " type " << typeName << "not found";
-    return { };
+    return {};
 }
 
-bool metaInfoIsCompatibleUnsafe(const NodeMetaInfo &sourceType, const NodeMetaInfo &targetType)
+NodeMetaInfo dynamicTypeMetaInfo(const AbstractProperty &property)
 {
-    if (sourceType.isVariant())
+    return dynamicTypeNameToNodeMetaInfo(property.dynamicTypeName(), property.model());
+}
+
+bool metaInfoIsCompatibleUnsafe(const NodeMetaInfo &target, const NodeMetaInfo &source)
+{
+    if (target.isVariant())
         return true;
 
-    if (sourceType.isBool() && targetType.isBool())
+    if (target == source)
         return true;
 
-    if (sourceType == targetType)
+    if (target.isBool() && source.isBool())
         return true;
 
-    if (sourceType.isNumber() && targetType.isNumber())
+    if (target.isNumber() && source.isNumber())
         return true;
 
-    if (sourceType.isString() && targetType.isString())
+    if (target.isString() && source.isString())
         return true;
 
-    if (sourceType.isUrl() && targetType.isUrl())
+    if (target.isUrl() && source.isUrl())
         return true;
 
-    if (sourceType.isColor() && targetType.isColor())
+    if (target.isColor() && source.isColor())
         return true;
 
     return false;
 }
 
-bool metaInfoIsCompatible(const NodeMetaInfo &sourceType, const PropertyMetaInfo &metaInfo)
+bool metaInfoIsCompatible(const NodeMetaInfo &targetType, const PropertyMetaInfo &sourceInfo)
 {
-    NodeMetaInfo targetType = metaInfo.propertyType();
+    NodeMetaInfo sourceType = sourceInfo.propertyType();
-    return metaInfoIsCompatibleUnsafe(sourceType, targetType);
+    return metaInfoIsCompatibleUnsafe(targetType, sourceType);
 }
 
 QVariant typeConvertVariant(const QVariant &variant, const QmlDesigner::TypeName &typeName)
@@ -194,7 +198,7 @@ void convertBindingToVariantProperty(const BindingProperty &property, const QVar
     convertPropertyType(property, value);
 }
 
-bool isBindingExpression(const QVariant& value)
+bool isBindingExpression(const QVariant &value)
 {
     if (value.metaType().id() != QMetaType::QString)
         return false;
@@ -250,11 +254,30 @@ QString defaultExpressionForType(const TypeName &type)
     return expression;
 }
 
+std::pair<QString, QString> splitExpression(const QString &expression)
+{
+    // ### Todo from original code (getExpressionStrings):
+    //      We assume no expressions yet
+    const QStringList stringList = expression.split(QLatin1String("."));
+
+    QString sourceNode = stringList.constFirst();
+    QString propertyName;
+    for (int i = 1; i < stringList.size(); ++i) {
+        propertyName += stringList.at(i);
+        if (i != stringList.size() - 1)
+            propertyName += QLatin1String(".");
+    }
+    if (propertyName.isEmpty())
+        std::swap(sourceNode, propertyName);
+
+    return {sourceNode, propertyName};
+}
+
 QStringList singletonsFromView(AbstractView *view)
 {
     RewriterView *rv = view->rewriterView();
     if (!rv)
-        return { };
+        return {};
 
     QStringList out;
     for (const QmlTypeData &data : rv->getQMLTypes()) {
@@ -264,6 +287,29 @@ QStringList singletonsFromView(AbstractView *view)
     return out;
 }
 
+std::vector<PropertyMetaInfo> propertiesFromSingleton(const QString &name, AbstractView *view)
+{
+    Model *model = view->model();
+    QTC_ASSERT(model, return {});
+
+    if (NodeMetaInfo metaInfo = model->metaInfo(name.toUtf8()); metaInfo.isValid())
+        return metaInfo.properties();
+
+    return {};
+}
+
+QList<AbstractProperty> dynamicPropertiesFromNode(const ModelNode &node)
+{
+    auto isDynamic = [](const AbstractProperty &p) { return p.isDynamic(); };
+    auto byName = [](const AbstractProperty &a, const AbstractProperty &b) {
+        return a.name() < b.name();
+    };
+
+    QList<AbstractProperty> dynamicProperties = Utils::filtered(node.properties(), isDynamic);
+    Utils::sort(dynamicProperties, byName);
+    return dynamicProperties;
+}
+
 QStringList availableSources(AbstractView *view)
 {
     QStringList sourceNodes;
@@ -295,7 +341,7 @@ QStringList availableTargetProperties(const BindingProperty &bindingProperty)
 
         return writableProperties;
     }
-    return { };
+    return {};
 }
 
 ModelNode getNodeByIdOrParent(AbstractView *view, const QString &id, const ModelNode &targetNode)
@@ -309,19 +355,17 @@ ModelNode getNodeByIdOrParent(AbstractView *view, const QString &id, const Model
     return {};
 }
 
-QStringList availableSourceProperties(const BindingProperty &bindingProperty, AbstractView *view)
+QStringList availableSourceProperties(const QString &id,
+                                      const BindingProperty &targetProperty,
+                                      AbstractView *view)
 {
-    const QString expression = bindingProperty.expression();
+    ModelNode modelNode = getNodeByIdOrParent(view, id, targetProperty.parentModelNode());
-    const QStringList stringlist = expression.split(QLatin1String("."));
 
-    const QString &id = stringlist.constFirst();
+    NodeMetaInfo targetType;
-    ModelNode modelNode = getNodeByIdOrParent(view, id, bindingProperty.parentModelNode());
+    if (targetProperty.isDynamic()) {
-
+        targetType = dynamicTypeMetaInfo(targetProperty);
-    NodeMetaInfo type;
+    } else if (auto metaInfo = targetProperty.parentModelNode().metaInfo(); metaInfo.isValid()) {
-    if (bindingProperty.isDynamic()) {
+        targetType = metaInfo.property(targetProperty.name()).propertyType();
-        type = dynamicTypeMetaInfo(bindingProperty);
-    } else if (auto metaInfo = bindingProperty.parentModelNode().metaInfo(); metaInfo.isValid()) {
-        type = metaInfo.property(bindingProperty.name()).propertyType();
     } else
         qWarning() << __FUNCTION__ << " no meta info for target node";
 
@@ -329,23 +373,19 @@ QStringList availableSourceProperties(const BindingProperty &bindingProperty, Ab
     if (!modelNode.isValid()) {
         QStringList singletons = singletonsFromView(view);
         if (singletons.contains(id)) {
-            Model *model = view->model();
+            for (const auto &property : propertiesFromSingleton(id, view)) {
-            QTC_ASSERT(model, return {});
+                if (metaInfoIsCompatible(targetType, property))
-            if (NodeMetaInfo metaInfo = model->metaInfo(id.toUtf8()); metaInfo.isValid()) {
+                    possibleProperties.push_back(QString::fromUtf8(property.name()));
-                for (const auto &property : metaInfo.properties()) {
-                    if (metaInfoIsCompatible(type, property))
-                        possibleProperties.push_back(QString::fromUtf8(property.name()));
-                }
             }
             return possibleProperties;
         }
-        qWarning() << __FUNCTION__ << " invalid model node";
+        qWarning() << __FUNCTION__ << " invalid model node: " << id;
         return {};
-   }
+    }
 
-    auto isCompatible = [type](const AbstractProperty& other) {
+    auto isCompatible = [targetType](const AbstractProperty &other) {
         auto otherType = dynamicTypeMetaInfo(other);
-        return metaInfoIsCompatibleUnsafe(type, otherType);
+        return metaInfoIsCompatibleUnsafe(targetType, otherType);
     };
 
     for (const VariantProperty &variantProperty : modelNode.variantProperties()) {
@@ -361,7 +401,7 @@ QStringList availableSourceProperties(const BindingProperty &bindingProperty, Ab
     NodeMetaInfo metaInfo = modelNode.metaInfo();
     if (metaInfo.isValid()) {
         for (const auto &property : metaInfo.properties()) {
-            if (metaInfoIsCompatible(type, property) )
+            if (metaInfoIsCompatible(targetType, property))
                 possibleProperties.push_back(QString::fromUtf8(property.name()));
         }
     } else {
@@ -371,35 +411,4 @@ QStringList availableSourceProperties(const BindingProperty &bindingProperty, Ab
     return possibleProperties;
 }
 
-QList<AbstractProperty> dynamicPropertiesFromNode(const ModelNode &node)
-{
-    auto isDynamic = [](const AbstractProperty &p) { return p.isDynamic(); };
-    auto byName = [](const AbstractProperty &a, const AbstractProperty &b) {
-        return a.name() < b.name();
-    };
-
-    QList<AbstractProperty> dynamicProperties = Utils::filtered(node.properties(), isDynamic);
-    Utils::sort(dynamicProperties, byName);
-    return dynamicProperties;
-}
-
-std::pair<QString, QString> splitExpression(const QString &expression)
-{
-    // ### Todo from original code (getExpressionStrings):
-    //      We assume no expressions yet
-    const QStringList stringList = expression.split(QLatin1String("."));
-
-    QString sourceNode = stringList.constFirst();
-    QString propertyName;
-    for (int i = 1; i < stringList.size(); ++i) {
-        propertyName += stringList.at(i);
-        if (i != stringList.size() - 1)
-            propertyName += QLatin1String(".");
-    }
-    if (propertyName.isEmpty())
-        std::swap(sourceNode, propertyName);
-
-    return {sourceNode, propertyName};
-}
-
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/connectioneditor/connectioneditorutils.h

@@ -24,21 +24,27 @@ void showErrorMessage(const QString &text);
 QString idOrTypeName(const ModelNode &modelNode);
 PropertyName uniquePropertyName(const PropertyName &suggestion, const ModelNode &modelNode);
 
-NodeMetaInfo dynamicTypeMetaInfo(const AbstractProperty& property);
+NodeMetaInfo dynamicTypeMetaInfo(const AbstractProperty &property);
+NodeMetaInfo dynamicTypeNameToNodeMetaInfo(const TypeName &typeName, Model *model);
+
 QVariant typeConvertVariant(const QVariant &variant, const QmlDesigner::TypeName &typeName);
 void convertVariantToBindingProperty(const VariantProperty &property, const QVariant &value);
 void convertBindingToVariantProperty(const BindingProperty &property, const QVariant &value);
 
-bool isBindingExpression(const QVariant& value);
+bool isBindingExpression(const QVariant &value);
 bool isDynamicVariantPropertyType(const TypeName &type);
 QVariant defaultValueForType(const TypeName &type);
 QString defaultExpressionForType(const TypeName &type);
-
-QStringList availableSources(AbstractView *view);
-QStringList availableTargetProperties(const BindingProperty &bindingProperty);
-QStringList availableSourceProperties(const BindingProperty &bindingProperty, AbstractView *view);
-QList<AbstractProperty> dynamicPropertiesFromNode(const ModelNode &node);
-
 std::pair<QString, QString> splitExpression(const QString &expression);
 
+QStringList singletonsFromView(AbstractView *view);
+std::vector<PropertyMetaInfo> propertiesFromSingleton(const QString &name, AbstractView *view);
+
+QList<AbstractProperty> dynamicPropertiesFromNode(const ModelNode &node);
+QStringList availableSources(AbstractView *view);
+QStringList availableTargetProperties(const BindingProperty &bindingProperty);
+QStringList availableSourceProperties(const QString &id,
+                                      const BindingProperty &targetProp,
+                                      AbstractView *view);
+
 } // namespace QmlDesigner