Merge pull request #1903 from paulvanbrenk/fileWatcherUpdate

File watcher update

Author: paulvanbrenk

Nodejs/Product/Nodejs/Nodejs.csproj

@@ -84,6 +84,9 @@
     <Reference Include="envdte80, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
       <EmbedInteropTypes>False</EmbedInteropTypes>
     </Reference>
+    <Reference Include="Microsoft.VisualStudio.Shell.Interop.15.7.DesignTime, Version=15.7.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <EmbedInteropTypes>True</EmbedInteropTypes>
+    </Reference>
     <Reference Include="microsoft.visualstudio.textmanager.interop.11.0, Version=11.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
       <EmbedInteropTypes>True</EmbedInteropTypes>
     </Reference>

Nodejs/Product/Nodejs/SharedProject/AssemblyReferenceNode.cs

@@ -400,7 +400,7 @@ protected virtual void OnAssemblyReferenceChangedOnDisk(object sender, FileChang
             }
 
             // We only care about file deletes, so check for one before enumerating references.
-            if ((e.FileChangeFlag & _VSFILECHANGEFLAGS.VSFILECHG_Del) == 0)
+            if (e.FileChange != WatcherChangeTypes.Deleted)
             {
                 return;
             }

Nodejs/Product/Nodejs/SharedProject/CommonProjectNode.cs

@@ -80,7 +80,7 @@ public CommonProjectNode(IServiceProvider serviceProvider, ImageList imageList)
             this.idleManager.OnIdle += this.OnIdle;
 
             this.fileWatcher = new FileChangeManager(serviceProvider);
-            this.fileWatcher.FolderChangedOnDisk += this.FolderChangedOnDisk;
+            this.fileWatcher.FileChangedOnDisk += this.FileChangedOnDisk;
         }
 
         public override int QueryService(ref Guid guidService, out object result)
@@ -462,7 +462,7 @@ protected override void Dispose(bool disposing)
                 this.idleManager.OnIdle -= this.OnIdle;
                 this.idleManager.Dispose();
 
-                this.fileWatcher.FolderChangedOnDisk -= this.FolderChangedOnDisk;
+                this.fileWatcher.FileChangedOnDisk -= this.FileChangedOnDisk;
                 this.fileWatcher.Dispose();
             }
 
@@ -728,17 +728,10 @@ protected override ReferenceContainerNode CreateReferenceContainerNode()
             return new CommonReferenceContainerNode(this);
         }
 
-        private void FolderChangedOnDisk(object sender, FolderChangedEventArgs e)
+        private void FileChangedOnDisk(object sender, FileChangedOnDiskEventArgs e)
         {
             var file = e.FileName;
-            if (File.Exists(file) || Directory.Exists(file))
-            {
-                this.QueueFileSystemChanges(new FileSystemChange(this, WatcherChangeTypes.Changed, file));
-            }
-            else
-            {
-                this.QueueFileSystemChanges(new FileSystemChange(this, WatcherChangeTypes.Deleted, file));
-            }
+            this.QueueFileSystemChanges(new FileSystemChange(this, e.FileChange, file));
         }
 
         private void QueueFileSystemChanges(params FileSystemChange[] changes)

Nodejs/Product/Nodejs/SharedProject/FileChangeManager.cs

@@ -9,26 +9,12 @@
 
 namespace Microsoft.VisualStudioTools.Project
 {
-    public sealed class FolderChangedEventArgs : EventArgs
-    {
-        public readonly string FolderName;
-        public readonly string FileName;
-        public _VSFILECHANGEFLAGS FileChangeFlag;
-
-        public FolderChangedEventArgs(string folderName, string fileName, _VSFILECHANGEFLAGS fileChangeFlag)
-        {
-            this.FolderName = folderName;
-            this.FileName = fileName;
-            this.FileChangeFlag = fileChangeFlag;
-        }
-    }
-
     /// <summary>
     /// This object is in charge of watching for changes to files and folders.
     /// </summary>
     internal sealed class FileChangeManager
     {
-        private sealed class FileChangeEvents : IVsFreeThreadedFileChangeEvents
+        private sealed class FileChangeEvents : IVsFreeThreadedFileChangeEvents2
         {
             private readonly FileChangeManager fileChangeManager;
 
@@ -56,58 +42,65 @@ public int FilesChanged(uint numberOfFilesChanged, string[] filesChanged, uint[]
                     throw new ArgumentNullException(nameof(flags));
                 }
 
-                for (var i = 0; i < numberOfFilesChanged; i++)
-                {
-                    var fullFileName = Utilities.CanonicalizeFileName(filesChanged[i]);
-                    if (this.fileChangeManager.observedFiles.TryGetValue(fullFileName, out var value))
-                    {
-                        var (ItemID, FileChangeCookie) = value;
-                        this.fileChangeManager.FileChangedOnDisk?.Invoke(this, new FileChangedOnDiskEventArgs(fullFileName, ItemID, (_VSFILECHANGEFLAGS)flags[i]));
-                    }
-                }
+                Debug.Assert(numberOfFilesChanged == filesChanged.Length && numberOfFilesChanged == flags.Length, "number of files changed doesn't match actual files reported.");
+
+                ProcessFileChanges(filesChanged, flags);
 
                 return VSConstants.S_OK;
             }
 
-            /// <summary>
-            /// Notifies clients of changes made to a directory. 
-            /// </summary>
-            /// <param name="directory">Name of the directory that had a change.</param>
-            /// <returns>If the method succeeds, it returns S_OK. If it fails, it returns an error code. </returns>
             public int DirectoryChanged(string directory)
             {
-                return VSConstants.S_OK;
+                // not called since we implement DirectoryChangedEx2
+                return VSConstants.E_NOTIMPL;
             }
 
             public int DirectoryChangedEx(string pszDirectory, string pszFile)
             {
-                if (pszDirectory == null)
+                // not called since we implement DirectoryChangedEx2
+                return VSConstants.E_NOTIMPL;
+            }
+
+            /// <summary>
+            /// Notifies clients of changes made to a directory. 
+            /// </summary>
+            /// <param name="directory">Name of the directory that had a change.</param>
+            /// <param name="numberOfFilesChanged">Number of files changed.</param>
+            /// <param name="filesChanged">Array of file names.</param>
+            /// <param name="flags">Array of flags indicating the type of changes. See _VSFILECHANGEFLAGS.</param>
+            public int DirectoryChangedEx2(string directory, uint numberOfFilesChanged, string[] filesChanged, uint[] flags)
+            {
+                if (filesChanged == null)
                 {
-                    throw new ArgumentNullException(nameof(pszDirectory));
+                    throw new ArgumentNullException(nameof(filesChanged));
                 }
 
-                if (pszFile == null)
+                if (flags == null)
                 {
-                    throw new ArgumentNullException(nameof(pszFile));
+                    throw new ArgumentNullException(nameof(flags));
                 }
 
-                this.fileChangeManager.FolderChangedOnDisk?.Invoke(this, new FolderChangedEventArgs(pszDirectory, pszFile,
-                    (_VSFILECHANGEFLAGS)0 /* default for now, until VS implements API that returns actual change */));
+                Debug.Assert(numberOfFilesChanged == filesChanged.Length && numberOfFilesChanged == flags.Length, "number of files changed doesn't match actual files reported.");
+
+                ProcessFileChanges(filesChanged, flags);
 
                 return VSConstants.S_OK;
             }
+
+            private void ProcessFileChanges(string[] filesChanged, uint[] flags)
+            {
+                for (var i = 0; i < filesChanged.Length; i++)
+                {
+                    this.fileChangeManager.FileChangedOnDisk?.Invoke(this, new FileChangedOnDiskEventArgs(filesChanged[i], (_VSFILECHANGEFLAGS)flags[i]));
+                }
+            }
         }
 
         /// <summary>
         /// Event that is raised when one of the observed files have changed on disk.
         /// </summary>
         public event EventHandler<FileChangedOnDiskEventArgs> FileChangedOnDisk;
 
-        /// <summary>
-        /// Event that is raised when one of the observed folders have changed on disk.
-        /// </summary>
-        public event EventHandler<FolderChangedEventArgs> FolderChangedOnDisk;
-
         /// <summary>
         /// Reference to the FileChange service.
         /// </summary>
@@ -117,7 +110,7 @@ public int DirectoryChangedEx(string pszDirectory, string pszFile)
         /// Maps between the observed file identified by its filename (in canonicalized form) and the cookie used for subscribing 
         /// to the events.
         /// </summary>
-        private readonly ConcurrentDictionary<string, (uint ItemID, uint FileChangeCookie)> observedFiles = new ConcurrentDictionary<string, (uint, uint)>();
+        private readonly ConcurrentDictionary<string, uint> observedFiles = new ConcurrentDictionary<string, uint>();
 
         /// <summary>
         /// Maps between the observer folder identified by its foldername (in canonicalized form) and the cookie used for subscribing 
@@ -171,9 +164,9 @@ public void Dispose()
             this.disposed = true;
 
             // Unsubscribe from the observed source files.
-            foreach (var (ItemID, FileChangeCookie) in this.observedFiles.Values)
+            foreach (var fileChangeCookie in this.observedFiles.Values)
             {
-                var hr = this.fileChangeService.UnadviseFileChange(FileChangeCookie);
+                var hr = this.fileChangeService.UnadviseFileChange(fileChangeCookie);
                 // don't want to crash VS during cleanup
                 Debug.Assert(ErrorHandler.Succeeded(hr), "UnadviseFileChange failed");
                 if (ErrorHandler.Failed(hr)) { break; }
@@ -195,23 +188,12 @@ public void Dispose()
             this.observedFolders.Clear();
         }
 
-        /// <summary>
-        /// Observe when the given file is updated on disk. In this case we do not care about the item id that represents the file in the hierarchy.
-        /// </summary>
-        /// <param name="fileName">File to observe.</param>
-        public void ObserveFile(string fileName)
-        {
-            this.CheckDisposed();
-
-            this.ObserveFile(fileName, VSConstants.VSITEMID_NIL);
-        }
-
         /// <summary>
         /// Observe when the given file is updated on disk.
         /// </summary>
         /// <param name="fileName">File to observe.</param>
         /// <param name="id">The item id of the item to observe.</param>
-        public void ObserveFile(string fileName, uint id)
+        public void ObserveFile(string fileName)
         {
             this.CheckDisposed();
 
@@ -227,7 +209,7 @@ public void ObserveFile(string fileName, uint id)
                 ErrorHandler.ThrowOnFailure(this.fileChangeService.AdviseFileChange(fullFileName, (uint)(_VSFILECHANGEFLAGS.VSFILECHG_Time | _VSFILECHANGEFLAGS.VSFILECHG_Del), this.fileChangeEvents, out var fileChangeCookie));
 
                 // Remember that we're observing this file (used in FilesChanged event handler)
-                this.observedFiles.TryAdd(fullFileName, (id, fileChangeCookie));
+                this.observedFiles.TryAdd(fullFileName, fileChangeCookie);
             }
         }
 
@@ -291,13 +273,10 @@ public bool StopObservingFile(string fileName)
             // Remove the file from our observed list. It's important that this is done before the call to 
             // UnadviseFileChange, because for some reason, the call to UnadviseFileChange can trigger a 
             // FilesChanged event, and we want to be able to filter that event away.
-            if (this.observedFiles.TryRemove(fullFileName, out var value))
+            if (this.observedFiles.TryRemove(fullFileName, out var fileChangeCookie))
             {
-                // Get the cookie that was used for this.observedItems to this file.
-                var (ItemID, FileChangeCookie) = value;
-
                 // Stop observing the file
-                return ErrorHandler.Succeeded(this.fileChangeService.UnadviseFileChange(FileChangeCookie));
+                return ErrorHandler.Succeeded(this.fileChangeService.UnadviseFileChange(fileChangeCookie));
             }
             return false;
         }

Nodejs/Product/Nodejs/SharedProject/StructuresEnums.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Diagnostics;
+using System.IO;
 using System.Runtime.InteropServices;
 using Microsoft.VisualStudio.Shell.Interop;
 
@@ -296,26 +298,39 @@ public sealed class FileChangedOnDiskEventArgs : EventArgs
         /// </summary>
         public readonly string FileName;
 
-        /// <summary>
-        /// The item ide of the file that has changed.
-        /// </summary>
-        public readonly uint ItemID;
-
         /// <summary>
         /// The reason the file has changed on disk.
         /// </summary>
-        public readonly _VSFILECHANGEFLAGS FileChangeFlag;
+        public readonly WatcherChangeTypes FileChange;
 
         /// <summary>
         /// Constructs a new event args.
         /// </summary>
         /// <param name="fileName">File name that was changed on disk.</param>
         /// <param name="id">The item id of the file that was changed on disk.</param>
-        internal FileChangedOnDiskEventArgs(string fileName, uint id, _VSFILECHANGEFLAGS flag)
+        internal FileChangedOnDiskEventArgs(string fileName, _VSFILECHANGEFLAGS flag)
         {
             this.FileName = fileName;
-            this.ItemID = id;
-            this.FileChangeFlag = flag;
+            this.FileChange = ConvertVSFILECHANGEFLAGS((uint)flag);
+        }
+
+        private static WatcherChangeTypes ConvertVSFILECHANGEFLAGS(uint flag)
+        {
+            if ((flag & (uint)(_VSFILECHANGEFLAGS.VSFILECHG_Size | _VSFILECHANGEFLAGS.VSFILECHG_Time | _VSFILECHANGEFLAGS.VSFILECHG_Attr)) != 0)
+            {
+                return WatcherChangeTypes.Changed;
+            }
+            if ((flag & (uint)_VSFILECHANGEFLAGS.VSFILECHG_Add) != 0)
+            {
+                return WatcherChangeTypes.Created;
+            }
+            if ((flag & (uint)_VSFILECHANGEFLAGS.VSFILECHG_Del) != 0)
+            {
+                return WatcherChangeTypes.Deleted;
+            }
+
+            Debug.Fail($"Unexpected value for the file changed event: \'{flag}\'");
+            return WatcherChangeTypes.Changed;
         }
     }
 }

Nodejs/Product/TestAdapterImpl/TestAdapterImpl.csproj

@@ -41,6 +41,12 @@
     <Reference Include="envdte90, Version=9.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
       <EmbedInteropTypes>False</EmbedInteropTypes>
     </Reference>
+    <Reference Include="Microsoft.VisualStudio.Shell.Interop.15.0.DesignTime, Version=15.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <EmbedInteropTypes>True</EmbedInteropTypes>
+    </Reference>
+    <Reference Include="Microsoft.VisualStudio.Shell.Interop.15.7.DesignTime, Version=15.7.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <EmbedInteropTypes>True</EmbedInteropTypes>
+    </Reference>
     <Reference Include="Microsoft.VisualStudio.TestPlatform.ObjectModel">
       <HintPath>$(DevEnvDir)CommonExtensions\Microsoft\TestWindow\Microsoft.VisualStudio.TestPlatform.ObjectModel.dll</HintPath>
       <Private>False</Private>

Nodejs/Product/TestAdapterImpl/TestContainerDiscoverer.cs

@@ -360,13 +360,7 @@ public IEnumerable<ITestContainer> GetTestContainers(IVsProject project)
                         var ft = File.GetLastWriteTimeUtc(filePath);
                         return (ft > latest) ? ft : latest;
                     }
-                    catch (UnauthorizedAccessException)
-                    {
-                    }
-                    catch (ArgumentException)
-                    {
-                    }
-                    catch (IOException)
+                    catch (Exception exc) when (exc is UnauthorizedAccessException || exc is ArgumentException || exc is IOException)
                     {
                     }
                     return latest;
@@ -530,13 +524,13 @@ private void OnProjectItemChanged(object sender, TestFileChangedEventArgs e)
             if (e != null && ShouldDiscover(e.File))
             {
                 string root = null;
+                var project = e.Project ?? GetTestProjectFromFile(e.File);
                 switch (e.ChangedReason)
                 {
-                    case TestFileChangedReason.Added:
-                        Debug.Assert(e.Project != null);
-                        if (e.Project.IsTestProject(Guids.NodejsBaseProjectFactory))
+                    case WatcherChangeTypes.Created:
+                        if (project.IsTestProject(Guids.NodejsBaseProjectFactory))
                         {
-                            root = e.Project.GetProjectHome();
+                            root = project.GetProjectHome();
 
                             if (!string.IsNullOrEmpty(root) && CommonUtils.IsSubpathOf(root, e.File))
                             {
@@ -548,12 +542,10 @@ private void OnProjectItemChanged(object sender, TestFileChangedEventArgs e)
                                 this.testFilesUpdateWatcher.AddFileWatch(e.File);
                             }
 
-                            OnTestContainersChanged(e.Project);
+                            OnTestContainersChanged(project);
                         }
                         break;
-                    case TestFileChangedReason.Removed:
-                        Debug.Assert(e.Project != null);
-
+                    case WatcherChangeTypes.Deleted:
                         if (this.fileRootMap.TryGetValue(e.File, out root))
                         {
                             this.fileRootMap.Remove(e.File);
@@ -571,15 +563,15 @@ private void OnProjectItemChanged(object sender, TestFileChangedEventArgs e)
                         // track the last delete as an update as our file system scan won't see it
                         this.lastWrite = DateTime.Now.ToUniversalTime();
 
-                        OnTestContainersChanged(e.Project);
+                        OnTestContainersChanged(project);
                         break;
 
                     // Dev12 renames files instead of overwriting them when
                     // saving, so we need to listen for renames where the new
                     // path is part of the project.
-                    case TestFileChangedReason.Renamed:
-                    case TestFileChangedReason.Changed:
-                        OnTestContainersChanged(GetTestProjectFromFile(e.File));
+                    case WatcherChangeTypes.Renamed:
+                    case WatcherChangeTypes.Changed:
+                        OnTestContainersChanged(project);
                         break;
                 }
             }
@@ -600,7 +592,7 @@ private IVsProject GetTestProjectFromFile(string filename)
                     CommonUtils.IsSamePath(projectPath, filename) ||
                     (hierarchy != null &&
                     project.IsTestProject(Guids.NodejsBaseProjectFactory) &&
-                    ErrorHandler.Succeeded(hierarchy.ParseCanonicalName(filename, out var itemid))))
+                    ErrorHandler.Succeeded(hierarchy.ParseCanonicalName(filename, out _))))
                 {
                     return project;
                 }