fix(mps-model-adapters): fix MPSNode.replaceNode

mps-model-adapters-plugin/src/test/kotlin/org/modelix/model/mpsadapters/ReplaceNodeTest.kt

@@ -1,40 +1,119 @@
 package org.modelix.model.mpsadapters
 
-import org.junit.Ignore
+import jetbrains.mps.smodel.SNode
+import jetbrains.mps.smodel.adapter.MetaAdapterByDeclaration
 import org.modelix.model.api.BuiltinLanguages
 import org.modelix.model.api.ConceptReference
 import org.modelix.model.api.INode
 import org.modelix.model.api.IReplaceableNode
 
-@Ignore("Replacing a node through MPS-model-adapters is broken. See MODELIX-920")
 class ReplaceNodeTest : MpsAdaptersTestBase("SimpleProject") {
 
-    fun testReplaceNode() {
-        readAction {
-            assertEquals(1, mpsProject.projectModules.size)
-        }
+    private val sampleConcept = MetaAdapterByDeclaration.asInstanceConcept(
+        MPSConcept.tryParseUID(BuiltinLanguages.jetbrains_mps_lang_core.BaseConcept.getUID())!!.concept,
+    )
+
+    fun `test replace node with parent and module (aka regular node)`() = runCommandOnEDT {
+        val rootNode = getRootUnderTest()
+        val nodeToReplace = rootNode.allChildren.first() as IReplaceableNode
+        val oldContainmentLink = nodeToReplace.getContainmentLink()
+        val nodesToKeep = rootNode.allChildren.drop(1)
+        val oldProperties = nodeToReplace.getAllProperties().toSet()
+        check(oldProperties.isNotEmpty()) { "Test should replace node with properties." }
+        val oldReferences = nodeToReplace.getAllReferenceTargetRefs().toSet()
+        check(oldReferences.isNotEmpty()) { "Test should replace node with references." }
+        val oldChildren = nodeToReplace.allChildren.toList()
+        check(oldChildren.isNotEmpty()) { "Test should replace node with children." }
+        val newConcept = ConceptReference("mps:f3061a53-9226-4cc5-a443-f952ceaf5816/1083245097125")
+
+        val newNode = nodeToReplace.replaceNode(newConcept)
+
+        assertEquals(listOf(newNode) + nodesToKeep, rootNode.allChildren.toList())
+        assertEquals((nodeToReplace as MPSNode).node.nodeId, (newNode as MPSNode).node.nodeId)
+        assertEquals(oldContainmentLink, newNode.getContainmentLink())
+        assertEquals(newConcept, newNode.getConceptReference())
+        assertEquals(oldProperties, newNode.getAllProperties().toSet())
+        assertEquals(oldReferences, newNode.getAllReferenceTargetRefs().toSet())
+        assertEquals(oldChildren, newNode.allChildren.toList())
+    }
+
+    fun `test replace node without parent but with module (aka root node)`() = runCommandOnEDT {
+        val rootNode = getRootUnderTest()
+        val oldContainmentLink = rootNode.getContainmentLink()
+        val model = getModelUnderTest()
+        val newConcept = ConceptReference("mps:f3061a53-9226-4cc5-a443-f952ceaf5816/1083245097125")
+
+        val newNode = rootNode.replaceNode(newConcept)
+
+        assertEquals(listOf(newNode), model.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes))
+        assertEquals((rootNode as MPSNode).node.nodeId, (newNode as MPSNode).node.nodeId)
+        assertEquals(oldContainmentLink, newNode.getContainmentLink())
+        assertEquals(newConcept, newNode.getConceptReference())
+    }
+
+    fun `test replace node without parent and without module (aka free-floating node)`() = runCommandOnEDT {
+        val untouchedRootNode = getRootUnderTest()
+        val model = getModelUnderTest()
+        val freeFloatingSNode = SNode(sampleConcept)
+        val freeFloatingNode = MPSNode(freeFloatingSNode)
+        val oldContainmentLink = freeFloatingNode.getContainmentLink()
+        val newConcept = ConceptReference("mps:f3061a53-9226-4cc5-a443-f952ceaf5816/1083245097125")
 
-        val repositoryNode: INode = MPSRepositoryAsNode(mpsProject.repository)
+        val newNode = freeFloatingNode.replaceNode(newConcept)
 
-        runCommandOnEDT {
-            val module = repositoryNode.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Repository.modules)
-                .single { it.getPropertyValue(BuiltinLanguages.jetbrains_mps_lang_core.INamedConcept.name) == "Solution1" }
-            val model = module.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Module.models)
-                .single { it.getPropertyValue(BuiltinLanguages.jetbrains_mps_lang_core.INamedConcept.name) == "Solution1.model1" }
+        assertEquals(listOf(untouchedRootNode), model.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes))
+        assertEquals(freeFloatingNode.node.nodeId, (newNode as MPSNode).node.nodeId)
+        assertEquals(oldContainmentLink, newNode.getContainmentLink())
+        assertEquals(newConcept, newNode.getConceptReference())
+    }
+
+    fun `test replace node with parent but without module (aka descendant of free-floating node)`() = runCommandOnEDT {
+        val freeFloatingSNode = SNode(sampleConcept)
+        val freeFloatingNode = MPSNode(freeFloatingSNode)
+        val nodeToReplace = freeFloatingNode.addNewChild(
+            BuiltinLanguages.MPSRepositoryConcepts.Model.usedLanguages,
+            -1,
+            BuiltinLanguages.jetbrains_mps_lang_core.BaseConcept,
+        ) as IReplaceableNode
+        val oldContainmentLink = nodeToReplace.getContainmentLink()
+        val newConcept = ConceptReference("mps:f3061a53-9226-4cc5-a443-f952ceaf5816/1083245097125")
+
+        val newNode = nodeToReplace.replaceNode(newConcept)
+
+        assertEquals(listOf(newNode), freeFloatingNode.allChildren.toList())
+        assertEquals((nodeToReplace as MPSNode).node.nodeId, (newNode as MPSNode).node.nodeId)
+        assertEquals(oldContainmentLink, newNode.getContainmentLink())
+        assertEquals(newConcept, newNode.getConceptReference())
+    }
 
-            val rootNode = model.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes).single() as IReplaceableNode
+    fun `test fail to replace node with null concept`() = runCommandOnEDT {
+        val rootNode = getRootUnderTest()
+        val nodeToReplace = rootNode.allChildren.first() as IReplaceableNode
 
-            val oldProperties = rootNode.getAllProperties().toSet()
-            val oldReferences = rootNode.getAllReferenceTargetRefs().toSet()
-            val oldChildren = rootNode.allChildren.toList()
+        val expectedMessage = "Cannot replace node `method1` with a null concept. Explicitly specify a concept (e.g., `BaseConcept`)."
+        assertThrows(IllegalArgumentException::class.java, expectedMessage) {
+            nodeToReplace.replaceNode(null)
+        }
+    }
 
-            val newConcept = ConceptReference("mps:f3061a53-9226-4cc5-a443-f952ceaf5816/1083245097125")
-            val newNode = rootNode.replaceNode(newConcept)
+    fun `test fail to replace node with non mps concept`() = runCommandOnEDT {
+        val rootNode = getRootUnderTest()
+        val nodeToReplace = rootNode.allChildren.first() as IReplaceableNode
+        val newConcept = ConceptReference("notMpsConcept")
 
-            assertEquals(oldProperties, newNode.getAllProperties().toSet())
-            assertEquals(oldReferences, newNode.getAllReferenceTargetRefs().toSet())
-            assertEquals(oldChildren, newNode.allChildren.toList())
-            assertEquals(newConcept, newNode.getConceptReference())
+        val expectedMessage = "Concept UID `notMpsConcept` cannot be parsed as MPS concept."
+        assertThrows(IllegalArgumentException::class.java, expectedMessage) {
+            nodeToReplace.replaceNode(newConcept)
         }
     }
+
+    private fun getModelUnderTest(): INode {
+        val repositoryNode = MPSRepositoryAsNode(mpsProject.repository)
+        val module = repositoryNode.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Repository.modules)
+            .single { it.getPropertyValue(BuiltinLanguages.jetbrains_mps_lang_core.INamedConcept.name) == "Solution1" }
+        return module.getChildren(BuiltinLanguages.MPSRepositoryConcepts.Module.models).single()
+    }
+
+    private fun getRootUnderTest(): IReplaceableNode = getModelUnderTest()
+        .getChildren(BuiltinLanguages.MPSRepositoryConcepts.Model.rootNodes).single() as IReplaceableNode
 }

mps-model-adapters-plugin/testdata/SimpleProject/solutions/Solution1/models/Solution1.model1.mps

@@ -14,10 +14,12 @@
       </concept>
       <concept id="1068580123165" name="jetbrains.mps.baseLanguage.structure.InstanceMethodDeclaration" flags="ig" index="3clFb_" />
       <concept id="1068580123136" name="jetbrains.mps.baseLanguage.structure.StatementList" flags="sn" stub="5293379017992965193" index="3clFbS" />
-      <concept id="1068581517677" name="jetbrains.mps.baseLanguage.structure.VoidType" flags="in" index="3cqZAl" />
       <concept id="1107461130800" name="jetbrains.mps.baseLanguage.structure.Classifier" flags="ng" index="3pOWGL">
         <child id="5375687026011219971" name="member" index="jymVt" unordered="true" />
       </concept>
+      <concept id="1107535904670" name="jetbrains.mps.baseLanguage.structure.ClassifierType" flags="in" index="3uibUv">
+        <reference id="1107535924139" name="classifier" index="3uigEE" />
+      </concept>
       <concept id="1178549954367" name="jetbrains.mps.baseLanguage.structure.IVisible" flags="ng" index="1B3ioH">
         <child id="1178549979242" name="visibility" index="1B3o_S" />
       </concept>
@@ -33,7 +35,7 @@
     <property role="TrG5h" value="Class1" />
     <node concept="3clFb_" id="3cIAtmcX1Te" role="jymVt">
       <property role="TrG5h" value="method1" />
-      <node concept="3cqZAl" id="3cIAtmcX1Tg" role="3clF45" />
+      <ref role="3uigEE" node="3cIAtmcX1Sw" resolve="Class1" />
       <node concept="3Tm1VV" id="3cIAtmcX1Th" role="1B3o_S" />
       <node concept="3clFbS" id="3cIAtmcX1Ti" role="3clF47" />
     </node>

mps-model-adapters/src/main/kotlin/org/modelix/model/mpsadapters/MPSNode.kt

@@ -3,11 +3,9 @@ package org.modelix.model.mpsadapters
 import jetbrains.mps.lang.smodel.generator.smodelAdapter.SNodeOperations
 import jetbrains.mps.smodel.MPSModuleRepository
 import jetbrains.mps.smodel.adapter.MetaAdapterByDeclaration
-import jetbrains.mps.smodel.adapter.ids.SConceptId
 import jetbrains.mps.smodel.adapter.ids.SContainmentLinkId
 import jetbrains.mps.smodel.adapter.ids.SPropertyId
 import jetbrains.mps.smodel.adapter.ids.SReferenceLinkId
-import jetbrains.mps.smodel.adapter.structure.concept.SConceptAdapterById
 import jetbrains.mps.smodel.adapter.structure.link.SContainmentLinkAdapterById
 import jetbrains.mps.smodel.adapter.structure.property.SPropertyAdapterById
 import jetbrains.mps.smodel.adapter.structure.ref.SReferenceLinkAdapterById
@@ -62,21 +60,40 @@ data class MPSNode(val node: SNode) : IDefaultNodeAdapter, IReplaceableNode {
         }
 
     override fun replaceNode(concept: ConceptReference?): INode {
-        requireNotNull(concept) { "Can't replace $node with null concept. Use BaseConcept explicitly." }
+        requireNotNull(concept) { "Cannot replace node `$node` with a null concept. Explicitly specify a concept (e.g., `BaseConcept`)." }
+        val mpsConcept = MPSConcept.tryParseUID(concept.uid)
+        requireNotNull(mpsConcept) { "Concept UID `${concept.uid}` cannot be parsed as MPS concept." }
+        val sConcept = MetaAdapterByDeclaration.asInstanceConcept(mpsConcept.concept)
+
+        val maybeModel = node.model
+        val maybeParent = node.parent
+        val containmentLink = getMPSContainmentLink(getContainmentLink())
+        val maybeNextSibling = node.nextSibling
+        // The existing node needs to be deleted before the replacing node is created,
+        // because `SModel.createNode` will not use the provided ID if it already exists.
+        node.delete()
+
+        val newNode = if (maybeModel != null) {
+            maybeModel.createNode(sConcept, node.nodeId)
+        } else {
+            jetbrains.mps.smodel.SNode(sConcept, node.nodeId)
+        }
+
+        if (maybeParent != null) {
+            // When `maybeNextSibling` is `null`, `replacingNode` is inserted as a last child.
+            maybeParent.insertChildBefore(containmentLink, newNode, maybeNextSibling)
+        } else if (maybeModel != null) {
+            maybeModel.addRootNode(newNode)
+        }
 
-        val id = node.nodeId
-        val model = checkNotNull(node.model) { "Node is not part of a model" }
-        val newNode = model.createNode(SConceptAdapterById(SConceptId.deserialize(concept.uid), ""), id)
         node.properties.forEach { newNode.setProperty(it, node.getProperty(it)) }
         node.references.forEach { newNode.setReference(it.link, it.targetNodeReference) }
         node.children.forEach { child ->
             val link = checkNotNull(child.containmentLink) { "Containment link of child node not found" }
+            node.removeChild(child)
             newNode.addChild(link, child)
         }
 
-        val parent = checkNotNull(node.parent) { "Cannot replace node without a parent" }
-        parent.insertChildBefore(getMPSContainmentLink(getContainmentLink()), node, newNode)
-        node.delete()
         return MPSNode(newNode)
     }