From 96eb7eaf8da20bc2e2c1c3cfd6e488b48bfe15e6 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 5 Jul 2018 15:02:04 -0700 Subject: [PATCH 1/2] Fixes diagnostics not appearing. Fixes too many task comments appearing Remove unused names. --- .../LanguageServer/DiagnosticsErrorSink.cs | 6 +- Python/Product/Analysis/Parsing/Parser.cs | 2 +- .../Intellisense/OutOfProcProjectAnalyzer.cs | 5 -- .../Intellisense/ProjectAnalyzer.cs | 85 ++++++++----------- .../PythonTools/Intellisense/TaskProvider.cs | 35 +++++--- 5 files changed, 64 insertions(+), 69 deletions(-) diff --git a/Python/Product/Analysis/LanguageServer/DiagnosticsErrorSink.cs b/Python/Product/Analysis/LanguageServer/DiagnosticsErrorSink.cs index 86324a0c8a..d32e1fdedb 100644 --- a/Python/Product/Analysis/LanguageServer/DiagnosticsErrorSink.cs +++ b/Python/Product/Analysis/LanguageServer/DiagnosticsErrorSink.cs @@ -53,15 +53,19 @@ public void ProcessTaskComment(object sender, CommentEventArgs e) { source = _source }; + bool found = false; foreach (var kv in _taskCommentMap.MaybeEnumerate().OrderByDescending(kv => kv.Key.Length)) { if (text.IndexOfOrdinal(kv.Key, ignoreCase: true) >= 0) { d.code = kv.Key; d.severity = kv.Value; + found = true; break; } } - _onDiagnostic(d); + if (found) { + _onDiagnostic(d); + } } internal static DiagnosticSeverity GetSeverity(Severity severity) { diff --git a/Python/Product/Analysis/Parsing/Parser.cs b/Python/Product/Analysis/Parsing/Parser.cs index 95f4448ee2..b0632d2a40 100644 --- a/Python/Product/Analysis/Parsing/Parser.cs +++ b/Python/Product/Analysis/Parsing/Parser.cs @@ -4965,7 +4965,7 @@ public override void Add(string message, SourceSpan span, int errorCode, Severit public static TextReader ReadStreamWithEncoding(Stream stream, PythonLanguageVersion version) { var defaultEncoding = version.Is3x() ? new UTF8Encoding(false) : DefaultEncoding; - return GetStreamReaderWithEncoding(stream, defaultEncoding, null); + return GetStreamReaderWithEncoding(stream, defaultEncoding, ErrorSink.Null); } /// diff --git a/Python/Product/Analyzer/Intellisense/OutOfProcProjectAnalyzer.cs b/Python/Product/Analyzer/Intellisense/OutOfProcProjectAnalyzer.cs index 8b1d7674b5..c8f919bd97 100644 --- a/Python/Product/Analyzer/Intellisense/OutOfProcProjectAnalyzer.cs +++ b/Python/Product/Analyzer/Intellisense/OutOfProcProjectAnalyzer.cs @@ -57,11 +57,6 @@ sealed class OutOfProcProjectAnalyzer : IDisposable { private bool _isDisposed; - // Moniker strings allow the task provider to distinguish between - // different sources of items for the same file. - private const string ParserTaskMoniker = "Parser"; - internal const string UnresolvedImportMoniker = "UnresolvedImport"; - private readonly Connection _connection; internal OutOfProcProjectAnalyzer(Stream writer, Stream reader, Action log) { diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs b/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs index 3b44c0099a..1d6f4d6d14 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs @@ -80,9 +80,10 @@ sealed class VsProjectAnalyzer : ProjectAnalyzer, IDisposable { internal readonly HashSet _hasParseErrors = new HashSet(); internal readonly object _hasParseErrorsLock = new object(); - private const string ParserTaskMoniker = "Parser"; - internal const string UnresolvedImportMoniker = "UnresolvedImport"; + private const string ParserTaskMoniker = "Python (parser)"; + private const string AnalyzerTaskMoniker = "Python (analysis)"; internal const string InvalidEncodingMoniker = "InvalidEncoding"; + private const string TaskCommentMoniker = "Task comment"; internal bool _analysisComplete; private AP.AnalysisOptions _analysisOptions; @@ -526,9 +527,9 @@ public void Dispose() { foreach (var path in _projectFiles.Keys) { _services.ErrorTaskProvider?.Clear(path, ParserTaskMoniker); - _services.ErrorTaskProvider?.Clear(path, UnresolvedImportMoniker); + _services.ErrorTaskProvider?.Clear(path, AnalyzerTaskMoniker); _services.ErrorTaskProvider?.Clear(path, InvalidEncodingMoniker); - _services.CommentTaskProvider?.Clear(path, ParserTaskMoniker); + _services.CommentTaskProvider?.Clear(path, TaskCommentMoniker); } Debug.WriteLine(String.Format("Disposing of parser {0}", _analysisProcess)); @@ -1033,9 +1034,9 @@ internal static void ConnectErrorList(PythonTextBufferInfo buffer) { } buffer.Services.ErrorTaskProvider?.AddBufferForErrorSource(buffer.Filename, ParserTaskMoniker, buffer.Buffer); - buffer.Services.ErrorTaskProvider?.AddBufferForErrorSource(buffer.Filename, UnresolvedImportMoniker, buffer.Buffer); + buffer.Services.ErrorTaskProvider?.AddBufferForErrorSource(buffer.Filename, AnalyzerTaskMoniker, buffer.Buffer); buffer.Services.ErrorTaskProvider?.AddBufferForErrorSource(buffer.Filename, InvalidEncodingMoniker, buffer.Buffer); - buffer.Services.CommentTaskProvider?.AddBufferForErrorSource(buffer.Filename, ParserTaskMoniker, buffer.Buffer); + buffer.Services.CommentTaskProvider?.AddBufferForErrorSource(buffer.Filename, TaskCommentMoniker, buffer.Buffer); buffer.Services.InvalidEncodingSquiggleProvider?.AddBuffer(buffer); } @@ -1047,9 +1048,9 @@ internal static void DisconnectErrorList(PythonTextBufferInfo buffer) { // Use Maybe* variants, since if they haven't been created we don't need to // remove our sources. buffer.Services.MaybeErrorTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, ParserTaskMoniker, buffer.Buffer); - buffer.Services.MaybeErrorTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, UnresolvedImportMoniker, buffer.Buffer); + buffer.Services.MaybeErrorTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, AnalyzerTaskMoniker, buffer.Buffer); buffer.Services.MaybeErrorTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, InvalidEncodingMoniker, buffer.Buffer); - buffer.Services.MaybeCommentTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, ParserTaskMoniker, buffer.Buffer); + buffer.Services.MaybeCommentTaskProvider?.RemoveBufferForErrorSource(buffer.Filename, TaskCommentMoniker, buffer.Buffer); buffer.Services.MaybeInvalidEncodingSquiggleProvider?.RemoveBuffer(buffer); } @@ -1073,9 +1074,9 @@ internal void BufferDetached(AnalysisEntry entry, ITextBuffer buffer) { if (!entry.SuppressErrorList) { _services.ErrorTaskProvider?.ClearErrorSource(entry.Path, ParserTaskMoniker); - _services.ErrorTaskProvider?.ClearErrorSource(entry.Path, UnresolvedImportMoniker); + _services.ErrorTaskProvider?.ClearErrorSource(entry.Path, AnalyzerTaskMoniker); _services.ErrorTaskProvider?.ClearErrorSource(entry.Path, InvalidEncodingMoniker); - _services.CommentTaskProvider?.ClearErrorSource(entry.Path, ParserTaskMoniker); + _services.CommentTaskProvider?.ClearErrorSource(entry.Path, TaskCommentMoniker); } if (entry.IsTemporaryFile) { @@ -1538,51 +1539,39 @@ private void OnDiagnostics(AnalysisEntry entry, AP.DiagnosticsEvent diagnostics) // Update the warn-on-launch state for this entry var hasErrors = diagnostics.diagnostics.MaybeEnumerate() - .Any(d => d.source == "Python" && d.severity == LS.DiagnosticSeverity.Error); + .Any(d => d.severity == LS.DiagnosticSeverity.Error); // Update the parser warnings/errors. var errorTask = entry.SuppressErrorList ? null : _services.ErrorTaskProvider; + var commentTask = entry.SuppressErrorList ? null : _services.CommentTaskProvider; - if (errorTask != null) { + if (errorTask != null || commentTask != null) { var pyErrors = diagnostics.diagnostics.MaybeEnumerate() - .Where(d => d.source == "Python") - .GroupBy(d => d.severity); + .GroupBy(d => d.source); if (!pyErrors.Any()) { - errorTask.Clear(entry.Path, ParserTaskMoniker); - } else { - var factory = new TaskProviderItemFactory(bi?.LocationTracker, diagnostics.version); - var items = pyErrors.SelectMany(ge => ge.Select(e => factory.FromDiagnostic( - _services.Site, - e, - ge.Key, - VSTASKCATEGORY.CAT_CODESENSE, - true - ))).ToList(); - - errorTask.ReplaceItems(entry.Path, ParserTaskMoniker, items); - } - } - - var commentTask = entry.SuppressErrorList ? null : _services.CommentTaskProvider; - if (commentTask != null) { - var comments = diagnostics.diagnostics.MaybeEnumerate() - .Where(d => d.source == "Task comment") - .GroupBy(d => d.severity); - - if (!comments.Any()) { - commentTask.Clear(entry.Path, ParserTaskMoniker); + errorTask?.Clear(entry.Path, ParserTaskMoniker); + errorTask?.Clear(entry.Path, AnalyzerTaskMoniker); + commentTask?.Clear(entry.Path, TaskCommentMoniker); } else { var factory = new TaskProviderItemFactory(bi?.LocationTracker, diagnostics.version); - var items = comments.SelectMany(ge => ge.Select(e => factory.FromDiagnostic( - _services.Site, - e, - ge.Key, - VSTASKCATEGORY.CAT_COMMENTS, - false - ))).ToList(); - - commentTask.ReplaceItems(entry.Path, ParserTaskMoniker, items); + foreach (var g in pyErrors) { + Debug.Assert(g.Key == ParserTaskMoniker || g.Key == AnalyzerTaskMoniker || g.Key == TaskCommentMoniker, $"Unexpected source {g.Key}"); + + bool isComment = (g.Key == TaskCommentMoniker); + var items = g.Select(e => factory.FromDiagnostic( + _services.Site, + e, + isComment ? VSTASKCATEGORY.CAT_COMMENTS : VSTASKCATEGORY.CAT_CODESENSE, + !isComment + )).ToList(); + + if (isComment) { + commentTask?.ReplaceItems(entry.Path, g.Key, items); + } else { + errorTask?.ReplaceItems(entry.Path, g.Key, items); + } + } } } @@ -1701,9 +1690,9 @@ internal async Task UnloadFileAsync(AnalysisEntry entry) { bp.ClearBuffers(); } else { _services.MaybeErrorTaskProvider?.ClearErrorSource(entry.Path, ParserTaskMoniker); - _services.MaybeErrorTaskProvider?.ClearErrorSource(entry.Path, UnresolvedImportMoniker); + _services.MaybeErrorTaskProvider?.ClearErrorSource(entry.Path, AnalyzerTaskMoniker); _services.MaybeErrorTaskProvider?.ClearErrorSource(entry.Path, InvalidEncodingMoniker); - _services.MaybeCommentTaskProvider?.ClearErrorSource(entry.Path, ParserTaskMoniker); + _services.MaybeCommentTaskProvider?.ClearErrorSource(entry.Path, TaskCommentMoniker); } if (entry?.Path != null) { _projectFiles.TryRemove(entry.Path, out _); diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs b/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs index 84eb3fa12e..a5d778527a 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs @@ -86,7 +86,7 @@ private string ErrorType { #region Conversion Functions - public bool IsValid => _squiggle && !string.IsNullOrEmpty(ErrorType); + public bool ShowSquiggle => _squiggle && !string.IsNullOrEmpty(ErrorType); public void CreateSquiggleSpan(SimpleTagger tagger) { if (_rawSpan.Start >= _rawSpan.End || _spanTranslator == null) { @@ -140,12 +140,11 @@ public TaskProviderItemFactory(LocationTracker spanTranslator, int fromVersion) internal TaskProviderItem FromDiagnostic( IServiceProvider site, Diagnostic diagnostic, - DiagnosticSeverity severity, VSTASKCATEGORY category, bool squiggle ) { var priority = VSTASKPRIORITY.TP_LOW; - switch (severity) { + switch (diagnostic.severity) { case DiagnosticSeverity.Error: priority = VSTASKPRIORITY.TP_HIGH; break; @@ -618,18 +617,16 @@ private async Task RefreshAsync(CancellationToken cancellationToken) { lock (_errorSources) { cancellationToken.ThrowIfCancellationRequested(); foreach (var kv in _errorSources) { - List items; buffers.UnionWith(kv.Value); lock (_itemsLock) { - if (!_items.TryGetValue(kv.Key, out items)) { + if (!_items.TryGetValue(kv.Key, out var items)) { continue; } foreach (var item in items) { - if (item.IsValid && item.TextBuffer != null) { - List itemList; - if (!bufferToErrorList.TryGetValue(item.TextBuffer, out itemList)) { + if (item.ShowSquiggle && item.TextBuffer != null) { + if (!bufferToErrorList.TryGetValue(item.TextBuffer, out var itemList)) { bufferToErrorList[item.TextBuffer] = itemList = new List(); } @@ -994,8 +991,21 @@ public int GetHierarchy(out IVsHierarchy ppProject) { } public int GetCategory(out uint pCategory) { - pCategory = 0; - return VSConstants.E_NOTIMPL; + switch (Priority) { + case VSTASKPRIORITY.TP_HIGH: + pCategory = (uint)__VSERRORCATEGORY.EC_ERROR; + break; + case VSTASKPRIORITY.TP_NORMAL: + pCategory = (uint)__VSERRORCATEGORY.EC_WARNING; + break; + case VSTASKPRIORITY.TP_LOW: + pCategory = (uint)__VSERRORCATEGORY.EC_MESSAGE; + break; + default: + pCategory = 0; + break; + } + return VSConstants.S_OK; } } @@ -1071,10 +1081,7 @@ private void RefreshTokens() { _tokens = newTokens; - var tokensChanged = TokensChanged; - if (tokensChanged != null) { - tokensChanged(this, EventArgs.Empty); - } + TokensChanged?.Invoke(this, EventArgs.Empty); } } } From b20ed43e78897a593ac3d624eb8634d50ed8ec6d Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 5 Jul 2018 16:34:34 -0700 Subject: [PATCH 2/2] Fixes #3829 Squiggles disappear when file is reopened Update task providers to only clear tags associated with their own monikers. Simplifies disposal of task providers Fixes tests --- .../Editor/PythonTextBufferInfo.cs | 6 +- .../Intellisense/IntellisenseController.cs | 4 +- .../InvalidEncodingSquiggleProvider.cs | 1 + .../Intellisense/ProjectAnalyzer.cs | 11 +- .../PythonTools/Intellisense/TaskProvider.cs | 106 ++++++++++-------- Python/Tests/Core/ExtractMethodTests.cs | 2 +- .../MockPythonToolsPackage.cs | 5 +- .../PythonToolsMockTests/SquiggleTests.cs | 27 ++--- .../PythonToolsTestUtilities.cs | 16 +-- 9 files changed, 88 insertions(+), 90 deletions(-) diff --git a/Python/Product/PythonTools/PythonTools/Editor/PythonTextBufferInfo.cs b/Python/Product/PythonTools/PythonTools/Editor/PythonTextBufferInfo.cs index 4cd27470b9..9f1de24cfb 100644 --- a/Python/Product/PythonTools/PythonTools/Editor/PythonTextBufferInfo.cs +++ b/Python/Product/PythonTools/PythonTools/Editor/PythonTextBufferInfo.cs @@ -446,7 +446,7 @@ public ITextSnapshot AddSentSnapshot(ITextSnapshot sent) { public bool UpdateLastReceivedParse(int version) { lock (_lock) { - Trace("UpdateLastReceivedParse", version, _expectParse.ContainsKey(version)); + Trace("UpdateLastReceivedParse", version, _expectParse.ContainsKey(version) ? "expected" : "unexpected"); var toRemove = _expectParse.Keys.TakeWhile(k => k < version).ToArray(); foreach (var i in toRemove) { Debug.WriteLine($"Skipped parse for version {i}"); @@ -459,7 +459,7 @@ public bool UpdateLastReceivedParse(int version) { public bool UpdateLastReceivedAnalysis(int version) { lock (_lock) { - Trace("UpdateLastReceivedAnalysis", version, _expectAnalysis.ContainsKey(version)); + Trace("UpdateLastReceivedAnalysis", version, _expectAnalysis.ContainsKey(version) ? "expected" : "unexpected"); var toRemove = _expectAnalysis.Keys.TakeWhile(k => k < version).ToArray(); foreach (var i in toRemove) { @@ -748,7 +748,7 @@ private AnalysisLogWriter OpenTraceLog() { ); } - private void Trace(string eventName, params object[] args) { + internal void Trace(string eventName, params object[] args) { _traceLog?.Log(eventName, args); } diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/IntellisenseController.cs b/Python/Product/PythonTools/PythonTools/Intellisense/IntellisenseController.cs index ce1ac98b72..55f892ba23 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/IntellisenseController.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/IntellisenseController.cs @@ -217,7 +217,7 @@ private static async Task AnalyzeBufferAsync(ITextView textView, } bool suppressErrorList = textView.Properties.ContainsProperty(SuppressErrorLists); - var entry = await vsAnalyzer.AnalyzeFileAsync(bufferInfo.DocumentUri, isTemporaryFile, suppressErrorList); + var entry = await vsAnalyzer.AnalyzeFileAsync(bufferInfo.DocumentUri, bufferInfo.Filename, isTemporaryFile, suppressErrorList); if (entry != null && followDefaultEnvironment) { entry.Properties[FollowDefaultEnvironment] = true; } @@ -300,7 +300,7 @@ private async Task DefaultInterpreterChanged() { oldAnalyzer.Dispose(); } - var newEntry = await analyzer.AnalyzeFileAsync(bi.DocumentUri, true, bi.Buffer.Properties.ContainsProperty(SuppressErrorLists)); + var newEntry = await analyzer.AnalyzeFileAsync(bi.DocumentUri, bi.Filename, true, bi.Buffer.Properties.ContainsProperty(SuppressErrorLists)); newEntry.Properties[FollowDefaultEnvironment] = true; bi.TrySetAnalysisEntry(newEntry, null); } diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/InvalidEncodingSquiggleProvider.cs b/Python/Product/PythonTools/PythonTools/Intellisense/InvalidEncodingSquiggleProvider.cs index 019af7b421..9bda23dc03 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/InvalidEncodingSquiggleProvider.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/InvalidEncodingSquiggleProvider.cs @@ -67,6 +67,7 @@ protected override async Task OnNewAnalysis(PythonTextBufferInfo bi, AnalysisEnt new List { new TaskProviderItem( Services.Site, + VsProjectAnalyzer.InvalidEncodingMoniker, message, span, VSTASKPRIORITY.TP_NORMAL, diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs b/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs index 1d6f4d6d14..067c7a9131 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/ProjectAnalyzer.cs @@ -80,10 +80,10 @@ sealed class VsProjectAnalyzer : ProjectAnalyzer, IDisposable { internal readonly HashSet _hasParseErrors = new HashSet(); internal readonly object _hasParseErrorsLock = new object(); - private const string ParserTaskMoniker = "Python (parser)"; - private const string AnalyzerTaskMoniker = "Python (analysis)"; + internal const string ParserTaskMoniker = "Python (parser)"; + internal const string AnalyzerTaskMoniker = "Python (analysis)"; internal const string InvalidEncodingMoniker = "InvalidEncoding"; - private const string TaskCommentMoniker = "Task comment"; + internal const string TaskCommentMoniker = "Task comment"; internal bool _analysisComplete; private AP.AnalysisOptions _analysisOptions; @@ -1125,6 +1125,7 @@ internal async Task AnalyzeFileAsync( internal async Task AnalyzeFileAsync( Uri uri, + string path, bool isTemporaryFile = false, bool suppressErrorList = false ) { @@ -1142,8 +1143,7 @@ internal async Task AnalyzeFileAsync( return entry; } - string path = null; - if (uri.IsFile) { + if (string.IsNullOrEmpty(path) && uri.IsFile) { path = uri.LocalPath; } @@ -1536,6 +1536,7 @@ private void OnDiagnostics(AnalysisEntry entry, AP.DiagnosticsEvent diagnostics) if (entry.Path == null) { return; } + bi?.Trace("OnDiagnostics", diagnostics.diagnostics?.Length ?? 0); // Update the warn-on-launch state for this entry var hasErrors = diagnostics.diagnostics.MaybeEnumerate() diff --git a/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs b/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs index a5d778527a..ccb5a30931 100644 --- a/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs +++ b/Python/Product/PythonTools/PythonTools/Intellisense/TaskProvider.cs @@ -39,6 +39,7 @@ namespace Microsoft.PythonTools.Intellisense { using AP = AnalysisProtocol; class TaskProviderItem { + private readonly string _moniker; private readonly string _message; private readonly SourceSpan _rawSpan; private readonly VSTASKPRIORITY _priority; @@ -50,6 +51,7 @@ class TaskProviderItem { internal TaskProviderItem( IServiceProvider serviceProvider, + string moniker, string message, SourceSpan rawSpan, VSTASKPRIORITY priority, @@ -59,6 +61,7 @@ internal TaskProviderItem( int fromVersion ) { _serviceProvider = serviceProvider; + _moniker = moniker; _message = message; _rawSpan = rawSpan; _spanTranslator = spanTranslator; @@ -106,7 +109,7 @@ public void CreateSquiggleSpan(SimpleTagger tagger) { SpanTrackingMode.EdgeInclusive ); - tagger.CreateTagSpan(tagSpan, new ErrorTag(ErrorType, _message)); + tagger.CreateTagSpan(tagSpan, new ErrorTagWithMoniker(ErrorType, _message, _moniker)); } public ITextBuffer TextBuffer => _spanTranslator?.TextBuffer; @@ -126,6 +129,14 @@ public ErrorTaskItem ToErrorTaskItem(EntryKey key) { #endregion } + sealed class ErrorTagWithMoniker : ErrorTag { + public ErrorTagWithMoniker(string errorType, object toolTipContent, string moniker) : base(errorType, toolTipContent) { + Moniker = moniker; + } + + public string Moniker { get; } + } + sealed class TaskProviderItemFactory { private readonly LocationTracker _spanTranslator; private readonly int _fromVersion; @@ -155,6 +166,7 @@ bool squiggle return new TaskProviderItem( site, + diagnostic.source, diagnostic.message, diagnostic.range, priority, @@ -311,7 +323,8 @@ internal sealed class AbortMessage : WorkerMessage { public AbortMessage() : base() { } public override bool Apply(Dictionary> items, object itemsLock) { - throw new OperationCanceledException(); + // Indicates that we should stop work immediately and not try to recover + throw new ObjectDisposedException(nameof(TaskProvider)); } } } @@ -319,6 +332,7 @@ public override bool Apply(Dictionary> items, o class TaskProvider : IVsTaskProvider, IDisposable { private readonly Dictionary> _items; private readonly Dictionary> _errorSources; + private readonly HashSet _monikers; private readonly object _itemsLock = new object(); private uint _cookie; private readonly IVsTaskList _taskList; @@ -329,22 +343,18 @@ class TaskProvider : IVsTaskProvider, IDisposable { private readonly Queue _workerQueue = new Queue(); private readonly ManualResetEventSlim _workerQueueChanged = new ManualResetEventSlim(); - public TaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider) { + public TaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider, IEnumerable monikers) { _serviceProvider = serviceProvider; _items = new Dictionary>(); _errorSources = new Dictionary>(); + _monikers = new HashSet(monikers.MaybeEnumerate(), StringComparer.OrdinalIgnoreCase); _taskList = taskList; _errorProvider = errorProvider; } - public void Dispose() { - Dispose(true); - GC.SuppressFinalize(this); - } - #if DEBUG - private static bool Quiet = true; + private static bool Quiet = false; #endif [Conditional("DEBUG")] @@ -356,35 +366,20 @@ private static void Log(string fmt, params object[] args) { #endif } - private void Dispose(bool disposing) { - if (disposing) { - var worker = _worker; - if (worker != null) { - Log("Sending abort... {0}", DateTime.Now); - lock (_workerQueue) { - _workerQueue.Clear(); - _workerQueue.Enqueue(WorkerMessage.Abort()); - _workerQueueChanged.Set(); - } - Log("Waiting for abort... {0}", DateTime.Now); - bool stopped = worker.Join(10000); - Log("Done Waiting for abort... {0} {1}", DateTime.Now, stopped); - Debug.Assert(stopped, "Failed to terminate TaskProvider worker thread"); - } - - lock (_itemsLock) { - _items.Clear(); - } - if (_taskList != null) { - _taskList.UnregisterTaskProvider(_cookie); - } - + public virtual void Dispose() { + try { + // Disposing the main wait object will terminate the thread + // as best as we can _workerQueueChanged.Dispose(); + } catch (ObjectDisposedException) { } - } - ~TaskProvider() { - Dispose(false); + lock (_itemsLock) { + _items.Clear(); + } + if (_taskList != null) { + _taskList.UnregisterTaskProvider(_cookie); + } } public uint Cookie { @@ -497,7 +492,7 @@ private void Worker(object param) { try { WorkerWorker(); } catch (OperationCanceledException) { - Log("Operation cancellled... {0}", DateTime.Now); + Log("Operation canceled... {0}", DateTime.Now); } catch (ObjectDisposedException ex) { Trace.TraceError(ex.ToString()); @@ -646,7 +641,10 @@ private async Task RefreshAsync(CancellationToken cancellationToken) { using (tagger.Update()) { if (buffers.Remove(kv.Key)) { - tagger.RemoveTagSpans(span => span.Span.TextBuffer == kv.Key); + tagger.RemoveTagSpans(span => + _monikers.Contains((span.Tag as ErrorTagWithMoniker)?.Moniker) && + span.Span.TextBuffer == kv.Key + ); } foreach (var taskProviderItem in kv.Value) { @@ -659,7 +657,10 @@ private async Task RefreshAsync(CancellationToken cancellationToken) { // Clear tags for any remaining buffers. foreach (var buffer in buffers) { var tagger = _errorProvider.GetErrorTagger(buffer); - tagger.RemoveTagSpans(span => span.Span.TextBuffer == buffer); + tagger.RemoveTagSpans(span => + _monikers.Contains((span.Tag as ErrorTagWithMoniker)?.Moniker) && + span.Span.TextBuffer == buffer + ); } } @@ -678,12 +679,15 @@ await _serviceProvider.GetUIThread().InvokeAsync(() => { } private void SendMessage(WorkerMessage message) { - lock (_workerQueue) { - _workerQueue.Enqueue(message); - _workerQueueChanged.Set(); - } + try { + lock (_workerQueue) { + _workerQueue.Enqueue(message); + _workerQueueChanged.Set(); + } - StartWorker(); + StartWorker(); + } catch (ObjectDisposedException) { + } } #endregion @@ -1010,8 +1014,8 @@ public int GetCategory(out uint pCategory) { } sealed class ErrorTaskProvider : TaskProvider { - internal ErrorTaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider) - : base(serviceProvider, taskList, errorProvider) { + internal ErrorTaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider, IEnumerable monikers) + : base(serviceProvider, taskList, errorProvider, monikers) { } public static object CreateService(IServiceProvider container, Type serviceType) { @@ -1019,7 +1023,11 @@ public static object CreateService(IServiceProvider container, Type serviceType) var errorList = container.GetService(typeof(SVsErrorList)) as IVsTaskList; var model = container.GetComponentModel(); var errorProvider = model != null ? model.GetService() : null; - return new ErrorTaskProvider(container, errorList, errorProvider); + return new ErrorTaskProvider(container, errorList, errorProvider, new[] { + VsProjectAnalyzer.ParserTaskMoniker, + VsProjectAnalyzer.AnalyzerTaskMoniker, + VsProjectAnalyzer.InvalidEncodingMoniker, + }); } return null; } @@ -1028,8 +1036,8 @@ public static object CreateService(IServiceProvider container, Type serviceType) sealed class CommentTaskProvider : TaskProvider, IVsTaskListEvents { private volatile Dictionary _tokens; - internal CommentTaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider) - : base(serviceProvider, taskList, errorProvider) { + internal CommentTaskProvider(IServiceProvider serviceProvider, IVsTaskList taskList, IErrorProviderFactory errorProvider, IEnumerable monikers) + : base(serviceProvider, taskList, errorProvider, monikers) { RefreshTokens(); } @@ -1038,7 +1046,7 @@ public static object CreateService(IServiceProvider container, Type serviceType) var errorList = container.GetService(typeof(SVsTaskList)) as IVsTaskList; var model = container.GetComponentModel(); var errorProvider = model != null ? model.GetService() : null; - return new CommentTaskProvider(container, errorList, errorProvider); + return new CommentTaskProvider(container, errorList, errorProvider, new[] { VsProjectAnalyzer.TaskCommentMoniker }); } return null; } diff --git a/Python/Tests/Core/ExtractMethodTests.cs b/Python/Tests/Core/ExtractMethodTests.cs index e4cf6465e1..9cd3105c0c 100644 --- a/Python/Tests/Core/ExtractMethodTests.cs +++ b/Python/Tests/Core/ExtractMethodTests.cs @@ -1709,7 +1709,7 @@ private async Task ExtractMethodTest(string input, Func extract, TestResul var bi = services.GetBufferInfo(buffer); bi.ParseImmediately = true; - var entry = await analyzer.AnalyzeFileAsync(bi.DocumentUri); + var entry = await analyzer.AnalyzeFileAsync(bi.DocumentUri, bi.Filename); Assert.AreEqual(entry, bi.TrySetAnalysisEntry(entry, null)); var bp = entry.GetOrCreateBufferParser(services); bp.AddBuffer(buffer); diff --git a/Python/Tests/PythonToolsMockTests/MockPythonToolsPackage.cs b/Python/Tests/PythonToolsMockTests/MockPythonToolsPackage.cs index d7007dd7ef..16b3d8b0ba 100644 --- a/Python/Tests/PythonToolsMockTests/MockPythonToolsPackage.cs +++ b/Python/Tests/PythonToolsMockTests/MockPythonToolsPackage.cs @@ -83,11 +83,10 @@ private static object CreateTaskProviderService(IServiceContainer container, Typ if (SuppressTaskProvider) { return null; } - var errorProvider = container.GetComponentModel().GetService(); if (typeof(ErrorTaskProvider).IsEquivalentTo(type) || typeof(ErrorTaskProvider).GUID == type.GUID) { - return new ErrorTaskProvider(container, null, errorProvider); + return ErrorTaskProvider.CreateService(container, typeof(ErrorTaskProvider)); } else if (typeof(CommentTaskProvider).IsEquivalentTo(type) || typeof(CommentTaskProvider).GUID == type.GUID) { - return new CommentTaskProvider(container, null, errorProvider); + return CommentTaskProvider.CreateService(container, typeof(CommentTaskProvider)); } else { return null; } diff --git a/Python/Tests/PythonToolsMockTests/SquiggleTests.cs b/Python/Tests/PythonToolsMockTests/SquiggleTests.cs index 884c0526d0..cae85bef53 100644 --- a/Python/Tests/PythonToolsMockTests/SquiggleTests.cs +++ b/Python/Tests/PythonToolsMockTests/SquiggleTests.cs @@ -41,9 +41,10 @@ public static void DoDeployment(TestContext context) { } private static string FormatErrorTag(TrackingTagSpan tag) { - return string.Format("{0}: {1} ({2})", + return string.Format("{0}: {1} ({2}:{3})", tag.Tag.ErrorType, tag.Tag.ToolTipContent, + (tag.Tag as ErrorTagWithMoniker)?.Moniker ?? "no moniker", FormatErrorTagSpan(tag) ); } @@ -60,7 +61,7 @@ private static string FormatErrorTagSpan(TrackingTagSpan tag) { public async Task UnresolvedImportSquiggle() { List squiggles; - using (var view = new PythonEditor("import fob, oar\r\nfrom baz import *\r\nfrom .spam import eggs")) { + using (var view = new PythonEditor("import fob, oar\r\nfrom baz import *\r\nfrom spam import eggs")) { var errorProvider = view.VS.ServiceProvider.GetComponentModel().GetService(); var tagger = errorProvider.GetErrorTagger(view.View.TextView.TextBuffer); // Ensure all tasks have been updated @@ -82,10 +83,10 @@ public async Task UnresolvedImportSquiggle() { int i = 0; foreach (var expected in new[] { // Ensure that the warning includes the module name - @".*warning:.*fob.*\(7-10\)", - @".*warning:.*oar.*\(12-15\)", - @".*warning:.*baz.*\(22-25\)", - @".*warning:.*\.spam.*\(41-46\)" + @".*warning:.*fob.*\(Python.+:7-10\)", + @".*warning:.*oar.*\(Python.+:12-15\)", + @".*warning:.*baz.*\(Python.+:22-25\)", + @".*warning:.*spam.*\(Python.+:41-45\)" }) { Assert.IsTrue(i < squiggles.Count, "Not enough squiggles"); AssertUtil.AreEqual(new Regex(expected, RegexOptions.IgnoreCase | RegexOptions.Singleline), squiggles[i]); @@ -94,8 +95,13 @@ public async Task UnresolvedImportSquiggle() { } [TestMethod, Priority(2)] - public void HandledImportSquiggle() { + public async Task HandledImportSquiggle() { var testCases = new List>(); + testCases.Add(Tuple.Create( + "try:\r\n import spam\r\nexcept ValueError:\r\n pass\r\n", + new[] { @".*warning:.*spam.*\(Python.+:17-21\)" } + )); + testCases.AddRange( new[] { "", " BaseException", " Exception", " ImportError", " (ValueError, ImportError)" } .Select(ex => Tuple.Create( @@ -104,11 +110,6 @@ public void HandledImportSquiggle() { )) ); - testCases.Add(Tuple.Create( - "try:\r\n import spam\r\nexcept ValueError:\r\n pass\r\n", - new[] { @".*warning:.*spam.*\(17-21\)" } - )); - using (var view = new PythonEditor()) { var errorProvider = view.VS.ServiceProvider.GetComponentModel().GetService(); var tagger = errorProvider.GetErrorTagger(view.View.TextView.TextBuffer); @@ -118,7 +119,7 @@ public void HandledImportSquiggle() { foreach (var testCase in testCases) { view.Text = testCase.Item1; - var time = taskProvider.FlushAsync().GetAwaiter().GetResult(); + var time = await taskProvider.FlushAsync(); Console.WriteLine("TaskProvider.FlushAsync took {0}ms", time.TotalMilliseconds); var squiggles = tagger.GetTaggedSpans(new SnapshotSpan(view.CurrentSnapshot, 0, view.CurrentSnapshot.Length)) diff --git a/Python/Tests/Utilities.Python/PythonToolsTestUtilities.cs b/Python/Tests/Utilities.Python/PythonToolsTestUtilities.cs index 5b5d27defc..2ccb424f69 100644 --- a/Python/Tests/Utilities.Python/PythonToolsTestUtilities.cs +++ b/Python/Tests/Utilities.Python/PythonToolsTestUtilities.cs @@ -91,8 +91,8 @@ public static MockServiceProvider CreateMockServiceProvider( serviceProvider.AddService(typeof(ErrorTaskProvider), null, true); serviceProvider.AddService(typeof(CommentTaskProvider), null, true); } else { - serviceProvider.AddService(typeof(ErrorTaskProvider), CreateTaskProviderService, true); - serviceProvider.AddService(typeof(CommentTaskProvider), CreateTaskProviderService, true); + serviceProvider.AddService(typeof(ErrorTaskProvider), ErrorTaskProvider.CreateService, true); + serviceProvider.AddService(typeof(CommentTaskProvider), CommentTaskProvider.CreateService, true); } serviceProvider.AddService(typeof(UIThreadBase), new MockUIThread()); var optionsService = new MockPythonToolsOptionsService(); @@ -130,17 +130,5 @@ private static PythonEditorServices CreatePythonEditorServices(IServiceContainer } return services; } - - private static object CreateTaskProviderService(IServiceContainer container, Type type) { - var errorProvider = container.GetComponentModel().GetService(); - if (type == typeof(ErrorTaskProvider)) { - return new ErrorTaskProvider(container, null, errorProvider); - } else if (type == typeof(CommentTaskProvider)) { - return new CommentTaskProvider(container, null, errorProvider); - } else { - return null; - } - } - } }