Fixes for project file removal support

src/OmniSharp.Abstractions/FileWatching/IFileSystemWatcher.cs

@@ -6,6 +6,8 @@ namespace OmniSharp.FileWatching
 
     public interface IFileSystemWatcher
     {
+        void Unwatch(string pathOrExtension);
+
         /// <summary>
         /// Call to watch a file or directory path for changes.
         /// </summary>

src/OmniSharp.Host/FileWatching/ManualFileSystemWatcher.cs

@@ -5,7 +5,7 @@
 
 namespace OmniSharp.FileWatching
 {
-    internal partial class ManualFileSystemWatcher : IFileSystemWatcher, IFileSystemNotifier
+    internal class ManualFileSystemWatcher : IFileSystemWatcher, IFileSystemNotifier
     {
         private readonly object _gate = new object();
         private readonly Dictionary<string, Callbacks> _callbacksMap;
@@ -73,5 +73,43 @@ public void Watch(string pathOrExtension, FileSystemNotificationCallback callbac
                 callbacks.Add(callback);
             }
         }
+
+        public void Unwatch(string pathOrExtension)
+        {
+            if (pathOrExtension == null)
+            {
+                throw new ArgumentNullException(nameof(pathOrExtension));
+            }
+
+            lock (_gate)
+            {
+                if (_callbacksMap.ContainsKey(pathOrExtension))
+                {
+                    _callbacksMap.Remove(pathOrExtension);
+                }
+            }
+        }
+
+        private class Callbacks
+        {
+            private readonly List<FileSystemNotificationCallback> _callbacks = new List<FileSystemNotificationCallback>();
+            private readonly HashSet<FileSystemNotificationCallback> _callbackSet = new HashSet<FileSystemNotificationCallback>();
+
+            public void Add(FileSystemNotificationCallback callback)
+            {
+                if (_callbackSet.Add(callback))
+                {
+                    _callbacks.Add(callback);
+                }
+            }
+
+            public void Invoke(string filePath, FileChangeType changeType)
+            {
+                foreach (var callback in _callbacks)
+                {
+                    callback(filePath, changeType);
+                }
+            }
+        }
     }
 }

src/OmniSharp.MSBuild/ProjectManager.cs

@@ -10,21 +10,17 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.Extensions.Logging;
-using Newtonsoft.Json.Linq;
 using OmniSharp.Eventing;
 using OmniSharp.FileWatching;
 using OmniSharp.Models.UpdateBuffer;
 using OmniSharp.MSBuild.Logging;
 using OmniSharp.MSBuild.Models.Events;
 using OmniSharp.MSBuild.Notification;
 using OmniSharp.MSBuild.ProjectFile;
-using OmniSharp.Roslyn.CSharp.Services.Diagnostics;
-using OmniSharp.Roslyn.CSharp.Services.Refactoring.V2;
 using OmniSharp.Options;
 using OmniSharp.Roslyn.Utilities;
 using OmniSharp.Services;
 using OmniSharp.Utilities;
-using System.Reflection;
 using Microsoft.CodeAnalysis.Diagnostics;
 using OmniSharp.Roslyn.EditorConfig;
 
@@ -37,11 +33,13 @@ private class ProjectToUpdate
             public ProjectIdInfo ProjectIdInfo;
             public string FilePath { get; }
             public bool AllowAutoRestore { get; set; }
+            public FileChangeType ChangeType { get; }
             public ProjectLoadedEventArgs LoadedEventArgs { get; set; }
 
-            public ProjectToUpdate(string filePath, bool allowAutoRestore, ProjectIdInfo projectIdInfo)
+            public ProjectToUpdate(string filePath, bool allowAutoRestore, ProjectIdInfo projectIdInfo, FileChangeType changeType)
             {
                 ProjectIdInfo = projectIdInfo ?? throw new ArgumentNullException(nameof(projectIdInfo));
+                ChangeType = changeType;
                 FilePath = filePath ?? throw new ArgumentNullException(nameof(filePath));
                 AllowAutoRestore = allowAutoRestore;
             }
@@ -123,7 +121,7 @@ private async Task WaitForProjectModelReadyAsync(string documentPath)
                         if (_projectsRequestedOnDemand.TryAdd(csProjFile, 0 /*unused*/))
                         {
                             var projectIdInfo = new ProjectIdInfo(ProjectId.CreateNewId(csProjFile), false);
-                            QueueProjectUpdate(csProjFile, allowAutoRestore: true, projectId: projectIdInfo);
+                            QueueProjectUpdate(csProjFile, allowAutoRestore: true, projectId: projectIdInfo, FileChangeType.Unspecified);
                         }
                     }
 
@@ -154,10 +152,10 @@ protected override void DisposeCore(bool disposing)
         public IEnumerable<ProjectFileInfo> GetAllProjects() => _projectFiles.GetItems();
         public bool TryGetProject(string projectFilePath, out ProjectFileInfo projectFileInfo) => _projectFiles.TryGetValue(projectFilePath, out projectFileInfo);
 
-        public void QueueProjectUpdate(string projectFilePath, bool allowAutoRestore, ProjectIdInfo projectId)
+        public void QueueProjectUpdate(string projectFilePath, bool allowAutoRestore, ProjectIdInfo projectId, FileChangeType changeType = FileChangeType.Unspecified)
         {
             _logger.LogInformation($"Queue project update for '{projectFilePath}'");
-            _queue.Post(new ProjectToUpdate(projectFilePath, allowAutoRestore, projectId));
+            _queue.Post(new ProjectToUpdate(projectFilePath, allowAutoRestore, projectId, changeType));
         }
 
         public async Task WaitForQueueEmptyAsync()
@@ -189,8 +187,8 @@ private void ProcessQueue(CancellationToken cancellationToken)
             _processingQueue = true;
             try
             {
-                Dictionary<string, ProjectToUpdate> projectByFilePathMap = null;
-                List<ProjectToUpdate> projectList = null;
+                var projectByFilePathMap = new Dictionary<string, ProjectToUpdate>(StringComparer.OrdinalIgnoreCase);
+                var projectList = new List<ProjectToUpdate>();
 
                 while (_queue.TryReceive(out var currentProject))
                 {
@@ -199,12 +197,6 @@ private void ProcessQueue(CancellationToken cancellationToken)
                         break;
                     }
 
-                    if (projectByFilePathMap == null)
-                    {
-                        projectByFilePathMap = new Dictionary<string, ProjectToUpdate>(StringComparer.OrdinalIgnoreCase);
-                        projectList = new List<ProjectToUpdate>();
-                    }
-
                     // Ensure that we don't process the same project twice. However, if a project *does*
                     // appear more than once in the update queue, ensure that AllowAutoRestore is set to true
                     // if any of the updates requires it.
@@ -218,7 +210,11 @@ private void ProcessQueue(CancellationToken cancellationToken)
                         continue;
                     }
 
-                    // TODO: Handle removing project
+                    if(currentProject.ChangeType == FileChangeType.Delete)
+                    {
+                        RemoveProject(currentProject.FilePath);
+                        continue;
+                    }
 
                     projectByFilePathMap.Add(currentProject.FilePath, currentProject);
                     projectList.Add(currentProject);
@@ -254,35 +250,32 @@ private void ProcessQueue(CancellationToken cancellationToken)
                     }
                 }
 
-                if (projectByFilePathMap != null)
+                foreach (var project in projectList)
                 {
-                    foreach (var project in projectList)
-                    {
-                        UpdateProject(project.FilePath);
+                    UpdateProject(project.FilePath);
 
-                        // Fire loaded events
-                        if (project.LoadedEventArgs != null)
+                    // Fire loaded events
+                    if (project.LoadedEventArgs != null)
+                    {
+                        foreach (var eventSink in _eventSinks)
                         {
-                            foreach (var eventSink in _eventSinks)
+                            try
+                            {
+                                eventSink.ProjectLoaded(project.LoadedEventArgs);
+                            }
+                            catch (Exception ex)
                             {
-                                try
-                                {
-                                    eventSink.ProjectLoaded(project.LoadedEventArgs);
-                                }
-                                catch (Exception ex)
-                                {
-                                    _logger.LogError(ex, "Exception thrown while calling event sinks");
-                                }
+                                _logger.LogError(ex, "Exception thrown while calling event sinks");
                             }
                         }
                     }
+                }
 
-                    foreach (var project in projectList)
+                foreach (var project in projectList)
+                {
+                    if (_projectFiles.TryGetValue(project.FilePath, out var projectFileInfo))
                     {
-                        if (_projectFiles.TryGetValue(project.FilePath, out var projectFileInfo))
-                        {
-                            _packageDependencyChecker.CheckForUnresolvedDependences(projectFileInfo, project.AllowAutoRestore);
-                        }
+                        _packageDependencyChecker.CheckForUnresolvedDependences(projectFileInfo, project.AllowAutoRestore);
                     }
                 }
             }
@@ -344,9 +337,10 @@ private bool RemoveProject(string projectFilePath)
             if (!_workspace.TryApplyChanges(newSolution))
             {
                 _logger.LogError($"Failed to remove project from workspace: '{projectFileInfo.FilePath}'");
+                return false;
             }
 
-            // TODO: Stop watching project files
+            UnwatchProjectFiles(projectFileInfo);
 
             return true;
         }
@@ -371,11 +365,9 @@ private void AddProject(ProjectFileInfo projectFileInfo)
 
         private void WatchProjectFiles(ProjectFileInfo projectFileInfo)
         {
-            // TODO: This needs some improvement. Currently, it tracks both deletions and changes
-            // as "updates". We should properly remove projects that are deleted.
             _fileSystemWatcher.Watch(projectFileInfo.FilePath, (file, changeType) =>
             {
-                QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo);
+                QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo, changeType);
             });
 
             if (_workspace.EditorConfigEnabled)
@@ -406,38 +398,50 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)
             {
                 _fileSystemWatcher.Watch(projectFileInfo.RuleSet.FilePath, (file, changeType) =>
                 {
-                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
+                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, changeType);
                 });
             }
 
-            if (!string.IsNullOrEmpty(projectFileInfo.ProjectAssetsFile))
+            foreach(var projectAssetFile in GetProjectAssetFilePaths(projectFileInfo))
             {
-                _fileSystemWatcher.Watch(projectFileInfo.ProjectAssetsFile, (file, changeType) =>
+                _fileSystemWatcher.Watch(projectAssetFile, (file, changeType) =>
                 {
-                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
+                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, changeType);
                 });
+            }
+        }
 
-                var restoreDirectory = Path.GetDirectoryName(projectFileInfo.ProjectAssetsFile);
-                var nugetFileBase = Path.Combine(restoreDirectory, Path.GetFileName(projectFileInfo.FilePath) + ".nuget");
-                var nugetCacheFile = nugetFileBase + ".cache";
-                var nugetPropsFile = nugetFileBase + ".g.props";
-                var nugetTargetsFile = nugetFileBase + ".g.targets";
+        private void UnwatchProjectFiles(ProjectFileInfo projectFileInfo)
+        {
+            _fileSystemWatcher.Unwatch(projectFileInfo.FilePath);
 
-                _fileSystemWatcher.Watch(nugetCacheFile, (file, changeType) =>
-                {
-                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
-                });
+            if (projectFileInfo.RuleSet?.FilePath != null)
+            {
+                _fileSystemWatcher.Unwatch(projectFileInfo.RuleSet.FilePath);
+            }
 
-                _fileSystemWatcher.Watch(nugetPropsFile, (file, changeType) =>
-                {
-                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
-                });
+            foreach(var projectAssetFile in GetProjectAssetFilePaths(projectFileInfo))
+            {
+                _fileSystemWatcher.Unwatch(projectAssetFile);
+            }
+        }
 
-                _fileSystemWatcher.Watch(nugetTargetsFile, (file, changeType) =>
-                {
-                    QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
-                });
+        private static IEnumerable<string> GetProjectAssetFilePaths(ProjectFileInfo projectFileInfo)
+        {
+            if (string.IsNullOrEmpty(projectFileInfo.ProjectAssetsFile))
+            {
+                return new string[] {};
             }
+
+            var restoreDirectory = Path.GetDirectoryName(projectFileInfo.ProjectAssetsFile);
+            var nugetFileBase = Path.Combine(restoreDirectory, Path.GetFileName(projectFileInfo.FilePath) + ".nuget");
+            return new[]
+            {
+                projectFileInfo.ProjectAssetsFile,
+                nugetFileBase + ".cache",
+                nugetFileBase + ".g.props",
+                nugetFileBase + ".g.targets"
+            };
         }
 
         private void UpdateProject(string projectFilePath)
@@ -648,7 +652,7 @@ private void UpdateProjectReferences(Project project, ImmutableArray<string> pro
                         referencedProject = ProjectFileInfo.CreateNoBuild(projectReferencePath, _projectLoader);
                         AddProject(referencedProject);
 
-                        QueueProjectUpdate(projectReferencePath, allowAutoRestore: true, referencedProject.ProjectIdInfo);
+                        QueueProjectUpdate(projectReferencePath, allowAutoRestore: true, referencedProject.ProjectIdInfo, FileChangeType.Unspecified);
                     }
                 }
 

src/OmniSharp.MSBuild/ProjectSystem.cs

@@ -86,7 +86,7 @@ internal class ProjectSystem : IProjectSystem
         public void Initalize(IConfiguration configuration)
         {
             if (Initialized) return;
-            
+
             _options = new MSBuildOptions();
             ConfigurationBinder.Bind(configuration, _options);