[Ide] Fix Remove item added for file that is deleted from disk

Deleting a file from the Solution pad would sometimes add a Remove
item for that file which should only happen if the file was excluded
from the project. A way to reproduce this is to start the IDE,
create a new .NET Standard project, then right click the Class1.cs
file in the Solution pad, and select Delete. Loading an existing
solution does not seem to reproduce this.

The problem is that the file is deleted from disk after the file is
removed from the Project.Files collection. The removal of the file
causes the type system service to request updated project information.
This can then cause the project file to be updated in memory before
the file is deleted from disk. Since the file exists on disk the
Project.SaveProjectItems method adds a Remove item since it is
treating the file as been excluded. This Remove item will not be
removed when the file is deleted from disk and the project is saved
later since it is not possible to distinguish this case from the case
where a file has been explicitly excluded.

To fix this the simplest approach is to delete the file before
modifying the project's Files collection. Also fixed this problem
that can occur when deleting a folder.

Fixes VSTS #593642 - Deleting file from SDK style project adds
unnecessary data into csproj file

2 files changed, 19 insertions, 15 deletions

diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFileNodeBuilder.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFileNodeBuilder.cs
index d7cb48b503..4fe44f4e70 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFileNodeBuilder.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFileNodeBuilder.cs
@@ -366,19 +366,22 @@ namespace MonoDevelop.Ide.Gui.Pads.ProjectPad
 					folderFile.Subtype = Subtype.Directory;
 					project.Files.Add (folderFile);
 				}
-
 				var children = FileNestingService.GetDependentOrNestedTree (file);
 				if (children != null) {
 					foreach (var child in children.ToArray ()) {
-						project.Files.Remove (child);
-						if(delete)
+						// Delete file before removing them from the project to avoid Remove items being added
+						// if the project is currently being saved in memory or to disk.
+						if (delete)
 							FileService.DeleteFile (child.Name);
+						project.Files.Remove (child);
 					}
 				}
 
-				project.Files.Remove (file);
+				// Delete file before removing them from the project to avoid Remove items being added
+				// if the project is currently being saved in memory or to disk.
 				if (delete && !file.IsLink)
 					FileService.DeleteFile (file.Name);
+				project.Files.Remove (file);
 			}
 		}
 
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFolderNodeBuilder.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFolderNodeBuilder.cs
index 5070f796f1..f0404a5e03 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFolderNodeBuilder.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.Gui.Pads.ProjectPad/ProjectFolderNodeBuilder.cs
@@ -230,19 +230,20 @@ namespace MonoDevelop.Ide.Gui.Pads.ProjectPad
 						break;
 					
 					projects.Add (project);
-					
-					//remove the files and link files in the directory
-					foreach (var f in files)
-						project.Files.Remove (f);
-					
-					// also remove the folder's own ProjectFile, if it exists 
-					// FIXME: it probably was already in the files list
-					if (folderPf != null)
-						project.Files.Remove (folderPf);
 				}
-				
+				// Delete folder before removing it and its files from the project to avoid Remove items being
+				// added to the project if the project is currently being saved in memory or to disk.
 				DeleteFolder (folder);
-				
+
+				//remove the files and link files in the directory
+				foreach (var f in files)
+					project.Files.Remove (f);
+
+				// also remove the folder's own ProjectFile, if it exists
+				// FIXME: it probably was already in the files list
+				if (folderPf != null)
+					project.Files.Remove (folderPf);
+
 				if (isProjectFolder && folder.Path.ParentDirectory != project.BaseDirectory) {
 					// If it's the last item in the parent folder, make sure we keep a reference to the parent 
 					// folder, so it is not deleted from the tree.