Merge pull request microsoft#96 from MikhailArkhipov/master

Better progress reporting

src/Analysis/Engine/Impl/ProjectEntry.cs

@@ -159,6 +159,7 @@ internal void SetCompleteAnalysis() {
             lock (this) {
                 _analysisTcs.TrySetResult(Analysis);
             }
+            RaiseNewAnalysis();
         }
 
         internal void ResetCompleteAnalysis() {
@@ -216,7 +217,7 @@ public void Analyze(CancellationToken cancel, bool enqueueOnly) {
             }
         }
 
-        internal void RaiseNewAnalysis() => NewAnalysis?.Invoke(this, EventArgs.Empty);
+        private void RaiseNewAnalysis() => NewAnalysis?.Invoke(this, EventArgs.Empty);
 
         public int AnalysisVersion { get; private set; }
 

src/Analysis/Engine/Impl/PythonAnalyzer.cs

@@ -925,7 +925,6 @@ public void AnalyzeQueuedEntries(CancellationToken cancel) {
             ddg.Analyze(Queue, cancel, _reportQueueSize, _reportQueueInterval);
             foreach (ProjectEntry entry in ddg.AnalyzedEntries) {
                 entry.SetCompleteAnalysis();
-                entry.RaiseNewAnalysis();
             }
         }
 

src/Analysis/Engine/Impl/Values/Definitions/IAnalysisIterableValue.cs

@@ -17,7 +17,7 @@
 using System.Collections.Generic;
 
 namespace Microsoft.PythonTools.Analysis.Values {
-    interface IAnalysisIterableValue: IAnalysisValue {
+    public interface IAnalysisIterableValue: IAnalysisValue {
         IEnumerable<IVariableDefinition> IndexTypes { get; }
     }
 }

src/Analysis/Engine/Test/AnalysisTest.cs

@@ -685,7 +685,6 @@ public async Task ImportTrailingComma() {
         }
 
         [TestMethod, Priority(0)]
-        [Ignore("https://github.com/Microsoft/python-language-server/issues/49")]
         public async Task ImportStarCorrectRefs() {
             var text1 = @"
 from mod2 import *
@@ -3212,7 +3211,6 @@ def f(a):
         }
 
         [TestMethod, Priority(0)]
-        [Ignore("https://github.com/Microsoft/python-language-server/issues/51")]
         public async Task ReferencesCrossModule() {
             var fobText = @"
 from oar import abc
@@ -3239,7 +3237,6 @@ from oar import abc
         }
 
         [TestMethod, Priority(0)]
-        [Ignore("https://github.com/Microsoft/python-language-server/issues/53")]
         public async Task SuperclassMemberReferencesCrossModule() {
             // https://github.com/Microsoft/PTVS/issues/2271
 

src/LanguageServer/Impl/Definitions/ILanguageServerExtensionProvider.cs

@@ -17,9 +17,8 @@
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.Python.LanguageServer.Extensions;
 
-namespace Microsoft.Python.LanguageServer {
+namespace Microsoft.Python.LanguageServer.Extensions {
     /// <summary>
     /// Implemented on a class that can create a language server extension.
     /// This class must have a default constructor.

src/LanguageServer/Impl/Implementation/AnalysisProgressReporter.cs

@@ -0,0 +1,86 @@
+// Python Tools for Visual Studio
+// Copyright(c) Microsoft Corporation
+// All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the License); you may not use
+// this file except in compliance with the License. You may obtain a copy of the
+// License at http://www.apache.org/licenses/LICENSE-2.0
+//
+// THIS CODE IS PROVIDED ON AN  *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
+// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
+// MERCHANTABLITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using System;
+using System.Threading.Tasks;
+using Microsoft.PythonTools.Analysis.Infrastructure;
+
+namespace Microsoft.Python.LanguageServer.Implementation {
+    sealed class AnalysisProgressReporter : IDisposable {
+        private readonly DisposableBag _disposables = new DisposableBag(nameof(AnalysisProgressReporter));
+        private readonly IProgressService _progressService;
+        private readonly ILogger _logger;
+        private readonly Server _server;
+        private readonly object _lock = new object();
+        private IProgress _progress;
+        private Task _queueMonitoringTask;
+
+        public AnalysisProgressReporter(Server server, IProgressService progressService, ILogger logger) {
+            _progressService = progressService;
+            _logger = logger;
+
+            _server = server;
+            _server.OnAnalysisQueued += OnAnalysisQueued;
+            _server.OnAnalysisComplete += OnAnalysisComplete;
+            _disposables
+                .Add(() => _server.OnAnalysisQueued -= OnAnalysisQueued)
+                .Add(() => _server.OnAnalysisComplete -= OnAnalysisComplete)
+                .Add(() => _progress?.Dispose());
+        }
+
+        public void Dispose() => _disposables.TryDispose();
+
+        private void OnAnalysisQueued(object sender, AnalysisQueuedEventArgs e) {
+            lock (_lock) {
+                UpdateProgressMessage();
+                _queueMonitoringTask = _queueMonitoringTask ?? QueueMonitoringTask();
+            }
+        }
+        private void OnAnalysisComplete(object sender, AnalysisCompleteEventArgs e) {
+            lock (_lock) {
+                UpdateProgressMessage();
+            }
+        }
+
+        private void UpdateProgressMessage() {
+            var count = _server.EstimateRemainingWork();
+            if (count > 0) {
+                _progress = _progress ?? _progressService.BeginProgress();
+                _progress.Report(count == 1
+                    ? Resources.AnalysisProgress_SingleItemRemaining
+                    : Resources.AnalysisProgress_MultipleItemsRemaining.FormatInvariant(count)).DoNotWait();
+            } else {
+                EndProgress();
+            }
+        }
+
+        private async Task QueueMonitoringTask() {
+            try {
+                await _server.WaitForCompleteAnalysisAsync();
+            } finally {
+                EndProgress();
+            }
+        }
+
+        private void EndProgress() {
+            lock (_lock) {
+                _progress?.Dispose();
+                _progress = null;
+                _queueMonitoringTask = null;
+            }
+        }
+    }
+}

src/LanguageServer/Impl/Implementation/Server.Extensions.cs

@@ -77,8 +77,7 @@ private async Task InvokeExtensionsAsync(Func<ILanguageServerExtension, Cancella
                         break;
                     }
                     await action(ext.Value, cancellationToken);
-                } catch (Exception ex) when (!ex.IsCriticalException()) {
-                    // We do not replace res in this case.
+                } catch (Exception ex) when (!ex.IsCriticalException() && !(ex is OperationCanceledException)) {
                     LogMessage(MessageType.Error, $"Error invoking extension '{ext.Key}': {ex}");
                 }
             }

src/LanguageServer/Impl/Implementation/Server.cs

@@ -123,11 +123,13 @@ private void Analysis_UnhandledException(object sender, UnhandledExceptionEventA
 
         public void Dispose() => _disposableBag.TryDispose();
 
+        #region ILogger
         public void TraceMessage(IFormattable message) {
             if (_traceLogging) {
                 LogMessage(MessageType.Log, message.ToString());
             }
         }
+        #endregion
 
         #region Client message handling
         internal InitializeResult GetInitializeResult() => new InitializeResult {

src/LanguageServer/Impl/LanguageServer.cs

@@ -43,31 +43,29 @@ public sealed partial class LanguageServer: IDisposable {
         private IServiceContainer _services;
         private IUIService _ui;
         private ITelemetryService _telemetry;
-        private IProgressService _progress;
 
         private JsonRpc _rpc;
         private bool _filesLoaded;
-        private Task _progressReportingTask;
         private PathsWatcher _pathsWatcher;
         private IdleTimeTracker _idleTimeTracker;
+        private AnalysisProgressReporter _analysisProgressReporter;
 
         public CancellationToken Start(IServiceContainer services, JsonRpc rpc) {
             _services = services;
             _ui = services.GetService<IUIService>();
             _telemetry = services.GetService<ITelemetryService>();
-            _progress = services.GetService<IProgressService>();
-
             _rpc = rpc;
 
+            var progress = services.GetService<IProgressService>();
+            _analysisProgressReporter = new AnalysisProgressReporter(_server, progress, _server);
+
             _server.OnLogMessage += OnLogMessage;
             _server.OnShowMessage += OnShowMessage;
             _server.OnTelemetry += OnTelemetry;
             _server.OnPublishDiagnostics += OnPublishDiagnostics;
             _server.OnApplyWorkspaceEdit += OnApplyWorkspaceEdit;
             _server.OnRegisterCapability += OnRegisterCapability;
             _server.OnUnregisterCapability += OnUnregisterCapability;
-            _server.OnAnalysisQueued += OnAnalysisQueued;
-            _server.OnAnalysisComplete += OnAnalysisComplete;
 
             _disposables
                 .Add(() => _server.OnLogMessage -= OnLogMessage)
@@ -77,40 +75,14 @@ public CancellationToken Start(IServiceContainer services, JsonRpc rpc) {
                 .Add(() => _server.OnApplyWorkspaceEdit -= OnApplyWorkspaceEdit)
                 .Add(() => _server.OnRegisterCapability -= OnRegisterCapability)
                 .Add(() => _server.OnUnregisterCapability -= OnUnregisterCapability)
-                .Add(() => _server.OnAnalysisQueued -= OnAnalysisQueued)
-                .Add(() => _server.OnAnalysisComplete -= OnAnalysisComplete)
-                .Add(_prioritizer);
+                .Add(_prioritizer)
+                .Add(_analysisProgressReporter)
+                .Add(() => _pathsWatcher?.Dispose());
 
             return _sessionTokenSource.Token;
         }
 
-        private void OnAnalysisQueued(object sender, AnalysisQueuedEventArgs e) => HandleAnalysisQueueEvent();
-        private void OnAnalysisComplete(object sender, AnalysisCompleteEventArgs e) => HandleAnalysisQueueEvent();
-
-        private void HandleAnalysisQueueEvent()
-            => _progressReportingTask = _progressReportingTask ?? ProgressWorker();
-
-        private async Task ProgressWorker() {
-            await Task.Delay(1000);
-
-            var remaining = _server.EstimateRemainingWork();
-            if (remaining > 0) {
-                using (var p = _progress.BeginProgress()) {
-                    while (remaining > 0) {
-                        var items = remaining > 1 ? "items" : "item";
-                        // TODO: in localization this needs to be two different messages 
-                        // since not all languages allow sentence construction.
-                        await p.Report($"Analyzing workspace, {remaining} {items} remaining...");
-                        await Task.Delay(100);
-                        remaining = _server.EstimateRemainingWork();
-                    }
-                }
-            }
-            _progressReportingTask = null;
-        }
-
         public void Dispose() {
-            _pathsWatcher?.Dispose();
             _disposables.TryDispose();
             _server.Dispose();
         }

src/LanguageServer/Impl/Resources.resx

@@ -117,6 +117,12 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="AnalysisProgress_MultipleItemsRemaining" xml:space="preserve">
+    <value>Analyzing workspace, {0} items remaining...</value>
+  </data>
+  <data name="AnalysisProgress_SingleItemRemaining" xml:space="preserve">
+    <value>Analyzing workspace, 1 item remaining...</value>
+  </data>
   <data name="RenameVariable_CannotRename" xml:space="preserve">
     <value>Cannot rename</value>
   </data>

src/LanguageServer/Impl/Services/UIService.cs

@@ -63,7 +63,8 @@ public Task LogMessage(string message, MessageType messageType) {
         public Task SetStatusBarMessage(string message) 
             => _rpc.NotifyWithParameterObjectAsync("window/setStatusBarMessage", message);
 
-        public void TraceMessage(IFormattable message) => LogMessage(message.ToString(), MessageType.Info);
+        public void TraceMessage(string message) => LogMessage(message.ToString(), MessageType.Info);
+        public void TraceMessage(IFormattable message) => TraceMessage(message.ToString());
 
         public void SetLogLevel(MessageType logLevel) => _logLevel = logLevel;
     }