Fixed compression bugs in text refactoring.

Task-number: BAUHAUS-729, BAUHAUS-731

4 files changed, 90 insertions, 48 deletions

diff --git a/src/plugins/qmldesigner/designercore/model/modeltotextmerger.cpp b/src/plugins/qmldesigner/designercore/model/modeltotextmerger.cpp
index 8592282692..2afdaec7d1 100644
--- a/src/plugins/qmldesigner/designercore/model/modeltotextmerger.cpp
+++ b/src/plugins/qmldesigner/designercore/model/modeltotextmerger.cpp
@@ -101,7 +101,7 @@ void ModelToTextMerger::propertiesChanged(const QList<AbstractProperty>& propert
 
             schedule(new AddPropertyRewriteAction(property,
                                                   propertyTextValue,
-                                                  propertyType(property),
+                                                  propertyType(property, propertyTextValue),
                                                   containedModelNode));
             break;
 
@@ -111,7 +111,7 @@ void ModelToTextMerger::propertiesChanged(const QList<AbstractProperty>& propert
 
             schedule(new ChangePropertyRewriteAction(property,
                                                      propertyTextValue,
-                                                     propertyType(property),
+                                                     propertyType(property, propertyTextValue),
                                                      containedModelNode));
             break;
 
@@ -149,7 +149,10 @@ void ModelToTextMerger::removeImport(const Import &import)
 void ModelToTextMerger::nodeReparented(const ModelNode &node, const NodeAbstractProperty &newPropertyParent, const NodeAbstractProperty &oldPropertyParent, AbstractView::PropertyChangeFlags propertyChange)
 {
     if (isInHierarchy(oldPropertyParent) && isInHierarchy(newPropertyParent)) { // the node is moved
-        schedule(new ReparentNodeRewriteAction(node, oldPropertyParent.parentModelNode(), newPropertyParent, propertyType(newPropertyParent)));
+        schedule(new ReparentNodeRewriteAction(node,
+                                               oldPropertyParent.parentModelNode(),
+                                               newPropertyParent,
+                                               propertyType(newPropertyParent)));
     } else if (isInHierarchy(oldPropertyParent) && !isInHierarchy(newPropertyParent)) { // the node is removed from hierarchy
         if (oldPropertyParent.isNodeProperty()) {
             // ignore, the subsequent remove property will take care of all
@@ -165,11 +168,17 @@ void ModelToTextMerger::nodeReparented(const ModelNode &node, const NodeAbstract
     } else if (!isInHierarchy(oldPropertyParent) && isInHierarchy(newPropertyParent)) { // the node is inserted into to hierarchy
         switch (propertyChange) {
         case AbstractView::PropertiesAdded:
-            schedule(new AddPropertyRewriteAction(newPropertyParent, QmlTextGenerator(getPropertyOrder())(node), propertyType(newPropertyParent), node));
+            schedule(new AddPropertyRewriteAction(newPropertyParent,
+                                                  QmlTextGenerator(getPropertyOrder())(node),
+                                                  propertyType(newPropertyParent),
+                                                  node));
             break;
 
         case AbstractView::NoAdditionalChanges:
-            schedule(new ChangePropertyRewriteAction(newPropertyParent, QmlTextGenerator(getPropertyOrder())(node), propertyType(newPropertyParent), node));
+            schedule(new ChangePropertyRewriteAction(newPropertyParent,
+                                                     QmlTextGenerator(getPropertyOrder())(node),
+                                                     propertyType(newPropertyParent),
+                                                     node));
             break;
 
         case AbstractView::EmptyPropertiesRemoved:
@@ -307,11 +316,18 @@ void ModelToTextMerger::schedule(RewriteAction *action)
     m_rewriteActions.append(action);
 }
 
-QmlDesigner::QmlRefactoring::PropertyType ModelToTextMerger::propertyType(const AbstractProperty &property)
+QmlDesigner::QmlRefactoring::PropertyType ModelToTextMerger::propertyType(const AbstractProperty &property, const QString &textValue)
 {
-    if (property.isBindingProperty())
-        return QmlDesigner::QmlRefactoring::ObjectBinding;
-    else if (property.isNodeListProperty())
+    if (property.isBindingProperty()) {
+        QString val = textValue.trimmed();
+        if (val.isEmpty())
+            return QmlDesigner::QmlRefactoring::ObjectBinding;
+        const QChar lastChar = val.at(val.size() - 1);
+        if (lastChar == '}' || lastChar == ';')
+            return QmlDesigner::QmlRefactoring::ObjectBinding;
+        else
+            return QmlDesigner::QmlRefactoring::ScriptBinding;
+    } else if (property.isNodeListProperty())
         return QmlDesigner::QmlRefactoring::ArrayBinding;
     else if (property.isNodeProperty())
         return QmlDesigner::QmlRefactoring::ObjectBinding;
diff --git a/src/plugins/qmldesigner/designercore/model/modeltotextmerger.h b/src/plugins/qmldesigner/designercore/model/modeltotextmerger.h
index 2c71a0c582..9af8f390bd 100644
--- a/src/plugins/qmldesigner/designercore/model/modeltotextmerger.h
+++ b/src/plugins/qmldesigner/designercore/model/modeltotextmerger.h
@@ -89,7 +89,7 @@ protected:
     QList<RewriteAction *> scheduledRewriteActions() const
     { return m_rewriteActions; }
 
-    static QmlDesigner::QmlRefactoring::PropertyType propertyType(const AbstractProperty &property);
+    static QmlDesigner::QmlRefactoring::PropertyType propertyType(const AbstractProperty &property, const QString &textValue = QString());
     static QStringList getPropertyOrder();
 
     static bool isInHierarchy(const AbstractProperty &property);
diff --git a/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.cpp b/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.cpp
index d3e52f1273..9431545a7f 100644
--- a/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.cpp
+++ b/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.cpp
@@ -67,6 +67,7 @@ void RewriteActionCompressor::operator()(QList<RewriteAction *> &actions) const
 
 void RewriteActionCompressor::compressImports(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QHash<Import, RewriteAction *> addedImports;
     QHash<Import, RewriteAction *> removedImports;
 
@@ -78,36 +79,42 @@ void RewriteActionCompressor::compressImports(QList<RewriteAction *> &actions) c
         if (RemoveImportRewriteAction *removeImportAction = action->asRemoveImportRewriteAction()) {
             const Import import = removeImportAction->import();
             if (removedImports.contains(import)) {
-                remove(iter);
+                actionsToRemove.append(action);
             } else if (RewriteAction *addImportAction = addedImports.value(import, 0)) {
-                actions.removeOne(addImportAction);
+                actionsToRemove.append(action);
+                actionsToRemove.append(addImportAction);
                 addedImports.remove(import);
                 delete addImportAction;
-                remove(iter);
             } else {
                 removedImports.insert(import, action);
             }
         } else if (AddImportRewriteAction *addImportAction = action->asAddImportRewriteAction()) {
             const Import import = addImportAction->import();
             if (RewriteAction *duplicateAction = addedImports.value(import, 0)) {
-                actions.removeOne(duplicateAction);
+                actionsToRemove.append(duplicateAction);
                 addedImports.remove(import);
                 delete duplicateAction;
                 addedImports.insert(import, action);
             } else if (RewriteAction *removeAction = removedImports.value(import, 0)) {
-                actions.removeOne(removeAction);
+                actionsToRemove.append(action);
+                actionsToRemove.append(removeAction);
                 removedImports.remove(import);
                 delete removeAction;
-                remove(iter);
             } else {
                 addedImports.insert(import, action);
             }
         }
     }
+
+    foreach (RewriteAction *action, actionsToRemove) {
+        actions.removeOne(action);
+        delete action;
+    }
 }
 
 void RewriteActionCompressor::compressRereparentActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QHash<ModelNode, ReparentNodeRewriteAction *> reparentedNodes;
 
     QMutableListIterator<RewriteAction*> iter(actions);
@@ -120,16 +127,22 @@ void RewriteActionCompressor::compressRereparentActions(QList<RewriteAction *> &
 
             if (ReparentNodeRewriteAction *otherAction = reparentedNodes.value(reparentedNode, 0)) {
                 otherAction->setOldParent(reparentAction->oldParent());
-                remove(iter);
+                actionsToRemove.append(action);
             } else {
                 reparentedNodes.insert(reparentedNode, reparentAction);
             }
         }
     }
+
+    foreach (RewriteAction *action, actionsToRemove) {
+        actions.removeOne(action);
+        delete action;
+    }
 }
 
 void RewriteActionCompressor::compressReparentIntoSameParentActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QMutableListIterator<RewriteAction *> iter(actions);
     iter.toBack();
     while (iter.hasPrevious()) {
@@ -139,15 +152,20 @@ void RewriteActionCompressor::compressReparentIntoSameParentActions(QList<Rewrit
             const ModelNode targetNode = reparentAction->targetProperty().parentModelNode();
             const ModelNode oldParent = reparentAction->oldParent();
             if (targetNode == oldParent)
-                remove(iter);
+                actionsToRemove.append(action);
         }
     }
+
+    foreach (RewriteAction *action, actionsToRemove) {
+        actions.removeOne(action);
+        delete action;
+    }
 }
 
 void RewriteActionCompressor::compressAddEditRemoveNodeActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QHash<ModelNode, RewriteAction *> removedNodes;
-    QSet<RewriteAction *> removeActionsToRemove;
 
     QMutableListIterator<RewriteAction*> iter(actions);
     iter.toBack();
@@ -158,7 +176,7 @@ void RewriteActionCompressor::compressAddEditRemoveNodeActions(QList<RewriteActi
             const ModelNode modelNode = removeNodeAction->node();
 
             if (removedNodes.contains(modelNode))
-                remove(iter);
+                actionsToRemove.append(action);
             else
                 removedNodes.insert(modelNode, action);
         } else if (action->asAddPropertyRewriteAction() || action->asChangePropertyRewriteAction()) {
@@ -172,30 +190,30 @@ void RewriteActionCompressor::compressAddEditRemoveNodeActions(QList<RewriteActi
                 containedModelNode = action->asChangePropertyRewriteAction()->containedModelNode();
             }
 
-            if (removedNodes.contains(property.parentModelNode()))
-                remove(iter);
-            else if (removedNodes.contains(containedModelNode)) {
-                remove(iter);
-                removeActionsToRemove.insert(removedNodes[containedModelNode]);
+            if (removedNodes.contains(property.parentModelNode())) {
+                actionsToRemove.append(action);
+            } else if (RewriteAction *removeAction = removedNodes.value(containedModelNode, 0)) {
+                actionsToRemove.append(action);
+                actionsToRemove.append(removeAction);
             }
         } else if (RemovePropertyRewriteAction *removePropertyAction = action->asRemovePropertyRewriteAction()) {
             const AbstractProperty property = removePropertyAction->property();
 
             if (removedNodes.contains(property.parentModelNode()))
-                remove(iter);
+                actionsToRemove.append(action);
         } else if (ChangeIdRewriteAction *changeIdAction = action->asChangeIdRewriteAction()) {
             if (removedNodes.contains(changeIdAction->node()))
-                remove(iter);
+                actionsToRemove.append(action);
         } else if (ChangeTypeRewriteAction *changeTypeAction = action->asChangeTypeRewriteAction()) {
             if (removedNodes.contains(changeTypeAction->node()))
-                remove(iter);
+                actionsToRemove.append(action);
         } else if (ReparentNodeRewriteAction *reparentAction = action->asReparentNodeRewriteAction()) {
             if (removedNodes.contains(reparentAction->reparentedNode()))
-                remove(iter);
+                actionsToRemove.append(action);
         }
     }
 
-    foreach (RewriteAction *action, removeActionsToRemove) {
+    foreach (RewriteAction *action, actionsToRemove) {
         actions.removeOne(action);
         delete action;
     }
@@ -203,6 +221,7 @@ void RewriteActionCompressor::compressAddEditRemoveNodeActions(QList<RewriteActi
 
 void RewriteActionCompressor::compressPropertyActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QHash<AbstractProperty, RewriteAction *> removedProperties;
     QHash<AbstractProperty, ChangePropertyRewriteAction *> changedProperties;
     QSet<AbstractProperty> addedProperties;
@@ -218,10 +237,10 @@ void RewriteActionCompressor::compressPropertyActions(QList<RewriteAction *> &ac
             const AbstractProperty property = changeAction->property();
 
             if (removedProperties.contains(property)) {
-                remove(iter);
+                actionsToRemove.append(action);
             } else if (changedProperties.contains(property)) {
                 if (!property.isValid() || !property.isDefaultProperty())
-                    remove(iter);
+                    actionsToRemove.append(action);
             } else {
                 changedProperties.insert(property, changeAction);
             }
@@ -229,10 +248,9 @@ void RewriteActionCompressor::compressPropertyActions(QList<RewriteAction *> &ac
             const AbstractProperty property = addAction->property();
 
             if (RewriteAction *removeAction = removedProperties.value(property, 0)) {
-                actions.removeOne(removeAction);
+                actionsToRemove.append(action);
+                actionsToRemove.append(removeAction);
                 removedProperties.remove(property);
-                delete removeAction;
-                remove(iter);
             } else {
                 if (changedProperties.contains(property))
                     changedProperties.remove(property);
@@ -241,10 +259,16 @@ void RewriteActionCompressor::compressPropertyActions(QList<RewriteAction *> &ac
             }
         }
     }
+
+    foreach (RewriteAction *action, actionsToRemove){
+        actions.removeOne(action);
+        delete action;
+    }
 }
 
 void RewriteActionCompressor::compressAddEditActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QSet<ModelNode> addedNodes;
     QSet<RewriteAction *> dirtyActions;
 
@@ -265,7 +289,7 @@ void RewriteActionCompressor::compressAddEditActions(QList<RewriteAction *> &act
             }
 
             if (property.isValid() && addedNodes.contains(property.parentModelNode())) {
-                remove(iter);
+                actionsToRemove.append(action);
                 continue;
             }
 
@@ -273,22 +297,27 @@ void RewriteActionCompressor::compressAddEditActions(QList<RewriteAction *> &act
                 continue;
 
             if (nodeOrParentInSet(containedNode, addedNodes)) {
-                remove(iter);
+                actionsToRemove.append(action);
             } else {
                 addedNodes.insert(containedNode);
                 dirtyActions.insert(action);
             }
         } else if (ChangeIdRewriteAction *changeIdAction = action->asChangeIdRewriteAction()) {
             if (nodeOrParentInSet(changeIdAction->node(), addedNodes)) {
-                remove(iter);
+                actionsToRemove.append(action);
             }
         } else if (ChangeTypeRewriteAction *changeTypeAction = action->asChangeTypeRewriteAction()) {
             if (nodeOrParentInSet(changeTypeAction->node(), addedNodes)) {
-                remove(iter);
+                actionsToRemove.append(action);
             }
         }
     }
 
+    foreach (RewriteAction *action, actionsToRemove){
+        actions.removeOne(action);
+        delete action;
+    }
+
     QmlTextGenerator gen(m_propertyOrder);
     foreach (RewriteAction *action, dirtyActions) {
         RewriteAction *newAction = 0;
@@ -313,6 +342,7 @@ void RewriteActionCompressor::compressAddEditActions(QList<RewriteAction *> &act
 
 void RewriteActionCompressor::compressAddReparentActions(QList<RewriteAction *> &actions) const
 {
+    QList<RewriteAction *> actionsToRemove;
     QMap<ModelNode, RewriteAction*> addedNodes;
 
     QMutableListIterator<RewriteAction*> iter(actions);
@@ -335,7 +365,7 @@ void RewriteActionCompressor::compressAddReparentActions(QList<RewriteAction *>
         } else if (ReparentNodeRewriteAction *reparentAction = action->asReparentNodeRewriteAction()) {
             if (addedNodes.contains(reparentAction->reparentedNode())) {
                 RewriteAction *previousAction = addedNodes[reparentAction->reparentedNode()];
-                actions.removeOne(previousAction);
+                actionsToRemove.append(previousAction);
 
                 RewriteAction *replacementAction = 0;
                 if (AddPropertyRewriteAction *addAction = previousAction->asAddPropertyRewriteAction()) {
@@ -351,15 +381,13 @@ void RewriteActionCompressor::compressAddReparentActions(QList<RewriteAction *>
                 }
 
                 iter.setValue(replacementAction);
-                delete previousAction;
                 delete action;
             }
         }
     }
-}
 
-void RewriteActionCompressor::remove(QMutableListIterator<RewriteAction*> &iter) const
-{
-    delete iter.value();
-    iter.remove();
+    foreach (RewriteAction *action, actionsToRemove){
+        actions.removeOne(action);
+        delete action;
+    }
 }
diff --git a/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.h b/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.h
index 84aaecdadb..ce0e9eeef0 100644
--- a/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.h
+++ b/src/plugins/qmldesigner/designercore/model/rewriteactioncompressor.h
@@ -54,8 +54,6 @@ private:
     void compressAddEditActions(QList<RewriteAction *> &actions) const;
     void compressAddReparentActions(QList<RewriteAction *> &actions) const;
 
-    void remove(QMutableListIterator<RewriteAction*> &iter) const;
-
 private:
     QStringList m_propertyOrder;
 };