Reenable file path watcher, watching interpreter paths (#1306)

* first working path classfier, breaks existing path code for now

* inherit Comparer instead of implementing both

* don't do path depth comparison, preserve user ordering as before

* switch to using new path classification code in main module resolution

* fix typo

* clean up comment about paths in Server.cs

* use String.Split instead of regex when parsing script output

* test ordering of user provided paths

* add new normalize and trim helper, check preconditions as debug asserts rather than commenting on them

* Bring back search path watcher for interpreter paths

* make watchSearchPaths change code more obvious, remove stray semicolon

* perform check only once

* filter files to python-ish ones, up buffer size to maximum

* avoid reanalysis of non-existent files on reload by removing all unopened files from the table

* fix build

* check parsed path instead of unparsed line

* move RDT reloading into RDT

Author: jakebailey

src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs

@@ -70,6 +70,12 @@ public interface IRunningDocumentTable {
         /// <returns>New lock count or -1 if document was not found.</returns>
         int UnlockDocument(Uri uri);
 
+        /// <summary>
+        /// Reloads the table by removing all unopened files (which would have been loaded from disk),
+        /// and resetting the content of all other files to trigger reanalysis.
+        /// </summary>
+        void ReloadAll();
+
         /// <summary>
         /// Fires when document is opened.
         /// </summary>

src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs

@@ -21,6 +21,7 @@
 using Microsoft.Python.Analysis.Analyzer;
 using Microsoft.Python.Analysis.Modules;
 using Microsoft.Python.Core;
+using Microsoft.Python.Core.Collections;
 using Microsoft.Python.Core.Logging;
 
 namespace Microsoft.Python.Analysis.Documents {
@@ -189,6 +190,29 @@ public void CloseDocument(Uri documentUri) {
             }
         }
 
+        public void ReloadAll() {
+            ImmutableArray<KeyValuePair<Uri, DocumentEntry>> opened;
+            ImmutableArray<KeyValuePair<Uri, DocumentEntry>> closed;
+
+            lock (_lock) {
+                _documentsByUri.Split(kvp => kvp.Value.Document.IsOpen, out opened, out closed);
+
+                foreach (var (uri, entry) in closed) {
+                    _documentsByUri.Remove(uri);
+                    entry.Document.Dispose();
+                }
+            }
+
+            foreach (var (_, entry) in closed) {
+                Closed?.Invoke(this, new DocumentEventArgs(entry.Document));
+                Removed?.Invoke(this, new DocumentEventArgs(entry.Document));
+            }
+
+            foreach (var (_, entry) in opened) {
+                entry.Document.Reset(null);
+            }
+        }
+
         public void Dispose() {
             lock (_lock) {
                 foreach (var d in _documentsByUri.Values.OfType<IDisposable>()) {

src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs

@@ -38,6 +38,8 @@ internal sealed class MainModuleResolution : ModuleResolutionBase, IModuleManage
         private readonly ConcurrentDictionary<string, IPythonModule> _specialized = new ConcurrentDictionary<string, IPythonModule>();
         private IRunningDocumentTable _rdt;
 
+        private IEnumerable<string> _userPaths = Enumerable.Empty<string>();
+
         public MainModuleResolution(string root, IServiceContainer services)
             : base(root, services) { }
 
@@ -75,7 +77,7 @@ protected override IPythonModule CreateModule(string name) {
                     return module;
                 }
             }
-            
+
             // If there is a stub, make sure it is loaded and attached
             // First check stub next to the module.
             if (!TryCreateModuleStub(name, moduleImport.ModulePath, out var stub)) {
@@ -169,14 +171,34 @@ internal async Task LoadBuiltinTypesAsync(CancellationToken cancellationToken =
             }
         }
 
+        internal async Task ReloadSearchPaths(CancellationToken cancellationToken = default) {
+            var ps = _services.GetService<IProcessServices>();
+
+            var paths = await GetInterpreterSearchPathsAsync(cancellationToken);
+            var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(Root, _fs, paths, Configuration.SearchPaths);
+
+            InterpreterPaths = interpreterPaths.Select(p => p.Path);
+            _userPaths = userPaths.Select(p => p.Path);
+
+            _log?.Log(TraceEventType.Information, "Interpreter search paths:");
+            foreach (var s in InterpreterPaths) {
+                _log?.Log(TraceEventType.Information, $"    {s}");
+            }
+
+            _log?.Log(TraceEventType.Information, "User search paths:");
+            foreach (var s in _userPaths) {
+                _log?.Log(TraceEventType.Information, $"    {s}");
+            }
+        }
+
         public async Task ReloadAsync(CancellationToken cancellationToken = default) {
             foreach (var uri in Modules
                 .Where(m => m.Value.Value?.Name != BuiltinModuleName)
                 .Select(m => m.Value.Value?.Uri)
                 .ExcludeDefault()) {
                 GetRdt()?.UnlockDocument(uri);
             }
-            
+
             // Preserve builtins, they don't need to be reloaded since interpreter does not change.
             var builtins = Modules[BuiltinModuleName];
             Modules.Clear();
@@ -187,26 +209,10 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) {
             var addedRoots = new HashSet<string>();
             addedRoots.UnionWith(PathResolver.SetRoot(Root));
 
-            var ps = _services.GetService<IProcessServices>();
-
-            var paths = await GetInterpreterSearchPathsAsync(cancellationToken);
-            var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(Root, _fs, paths, Configuration.SearchPaths);
-
-            InterpreterPaths = interpreterPaths.Select(p => p.Path);
-            var userSearchPaths = userPaths.Select(p => p.Path);
-
-            _log?.Log(TraceEventType.Information, "Interpreter search paths:");
-            foreach (var s in InterpreterPaths) {
-                _log?.Log(TraceEventType.Information, $"    {s}");
-            }
-
-            _log?.Log(TraceEventType.Information, "User search paths:");
-            foreach (var s in userSearchPaths) {
-                _log?.Log(TraceEventType.Information, $"    {s}");
-            }
+            await ReloadSearchPaths(cancellationToken);
 
             addedRoots.UnionWith(PathResolver.SetInterpreterSearchPaths(InterpreterPaths));
-            addedRoots.UnionWith(PathResolver.SetUserSearchPaths(userSearchPaths));
+            addedRoots.UnionWith(PathResolver.SetUserSearchPaths(_userPaths));
             ReloadModulePaths(addedRoots);
         }
 

src/LanguageServer/Impl/Implementation/Server.cs

@@ -49,6 +49,10 @@ public sealed partial class Server : IDisposable {
         private IIndexManager _indexManager;
         private string _rootDir;
 
+        private bool _watchSearchPaths;
+        private PathsWatcher _pathsWatcher;
+        private string[] _searchPaths;
+
         public Server(IServiceManager services) {
             _services = services;
 
@@ -58,6 +62,7 @@ public Server(IServiceManager services) {
                         ext.Dispose();
                     }
                 })
+                .Add(() => _pathsWatcher?.Dispose())
                 .Add(() => _shutdownCts.Cancel());
         }
 
@@ -173,11 +178,11 @@ public void DidChangeConfiguration(DidChangeConfigurationParams @params, Cancell
             _disposableBag.ThrowIfDisposed();
             switch (@params.settings) {
                 case ServerSettings settings: {
-                    if (HandleConfigurationChanges(settings)) {
-                        RestartAnalysis();
+                        if (HandleConfigurationChanges(settings)) {
+                            RestartAnalysis();
+                        }
+                        break;
                     }
-                    break;
-                }
                 default:
                     _log?.Log(TraceEventType.Error, "change configuration notification sent unsupported settings");
                     break;
@@ -233,7 +238,34 @@ private IDocumentationSource ChooseDocumentationSource(string[] kinds) {
         }
         #endregion
 
-        public void NotifyPackagesChanged(CancellationToken cancellationToken) {
+        public void HandleWatchPathsChange(bool watchSearchPaths) {
+            if (watchSearchPaths == _watchSearchPaths) {
+                return;
+            }
+
+            _watchSearchPaths = watchSearchPaths;
+
+            if (!_watchSearchPaths) {
+                _searchPaths = null;
+                _pathsWatcher?.Dispose();
+                _pathsWatcher = null;
+                return;
+            }
+
+            ResetPathWatcher();
+        }
+
+        private void ResetPathWatcher() {
+            var paths = _interpreter.ModuleResolution.InterpreterPaths.ToArray();
+
+            if (_searchPaths == null || !_searchPaths.SequenceEqual(paths)) {
+                _searchPaths = paths;
+                _pathsWatcher?.Dispose();
+                _pathsWatcher = new PathsWatcher(_searchPaths, () => NotifyPackagesChanged(), _log);
+            }
+        }
+
+        public void NotifyPackagesChanged(CancellationToken cancellationToken = default) {
             var interpreter = _services.GetService<IPythonInterpreter>();
             _log?.Log(TraceEventType.Information, Resources.ReloadingModules);
             // No need to reload typeshed resolution since it is a static storage.
@@ -242,17 +274,20 @@ public void NotifyPackagesChanged(CancellationToken cancellationToken) {
             interpreter.ModuleResolution.ReloadAsync(cancellationToken).ContinueWith(t => {
                 _log?.Log(TraceEventType.Information, Resources.Done);
                 _log?.Log(TraceEventType.Information, Resources.AnalysisRestarted);
+
                 RestartAnalysis();
+                
+                if (_watchSearchPaths) {
+                    ResetPathWatcher();
+                }
             }, cancellationToken).DoNotWait();
 
         }
 
         private void RestartAnalysis() {
-            var analyzer = Services.GetService<IPythonAnalyzer>();;
+            var analyzer = Services.GetService<IPythonAnalyzer>();
             analyzer.ResetAnalyzer();
-            foreach (var doc in _rdt.GetDocuments()) {
-                doc.Reset(null);
-            }
+            _rdt.ReloadAll();
         }
     }
 }

src/LanguageServer/Impl/LanguageServer.Configuration.cs

@@ -51,7 +51,7 @@ public async Task DidChangeConfiguration(JToken token, CancellationToken cancell
                 settings.symbolsHierarchyMaxSymbols = GetSetting(analysis, "symbolsHierarchyMaxSymbols", 1000);
 
                 _logger.LogLevel = GetLogLevel(analysis).ToTraceEventType();
-                HandlePathWatchChanges(token, cancellationToken);
+                HandlePathWatchChanges(token);
                 HandleDiagnosticsChanges(pythonSection, settings);
 
                 _server.DidChangeConfiguration(new DidChangeConfigurationParams { settings = settings }, cancellationToken);

src/LanguageServer/Impl/LanguageServer.cs

@@ -55,12 +55,8 @@ public sealed partial class LanguageServer : IDisposable {
 
         private JsonRpc _rpc;
         private JsonSerializer _jsonSerializer;
-        private PathsWatcher _pathsWatcher;
         private IIdleTimeTracker _idleTimeTracker;
 
-        private bool _watchSearchPaths;
-        private string[] _searchPaths = Array.Empty<string>();
-
         public CancellationToken Start(IServiceManager services, JsonRpc rpc) {
             _server = new Server(services);
             _services = services;
@@ -78,7 +74,6 @@ public CancellationToken Start(IServiceManager services, JsonRpc rpc) {
             _disposables
                 .Add(() => _shutdownCts.Cancel())
                 .Add(_prioritizer)
-                .Add(() => _pathsWatcher?.Dispose())
                 .Add(() => _rpc.TraceSource.Listeners.Remove(rpcTraceListener));
 
             services.AddService(_optionsProvider);
@@ -357,30 +352,8 @@ private MessageType GetLogLevel(JToken analysisKey) {
             return MessageType.Error;
         }
 
-        private void HandlePathWatchChanges(JToken section, CancellationToken cancellationToken) {
-            var watchSearchPaths = GetSetting(section, "watchSearchPaths", true);
-            if (!watchSearchPaths) {
-                // No longer watching.
-                _pathsWatcher?.Dispose();
-                _searchPaths = Array.Empty<string>();
-                _watchSearchPaths = false;
-                return;
-            }
-
-            // Now watching.
-            if (!_watchSearchPaths || (_watchSearchPaths && _searchPaths.SetEquals(_initParams.initializationOptions.searchPaths))) {
-                // Were not watching OR were watching but paths have changed. Recreate the watcher.
-                _pathsWatcher?.Dispose();
-                _pathsWatcher = new PathsWatcher(
-                    _initParams.initializationOptions.searchPaths,
-                    () =>_server.NotifyPackagesChanged(cancellationToken),
-                        _services.GetService<ILogger>()
-                    );
-
-                _watchSearchPaths = true;
-                _searchPaths = _initParams.initializationOptions.searchPaths;
-            }
-        }
+        private void HandlePathWatchChanges(JToken section)
+            => _server.HandleWatchPathsChange(GetSetting(section, "watchSearchPaths", true));
 
         private static CancellationToken GetToken(CancellationToken original)
                 => Debugger.IsAttached ? CancellationToken.None : original;

src/LanguageServer/Impl/PathsWatcher.cs

@@ -43,6 +43,7 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) {
             _onChanged = onChanged;
 
             var reduced = ReduceToCommonRoots(paths);
+
             foreach (var p in reduced) {
                 try {
                     if (!Directory.Exists(p)) {
@@ -53,20 +54,29 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) {
                     continue;
                 }
 
+                _log.Log(TraceEventType.Verbose, $"Watching {p}");
+
                 try {
                     var fsw = new System.IO.FileSystemWatcher(p) {
                         IncludeSubdirectories = true,
                         EnableRaisingEvents = true,
-                        NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName
+                        NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName | NotifyFilters.LastWrite,
+                        InternalBufferSize = 1 << 16, // Max buffer size of 64 KB
                     };
 
+                    fsw.Changed += OnChanged;
                     fsw.Created += OnChanged;
                     fsw.Deleted += OnChanged;
+                    fsw.Renamed += OnChanged;
+
+                    fsw.Filter = "*.p*"; // .py, .pyc, .pth - TODO: Use Filters in .NET Core 3.0.
 
                     _disposableBag
                         .Add(() => _throttleTimer?.Dispose())
+                        .Add(() => fsw.Changed -= OnChanged)
                         .Add(() => fsw.Created -= OnChanged)
                         .Add(() => fsw.Deleted -= OnChanged)
+                        .Add(() => fsw.Renamed -= OnChanged)
                         .Add(() => fsw.EnableRaisingEvents = false)
                         .Add(fsw);
                 } catch (ArgumentException ex) {