Merge pull request #81 from microsoft/users/richsta/removefolderrecursive

Change RemoveFolder behavior to recursively remove child items.

Author: richardstanton

src/Microsoft.VisualStudio.SolutionPersistence/Model/SolutionModel.cs

@@ -224,26 +224,16 @@ public SolutionProjectModel AddProject(string filePath, string? projectTypeName
     }
 
     /// <summary>
-    /// Remove a solution folder from the solution model.
+    /// Remove a solution folder from the solution model. This includes any child folders and projects.
     /// </summary>
     /// <param name="folder">The folder to remove.</param>
     /// <returns><see langword="true"/> if the folder was found and removed.</returns>
     public bool RemoveFolder(SolutionFolderModel folder)
     {
         Argument.ThrowIfNull(folder, nameof(folder));
         this.ValidateInModel(folder);
-        _ = this.solutionFolders.Remove(folder);
 
-        // Remove any children of this folder.
-        foreach (SolutionItemModel existingItem in this.SolutionItems)
-        {
-            if (ReferenceEquals(existingItem.Parent, folder))
-            {
-                existingItem.MoveToFolder(folder.Parent);
-            }
-        }
-
-        return this.RemoveItem(folder);
+        return this.RemoveFolder(folder, this.SolutionItems.ToArray());
     }
 
     /// <summary>
@@ -637,6 +627,29 @@ private SolutionFolderModel AddFolder(StringSpan name, string? parentItemRef)
         return folder;
     }
 
+    // Remove a solution folder from the solution model. This includes any child folders and projects.
+    // Recursive call reuses the solutionItems array to avoid creating a new array for each recursive call.
+    private bool RemoveFolder(SolutionFolderModel folder, SolutionItemModel[] solutionItems)
+    {
+        _ = this.solutionFolders.Remove(folder);
+
+        // Remove any children of this folder.
+        foreach (SolutionItemModel existingItem in solutionItems)
+        {
+            if (ReferenceEquals(existingItem.Parent, folder))
+            {
+                _ = existingItem switch
+                {
+                    SolutionFolderModel childFolder => this.RemoveFolder(childFolder, solutionItems),
+                    SolutionProjectModel childProject => this.RemoveProject(childProject),
+                    _ => throw new InvalidOperationException(),
+                };
+            }
+        }
+
+        return this.RemoveItem(folder);
+    }
+
     private bool RemoveItem(SolutionItemModel item)
     {
         _ = this.solutionItemsById.Remove(item.Id);

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/Folders.cs

@@ -28,6 +28,8 @@ public void RemoveFolder()
         Assert.NotNull(folderNested);
         Assert.NotNull(folderFolder);
 
+        SolutionProjectModel projectInIs = solution.AddProject("ProjectInThis.csproj", folder: folderIs);
+        Assert.NotNull(projectInIs);
         SolutionProjectModel projectInA = solution.AddProject("ProjectInA.csproj", folder: folderA);
         Assert.NotNull(projectInA);
         SolutionProjectModel projectInFolder = solution.AddProject("ProjectInFolder.csproj", folder: folderFolder);
@@ -36,28 +38,31 @@ public void RemoveFolder()
         // Remove the middle 'A' folder.
         Assert.True(solution.RemoveFolder(folderA));
 
-        // Make sure remaining folders have updated references.
+        // Make sure child folders were removed.
         Assert.Equal("/This/", folderThis.ItemRef);
         Assert.Equal("/This/Is/", folderIs.ItemRef);
-        Assert.Equal("/This/Is/Nested/", folderNested.ItemRef);
-        Assert.Equal("/This/Is/Nested/Folder/", folderFolder.ItemRef);
-
-        // Make sure projects have updated references.
-        Assert.NotNull(projectInA.Parent);
-        Assert.Equal("/This/Is/", projectInA.Parent.ItemRef);
-
-        Assert.NotNull(projectInFolder.Parent);
-        Assert.Equal("/This/Is/Nested/Folder/", projectInFolder.Parent.ItemRef);
-
-        // Remove all folders.
-        Assert.True(solution.RemoveFolder(folderThis));
+        Assert.Null(solution.FindFolder(folderNested.ItemRef));
+        Assert.Null(solution.FindFolder(folderFolder.ItemRef));
+
+        // Make sure child projects were removed.
+        Assert.Null(solution.FindProject(projectInA.ItemRef));
+        Assert.Null(solution.FindProject(projectInFolder.ItemRef));
+
+        // Make sure project in 'Is' folder was not removed.
+        Assert.NotNull(projectInIs.Parent);
+        Assert.NotNull(solution.FindProject(projectInIs.ItemRef));
+        Assert.NotNull(projectInIs.Parent);
+        Assert.Equal("/This/Is/", projectInIs.Parent.ItemRef);
+
+        // Remove all folders in reverse.
+        Assert.False(solution.RemoveFolder(folderFolder));
+        Assert.False(solution.RemoveFolder(folderNested));
         Assert.True(solution.RemoveFolder(folderIs));
-        Assert.True(solution.RemoveFolder(folderNested));
-        Assert.True(solution.RemoveFolder(folderFolder));
+        Assert.True(solution.RemoveFolder(folderThis));
 
-        // Make sure projects are in root.
-        Assert.Null(projectInA.Parent);
-        Assert.Null(projectInFolder.Parent);
+        Assert.Empty(solution.SolutionItems);
+        Assert.Empty(solution.SolutionProjects);
+        Assert.Empty(solution.SolutionFolders);
     }
 
     /// <summary>