Improved project folder support (#2692)

* Significantly improved project folder support

Fixes #2178

* Better support for non-Windows style directory separators in .fsproj

* Enable support for "Add folder" on project node

* Fix copy/pasting files within subfolders

Fixes #2048

* "Add existing" on the project node now places the file in the correct subfolder

* "Add existing..." now creates any necessary intermediate directories

* Improved resilience of Add Above/Add Below with folders

* Localised new error messages

* Add "New Folder" to Add Above/Add Below

* Updated FileCannotBePlacedMultipleFiles localisation to be more user-friendly

* Fixed being unable to "Add existing" on files outside of the project hierarchy

* Rename AllChildren to AllDescendants

* Fix cut/paste folders resulting in an infinite loop

https://mpfproj10.codeplex.com/workitem/11618

* Fix test compile error

* Make InsertionLocation internal

* Add support for linked files

"Add Existing" on files outside of the project hierarchy will now copy them to the target node

* If the folder exists on disk use that instead

* Do not delete linked files when deleting folders

* Remove bogus test - it's not possible to add a file to the project that has the same name as an existing file

vsintegration/src/FSharp.ProjectSystem.Base/Project/AssemblyReferenceNode.cs

@@ -394,7 +394,7 @@ public override bool CanDeleteItem(__VSDELETEITEMOPERATION deleteOperation)
             return base.CanDeleteItem(deleteOperation);
         }
 
-        public override void Remove(bool removeFromStorage)
+        public override void Remove(bool removeFromStorage, bool promptSave = true)
         {
             // AssemblyReference doesn't backed by the document - its removal is simply modification of the project file
             // we disable IVsTrackProjectDocuments2 events to avoid confusing messages from SCC
@@ -403,7 +403,7 @@ public override void Remove(bool removeFromStorage)
             {
                 ProjectMgr.EventTriggeringFlag = oldFlag | ProjectNode.EventTriggering.DoNotTriggerTrackerEvents;
 
-                base.Remove(removeFromStorage);
+                base.Remove(removeFromStorage, promptSave);
 
                 // invoke ComputeSourcesAndFlags to refresh compiler flags
                 // it was the only useful thing performed by one of IVsTrackProjectDocuments2 listeners

vsintegration/src/FSharp.ProjectSystem.Base/Project/Automation/OAProjectItem.cs

@@ -350,7 +350,7 @@ public virtual void Remove()
                 extensibility.EnterAutomationFunction();
                 try
                 {
-                    this.node.Remove(false);
+                    this.node.Remove(removeFromStorage: false);
                 }
                 finally
                 {
@@ -381,7 +381,7 @@ public virtual void Delete()
 
                 try
                 {
-                    this.node.Remove(true);
+                    this.node.Remove(removeFromStorage: true, promptSave: false);
                 }
                 finally
                 {

vsintegration/src/FSharp.ProjectSystem.Base/Project/Automation/VSProject/OAReferenceBase.cs

@@ -158,7 +158,7 @@ public virtual string PublicKeyToken
         public virtual void Remove()
         {
             UIThread.DoOnUIThread(delegate(){
-                BaseReferenceNode.Remove(false);
+                BaseReferenceNode.Remove(removeFromStorage: false);
             });
         }
 

vsintegration/src/FSharp.ProjectSystem.Base/Project/FileNode.cs

@@ -210,6 +210,14 @@ internal FileNode(ProjectNode root, ProjectElement element, uint? hierarchyId =
             } 
         } 
 
+        public virtual string RelativeFilePath
+        {
+            get
+            {
+                return PackageUtilities.MakeRelativeIfRooted(this.Url, this.ProjectMgr.BaseURI);
+            }
+        }
+
         public override NodeProperties CreatePropertiesObject()
         {
             return new FileNodeProperties(this);

vsintegration/src/FSharp.ProjectSystem.Base/Project/FolderNode.cs

@@ -18,6 +18,7 @@
 using VsCommands2K = Microsoft.VisualStudio.VSConstants.VSStd2KCmdID;
 using System.Diagnostics.CodeAnalysis;
 using System.Collections.Generic;
+using System.Linq;
 
 namespace Microsoft.VisualStudio.FSharp.ProjectSystem
 {
@@ -93,21 +94,15 @@ public override int SetEditLabel(string label)
 
             string newPath = Path.Combine(new DirectoryInfo(this.Url).Parent.FullName, label);
 
-            // Verify that No Directory/file already exists with the new name among current children
+            // Verify that No Directory/file already exists with the new name among siblings
             for (HierarchyNode n = Parent.FirstChild; n != null; n = n.NextSibling)
             {
                 if (n != this && String.Compare(n.Caption, label, StringComparison.OrdinalIgnoreCase) == 0)
                 {
-                    return ShowFileOrFolderAlreadExistsErrorMessage(newPath);
+                    return ShowErrorMessage(SR.FileOrFolderAlreadyExists, newPath);
                 }
             }
 
-            // Verify that No Directory/file already exists with the new name on disk
-            if (Directory.Exists(newPath) || FSLib.Shim.FileSystem.SafeExists(newPath))
-            {
-                return ShowFileOrFolderAlreadExistsErrorMessage(newPath);
-            }
-
             try
             {
                 RenameFolder(label);
@@ -380,7 +375,8 @@ public virtual void RenameDirectory(string newPath)
             {
                 if (Directory.Exists(newPath))
                 {
-                    ShowFileOrFolderAlreadExistsErrorMessage(newPath);
+                    ShowErrorMessage(SR.FileOrFolderAlreadyExists, newPath);
+                    return;
                 }
 
                 Directory.Move(this.Url, newPath);
@@ -389,12 +385,34 @@ public virtual void RenameDirectory(string newPath)
 
         private void RenameFolder(string newName)
         {
-            // Do the rename (note that we only do the physical rename if the leaf name changed)
             string newPath = Path.Combine(this.Parent.VirtualNodeName, newName);
+            string newFullPath = Path.Combine(this.ProjectMgr.ProjectFolder, newPath);
+
+            // Only do the physical rename if the leaf name changed
             if (String.Compare(Path.GetFileName(VirtualNodeName), newName, StringComparison.Ordinal) != 0)
             {
-                this.RenameDirectory(Path.Combine(this.ProjectMgr.ProjectFolder, newPath));
+                // Verify that no directory/file already exists with the new name on disk.
+                // If it does, just subsume that name if our directory is empty.
+                if (Directory.Exists(newFullPath) || FSLib.Shim.FileSystem.SafeExists(newFullPath))
+                {
+                    // We can't delete our old directory as it is not empty
+                    if (Directory.EnumerateFileSystemEntries(this.Url).Any())
+                    {
+                        ShowErrorMessage(SR.FolderCannotBeRenamed, newPath);
+                        return;
+                    }
+
+                    // Try to delete the old (empty) directory.
+                    // Note that we don't want to delete recursively in case a file was added between
+                    // when we checked and when we went to delete (potential race condition).
+                    Directory.Delete(this.Url, false);
+                }
+                else
+                {
+                    this.RenameDirectory(newFullPath);
+                }
             }
+
             this.VirtualNodeName = newPath;
 
             this.ItemNode.Rename(VirtualNodeName);
@@ -422,13 +440,15 @@ private void RenameFolder(string newName)
         /// <summary>
         /// Show error message if not in automation mode, otherwise throw exception
         /// </summary>
-        /// <param name="newPath">path of file or folder already existing on disk</param>
+        /// <param name="parameter">Parameter for resource string format</param>
         /// <returns>S_OK</returns>
-        private int ShowFileOrFolderAlreadExistsErrorMessage(string newPath)
+        private int ShowErrorMessage(string resourceName, string parameter)
         {
-            //A file or folder with the name '{0}' already exists on disk at this location. Please choose another name.
-            //If this file or folder does not appear in the Solution Explorer, then it is not currently part of your project. To view files which exist on disk, but are not in the project, select Show All Files from the Project menu.
-            string errorMessage = (String.Format(CultureInfo.CurrentCulture, SR.GetString(SR.FileOrFolderAlreadyExists, CultureInfo.CurrentUICulture), newPath));
+            // Most likely the cause of:
+            // A file or folder with the name '{0}' already exists on disk at this location. Please choose another name.
+            // -or-
+            // This folder cannot be renamed to '{0}' as it already exists on disk.
+            string errorMessage = String.Format(CultureInfo.CurrentCulture, SR.GetStringWithCR(resourceName), parameter);
             if (!Utilities.IsInAutomationFunction(this.ProjectMgr.Site))
             {
                 string title = null;

vsintegration/src/FSharp.ProjectSystem.Base/Project/HierarchyNode.cs

@@ -524,8 +524,9 @@ public virtual void AddChild(HierarchyNode node)
             Object nodeWithSameID = this.projectMgr.ItemIdMap[node.hierarchyId];
             if (!Object.ReferenceEquals(node, nodeWithSameID as HierarchyNode))
             {
-                if (nodeWithSameID == null && node.ID <= this.ProjectMgr.ItemIdMap.Count)
-                { // reuse our hierarchy id if possible.
+                // reuse our hierarchy id if possible.
+                if (nodeWithSameID == null)
+                {
                     this.projectMgr.ItemIdMap.SetAt(node.hierarchyId, this);
                 }
                 else
@@ -928,7 +929,7 @@ public virtual string GetMkDocument()
         /// Removes items from the hierarchy. Project overwrites this
         /// </summary>
         /// <param name="removeFromStorage"></param>
-        public virtual void Remove(bool removeFromStorage)
+        public virtual void Remove(bool removeFromStorage, bool promptSave = true)
         {
             string documentToRemove = this.GetMkDocument();
 
@@ -944,7 +945,7 @@ public virtual void Remove(bool removeFromStorage)
             DocumentManager manager = this.GetDocumentManager();
             if (manager != null)
             {
-                if (manager.Close(!removeFromStorage ? __FRAMECLOSE.FRAMECLOSE_PromptSave : __FRAMECLOSE.FRAMECLOSE_NoSave) == VSConstants.E_ABORT)
+                if (manager.Close(promptSave ? __FRAMECLOSE.FRAMECLOSE_PromptSave : __FRAMECLOSE.FRAMECLOSE_NoSave) == VSConstants.E_ABORT)
                 {
                     // User cancelled operation in message box.
                     return;
@@ -963,7 +964,7 @@ public virtual void Remove(bool removeFromStorage)
             // Remove child if any before removing from the hierarchy
             for (HierarchyNode child = this.FirstChild; child != null; child = child.NextSibling)
             {
-                child.Remove(removeFromStorage);
+                child.Remove(removeFromStorage: false, promptSave: promptSave);
             }
 
             HierarchyNode thisParentNode = this.parentNode;
@@ -1114,7 +1115,7 @@ public virtual HierarchyNode GetDragTargetHandlerNode()
         /// Add a new Folder to the project hierarchy.
         /// </summary>
         /// <returns>S_OK if succeeded, otherwise an error</returns>
-        public virtual int AddNewFolder()
+        public virtual int AddNewFolder(Action<HierarchyNode> moveNode=null)
         {
             // Check out the project file.
             if (!this.ProjectMgr.QueryEditProjectFile(false))
@@ -1129,20 +1130,25 @@ public virtual int AddNewFolder()
                 ErrorHandler.ThrowOnFailure(this.projectMgr.GenerateUniqueItemName(this.hierarchyId, String.Empty, String.Empty, out newFolderName));
 
                 // create the project part of it, the project file
-                HierarchyNode child = this.ProjectMgr.CreateFolderNodes(Path.Combine(this.virtualNodeName, newFolderName));
+                HierarchyNode node = this.ProjectMgr.CreateFolderNodes(Path.Combine(this.virtualNodeName, newFolderName));
 
-                if (child is FolderNode)
+                if (node is FolderNode)
                 {
-                    ((FolderNode)child).CreateDirectory();
+                    ((FolderNode)node).CreateDirectory();
                 }
 
+                if (moveNode != null)
+                {
+                    moveNode(node);
+                }
+                
                 // If we are in automation mode then skip the ui part which is about renaming the folder
                 if (!Utilities.IsInAutomationFunction(this.projectMgr.Site))
                 {
                     IVsUIHierarchyWindow uiWindow = UIHierarchyUtilities.GetUIHierarchyWindow(this.projectMgr.Site, SolutionExplorer);
                     // we need to get into label edit mode now...
                     // so first select the new guy...
-                    ErrorHandler.ThrowOnFailure(uiWindow.ExpandItem(this.projectMgr.InteropSafeIVsUIHierarchy, child.hierarchyId, EXPANDFLAGS.EXPF_SelectItem));
+                    ErrorHandler.ThrowOnFailure(uiWindow.ExpandItem(this.projectMgr.InteropSafeIVsUIHierarchy, node.hierarchyId, EXPANDFLAGS.EXPF_SelectItem));
                     // them post the rename command to the shell. Folder verification and creation will
                     // happen in the setlabel code...
                     IVsUIShell shell = this.projectMgr.Site.GetService(typeof(SVsUIShell)) as IVsUIShell;
@@ -1214,7 +1220,7 @@ public virtual void DoDefaultAction()
         public virtual int ExcludeFromProject()
         {
             Debug.Assert(this.ProjectMgr != null, "The project item " + this.ToString() + " has not been initialised correctly. It has a null ProjectMgr");
-            this.Remove(false);
+            this.Remove(removeFromStorage: false);
             return VSConstants.S_OK;
         }
 
@@ -1403,14 +1409,19 @@ public virtual int ExecCommandOnNode(Guid cmdGroup, uint cmd, uint nCmdexecopt,
             }
             else if (cmdGroup == VsMenus.guidStandardCommandSet97)
             {
+                int result = -1;
                 HierarchyNode nodeToAddTo = this.GetDragTargetHandlerNode();
                 switch ((VsCommands)cmd)
                 {
                     case VsCommands.AddNewItem:
-                        return nodeToAddTo.AddItemToHierarchy(HierarchyAddType.AddNewItem);
+                        result = nodeToAddTo.AddItemToHierarchy(HierarchyAddType.AddNewItem);
+                        this.projectMgr.EnsureMSBuildAndSolutionExplorerAreInSync();
+                        return result;
 
                     case VsCommands.AddExistingItem:
-                        return nodeToAddTo.AddItemToHierarchy(HierarchyAddType.AddExistingItem);
+                        result = nodeToAddTo.AddItemToHierarchy(HierarchyAddType.AddExistingItem);
+                        this.projectMgr.EnsureMSBuildAndSolutionExplorerAreInSync();
+                        return result;
 
                     case VsCommands.NewFolder:
                         return nodeToAddTo.AddNewFolder();
@@ -2876,7 +2887,8 @@ public virtual int DeleteItem(uint delItemOp, uint itemId)
             HierarchyNode node = this.projectMgr.NodeFromItemId(itemId);
             if (node != null)
             {
-                node.Remove((delItemOp & (uint)__VSDELETEITEMOPERATION.DELITEMOP_DeleteFromStorage) != 0);
+                var removeFromStorage = (delItemOp & (uint)__VSDELETEITEMOPERATION.DELITEMOP_DeleteFromStorage) != 0;
+                node.Remove(removeFromStorage, promptSave: !removeFromStorage);
                 return VSConstants.S_OK;
             }
 
@@ -3294,5 +3306,38 @@ public int GetResourceItem(uint itemidDocument, string pszCulture, uint grfPRF,
         }
 
         public virtual __VSPROVISIONALVIEWINGSTATUS ProvisionalViewingStatus => __VSPROVISIONALVIEWINGSTATUS.PVS_Disabled;
+        
+        /// <summary>
+        /// All nodes that are direct children of this node.
+        /// </summary>
+        public virtual IEnumerable<HierarchyNode> AllChildren
+        {
+            get
+            {
+                for (var child = this.FirstChild; child != null; child = child.NextSibling)
+                {
+                    yield return child;
+                }
+            }
+        }
+
+        /// <summary>
+        /// All nodes that are my children, plus their children, ad infinitum.
+        /// </summary>
+        public virtual IEnumerable<HierarchyNode> AllDescendants
+        {
+            get
+            {
+                foreach (var child in this.AllChildren)
+                {
+                    yield return child;
+
+                    foreach (var descendant in child.AllDescendants)
+                    {
+                        yield return descendant;
+                    }
+                }
+            }
+        }
     }
 }

vsintegration/src/FSharp.ProjectSystem.Base/Project/LinkedFileNode.cs

@@ -158,6 +158,19 @@ public bool IsImported
         public override int MenuCommandId
         {
             get { return VsMenus.IDM_VS_CTXT_ITEMNODE; }
-        } 
+        }
+
+        public override string RelativeFilePath
+        {
+            get
+            {
+                string link = this.ItemNode.GetMetadata(ProjectFileConstants.Link);
+                if (string.IsNullOrEmpty(link))
+                {
+                    return base.RelativeFilePath;
+                }
+                return link;
+            }
+        }
     }
 }

vsintegration/src/FSharp.ProjectSystem.Base/Project/Microsoft.VisualStudio.Package.Project.cs

@@ -115,6 +115,7 @@ public sealed class SR
         public const string FileName = "FileName";
         public const string FileNameDescription = "FileNameDescription";
         public const string FileOrFolderAlreadyExists = "FileOrFolderAlreadyExists";
+        public const string FolderCannotBeRenamed = "FolderCannotBeRenamed";
         public const string FileOrFolderCannotBeFound = "FileOrFolderCannotBeFound";
         public const string FileProperties = "FileProperties";
         public const string FolderName = "FolderName";

vsintegration/src/FSharp.ProjectSystem.Base/Project/Microsoft.VisualStudio.Package.Project.resx

@@ -407,6 +407,9 @@
   <data name="FileOrFolderAlreadyExists" xml:space="preserve">
     <value>A file or folder with the name '{0}' already exists on disk at this location. Please choose another name.</value>
   </data>
+  <data name="FolderCannotBeRenamed" xml:space="preserve">
+    <value>This folder cannot be renamed to '{0}' as it already exists on disk.\n\nOnly empty folders can be renamed to existing folders. This folder contains files within it on disk.</value>
+  </data>
   <data name="BuildCaption" xml:space="preserve">
     <value>Build</value>
   </data>

vsintegration/src/FSharp.ProjectSystem.Base/Project/ProjectNode.CopyPaste.cs

@@ -555,7 +555,7 @@ public virtual void WalkSourceProjectAndAdd(IVsHierarchy sourceHierarchy, uint i
                     while (currentItemID != VSConstants.VSITEMID_NIL)
                     {
                         variant = null;
-                        ErrorHandler.ThrowOnFailure(sourceHierarchy.GetProperty(itemId, (int)__VSHPROPID.VSHPROPID_NextVisibleSibling, out variant));
+                        ErrorHandler.ThrowOnFailure(sourceHierarchy.GetProperty(currentItemID, (int)__VSHPROPID.VSHPROPID_NextVisibleSibling, out variant));
                         currentItemID = (uint)(int)variant;
                         WalkSourceProjectAndAdd(sourceHierarchy, currentItemID, targetNode, true);
                     }
@@ -974,7 +974,7 @@ public void CleanupSelectionDataObject(bool dropped, bool cut, bool moved, bool
                             }
                         }
 
-                        node.Remove(true);
+                        node.Remove(removeFromStorage: true, promptSave: false);
                     }
                     else if (w != null)
                     {

vsintegration/src/FSharp.ProjectSystem.Base/Project/ProjectNode.cs

@@ -1521,7 +1521,7 @@ public override int SetGuidProperty(int propid, ref Guid guid)
         /// Removes items from the hierarchy. 
         /// </summary>
         /// <devdoc>Project overwrites this.</devdoc>
-        public override void Remove(bool removeFromStorage)
+        public override void Remove(bool removeFromStorage, bool promptSave = true)
         {
             // the project will not be deleted from disk, just removed      
             if (removeFromStorage)
@@ -1529,6 +1529,8 @@ public override void Remove(bool removeFromStorage)
                 return;
             }
 
+            Debug.Assert(promptSave, "Non-save prompting removal is not supported");
+
             // Remove the entire project from the solution
             IVsSolution solution = this.Site.GetService(typeof(SVsSolution)) as IVsSolution;
             uint iOption = 1; // SLNSAVEOPT_PromptSave
@@ -3642,7 +3644,7 @@ internal virtual ProjectElement AddFileToMsBuild(string file)
         {
             ProjectElement newItem;
 
-            string itemPath = PackageUtilities.MakeRelativeIfRooted(file, this.BaseURI);
+            string itemPath = PackageUtilities.MakeRelative(this.BaseURI.AbsoluteUrl, file);
             Debug.Assert(!Path.IsPathRooted(itemPath), "Cannot add item with full path.");
 
             string defaultBuildAction = this.DefaultBuildAction(itemPath);
@@ -4830,7 +4832,7 @@ public virtual int GetMkDocument(uint itemId, out string mkDoc)
             return VSConstants.S_OK;
         }
 
-        public abstract void MoveFileToBottomIfNoOtherPendingMove(string relativeFilename);
+        public abstract void MoveFileToBottomIfNoOtherPendingMove(FileNode node);
 
         public int AddItem(uint itemIdLoc, VSADDITEMOPERATION op, string itemName, uint filesToOpen, string[] files, IntPtr dlgOwner, VSADDRESULT[] result)
         {
@@ -4840,37 +4842,10 @@ public int AddItem(uint itemIdLoc, VSADDITEMOPERATION op, string itemName, uint
         internal int DoAddItem(uint itemIdLoc, VSADDITEMOPERATION op, string itemName, uint filesToOpen, string[] files, IntPtr dlgOwner, VSADDRESULT[] result, AddItemContext addItemContext = AddItemContext.Unknown)
         {
             // Note that when executing UI actions from the F# project system, any pending 'moves' (for add/add above/add below) are already handled at another level (in Project.fs).
-            // Calls to MoveFileToBottomIfNoOtherPendingMove() in this method are for code paths hit directly by automation APIs.
             Guid empty = Guid.Empty;
 
             // When Adding an item, pass true to let AddItemWithSpecific know to fire the tracker events.
-            var r = AddItemWithSpecific(itemIdLoc, op, itemName, filesToOpen, files, dlgOwner, 0, ref empty, null, ref empty, result, true, context: addItemContext);
-            if (op == VSADDITEMOPERATION.VSADDITEMOP_RUNWIZARD)
-            {
-                HierarchyNode n = this.NodeFromItemId(itemIdLoc);
-                string relativeFolder = Path.GetDirectoryName(n.Url);
-                string relPath = PackageUtilities.MakeRelativeIfRooted(Path.Combine(relativeFolder, Path.GetFileName(itemName)), this.BaseURI);
-                MoveFileToBottomIfNoOtherPendingMove(relPath);
-            }
-            else if (op == VSADDITEMOPERATION.VSADDITEMOP_OPENFILE)
-            {
-                foreach (string file in files)
-                {
-                    HierarchyNode n = this.NodeFromItemId(itemIdLoc);
-                    string relativeFolder = Path.GetDirectoryName(n.Url);
-                    string relPath = PackageUtilities.MakeRelativeIfRooted(Path.Combine(relativeFolder, Path.GetFileName(file)), this.BaseURI);
-                    MoveFileToBottomIfNoOtherPendingMove(relPath);
-                }
-            }
-            else if (op == VSADDITEMOPERATION.VSADDITEMOP_LINKTOFILE)
-            {
-                // This does not seem to be reachable from automation APIs, no movement needed.
-            }
-            else if (op == VSADDITEMOPERATION.VSADDITEMOP_CLONEFILE)
-            {
-                // This seems to only be called as a sub-step of RUNWIZARD, no movement needed.
-            }
-            return r;
+            return AddItemWithSpecific(itemIdLoc, op, itemName, filesToOpen, files, dlgOwner, 0, ref empty, null, ref empty, result, true, context: addItemContext);
         }
 
         /// <summary>
@@ -4988,11 +4963,22 @@ internal int AddItemWithSpecific(uint itemIdLoc, VSADDITEMOPERATION op, string i
                     case VSADDITEMOPERATION.VSADDITEMOP_OPENFILE:
                         {
                             string fileName = Path.GetFileName(file);
-                            newFileName =
+                            
+                            if (context == AddItemContext.Paste && FindChild(file) != null)
+                            {
                                 // if we are doing 'Paste' and source file belongs to current project - generate fresh unique name
-                                context == AddItemContext.Paste && FindChild(file) != null
-                                    ? GenerateCopyOfFileName(baseDir, fileName)
-                                    : Path.Combine(baseDir, fileName);
+                                newFileName = GenerateCopyOfFileName(baseDir, fileName);
+                            }
+                            else if (!IsContainedWithinProjectDirectory(file))
+                            {
+                                // if the file isn't contained within the project directory,
+                                // copy it to be a child of the node we're adding to.
+                                newFileName = Path.Combine(baseDir, fileName);
+                            }
+                            else
+                            {
+                                newFileName = file;
+                            }
                         }
                         break;
                 }
@@ -5162,6 +5148,14 @@ internal int AddItemWithSpecific(uint itemIdLoc, VSADDITEMOPERATION op, string i
                 }
             }
 
+            for (int i = 0; i < actualFilesAddedIndex; ++i)
+            {
+                string absolutePath = actualFiles[i];
+                var fileNode = this.FindChild(absolutePath) as FileNode;
+                Debug.Assert(fileNode != null, $"Unable to find added child node {absolutePath}");
+                MoveFileToBottomIfNoOtherPendingMove(fileNode);
+            }
+
             return VSConstants.S_OK;
         }
 
@@ -5173,25 +5167,12 @@ public virtual int AddLinkedItem(HierarchyNode node, string[] files, VSADDRESULT
             // files[index] will be the absolute location to the linked file
             for (int index = 0; index < files.Length; index++)
             {
-                string relativeUri = PackageUtilities.GetPathDistance(this.ProjectMgr.BaseURI.Uri, new Uri(files[index]));
-                if (string.IsNullOrEmpty(relativeUri))
-                {
-                    return VSConstants.E_FAIL;
-                }
-
-                string fileName = Path.GetFileName(files[index]);
-                if (string.IsNullOrEmpty(fileName))
-                {
-                    return VSConstants.E_FAIL;
-                }
-
-                LinkedFileNode linkedNode = this.AddNewFileNodeToHierarchy(node, fileName) as LinkedFileNode;
+                LinkedFileNode linkedNode = this.AddNewFileNodeToHierarchyCore(node, files[index]) as LinkedFileNode;
                 if (linkedNode == null)
                 {
                     return VSConstants.E_FAIL;
                 }
-
-                linkedNode.ItemNode.Rename(relativeUri);
+                
                 if (node == this)
                 {
                     // parent we are adding to is project root
@@ -5205,6 +5186,12 @@ public virtual int AddLinkedItem(HierarchyNode node, string[] files, VSADDRESULT
                 }
                 linkedNode.SetIsLinkedFile(true);
                 linkedNode.OnInvalidateItems(node);
+
+                // fire the node added event now that we've set the item metadata
+                FireAddNodeEvent(files[index]);
+
+                MoveFileToBottomIfNoOtherPendingMove(linkedNode);
+
                 result[0] = VSADDRESULT.ADDRESULT_Success;
             }
             return VSConstants.S_OK;
@@ -5402,7 +5389,7 @@ public virtual int RemoveItem(uint reserved, uint itemId, out int result)
             {
                 throw new ArgumentException(SR.GetString(SR.ParameterMustBeAValidItemId, CultureInfo.CurrentUICulture), "itemId");
             }
-            n.Remove(true);
+            n.Remove(removeFromStorage: true, promptSave: false);
             result = 1;
             return VSConstants.S_OK;
         }
@@ -6395,6 +6382,15 @@ private static void CloseAllSubNodes(HierarchyNode node)
                 CloseAllNodes(n);
             }
         }
+
+        /// <summary>
+        /// Debug method to assert that the project file and the solution explorer are in sync.
+        /// </summary>
+        [Conditional("DEBUG")]
+        public virtual void EnsureMSBuildAndSolutionExplorerAreInSync()
+        {
+        }
+
         /// <summary>
         /// Get the project extensions
         /// </summary>

vsintegration/src/FSharp.ProjectSystem.Base/Project/ProjectReferenceNode.cs

@@ -660,14 +660,14 @@ public override bool AddReference()
         /// <summary>
         /// Overridden method. The method updates the build dependency list before removing the node from the hierarchy.
         /// </summary>
-        public override void Remove(bool removeFromStorage)
+        public override void Remove(bool removeFromStorage, bool promptSave = true)
         {
             if (this.ProjectMgr == null || !this.canRemoveReference)
             {
                 return;
             }
             this.ProjectMgr.RemoveBuildDependency(this.buildDependency);
-            base.Remove(removeFromStorage);
+            base.Remove(removeFromStorage, promptSave);
             
             // current reference is removed - delete associated error from list
             CleanProjectReferenceErrorState();

vsintegration/src/FSharp.ProjectSystem.Base/Project/VSProjectConstants.cs

@@ -20,6 +20,8 @@ internal sealed class VSProjectConstants
         public static readonly CommandID AddNewItemAbove = new CommandID(guidFSharpProjectCmdSet, 0x3005);
         public static readonly CommandID AddExistingItemAbove = new CommandID(guidFSharpProjectCmdSet, 0x3006);
         public static readonly CommandID MoveDownCmd = new CommandID(guidFSharpProjectCmdSet, 0x3007);
+        public static readonly CommandID NewFolderAbove = new CommandID(guidFSharpProjectCmdSet, 0x3008);
+        public static readonly CommandID NewFolderBelow = new CommandID(guidFSharpProjectCmdSet, 0x3009);
 
         public static readonly CommandID FSharpSendThisReferenceToInteractiveCmd = new CommandID(guidFSharpProjectCmdSet, 0x5004);
         public static readonly CommandID FSharpSendReferencesToInteractiveCmd = new CommandID(guidFSharpProjectCmdSet, 0x5005);

vsintegration/src/FSharp.ProjectSystem.FSharp/MSBuildUtilities.fs

@@ -25,7 +25,11 @@ type internal MSBuildUtilities() =
 
     static let GetItemType(item : ProjectItemElement) =
         item.ItemType
-        
+
+    /// Normalize path directory separator characters to the OS defacto
+    static let NormalizePath(path : string) =
+        path.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar)
+    
     // Gets the <... Include="path"> path for this item, except if the item is a link, then
     // gets the <Link>path</Link> value instead.
     // In other words, gets the location that will be displayed in the solution explorer.
@@ -35,6 +39,7 @@ type internal MSBuildUtilities() =
             item.EvaluatedInclude
         else
             strPath
+
     static let GetUnescapedUnevaluatedInclude(item : ProjectItemElement) =
         let mutable foundLink = None
         for m in item.Metadata do
@@ -45,17 +50,16 @@ type internal MSBuildUtilities() =
             match foundLink with
             | None -> item.Include
             | Some(link) -> link
-        ProjectCollection.Unescape(escaped)
+        ProjectCollection.Unescape(escaped) |> NormalizePath
 
     static let MattersForOrdering(bi : ProjectItemElement) =
         not (bi.ItemType = ProjectFileConstants.ProjectReference || bi.ItemType = ProjectFileConstants.Reference)
 
     // if 'path' is as in <... Include="path">, determine the relative path of the folder that contains this
     static let ComputeFolder(path : string, projectUrl : Url) =
-        Path.GetDirectoryName(PackageUtilities.MakeRelativeIfRooted(path, projectUrl)) + "\\"
+        Path.GetDirectoryName(PackageUtilities.MakeRelativeIfRooted(path, projectUrl)) + Path.DirectorySeparatorChar.ToString()
 
-    static let FolderComparer = StringComparer.OrdinalIgnoreCase 
-    static let FilenameComparer = StringComparer.OrdinalIgnoreCase 
+    static let FolderComparer = StringComparer.OrdinalIgnoreCase
 
     static let Same(x : ProjectItemElement, y : ProjectItemElement) =
         Object.ReferenceEquals(x,y)
@@ -191,13 +195,7 @@ type internal MSBuildUtilities() =
             match priorGroupWithAtLeastOneItemThatMattersForOrdering with
             | Some(g) -> EnsureProperFolderLogic msbuildProject g projectNode throwIfCannotRender
             | None -> msbuildProject.Xml.AddItemGroup()
-
-    static let CheckItemType(item, buildItemName) =
-        // It checks if this node item has the same BuildActionType as returned by DefaultBuildAction(), which only can see the file name.
-        // Additionally, return true when a node item has "None" as "default" build action to avoid "compile" or "publish". 
-        let itemType = GetItemType(item)
-        itemType = "None" || itemType = buildItemName
-
+            
     static member ThrowIfNotValidAndRearrangeIfNecessary (projectNode : ProjectNode) =
         EnsureValid projectNode.BuildProject projectNode true |> ignore
         
@@ -205,67 +203,40 @@ type internal MSBuildUtilities() =
         // TODO wildcards?
         big.RemoveChild(item)
         big.InsertBeforeChild(item, itemToMoveAbove)
-
-    /// Move <... Include='relativeFileName'> to above nodeToMoveAbove (from solution-explorer point-of-view)
-    static member MoveFileAbove(relativeFileName : string, nodeToMoveAbove : HierarchyNode, projectNode : ProjectNode) =  
-        let msbuildProject = projectNode.BuildProject
-        let buildItemName = projectNode.DefaultBuildAction(relativeFileName)
-        let big = EnsureValid msbuildProject projectNode true
-        let mutable itemToMove = None
-        for bi in EnumerateItems(big) do
-            if CheckItemType(bi, buildItemName) && 0=FilenameComparer.Compare(GetUnescapedUnevaluatedInclude(bi), relativeFileName) then
-                itemToMove <- Some(bi)
-        Debug.Assert(itemToMove.IsSome, "did not find item")
-        let itemToMoveAbove = nodeToMoveAbove.ItemNode.Item 
-        Debug.Assert(itemToMoveAbove <> null, "nodeToMoveAbove was unexpectedly virtual")  // add new/existing item above only works on files, not folders
-        MSBuildUtilities.MoveFileAboveHelper(itemToMove.Value, itemToMoveAbove.Xml, big, projectNode)
-
+        
     static member private MoveFileBelowHelper(item : ProjectItemElement, itemToMoveBelow : ProjectItemElement, big : ProjectItemGroupElement, _projectNode : ProjectNode) =  
         // TODO wildcards?
         big.RemoveChild(item)
         big.InsertAfterChild(item, itemToMoveBelow)
-
-    static member MoveFileBelowCore(relativeFileName : string, itemToMoveBelow : ProjectItemElement, projectNode : ProjectNode, throwIfCannotRender) =  
-        let msbuildProject = projectNode.BuildProject
-        let buildItemName = projectNode.DefaultBuildAction(relativeFileName)
-        let big = EnsureValid msbuildProject projectNode throwIfCannotRender
-        let mutable itemToMove = None
-        for bi in EnumerateItems(big) do
-            if CheckItemType(bi, buildItemName) && 0=FilenameComparer.Compare(GetUnescapedUnevaluatedInclude(bi), relativeFileName) then
-                itemToMove <- Some(bi)
-        Debug.Assert(itemToMove.IsSome, "did not find item")
-        Debug.Assert(itemToMoveBelow <> null, "nodeToMoveBelow was unexpectedly virtual")  // add new/existing item below only works on files, not folders
-        MSBuildUtilities.MoveFileBelowHelper(itemToMove.Value, itemToMoveBelow, big, projectNode)
-
-    /// Move <... Include='relativeFileName'> to below nodeToMoveBelow (from solution-explorer point-of-view)
-    static member MoveFileBelow(relativeFileName : string, nodeToMoveBelow : HierarchyNode, projectNode : ProjectNode) =  
-        let itemToMoveBelow = nodeToMoveBelow.ItemNode.Item 
-        MSBuildUtilities.MoveFileBelowCore(relativeFileName, itemToMoveBelow.Xml, projectNode, true)
-
-    /// Move <... Include='relativeFileName'> to the bottom of the list of items, except if this item has a subfolder that already exists, move it
-    /// to the bottom of that subforlder, rather than the very bottom.
-    static member MoveFileToBottomOfGroup(relativeFileName : string, projectNode : ProjectNode) =  
-        let dir = Path.GetDirectoryName(relativeFileName) + "\\"
-        let mutable lastItemInDir = null
+        
+    /// Move a file node to its correct place in the build project.
+    /// Its correct place is defined by where it sits in the hierarchy relative
+    /// to other files.
+    static member SyncWithHierarchy(fileNode : FileNode) =
+        let projectNode = fileNode.ProjectMgr
         let msbuildProject = projectNode.BuildProject
-        let buildItemName = projectNode.DefaultBuildAction(relativeFileName)
         let big = EnsureValid msbuildProject projectNode false
-        let mutable itemToMove = None
-        for bi in EnumerateItems(big) do
-            if CheckItemType(bi, buildItemName) && 0=FilenameComparer.Compare(GetUnescapedUnevaluatedInclude(bi), relativeFileName) then
-                itemToMove <- Some(bi)
-            else
-                // under else, as we don't want to try to move under _ourself_, only under _another_ existing item in same dir
-                if GetUnescapedUnevaluatedInclude(bi).StartsWith(dir, System.StringComparison.OrdinalIgnoreCase) then
-                    lastItemInDir <- bi
-        Debug.Assert(itemToMove.IsSome, "did not find item")
-        if lastItemInDir <> null then
-            MSBuildUtilities.MoveFileBelowCore(relativeFileName, lastItemInDir, projectNode, false)
-        else
-            big.RemoveChild(itemToMove.Value)
-            big.AppendChild(itemToMove.Value)
-
-
+        
+        let itemToMove = fileNode.ItemNode.Item.Xml
+        big.RemoveChild itemToMove
+            
+        let precedingFile =
+            projectNode.AllDescendants
+            |> Seq.choose (function :? FileNode as fileNode -> Some fileNode | _ -> None)
+            |> Seq.takeWhile ((<>) fileNode)
+            |> Seq.tryLast
+            
+        match precedingFile with
+        | None ->
+            // if there is no preceding file, it must be because we're
+            // the first file in the hierarchy - so put us first
+            big.PrependChild itemToMove
+        | Some precedingFile ->
+            big.InsertAfterChild (itemToMove, precedingFile.ItemNode.Item.Xml)
+        
+        // The project file should now be in a valid state
+        MSBuildUtilities.ThrowIfNotValidAndRearrangeIfNecessary projectNode
+        
     /// Given a HierarchyNode, compute the last BuildItem if we want to move something after it
     static member private FindLast(toMoveAfter : HierarchyNode, projectNode : ProjectNode) =
         match toMoveAfter with
@@ -334,8 +305,6 @@ type internal MSBuildUtilities() =
                 index := !index + 1]
         if !itemToMoveBeforeIndex = -1 then
             Debug.Assert(false, sprintf "did not find item to move before <%s Include=\"%s\">" (GetItemType itemToMoveBefore) (GetUnescapedUnevaluatedInclude itemToMoveBefore))
-        if itemsToMove.IsEmpty then
-            Debug.Assert(false, sprintf "did not find any item to move (anything in folder %s)" folderToBeMoved)
         for (item,i) in itemsToMove do
             Debug.Assert(i <> 0, "item is already at top")
             Debug.Assert(!itemToMoveBeforeIndex < i, "not moving up")
@@ -363,8 +332,6 @@ type internal MSBuildUtilities() =
                 index := !index + 1]
         if !itemToMoveAfterIndex = -1 then
             Debug.Assert(false, sprintf "did not find item to move after <%s Include=\"%s\">" (GetItemType itemToMoveAfter) (GetUnescapedUnevaluatedInclude itemToMoveAfter))
-        if itemsToMove.IsEmpty then
-            Debug.Assert(false, sprintf "did not find any item to move (anything in folder %s)" folderToBeMoved)
         for (item,i) in List.rev itemsToMove do
             Debug.Assert(i <> !index - 1, "item is already at bottom")
             Debug.Assert(!itemToMoveAfterIndex > i, "not moving down")

vsintegration/src/FSharp.ProjectSystem.FSharp/MenusAndCommands.vsct

@@ -4,7 +4,8 @@
   <Extern href="stdidcmd.h"/>
   <Extern href="vsshlids.h"/>
   <Include href="..\FSharp.VS.FSI\fsiCommands.vsct"/>    <!-- FSI-LINKAGE-POINT -->
-  
+  <Include href="KnownImageIds.vsct"/>
+
   <!-- Stake a claime to existing commands so they aren't marked as unused and removed. -->
   <UsedCommands>
     <UsedCommand guid="guidVSStd2K" id="ECMD_ADDREFERENCE" />
@@ -179,6 +180,28 @@
           <CommandName>Add Existing Item</CommandName>
         </Strings>
       </Button>
+      <Button guid="FSharpProjectCmdSet" id="NewFolderAbove" priority="0x200" type="Button">
+        <Parent guid ="FSharpProjectCmdSet" id ="FSharpGroupAddAbove"/>
+        <Icon guid="ImageCatalogGuid" id="NewFolder"/>
+        <CommandFlag>DynamicVisibility</CommandFlag>
+        <CommandFlag>DefaultInvisible</CommandFlag>
+        <CommandFlag>TextChanges</CommandFlag>
+        <CommandFlag>IconIsMoniker</CommandFlag>
+        <Strings>
+          <ButtonText>New Fol&amp;der</ButtonText>
+        </Strings>
+      </Button>
+      <Button guid="FSharpProjectCmdSet" id="NewFolderBelow" priority="0x200" type="Button">
+        <Parent guid ="FSharpProjectCmdSet" id ="FSharpGroupAddBelow"/>
+        <Icon guid="ImageCatalogGuid" id="NewFolder"/>
+        <CommandFlag>DynamicVisibility</CommandFlag>
+        <CommandFlag>DefaultInvisible</CommandFlag>
+        <CommandFlag>TextChanges</CommandFlag>
+        <CommandFlag>IconIsMoniker</CommandFlag>
+        <Strings>
+          <ButtonText>New Fol&amp;der</ButtonText>
+        </Strings>
+      </Button>
     </Buttons>
     <Bitmaps>
       <Bitmap guid="FSharpMoveUpBmp" href="Resources\MoveUp.bmp" usedList="moveUp"/>
@@ -204,6 +227,8 @@
       <IDSymbol name ="AddNewItemAbove" value ="0x3005"/>
       <IDSymbol name ="AddExistingItemAbove" value ="0x3006"/>
       <IDSymbol name ="MoveDownCmd" value ="0x3007"/>
+      <IDSymbol name ="NewFolderAbove" value ="0x3008"/>
+      <IDSymbol name ="NewFolderBelow" value ="0x3009"/>
 
       <IDSymbol name = "FSharpSendThisReferenceToInteractiveGroup" value = "0x5001"/>
       <IDSymbol name = "FSharpSendReferencesToInteractiveGroup" value = "0x5002"/>

vsintegration/src/FSharp.ProjectSystem.FSharp/ProjectPrelude.fs

@@ -221,7 +221,14 @@ namespace Microsoft.VisualStudio.FSharp.ProjectSystem
         let ComputingSourcesAndFlags = "ComputingSourcesAndFlags"
         [<Literal>]
         let UpdatingSolutionConfiguration = "UpdatingSolutionConfiguration"
-
+        [<Literal>]
+        let FileCannotBePlacedBodyAbove = "FileCannotBePlacedBodyAbove"
+        [<Literal>]
+        let FileCannotBePlacedBodyBelow = "FileCannotBePlacedBodyBelow"
+        [<Literal>]
+        let FileCannotBePlacedDifferentSubtree = "FileCannotBePlacedDifferentSubtree"
+        [<Literal>]
+        let FileCannotBePlacedMultipleFiles = "FileCannotBePlacedMultipleFiles"
 
         type private TypeInThisAssembly = class end
         let thisAssembly = typeof<TypeInThisAssembly>.Assembly 

vsintegration/src/FSharp.ProjectSystem.FSharp/VSPackage.resx

@@ -530,4 +530,16 @@
   <data name="UpdatingSolutionConfiguration" xml:space="preserve">
     <value>Updating solution configuration...</value>
   </data>
+  <data name="FileCannotBePlacedBodyAbove" xml:space="preserve">
+    <value>The file '{0}' cannot be placed above '{1}' in the Solution Explorer.\n\n{2}.</value>
+  </data>
+  <data name="FileCannotBePlacedBodyBelow" xml:space="preserve">
+    <value>The file '{0}' cannot be placed below '{1}' in the Solution Explorer.\n\n{2}.</value>
+  </data>
+  <data name="FileCannotBePlacedDifferentSubtree" xml:space="preserve">
+    <value>The file is in a different subtree</value>
+  </data>
+  <data name="FileCannotBePlacedMultipleFiles" xml:space="preserve">
+    <value>The '{0}' folder cannot be moved as it already exists in the Solution Explorer in a different location</value>
+  </data>
 </root>
\ No newline at end of file

vsintegration/tests/unittests/Tests.ProjectSystem.Miscellaneous.fs

@@ -88,14 +88,14 @@ type Miscellaneous() =
             )
             
     [<Test>]
-    member public this.``Miscellaneous.FSharpFileNode.GetRelativePath`` () =
+    member public this.``Miscellaneous.FSharpFileNode.RelativeFilePath`` () =
         this.MakeProjectAndDo(["orig1.fs"], [], "", (fun project ->
             let absFilePath = Path.Combine(project.ProjectFolder, "orig1.fs")
             let files = new List<FSharpFileNode>()
             project.FindNodesOfType(files)
             Assert.AreEqual(1, files.Count)
             let file = files.[0]
-            let path = file.GetRelativePath()
+            let path = file.RelativeFilePath
             Assert.AreEqual("orig1.fs", path)
             ))
            

vsintegration/tests/unittests/Tests.ProjectSystem.Project.fs

@@ -47,7 +47,7 @@ type Project() =
         l.[0]     
 
     [<Test>]    
-    member public this.NoNewFolderOnProjectMenu() =
+    member public this.NewFolderOnProjectMenu() =
         printfn "starting..."
         let package = new FSharpProjectPackage()
         let project = new FSharpProjectNode(package)
@@ -59,8 +59,8 @@ type Project() =
         let x = project.QueryStatusOnNode(guidCmdGroup, uint32(cmdEnum), pCmdText, &result)
         printfn "and..."
         AssertEqual x VSConstants.S_OK
-        if (result &&& QueryStatusResult.INVISIBLE) = enum 0 then
-            Assert.Fail("Unexpected: New Folder was not invisible")
+        if (result &&& QueryStatusResult.ENABLED) <> QueryStatusResult.ENABLED then
+            Assert.Fail("Unexpected: New Folder was not enabled")
         ()    
 
     [<Test>]

vsintegration/tests/unittests/Tests.ProjectSystem.ProjectItems.fs

@@ -75,26 +75,6 @@ type ProjectItems() =
                 File.Delete(absFilePath)
             ))
     
-    [<Test>]
-    member public this.``AddNewItem.ItemAppearsAtBottomOfFsprojFileEvenIfUnknownItemWithSameName``() =
-        this.MakeProjectAndDo([], [], @"
-                <ItemGroup>
-                    <Unknown Include=""a.fs"" />
-                    <Compile Include=""orig.fs"" />
-                </ItemGroup>
-            ", (fun project ->
-            let absFilePath = Path.Combine(project.ProjectFolder, "a.fs")
-            try
-                File.AppendAllText(absFilePath, "#light")
-                // Note: this is not the same code path as the UI, but it is close
-                project.MoveNewlyAddedFileToBottomOfGroup (fun () ->
-                    project.AddNewFileNodeToHierarchy(project,absFilePath) |> ignore)
-                let msbuildInfo = TheTests.MsBuildCompileItems(project.BuildProject)
-                AssertEqual ["orig.fs"; "a.fs"] msbuildInfo
-            finally
-                File.Delete(absFilePath)
-            ))
-    
     [<Test>]
     member public this.``AddNewItemBelow.ItemAppearsInRightSpot``() =
         this.MakeProjectAndDo(["orig1.fs"; "orig2.fs"], [], "", (fun project ->