QmlDesigner: Simplify InternalNode

Before we add new members the simple getter and setter without value are
removed because the model provides capsulation.

To  remove the weak pointer workaround std::enable_shared_from_this is
used which makes the class aware of its shared pointer. For that we
change to std::shared_ptr

Task-number: QDS-7343
Change-Id: Ic5f14ba8c1fd7af7633b8decb413538ee01c90d6
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/plugins/qmldesigner/designercore/include/abstractproperty.h

@@ -29,6 +29,8 @@
 #include <QSharedPointer>
 #include "qmldesignercorelib_global.h"
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 class QTextStream;
 QT_END_NAMESPACE
@@ -38,7 +40,7 @@ namespace QmlDesigner {
     class InternalNode;
     class InternalProperty;
 
-    using InternalNodePointer = QSharedPointer<InternalNode>;
+    using InternalNodePointer = std::shared_ptr<InternalNode>;
     using InternalPropertyPointer = QSharedPointer<InternalProperty>;
     }
 
@@ -122,7 +124,7 @@ public:
 
     friend auto qHash(const AbstractProperty &property)
     {
-        return ::qHash(property.m_internalNode.data()) ^ ::qHash(property.m_propertyName);
+        return ::qHash(property.m_internalNode.get()) ^ ::qHash(property.m_propertyName);
     }
 
 protected:

src/plugins/qmldesigner/designercore/include/abstractview.h

@@ -40,6 +40,7 @@
 #include <QPointer>
 
 #include <functional>
+#include <memory>
 
 QT_BEGIN_NAMESPACE
 class QStyle;
@@ -50,8 +51,8 @@ QT_END_NAMESPACE
 
 namespace QmlDesigner {
     namespace Internal {
-        class InternalNode;
+    class InternalNode;
-        using InternalNodePointer = QSharedPointer<InternalNode>;
+    using InternalNodePointer = std::shared_ptr<InternalNode>;
     }
 }
 

src/plugins/qmldesigner/designercore/include/modelnode.h

@@ -31,6 +31,8 @@
 #include <QVector>
 #include <QVariant>
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 class QTextStream;
 QT_END_NAMESPACE
@@ -43,7 +45,7 @@ namespace Internal {
     class InternalNode;
     class InternalProperty;
 
-    using InternalNodePointer = QSharedPointer<InternalNode>;
+    using InternalNodePointer = std::shared_ptr<InternalNode>;
     using InternalPropertyPointer = QSharedPointer<InternalProperty>;
 }
 class NodeMetaInfo;
@@ -251,7 +253,7 @@ public:
         swap(first.m_view, second.m_view);
     }
 
-    friend auto qHash(const ModelNode &node) { return ::qHash(node.m_internalNode.data()); }
+    friend auto qHash(const ModelNode &node) { return ::qHash(node.m_internalNode.get()); }
 
 private: // functions
     Internal::InternalNodePointer internalNode() const;

src/plugins/qmldesigner/designercore/include/nodeanchors.h

@@ -37,14 +37,14 @@ namespace QmlDesigner {
     class NodeState;
 
     namespace Internal {
-        class InternalNode;
+    class InternalNode;
-        using InternalNodePointer = QSharedPointer<InternalNode>;
+    using InternalNodePointer = std::shared_ptr<InternalNode>;
-        using InternalNodeWeakPointer = QWeakPointer<InternalNode>;
+    using InternalNodeWeakPointer = QWeakPointer<InternalNode>;
 
-        class InternalNodeState;
+    class InternalNodeState;
-        using InternalNodeStatePointer = QSharedPointer<InternalNodeState>;
+    using InternalNodeStatePointer = QSharedPointer<InternalNodeState>;
 
-        class TextToModelMerger;
+    class TextToModelMerger;
     }
 }
 

src/plugins/qmldesigner/designercore/model/abstractproperty.cpp

@@ -126,12 +126,8 @@ PropertyName AbstractProperty::name() const
 */
 bool AbstractProperty::isValid() const
 {
-    return !m_internalNode.isNull() &&
+    return m_internalNode && !m_model.isNull() && m_internalNode->isValid
-            !m_model.isNull() &&
+           && !m_propertyName.isEmpty() && !m_propertyName.contains(' ') && m_propertyName != "id";
-            m_internalNode->isValid() &&
-            !m_propertyName.isEmpty() &&
-            !m_propertyName.contains(' ') &&
-            m_propertyName != "id";
 }
 
 bool AbstractProperty::exists() const

src/plugins/qmldesigner/designercore/model/internalnode.cpp

@@ -35,89 +35,6 @@
 namespace QmlDesigner {
 namespace Internal {
 
-/*!
-  \class QmlDesigner::Internal::InternalNode
-
-  Represents one XML element.
-  */
-
-InternalNode::InternalNode() :
-    m_majorVersion(0),
-    m_minorVersion(0),
-    m_valid(false),
-    m_internalId(-1)
-{
-}
-
-InternalNode::InternalNode(const TypeName &typeName,int majorVersion, int minorVersion, qint32 internalId):
-        m_typeName(typeName),
-        m_majorVersion(majorVersion),
-        m_minorVersion(minorVersion),
-        m_valid(true),
-        m_internalId(internalId)
-{
-
-}
-
-InternalNode::Pointer InternalNode::create(const TypeName &type,int majorVersion, int minorVersion, qint32 internalId)
-{
-    auto newPointer(new InternalNode(type, majorVersion, minorVersion, internalId));
-    InternalNode::Pointer smartPointer(newPointer);
-
-    newPointer->setInternalWeakPointer(smartPointer);
-
-    return smartPointer;
-}
-
-InternalNode::Pointer InternalNode::internalPointer() const
-{
-    return m_internalPointer.toStrongRef();
-}
-void InternalNode::setInternalWeakPointer(const Pointer &pointer)
-{
-    m_internalPointer = pointer;
-}
-
-TypeName InternalNode::type() const
-{
-    return m_typeName;
-}
-
-void InternalNode::setType(const TypeName &newType)
-{
-    m_typeName = newType;
-}
-
-int InternalNode::minorVersion() const
-{
-    return m_minorVersion;
-}
-
-int InternalNode::majorVersion() const
-{
-    return m_majorVersion;
-}
-
-void InternalNode::setMinorVersion(int number)
-{
-    m_minorVersion = number;
-}
-
-void InternalNode::setMajorVersion(int number)
-{
-    m_majorVersion = number;
-}
-
-bool InternalNode::isValid() const
-{
-    return m_valid;
-}
-
-void InternalNode::setValid(bool valid)
-{
-    m_valid = valid;
-}
-
 InternalNodeAbstractProperty::Pointer InternalNode::parentProperty() const
 {
     return m_parentProperty;
@@ -126,47 +43,23 @@ void InternalNode::setParentProperty(const InternalNodeAbstractProperty::Pointer
 {
     InternalNodeAbstractProperty::Pointer parentProperty = m_parentProperty.toStrongRef();
     if (parentProperty)
-        parentProperty->remove(internalPointer());
+        parentProperty->remove(shared_from_this());
 
     Q_ASSERT(parent && parent->isValid());
     m_parentProperty = parent;
 
-    parent->add(internalPointer());
+    parent->add(shared_from_this());
 }
 
 void InternalNode::resetParentProperty()
 {
     InternalNodeAbstractProperty::Pointer parentProperty = m_parentProperty.toStrongRef();
     if (parentProperty)
-        parentProperty->remove(internalPointer());
+        parentProperty->remove(shared_from_this());
 
     m_parentProperty.clear();
 }
 
-QString InternalNode::id() const
-{
-    return m_id;
-}
-
-void InternalNode::setId(const QString& id)
-{
-    m_id = id;
-}
-
-bool InternalNode::hasId() const
-{
-    return !m_id.isEmpty();
-}
-
-
-size_t qHash(const InternalNodePointer& node)
-{
-    if (node.isNull())
-        return ::qHash(-1);
-
-    return ::qHash(node->internalId());
-}
-
 QVariant InternalNode::auxiliaryData(const PropertyName &name) const
 {
     return m_auxiliaryDataHash.value(name);
@@ -235,19 +128,21 @@ InternalVariantProperty::Pointer InternalNode::variantProperty(const PropertyNam
 
 void InternalNode::addBindingProperty(const PropertyName &name)
 {
-    InternalProperty::Pointer newProperty(InternalBindingProperty::create(name, internalPointer()));
+    InternalProperty::Pointer newProperty(InternalBindingProperty::create(name, shared_from_this()));
     m_namePropertyHash.insert(name, newProperty);
 }
 
 void InternalNode::addSignalHandlerProperty(const PropertyName &name)
 {
-    InternalProperty::Pointer newProperty(InternalSignalHandlerProperty::create(name, internalPointer()));
+    InternalProperty::Pointer newProperty(
+        InternalSignalHandlerProperty::create(name, shared_from_this()));
     m_namePropertyHash.insert(name, newProperty);
 }
 
 void InternalNode::addSignalDeclarationProperty(const PropertyName &name)
 {
-    InternalProperty::Pointer newProperty(InternalSignalDeclarationProperty::create(name, internalPointer()));
+    InternalProperty::Pointer newProperty(
+        InternalSignalDeclarationProperty::create(name, shared_from_this()));
     m_namePropertyHash.insert(name, newProperty);
 }
 
@@ -280,20 +175,20 @@ InternalNodeProperty::Pointer InternalNode::nodeProperty(const PropertyName &nam
 
 void InternalNode::addVariantProperty(const PropertyName &name)
 {
-    InternalProperty::Pointer newProperty(InternalVariantProperty::create(name, internalPointer()));
+    InternalProperty::Pointer newProperty(InternalVariantProperty::create(name, shared_from_this()));
     m_namePropertyHash.insert(name, newProperty);
 }
 
 void InternalNode::addNodeProperty(const PropertyName &name, const TypeName &dynamicTypeName)
 {
-    InternalNodeProperty::Pointer newProperty(InternalNodeProperty::create(name, internalPointer()));
+    InternalNodeProperty::Pointer newProperty(InternalNodeProperty::create(name, shared_from_this()));
     newProperty->setDynamicTypeName(dynamicTypeName);
     m_namePropertyHash.insert(name, newProperty);
 }
 
 void InternalNode::addNodeListProperty(const PropertyName &name)
 {
-    InternalProperty::Pointer newProperty(InternalNodeListProperty::create(name, internalPointer()));
+    InternalProperty::Pointer newProperty(InternalNodeListProperty::create(name, shared_from_this()));
     m_namePropertyHash.insert(name, newProperty);
 }
 
@@ -358,61 +253,5 @@ QList<InternalNode::Pointer> InternalNode::allDirectSubNodes() const
     return nodeList;
 }
 
-bool operator <(const InternalNode::Pointer &firstNode, const InternalNode::Pointer &secondNode)
+} // namespace Internal
-{
-    if (firstNode.isNull())
-        return true;
-
-    if (secondNode.isNull())
-        return false;
-
-    return firstNode->internalId() < secondNode->internalId();
-}
-
-void InternalNode::setScriptFunctions(const QStringList &scriptFunctionList)
-{
-    m_scriptFunctionList = scriptFunctionList;
-}
-
-QStringList InternalNode::scriptFunctions() const
-{
-    return m_scriptFunctionList;
-}
-
-qint32 InternalNode::internalId() const
-{
-    return m_internalId;
-}
-
-void InternalNode::setNodeSource(const QString &nodeSource)
-{
-    m_nodeSource = nodeSource;
-}
-
-QString InternalNode::nodeSource() const
-{
-    return m_nodeSource;
-}
-
-int InternalNode::nodeSourceType() const
-{
-    return m_nodeSourceType;
-}
-
-void InternalNode::setNodeSourceType(int i)
-{
-    m_nodeSourceType = i;
-}
-
-QString InternalNode::behaviorPropertyName() const
-{
-    return m_behaviorPropertyName;
-}
-
-void InternalNode::setBehaviorPropertyName(const QString &name)
-{
-    m_behaviorPropertyName = name;
-}
-
-}
 }

src/plugins/qmldesigner/designercore/model/internalnode_p.h

@@ -38,6 +38,8 @@
 #include "internalnodeproperty.h"
 #include "internalnodeabstractproperty.h"
 
+#include <memory>
+
 namespace QmlDesigner {
 
 namespace Internal {
@@ -45,31 +47,26 @@ namespace Internal {
 class InternalProperty;
 class InternalNode;
 
-using InternalNodePointer = QSharedPointer<InternalNode>;
+using InternalNodePointer = std::shared_ptr<InternalNode>;
 using InternalPropertyPointer = QSharedPointer<InternalProperty>;
 
-class InternalNode
+class InternalNode : public std::enable_shared_from_this<InternalNode>
 {
     friend InternalProperty;
 
 public:
-    using Pointer = QSharedPointer<InternalNode>;
+    using Pointer = std::shared_ptr<InternalNode>;
-    using WeakPointer = QWeakPointer<InternalNode>;
+    using WeakPointer = std::weak_ptr<InternalNode>;
 
-    explicit InternalNode();
+    explicit InternalNode() = default;
 
-    static Pointer create(const TypeName &typeName, int majorVersion, int minorVersion, qint32 internalId);
+    explicit InternalNode(TypeName typeName, int majorVersion, int minorVersion, qint32 internalId)
-
+        : typeName(std::move(typeName))
-    TypeName type() const;
+        , majorVersion(majorVersion)
-    void setType(const TypeName &newType);
+        , minorVersion(minorVersion)
-
+        , isValid(true)
-    int minorVersion() const;
+        , internalId(internalId)
-    int majorVersion() const;
+    {}
-    void setMinorVersion(int number);
-    void setMajorVersion(int number);
-
-    bool isValid() const;
-    void setValid(bool valid);
 
     InternalNodeAbstractProperty::Pointer parentProperty() const;
 
@@ -77,10 +74,6 @@ public:
     void setParentProperty(const InternalNodeAbstractProperty::Pointer &parent);
     void resetParentProperty();
 
-    QString id() const;
-    void setId(const QString& id);
-    bool hasId() const;
-
     QVariant auxiliaryData(const PropertyName &name) const;
     void setAuxiliaryData(const PropertyName &name, const QVariant &data);
     void removeAuxiliaryData(const PropertyName &name);
@@ -113,51 +106,46 @@ public:
     QList<InternalNode::Pointer> allSubNodes() const;
     QList<InternalNode::Pointer> allDirectSubNodes() const;
 
-    void setScriptFunctions(const QStringList &scriptFunctionList);
+    friend bool operator<(const InternalNode::Pointer &firstNode,
-    QStringList scriptFunctions() const;
+                          const InternalNode::Pointer &secondNode)
+    {
+        if (!firstNode)
+            return true;
 
-    qint32 internalId() const;
+        if (!secondNode)
+            return false;
 
-    void setNodeSource(const QString&);
+        return firstNode->internalId < secondNode->internalId;
-    QString nodeSource() const;
+    }
 
-    int nodeSourceType() const;
+    friend size_t qHash(const InternalNodePointer &node)
-    void setNodeSourceType(int i);
+    {
+        if (!node)
+            return ::qHash(-1);
 
-    QString behaviorPropertyName() const;
+        return ::qHash(node->internalId);
-    void setBehaviorPropertyName(const QString &name);
+    }
 
 protected:
-    Pointer internalPointer() const;
-    void setInternalWeakPointer(const Pointer &pointer);
     void removeProperty(const PropertyName &name);
-    explicit InternalNode(const TypeName &type, int majorVersion, int minorVersion, qint32 internalId);
+
+public:
+    TypeName typeName;
+    QString id;
+    int majorVersion = 0;
+    int minorVersion = 0;
+    bool isValid = false;
+    qint32 internalId = -1;
+    QString nodeSource;
+    int nodeSourceType = 0;
+    QString behaviorPropertyName;
+    QStringList scriptFunctions;
 
 private:
-    TypeName m_typeName;
-    QString m_id;
-
     QHash<PropertyName, QVariant> m_auxiliaryDataHash;
-
     InternalNodeAbstractProperty::WeakPointer m_parentProperty;
-    WeakPointer m_internalPointer;
-
-    int m_majorVersion;
-    int m_minorVersion;
-
-    bool m_valid;
-    qint32 m_internalId;
-
     QHash<PropertyName, InternalPropertyPointer> m_namePropertyHash;
-    QStringList m_scriptFunctionList;
-
-    QString m_nodeSource;
-    int m_nodeSourceType = 0;
-
-    QString m_behaviorPropertyName;
 };
 
-size_t qHash(const InternalNodePointer& node);
-bool operator <(const InternalNodePointer &firstNode, const InternalNodePointer &secondNode);
 } // Internal
 } // QtQmlDesigner

src/plugins/qmldesigner/designercore/model/internalnodelistproperty.cpp

@@ -62,7 +62,7 @@ int InternalNodeListProperty::count() const
 
 int InternalNodeListProperty::indexOf(const InternalNode::Pointer &node) const
 {
-    if (node.isNull())
+    if (!node)
         return -1;
 
     return m_nodeList.indexOf(node);

src/plugins/qmldesigner/designercore/model/internalnodeproperty.cpp

@@ -46,7 +46,7 @@ InternalNodeProperty::Pointer InternalNodeProperty::create(const PropertyName &n
 
 bool InternalNodeProperty::isEmpty() const
 {
-    return m_node.isNull();
+    return !m_node;
 }
 
 int InternalNodeProperty::count() const
@@ -59,7 +59,7 @@ int InternalNodeProperty::count() const
 
 int InternalNodeProperty::indexOf(const InternalNode::Pointer &node) const
 {
-    if (!node.isNull() && node == m_node)
+    if (node && node == m_node)
         return 0;
 
     return -1;
@@ -83,8 +83,7 @@ InternalNode::Pointer InternalNodeProperty::node() const
 void InternalNodeProperty::remove([[maybe_unused]] const InternalNode::Pointer &node)
 {
     Q_ASSERT(m_node == node);
-    m_node.clear();
+    m_node.reset();
-
 }
 
 void InternalNodeProperty::add(const InternalNode::Pointer &node)

src/plugins/qmldesigner/designercore/model/internalproperty.cpp

@@ -62,7 +62,7 @@ void InternalProperty::setInternalWeakPointer(const Pointer &pointer)
 
 bool InternalProperty::isValid() const
 {
-    return m_propertyOwner && !m_name.isEmpty();
+    return !m_propertyOwner.expired() && !m_name.isEmpty();
 }
 
 PropertyName InternalProperty::name() const
@@ -121,7 +121,7 @@ QSharedPointer<InternalVariantProperty> InternalProperty::toVariantProperty() co
 
 InternalNode::Pointer InternalProperty::propertyOwner() const
 {
-    return m_propertyOwner.toStrongRef();
+    return m_propertyOwner.lock();
 }
 
 QSharedPointer<InternalNodeListProperty> InternalProperty::toNodeListProperty() const
@@ -157,7 +157,7 @@ QSharedPointer<InternalSignalDeclarationProperty> InternalProperty::toSignalDecl
 void InternalProperty::remove()
 {
     propertyOwner()->removeProperty(name());
-    m_propertyOwner.clear();
+    m_propertyOwner.reset();
 }
 
 TypeName InternalProperty::dynamicTypeName() const

src/plugins/qmldesigner/designercore/model/internalproperty.h

@@ -30,6 +30,8 @@
 #include <QVariant>
 #include <QSharedPointer>
 
+#include <memory>
+
 namespace QmlDesigner {
 
 namespace Internal {
@@ -43,7 +45,7 @@ class InternalNodeProperty;
 class InternalNodeAbstractProperty;
 class InternalNode;
 
-using InternalNodePointer = QSharedPointer<InternalNode>;
+using InternalNodePointer = std::shared_ptr<InternalNode>;
 
 class QMLDESIGNERCORE_EXPORT InternalProperty
 {
@@ -90,8 +92,7 @@ private:
     QWeakPointer<InternalProperty> m_internalPointer;
     PropertyName m_name;
     TypeName m_dynamicType;
-    QWeakPointer<InternalNode> m_propertyOwner;
+    std::weak_ptr<InternalNode> m_propertyOwner;
-
 };
 
 } // namespace Internal

src/plugins/qmldesigner/designercore/model/model.cpp

@@ -243,9 +243,9 @@ void ModelPrivate::setFileUrl(const QUrl &fileUrl)
 void ModelPrivate::changeNodeType(const InternalNodePointer &node, const TypeName &typeName,
                                   int majorVersion, int minorVersion)
 {
-    node->setType(typeName);
+    node->typeName = typeName;
-    node->setMajorVersion(majorVersion);
+    node->majorVersion = majorVersion;
-    node->setMinorVersion(minorVersion);
+    node->minorVersion = minorVersion;
 
     try {
         notifyNodeTypeChanged(node, typeName, majorVersion, minorVersion);
@@ -272,10 +272,10 @@ InternalNodePointer ModelPrivate::createNode(const TypeName &typeName,
     if (!isRootNode)
         internalId = m_internalIdCounter++;
 
-    InternalNodePointer newNode = InternalNode::create(typeName, majorVersion, minorVersion, internalId);
+    auto newNode = std::make_shared<InternalNode>(typeName, majorVersion, minorVersion, internalId);
-    newNode->setNodeSourceType(nodeSourceType);
+    newNode->nodeSourceType = nodeSourceType;
 
-    newNode->setBehaviorPropertyName(behaviorPropertyName);
+    newNode->behaviorPropertyName = behaviorPropertyName;
 
     using PropertyPair = QPair<PropertyName, QVariant>;
 
@@ -288,10 +288,10 @@ InternalNodePointer ModelPrivate::createNode(const TypeName &typeName,
         newNode->setAuxiliaryData(propertyPair.first, propertyPair.second);
 
     m_nodeSet.insert(newNode);
-    m_internalIdNodeHash.insert(newNode->internalId(), newNode);
+    m_internalIdNodeHash.insert(newNode->internalId, newNode);
 
     if (!nodeSource.isNull())
-        newNode->setNodeSource(nodeSource);
+        newNode->nodeSource = nodeSource;
 
     notifyNodeCreated(newNode);
 
@@ -303,16 +303,16 @@ InternalNodePointer ModelPrivate::createNode(const TypeName &typeName,
 
 void ModelPrivate::removeNodeFromModel(const InternalNodePointer &node)
 {
-    Q_ASSERT(!node.isNull());
+    Q_ASSERT(node);
 
     node->resetParentProperty();
 
     m_selectedInternalNodeList.removeAll(node);
-    if (!node->id().isEmpty())
+    if (!node->id.isEmpty())
-        m_idNodeHash.remove(node->id());
+        m_idNodeHash.remove(node->id);
-    node->setValid(false);
+    node->isValid = false;
     m_nodeSet.remove(node);
-    m_internalIdNodeHash.remove(node->internalId());
+    m_internalIdNodeHash.remove(node->internalId);
 }
 
 const QList<QPointer<AbstractView>> ModelPrivate::enabledViews() const
@@ -328,7 +328,7 @@ void ModelPrivate::removeAllSubNodes(const InternalNodePointer &node)
 
 void ModelPrivate::removeNode(const InternalNodePointer &node)
 {
-    Q_ASSERT(!node.isNull());
+    Q_ASSERT(node);
 
     AbstractView::PropertyChangeFlags propertyChangeFlags = AbstractView::NoAdditionalChanges;
 
@@ -372,9 +372,9 @@ void ModelPrivate::setMetaInfo(const MetaInfo &metaInfo)
 
 void ModelPrivate::changeNodeId(const InternalNodePointer &node, const QString &id)
 {
-    const QString oldId = node->id();
+    const QString oldId = node->id;
 
-    node->setId(id);
+    node->id = id;
     if (!oldId.isEmpty())
         m_idNodeHash.remove(oldId);
     if (!id.isEmpty())
@@ -901,7 +901,7 @@ void ModelPrivate::notifyNodeAboutToBeReparent(const InternalNodePointer &node,
         NodeAbstractProperty newProperty;
         NodeAbstractProperty oldProperty;
 
-        if (!oldPropertyName.isEmpty() && oldParent->isValid())
+        if (!oldPropertyName.isEmpty() && oldParent->isValid)
             oldProperty = NodeAbstractProperty(oldPropertyName, oldParent, m_model, view);
 
         if (!newPropertyParent.isNull())
@@ -922,7 +922,7 @@ void ModelPrivate::notifyNodeReparent(const InternalNodePointer &node,
         NodeAbstractProperty newProperty;
         NodeAbstractProperty oldProperty;
 
-        if (!oldPropertyName.isEmpty() && oldParent->isValid())
+        if (!oldPropertyName.isEmpty() && oldParent->isValid)
             oldProperty = NodeAbstractProperty(oldPropertyName, oldParent, m_model, view);
 
         if (!newPropertyParent.isNull())
@@ -953,8 +953,8 @@ void ModelPrivate::notifyNodeOrderChanged(const InternalNodeListPropertyPointer
 
 void ModelPrivate::setSelectedNodes(const QList<InternalNodePointer> &selectedNodeList)
 {
-    QList<InternalNodePointer> sortedSelectedList = Utils::filtered(selectedNodeList,
+    auto sortedSelectedList = Utils::filtered(selectedNodeList,
-                                                                    &InternalNode::isValid);
+                                              [](const auto &node) { return node->isValid; });
 
     sortedSelectedList = Utils::toList(Utils::toSet(sortedSelectedList));
     Utils::sort(sortedSelectedList);
@@ -1039,7 +1039,7 @@ void ModelPrivate::changeSelectedNodes(const QList<InternalNodePointer> &newSele
 QList<InternalNodePointer> ModelPrivate::selectedNodes() const
 {
     for (const InternalNodePointer &node : std::as_const(m_selectedInternalNodeList)) {
-        if (!node->isValid())
+        if (!node->isValid)
             throw new InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
 
@@ -1239,23 +1239,23 @@ void ModelPrivate::clearParent(const InternalNodePointer &node)
 
 void ModelPrivate::changeRootNodeType(const TypeName &type, int majorVersion, int minorVersion)
 {
-    Q_ASSERT(!rootNode().isNull());
+    Q_ASSERT(rootNode());
-    rootNode()->setType(type);
+    rootNode()->typeName = type;
-    rootNode()->setMajorVersion(majorVersion);
+    rootNode()->majorVersion = majorVersion;
-    rootNode()->setMinorVersion(minorVersion);
+    rootNode()->minorVersion = minorVersion;
     notifyRootNodeTypeChanged(QString::fromUtf8(type), majorVersion, minorVersion);
 }
 
 void ModelPrivate::setScriptFunctions(const InternalNodePointer &node, const QStringList &scriptFunctionList)
 {
-    node->setScriptFunctions(scriptFunctionList);
+    node->scriptFunctions = scriptFunctionList;
 
     notifyScriptFunctionsChanged(node, scriptFunctionList);
 }
 
 void ModelPrivate::setNodeSource(const InternalNodePointer &node, const QString &nodeSource)
 {
-    node->setNodeSource(nodeSource);
+    node->nodeSource = nodeSource;
     notifyNodeSourceChanged(node, nodeSource);
 }
 
@@ -1343,7 +1343,7 @@ bool ModelPrivate::hasNodeForInternalId(qint32 internalId) const
 
 QList<InternalNodePointer> ModelPrivate::allNodes() const
 {
-    if (m_rootInternalNode.isNull() || !m_rootInternalNode->isValid())
+    if (!m_rootInternalNode || !m_rootInternalNode->isValid)
         return {};
 
     // the nodes must be ordered.

src/plugins/qmldesigner/designercore/model/model_p.h

@@ -59,7 +59,7 @@ class InternalVariantProperty;
 class InternalNodeAbstractProperty;
 class InternalNodeListProperty;
 
-using InternalNodePointer = QSharedPointer<InternalNode>;
+using InternalNodePointer = std::shared_ptr<InternalNode>;
 using InternalPropertyPointer = QSharedPointer<InternalProperty>;
 using InternalBindingPropertyPointer = QSharedPointer<InternalBindingProperty>;
 using InternalSignalHandlerPropertyPointer = QSharedPointer<InternalSignalHandlerProperty>;

src/plugins/qmldesigner/designercore/model/modelnode.cpp

@@ -137,7 +137,7 @@ QString ModelNode::id() const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return m_internalNode->id();
+    return m_internalNode->id;
 }
 
 QString ModelNode::validId()
@@ -243,15 +243,14 @@ bool ModelNode::hasId() const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return m_internalNode->hasId();
+    return !m_internalNode->id.isEmpty();
 }
 
 void ModelNode::setIdWithRefactoring(const QString& id)
 {
-    if (model()->rewriterView()
+    if (model()->rewriterView() && !id.isEmpty()
-            && !id.isEmpty()
+        && !m_internalNode->id.isEmpty()) { // refactor the id if they are not empty
-            && !m_internalNode->id().isEmpty()) { // refactor the id if they are not empty
+        model()->rewriterView()->renameId(m_internalNode->id, id);
-        model()->rewriterView()->renameId(m_internalNode->id(), id);
     } else {
         setIdWithoutRefactoring(id);
     }
@@ -268,14 +267,14 @@ void ModelNode::setIdWithoutRefactoring(const QString &id)
     if (!isValidId(id))
         throw InvalidIdException(__LINE__, __FUNCTION__, __FILE__, id.toUtf8(), InvalidIdException::InvalidCharacters);
 
-    if (id == m_internalNode->id())
+    if (id == m_internalNode->id)
         return;
 
 
     if (model()->hasId(id))
         throw InvalidIdException(__LINE__, __FUNCTION__, __FILE__, id.toUtf8(), InvalidIdException::DuplicateId);
 
-    m_model.data()->d->changeNodeId(internalNode(), id);
+    m_model.data()->d->changeNodeId(m_internalNode, id);
 }
 
 /*! \brief the fully-qualified type name of the node is represented as string
@@ -287,7 +286,7 @@ TypeName ModelNode::type() const
         Q_ASSERT_X(isValid(), Q_FUNC_INFO, "model node is invalid");
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
-    return m_internalNode->type();
+    return m_internalNode->typeName;
 }
 
 /*! \brief minor number of the QML type
@@ -299,7 +298,7 @@ int ModelNode::minorVersion() const
         Q_ASSERT_X(isValid(), Q_FUNC_INFO, "model node is invalid");
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
-    return m_internalNode->minorVersion();
+    return m_internalNode->minorVersion;
 }
 
 /*! \brief major number of the QML type
@@ -311,7 +310,7 @@ int ModelNode::majorVersion() const
         Q_ASSERT_X(isValid(), Q_FUNC_INFO, "model node is invalid");
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
-    return m_internalNode->majorVersion();
+    return m_internalNode->majorVersion;
 }
 
 /*! \return the short-hand type name of the node. */
@@ -343,7 +342,7 @@ A node might become invalid if e.g. it or one of its ancestors is deleted.
 */
 bool ModelNode::isValid() const
 {
-    return !m_model.isNull() && !m_view.isNull() && m_internalNode && m_internalNode->isValid();
+    return !m_model.isNull() && !m_view.isNull() && m_internalNode && m_internalNode->isValid;
 }
 
 /*!
@@ -435,8 +434,7 @@ void ModelNode::changeType(const TypeName &typeName, int majorVersion, int minor
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
 
-    model()->d->changeNodeType(internalNode(), typeName, majorVersion, minorVersion);
+    model()->d->changeNodeType(m_internalNode, typeName, majorVersion, minorVersion);
-
 }
 
 void ModelNode::setParentProperty(const ModelNode &newParentNode, const PropertyName &propertyName)
@@ -603,9 +601,9 @@ QList<AbstractProperty> ModelNode::properties() const
 
     QList<AbstractProperty> propertyList;
 
-    const QList<PropertyName> propertyNames = internalNode()->propertyNameList();
+    const QList<PropertyName> propertyNames = m_internalNode->propertyNameList();
     for (const PropertyName &propertyName : propertyNames) {
-        AbstractProperty property(propertyName, internalNode(), model(), view());
+        AbstractProperty property(propertyName, m_internalNode, model(), view());
         propertyList.append(property);
     }
 
@@ -707,8 +705,8 @@ void ModelNode::removeProperty(const PropertyName &name) const
 
     model()->d->checkPropertyName(name);
 
-    if (internalNode()->hasProperty(name))
+    if (m_internalNode->hasProperty(name))
-        model()->d->removeProperty(internalNode()->property(name));
+        model()->d->removeProperty(m_internalNode->property(name));
 }
 
 /*! \brief removes this node from the node tree
@@ -750,7 +748,7 @@ void ModelNode::destroy()
         throw InvalidArgumentException(__LINE__, __FUNCTION__, __FILE__, "rootNode");
 
     removeModelNodeFromSelection(*this);
-    model()->d->removeNode(internalNode());
+    model()->d->removeNode(m_internalNode);
 }
 //\}
 
@@ -814,7 +812,7 @@ properties.
 */
 QList<ModelNode> ModelNode::directSubModelNodes() const
 {
-    return toModelNodeList(internalNode()->allDirectSubNodes(), view());
+    return toModelNodeList(m_internalNode->allDirectSubNodes(), view());
 }
 
 QList<ModelNode> ModelNode::directSubModelNodesOfType(const TypeName &typeName) const
@@ -917,7 +915,7 @@ PropertyNameList ModelNode::propertyNames() const
 {
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
-    return internalNode()->propertyNameList();
+    return m_internalNode->propertyNameList();
 }
 
 /*! \brief test a if a property is set for this node
@@ -928,47 +926,50 @@ bool ModelNode::hasProperty(const PropertyName &name) const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return internalNode()->hasProperty(name);
+    return m_internalNode->hasProperty(name);
 }
 
 bool ModelNode::hasVariantProperty(const PropertyName &name) const
 {
-    return hasProperty(name) && internalNode()->property(name)->isVariantProperty();
+    return hasProperty(name) && m_internalNode->property(name)->isVariantProperty();
 }
 
 bool ModelNode::hasBindingProperty(const PropertyName &name) const
 {
-    return hasProperty(name) && internalNode()->property(name)->isBindingProperty();
+    return hasProperty(name) && m_internalNode->property(name)->isBindingProperty();
 }
 
 bool ModelNode::hasNodeAbstractProperty(const PropertyName &name) const
 {
-    return hasProperty(name) && internalNode()->property(name)->isNodeAbstractProperty();
+    return hasProperty(name) && m_internalNode->property(name)->isNodeAbstractProperty();
 }
 
 bool ModelNode::hasDefaultNodeAbstractProperty() const
 {
-    return hasProperty(metaInfo().defaultPropertyName()) && internalNode()->property(metaInfo().defaultPropertyName())->isNodeAbstractProperty();
+    return hasProperty(metaInfo().defaultPropertyName())
+           && m_internalNode->property(metaInfo().defaultPropertyName())->isNodeAbstractProperty();
 }
 
 bool ModelNode::hasDefaultNodeListProperty() const
 {
-    return hasProperty(metaInfo().defaultPropertyName()) && internalNode()->property(metaInfo().defaultPropertyName())->isNodeListProperty();
+    return hasProperty(metaInfo().defaultPropertyName())
+           && m_internalNode->property(metaInfo().defaultPropertyName())->isNodeListProperty();
 }
 
 bool ModelNode::hasDefaultNodeProperty() const
 {
-    return hasProperty(metaInfo().defaultPropertyName()) && internalNode()->property(metaInfo().defaultPropertyName())->isNodeProperty();
+    return hasProperty(metaInfo().defaultPropertyName())
+           && m_internalNode->property(metaInfo().defaultPropertyName())->isNodeProperty();
 }
 
 bool ModelNode::hasNodeProperty(const PropertyName &name) const
 {
-    return hasProperty(name) && internalNode()->property(name)->isNodeProperty();
+    return hasProperty(name) && m_internalNode->property(name)->isNodeProperty();
 }
 
 bool ModelNode::hasNodeListProperty(const PropertyName &name) const
 {
-    return hasProperty(name) && internalNode()->property(name)->isNodeListProperty();
+    return hasProperty(name) && m_internalNode->property(name)->isNodeListProperty();
 }
 
 static bool recursiveAncestor(const ModelNode &possibleAncestor, const ModelNode &node)
@@ -1054,24 +1055,24 @@ QVariant ModelNode::auxiliaryData(const PropertyName &name) const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return internalNode()->auxiliaryData(name);
+    return m_internalNode->auxiliaryData(name);
 }
 
 void ModelNode::setAuxiliaryData(const PropertyName &name, const QVariant &data) const
 {
     Internal::WriteLocker locker(m_model.data());
-    m_model.data()->d->setAuxiliaryData(internalNode(), name, data);
+    m_model.data()->d->setAuxiliaryData(m_internalNode, name, data);
 }
 
 void ModelNode::setAuxiliaryDataWithoutLock(const PropertyName &name, const QVariant &data) const
 {
-     m_model.data()->d->setAuxiliaryData(internalNode(), name, data);
+    m_model.data()->d->setAuxiliaryData(m_internalNode, name, data);
 }
 
 void ModelNode::removeAuxiliaryData(const PropertyName &name) const
 {
     Internal::WriteLocker locker(m_model.data());
-    m_model.data()->d->removeAuxiliaryData(internalNode(), name);
+    m_model.data()->d->removeAuxiliaryData(m_internalNode, name);
 }
 
 bool ModelNode::hasAuxiliaryData(const PropertyName &name) const
@@ -1079,7 +1080,7 @@ bool ModelNode::hasAuxiliaryData(const PropertyName &name) const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return internalNode()->hasAuxiliaryData(name);
+    return m_internalNode->hasAuxiliaryData(name);
 }
 
 const QHash<PropertyName, QVariant> &ModelNode::auxiliaryData() const
@@ -1087,7 +1088,7 @@ const QHash<PropertyName, QVariant> &ModelNode::auxiliaryData() const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return internalNode()->auxiliaryData();
+    return m_internalNode->auxiliaryData();
 }
 
 QString ModelNode::customId() const
@@ -1287,20 +1288,20 @@ bool ModelNode::isThisOrAncestorLocked(const ModelNode &node)
 
 void  ModelNode::setScriptFunctions(const QStringList &scriptFunctionList)
 {
-    model()->d->setScriptFunctions(internalNode(), scriptFunctionList);
+    model()->d->setScriptFunctions(m_internalNode, scriptFunctionList);
 }
 
 QStringList  ModelNode::scriptFunctions() const
 {
-    return internalNode()->scriptFunctions();
+    return m_internalNode->scriptFunctions;
 }
 
 qint32 ModelNode::internalId() const
 {
-    if (m_internalNode.isNull())
+    if (!m_internalNode)
         return -1;
 
-    return m_internalNode->internalId();
+    return m_internalNode->internalId;
 }
 
 void ModelNode::setNodeSource(const QString &newNodeSource)
@@ -1312,10 +1313,10 @@ void ModelNode::setNodeSource(const QString &newNodeSource)
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
 
-    if (internalNode()->nodeSource() == newNodeSource)
+    if (m_internalNode->nodeSource == newNodeSource)
         return;
 
-    m_model.data()->d->setNodeSource(internalNode(), newNodeSource);
+    m_model.data()->d->setNodeSource(m_internalNode, newNodeSource);
 }
 
 void ModelNode::setNodeSource(const QString &newNodeSource, NodeSourceType type)
@@ -1327,11 +1328,11 @@ void ModelNode::setNodeSource(const QString &newNodeSource, NodeSourceType type)
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
     }
 
-    if (internalNode()->nodeSourceType() == type && internalNode()->nodeSource() == newNodeSource)
+    if (m_internalNode->nodeSourceType == type && m_internalNode->nodeSource == newNodeSource)
         return;
 
-    internalNode()->setNodeSourceType(type); // Set type first as it doesn't trigger any notifies
+    m_internalNode->nodeSourceType = type; // Set type first as it doesn't trigger any notifies
-    m_model.data()->d->setNodeSource(internalNode(), newNodeSource);
+    m_model.data()->d->setNodeSource(m_internalNode, newNodeSource);
 }
 
 QString ModelNode::nodeSource() const
@@ -1339,7 +1340,7 @@ QString ModelNode::nodeSource() const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return internalNode()->nodeSource();
+    return m_internalNode->nodeSource;
 }
 
 QString ModelNode::convertTypeToImportAlias() const
@@ -1358,8 +1359,7 @@ ModelNode::NodeSourceType ModelNode::nodeSourceType() const
     if (!isValid())
         throw InvalidModelNodeException(__LINE__, __FUNCTION__, __FILE__);
 
-    return static_cast<ModelNode::NodeSourceType>(internalNode()->nodeSourceType());
+    return static_cast<ModelNode::NodeSourceType>(m_internalNode->nodeSourceType);
-
 }
 
 bool ModelNode::isComponent() const
@@ -1445,10 +1445,10 @@ QIcon ModelNode::typeIcon() const
 
 QString ModelNode::behaviorPropertyName() const
 {
-    if (m_internalNode.isNull())
+    if (!m_internalNode)
         return {};
 
-    return m_internalNode->behaviorPropertyName();
+    return m_internalNode->behaviorPropertyName;
 }
 
 }

src/plugins/qmldesigner/designercore/model/nodeabstractproperty.cpp

@@ -149,9 +149,7 @@ int NodeAbstractProperty::count() const
 
 QList<ModelNode> NodeAbstractProperty::allSubNodes()
 {
-    if (!internalNode()
+    if (!internalNode() || !internalNode()->isValid || !internalNode()->hasProperty(name())
-        || !internalNode()->isValid()
-        || !internalNode()->hasProperty(name())
         || !internalNode()->property(name())->isNodeAbstractProperty())
         return QList<ModelNode>();
 
@@ -161,9 +159,7 @@ QList<ModelNode> NodeAbstractProperty::allSubNodes()
 
 QList<ModelNode> NodeAbstractProperty::directSubNodes() const
 {
-    if (!internalNode()
+    if (!internalNode() || !internalNode()->isValid || !internalNode()->hasProperty(name())
-        || !internalNode()->isValid()
-        || !internalNode()->hasProperty(name())
         || !internalNode()->property(name())->isNodeAbstractProperty())
         return QList<ModelNode>();