From c610d516f6b7cbe33af8f77188f956c774478ea5 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 13 Oct 2020 19:40:28 -0700 Subject: [PATCH] Revert diagnostic worker changes. --- .../V2/CachingCodeFixProviderForProjects.cs | 10 +- .../Workers/Diagnostics/AnalyzerWorkQueue.cs | 28 +- .../Diagnostics/CSharpDiagnosticWorker.cs | 25 +- .../CSharpDiagnosticWorkerWithAnalyzers.cs | 257 ++++++++---------- .../CsharpDiagnosticWorkerComposer.cs | 9 +- .../Diagnostics/DocumentDiagnostics.cs | 13 +- .../Diagnostics/ICsDiagnosticWorker.cs | 7 +- .../AnalyzerWorkerQueueFacts.cs | 61 ++--- 8 files changed, 181 insertions(+), 229 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs index e3d919f6ef..ea230f0a16 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs @@ -41,14 +41,16 @@ public CachingCodeFixProviderForProjects(ILoggerFactory loggerFactory, OmniSharp }; } - public ImmutableArray GetAllCodeFixesForProject(Project project) + public ImmutableArray GetAllCodeFixesForProject(ProjectId projectId) { - if (_cache.ContainsKey(project.Id)) - return _cache[project.Id]; + if (_cache.ContainsKey(projectId)) + return _cache[projectId]; + + var project = _workspace.CurrentSolution.GetProject(projectId); if (project == null) { - _cache.TryRemove(project.Id, out _); + _cache.TryRemove(projectId, out _); return ImmutableArray.Empty; } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs index 6f1e818e83..0559544976 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs @@ -17,8 +17,8 @@ public Queue(TimeSpan throttling) Throttling = throttling; } - public ImmutableHashSet WorkWaitingToExecute { get; set; } = ImmutableHashSet.Empty; - public ImmutableHashSet WorkExecuting { get; set; } = ImmutableHashSet.Empty; + public ImmutableHashSet WorkWaitingToExecute { get; set; } = ImmutableHashSet.Empty; + public ImmutableHashSet WorkExecuting { get; set; } = ImmutableHashSet.Empty; public DateTime LastThrottlingBegan { get; set; } = DateTime.UtcNow; public TimeSpan Throttling { get; } public CancellationTokenSource WorkPendingToken { get; set; } @@ -44,7 +44,7 @@ public AnalyzerWorkQueue(ILoggerFactory loggerFactory, int timeoutForPendingWork _maximumDelayWhenWaitingForResults = timeoutForPendingWorkMs; } - public void PutWork(IReadOnlyCollection documents, AnalyzerWorkType workType) + public void PutWork(IReadOnlyCollection documentIds, AnalyzerWorkType workType) { lock (_queueLock) { @@ -56,21 +56,21 @@ public void PutWork(IReadOnlyCollection documents, AnalyzerWorkType wo if (queue.WorkPendingToken == null) queue.WorkPendingToken = new CancellationTokenSource(); - queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documents); + queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documentIds); } } - public IReadOnlyCollection TakeWork(AnalyzerWorkType workType) + public IReadOnlyCollection TakeWork(AnalyzerWorkType workType) { lock (_queueLock) { var queue = _queues[workType]; if (IsThrottlingActive(queue) || queue.WorkWaitingToExecute.IsEmpty) - return ImmutableHashSet.Empty; + return ImmutableHashSet.Empty; queue.WorkExecuting = queue.WorkWaitingToExecute; - queue.WorkWaitingToExecute = ImmutableHashSet.Empty; + queue.WorkWaitingToExecute = ImmutableHashSet.Empty; return queue.WorkExecuting; } } @@ -84,12 +84,12 @@ public void WorkComplete(AnalyzerWorkType workType) { lock (_queueLock) { - if (_queues[workType].WorkExecuting.IsEmpty) + if(_queues[workType].WorkExecuting.IsEmpty) return; _queues[workType].WorkPendingToken?.Cancel(); _queues[workType].WorkPendingToken = null; - _queues[workType].WorkExecuting = ImmutableHashSet.Empty; + _queues[workType].WorkExecuting = ImmutableHashSet.Empty; } } @@ -107,9 +107,15 @@ public Task WaitForegroundWorkComplete() .ContinueWith(task => LogTimeouts(task)); } - public void QueueDocumentForeground(Document document) + public bool TryPromote(DocumentId id) { - PutWork(new[] { document }, AnalyzerWorkType.Foreground); + if (_queues[AnalyzerWorkType.Background].WorkWaitingToExecute.Contains(id) || _queues[AnalyzerWorkType.Background].WorkExecuting.Contains(id)) + { + PutWork(new[] { id }, AnalyzerWorkType.Foreground); + return true; + } + + return false; } private void LogTimeouts(Task task) diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs index dab71967f1..fc63dc1981 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs @@ -145,17 +145,6 @@ public async Task> GetDiagnostics(ImmutableA .Select(docPath => _workspace.GetDocumentsFromFullProjectModelAsync(docPath))) ).SelectMany(s => s); - return await GetDiagnostics(documents); - } - - public Task> GetDiagnostics(ImmutableArray documents) - { - return GetDiagnostics((IEnumerable)documents); - } - - private async Task> GetDiagnostics(IEnumerable documents) - { - var results = new List(); foreach (var document in documents) { if(document?.Project?.Name == null) @@ -163,7 +152,7 @@ private async Task> GetDiagnostics(IEnumerab var projectName = document.Project.Name; var diagnostics = await GetDiagnosticsForDocument(document, projectName); - results.Add(new DocumentDiagnostics(document, diagnostics)); + results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics)); } return results.ToImmutableArray(); @@ -185,18 +174,18 @@ private static async Task> GetDiagnosticsForDocument( } } - public ImmutableArray QueueDocumentsForDiagnostics() + public ImmutableArray QueueDocumentsForDiagnostics() { - var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); + var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents); QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray()); - return documents; + return documents.Select(x => x.Id).ToImmutableArray(); } - public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) + public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) { - var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents).ToImmutableArray(); + var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents); QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray()); - return documents; + return documents.Select(x => x.Id).ToImmutableArray(); } public Task> GetAllDiagnosticsAsync() diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs index 13898530a1..be2d7cbe08 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs @@ -1,15 +1,15 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; -using System.Diagnostics; using System.Linq; using System.Reflection; -using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Options; using Microsoft.Extensions.Logging; using OmniSharp.Helpers; using OmniSharp.Models.Diagnostics; @@ -25,8 +25,8 @@ public class CSharpDiagnosticWorkerWithAnalyzers : ICsDiagnosticWorker, IDisposa private readonly AnalyzerWorkQueue _workQueue; private readonly ILogger _logger; - private readonly ConditionalWeakTable _currentDiagnosticResultLookup = - new ConditionalWeakTable(); + private readonly ConcurrentDictionary _currentDiagnosticResultLookup = + new ConcurrentDictionary(); private readonly ImmutableArray _providers; private readonly DiagnosticEventForwarder _forwarder; private readonly OmniSharpOptions _options; @@ -71,68 +71,42 @@ public void OnWorkspaceInitialized(bool isInitialized) { if (isInitialized) { - var documents = QueueDocumentsForDiagnostics(); - _logger.LogInformation($"Solution initialized -> queue all documents for code analysis. Initial document count: {documents.Length}."); + var documentIds = QueueDocumentsForDiagnostics(); + _logger.LogInformation($"Solution initialized -> queue all documents for code analysis. Initial document count: {documentIds.Length}."); } } public async Task> GetDiagnostics(ImmutableArray documentPaths) { - var documents = documentPaths - .Select(docPath => _workspace.GetDocument(docPath)) - .Where(x => x != default) - .ToImmutableArray(); - return await GetDiagnosticsByDocument(documents, waitForDocuments: true); - } + var documentIds = GetDocumentIdsFromPaths(documentPaths); - public async Task> GetDiagnostics(ImmutableArray documents) - { - return await GetDiagnosticsByDocument(documents, waitForDocuments: true); + return await GetDiagnosticsByDocumentIds(documentIds, waitForDocuments: true); } - private async Task> GetDiagnosticsByDocument(ImmutableArray documents, bool waitForDocuments) + private async Task> GetDiagnosticsByDocumentIds(ImmutableArray documentIds, bool waitForDocuments) { - if (documents.IsDefaultOrEmpty) return ImmutableArray.Empty; - - ImmutableArray.Builder resultsBuilder = ImmutableArray.CreateBuilder(documents.Length); - resultsBuilder.Count = documents.Length; - - bool foundAll = true; - - for (int i = 0; i < documents.Length; i++) + if (waitForDocuments) { - if (_currentDiagnosticResultLookup.TryGetValue(documents[i], out var diagnostics)) - { - resultsBuilder[i] = diagnostics; - } - else + foreach (var documentId in documentIds) { - _workQueue.QueueDocumentForeground(documents[i]); - foundAll = false; + _workQueue.TryPromote(documentId); } - } - if (foundAll) - { - return resultsBuilder.MoveToImmutable(); + await _workQueue.WaitForegroundWorkComplete(); } - await _workQueue.WaitForegroundWorkComplete(); - - for (int i = 0; i < documents.Length; i++) - { - if (_currentDiagnosticResultLookup.TryGetValue(documents[i], out var diagnostics)) - { - resultsBuilder[i] = diagnostics; - } - else - { - Debug.Fail("Should have diagnostics after waiting for work"); - resultsBuilder[i] = new DocumentDiagnostics(documents[i], ImmutableArray.Empty); - } - } + return documentIds + .Where(x => _currentDiagnosticResultLookup.ContainsKey(x)) + .Select(x => _currentDiagnosticResultLookup[x]) + .ToImmutableArray(); + } - return resultsBuilder.MoveToImmutable(); + private ImmutableArray GetDocumentIdsFromPaths(ImmutableArray documentPaths) + { + return documentPaths + .Select(docPath => _workspace.GetDocumentId(docPath)) + .Where(x => x != default) + .ToImmutableArray(); } private async Task Worker(AnalyzerWorkType workType) @@ -141,19 +115,22 @@ private async Task Worker(AnalyzerWorkType workType) { try { + var solution = _workspace.CurrentSolution; + var currentWorkGroupedByProjects = _workQueue .TakeWork(workType) - .Select(document => (project: document.Project, document)) - .GroupBy(x => x.project, x => x.document) + .Select(documentId => (projectId: solution.GetDocument(documentId)?.Project?.Id, documentId)) + .Where(x => x.projectId != null) + .GroupBy(x => x.projectId, x => x.documentId) .ToImmutableArray(); foreach (var projectGroup in currentWorkGroupedByProjects) { - var projectPath = projectGroup.Key.FilePath; + var projectPath = solution.GetProject(projectGroup.Key).FilePath; EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Started); - await AnalyzeProject(projectGroup); + await AnalyzeProject(solution, projectGroup); EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Ready); } @@ -175,9 +152,9 @@ private void EventIfBackgroundWork(AnalyzerWorkType workType, string projectPath _forwarder.ProjectAnalyzedInBackground(projectPath, status); } - private void QueueForAnalysis(ImmutableArray documents, AnalyzerWorkType workType) + private void QueueForAnalysis(ImmutableArray documentIds, AnalyzerWorkType workType) { - _workQueue.PutWork(documents, workType); + _workQueue.PutWork(documentIds, workType); } private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEvent) @@ -188,16 +165,20 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv case WorkspaceChangeKind.DocumentAdded: case WorkspaceChangeKind.DocumentReloaded: case WorkspaceChangeKind.DocumentInfoChanged: - QueueForAnalysis(ImmutableArray.Create(changeEvent.NewSolution.GetDocument(changeEvent.DocumentId)), AnalyzerWorkType.Foreground); + QueueForAnalysis(ImmutableArray.Create(changeEvent.DocumentId), AnalyzerWorkType.Foreground); break; case WorkspaceChangeKind.DocumentRemoved: + if (!_currentDiagnosticResultLookup.TryRemove(changeEvent.DocumentId, out _)) + { + _logger.LogDebug($"Tried to remove non existent document from analysis, document: {changeEvent.DocumentId}"); + } break; case WorkspaceChangeKind.ProjectAdded: case WorkspaceChangeKind.ProjectChanged: case WorkspaceChangeKind.ProjectReloaded: _logger.LogDebug($"Project {changeEvent.ProjectId} updated, reanalyzing its diagnostics."); - var projectDocuments = _workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.ToImmutableArray(); - QueueForAnalysis(projectDocuments, AnalyzerWorkType.Background); + var projectDocumentIds = _workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.Select(x => x.Id).ToImmutableArray(); + QueueForAnalysis(projectDocumentIds, AnalyzerWorkType.Background); break; case WorkspaceChangeKind.SolutionAdded: case WorkspaceChangeKind.SolutionChanged: @@ -208,20 +189,26 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv } } - private async Task AnalyzeProject(IGrouping documentsGroupedByProject) + private async Task AnalyzeProject(Solution solution, IGrouping documentsGroupedByProject) { try { - var project = documentsGroupedByProject.Key; - ImmutableArray allAnalyzers = GetProjectAnalyzers(project); + var project = solution.GetProject(documentsGroupedByProject.Key); - var compilation = await project.GetCompilationAsync(); + var allAnalyzers = _providers + .SelectMany(x => x.CodeDiagnosticAnalyzerProviders) + .Concat(project.AnalyzerReferences.SelectMany(x => x.GetAnalyzers(project.Language))) + .ToImmutableArray(); + + var compiled = await project + .GetCompilationAsync(); - var workspaceAnalyzerOptions = GetWorkspaceAnalyzerOptions(project); + var workspaceAnalyzerOptions = (AnalyzerOptions) _workspaceAnalyzerOptionsConstructor.Invoke(new object[] {project.AnalyzerOptions, project.Solution}); - foreach (var document in documentsGroupedByProject) + foreach (var documentId in documentsGroupedByProject) { - await AnalyzeAndUpdateDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document); + var document = project.GetDocument(documentId); + await AnalyzeDocument(project, allAnalyzers, compiled, workspaceAnalyzerOptions, document); } } catch (Exception ex) @@ -230,75 +217,56 @@ private async Task AnalyzeProject(IGrouping documentsGroupedB } } - private AnalyzerOptions GetWorkspaceAnalyzerOptions(Project project) - { - return (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution }); - } - - private ImmutableArray GetProjectAnalyzers(Project project) - { - return _providers - .SelectMany(x => x.CodeDiagnosticAnalyzerProviders) - .Concat(project.AnalyzerReferences.SelectMany(x => x.GetAnalyzers(project.Language))) - .ToImmutableArray(); - } - - private async Task AnalyzeAndUpdateDocument(Project project, ImmutableArray allAnalyzers, Compilation compilation, AnalyzerOptions workspaceAnalyzerOptions, Document document) + private async Task AnalyzeDocument(Project project, ImmutableArray allAnalyzers, Compilation compiled, AnalyzerOptions workspaceAnalyzerOptions, Document document) { try { - ImmutableArray diagnostics = await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document); - UpdateCurrentDiagnostics(document, diagnostics); - } - catch (Exception ex) - { - _logger.LogError($"Analysis of document {document.Name} failed or cancelled by timeout: {ex.Message}, analysers: {string.Join(", ", allAnalyzers)}"); - } - } + // There's real possibility that bug in analyzer causes analysis hang at document. + var perDocumentTimeout = + new CancellationTokenSource(_options.RoslynExtensionsOptions.DocumentAnalysisTimeoutMs); - private async Task> AnalyzeDocument(Project project, ImmutableArray allAnalyzers, Compilation compilation, AnalyzerOptions workspaceAnalyzerOptions, Document document) - { - // There's real possibility that bug in analyzer causes analysis hang at document. - var perDocumentTimeout = - new CancellationTokenSource(_options.RoslynExtensionsOptions.DocumentAnalysisTimeoutMs); + var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token); - var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token); + var diagnostics = ImmutableArray.Empty; - var diagnostics = ImmutableArray.Empty; + // Only basic syntax check is available if file is miscellanous like orphan .cs file. + // Those projects are on hard coded virtual project + if (project.Name == $"{Configuration.OmniSharpMiscProjectName}.csproj") + { + var syntaxTree = await document.GetSyntaxTreeAsync(); + diagnostics = syntaxTree.GetDiagnostics().ToImmutableArray(); + } + else if (allAnalyzers.Any()) // Analyzers cannot be called with empty analyzer list. + { + var compilationWithAnalyzers = compiled.WithAnalyzers(allAnalyzers, new CompilationWithAnalyzersOptions( + workspaceAnalyzerOptions, + onAnalyzerException: OnAnalyzerException, + concurrentAnalysis: false, + logAnalyzerExecutionTime: false, + reportSuppressedDiagnostics: false)); + + var semanticDiagnosticsWithAnalyzers = await compilationWithAnalyzers + .GetAnalyzerSemanticDiagnosticsAsync(documentSemanticModel, filterSpan: null, perDocumentTimeout.Token); + + var syntaxDiagnosticsWithAnalyzers = await compilationWithAnalyzers + .GetAnalyzerSyntaxDiagnosticsAsync(documentSemanticModel.SyntaxTree, perDocumentTimeout.Token); + + diagnostics = semanticDiagnosticsWithAnalyzers + .Concat(syntaxDiagnosticsWithAnalyzers) + .Concat(documentSemanticModel.GetDiagnostics()) + .ToImmutableArray(); + } + else + { + diagnostics = documentSemanticModel.GetDiagnostics().ToImmutableArray(); + } - // Only basic syntax check is available if file is miscellanous like orphan .cs file. - // Those projects are on hard coded virtual project - if (project.Name == $"{Configuration.OmniSharpMiscProjectName}.csproj") - { - var syntaxTree = await document.GetSyntaxTreeAsync(); - diagnostics = syntaxTree.GetDiagnostics().ToImmutableArray(); - } - else if (allAnalyzers.Any()) // Analyzers cannot be called with empty analyzer list. - { - var compilationWithAnalyzers = compilation.WithAnalyzers(allAnalyzers, new CompilationWithAnalyzersOptions( - workspaceAnalyzerOptions, - onAnalyzerException: OnAnalyzerException, - concurrentAnalysis: false, - logAnalyzerExecutionTime: false, - reportSuppressedDiagnostics: false)); - - var semanticDiagnosticsWithAnalyzers = await compilationWithAnalyzers - .GetAnalyzerSemanticDiagnosticsAsync(documentSemanticModel, filterSpan: null, perDocumentTimeout.Token); - - var syntaxDiagnosticsWithAnalyzers = await compilationWithAnalyzers - .GetAnalyzerSyntaxDiagnosticsAsync(documentSemanticModel.SyntaxTree, perDocumentTimeout.Token); - - diagnostics = semanticDiagnosticsWithAnalyzers - .Concat(syntaxDiagnosticsWithAnalyzers) - .Concat(documentSemanticModel.GetDiagnostics()) - .ToImmutableArray(); + UpdateCurrentDiagnostics(project, document, diagnostics); } - else + catch (Exception ex) { - diagnostics = documentSemanticModel.GetDiagnostics().ToImmutableArray(); + _logger.LogError($"Analysis of document {document.Name} failed or cancelled by timeout: {ex.Message}, analysers: {string.Join(", ", allAnalyzers)}"); } - - return diagnostics; } private void OnAnalyzerException(Exception ex, DiagnosticAnalyzer analyzer, Diagnostic diagnostic) @@ -309,19 +277,10 @@ private void OnAnalyzerException(Exception ex, DiagnosticAnalyzer analyzer, Diag $"\n exception: {ex.Message}"); } - private void UpdateCurrentDiagnostics(Document document, ImmutableArray diagnosticsWithAnalyzers) + private void UpdateCurrentDiagnostics(Project project, Document document, ImmutableArray diagnosticsWithAnalyzers) { - DocumentDiagnostics documentDiagnostics = new DocumentDiagnostics(document, diagnosticsWithAnalyzers); - try - { - _currentDiagnosticResultLookup.Add(document, documentDiagnostics); - } - catch (ArgumentException) - { - // The work for this document was already done. Solutions (and by extension Documents) are immutable, - // so this is fine to silently swallow, as we'll get the same results every time. - } - EmitDiagnostics(documentDiagnostics); + _currentDiagnosticResultLookup[document.Id] = new DocumentDiagnostics(document.Id, document.FilePath, project.Id, project.Name, diagnosticsWithAnalyzers); + EmitDiagnostics(_currentDiagnosticResultLookup[document.Id]); } private void EmitDiagnostics(DocumentDiagnostics results) @@ -332,7 +291,7 @@ private void EmitDiagnostics(DocumentDiagnostics results) { new DiagnosticResult { - FileName = results.Document.FilePath, QuickFixes = results.Diagnostics + FileName = results.DocumentPath, QuickFixes = results.Diagnostics .Select(x => x.ToDiagnosticLocation()) .ToList() } @@ -340,26 +299,26 @@ private void EmitDiagnostics(DocumentDiagnostics results) }); } - public ImmutableArray QueueDocumentsForDiagnostics() + public ImmutableArray QueueDocumentsForDiagnostics() { - var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); - QueueForAnalysis(documents, AnalyzerWorkType.Background); - return documents; + var documentIds = _workspace.CurrentSolution.Projects.SelectMany(x => x.DocumentIds).ToImmutableArray(); + QueueForAnalysis(documentIds, AnalyzerWorkType.Background); + return documentIds; } public async Task> GetAllDiagnosticsAsync() { - var allDocuments = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); - return await GetDiagnosticsByDocument(allDocuments, waitForDocuments: false); + var allDocumentsIds = _workspace.CurrentSolution.Projects.SelectMany(x => x.DocumentIds).ToImmutableArray(); + return await GetDiagnosticsByDocumentIds(allDocumentsIds, waitForDocuments: false); } - public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) + public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) { - var documents = projectIds - .SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents) + var documentIds = projectIds + .SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents.Select(x => x.Id)) .ToImmutableArray(); - QueueForAnalysis(documents, AnalyzerWorkType.Background); - return documents; + QueueForAnalysis(documentIds, AnalyzerWorkType.Background); + return documentIds; } public void Dispose() diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs index f509c2510b..9916347c3c 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs @@ -77,17 +77,12 @@ public Task> GetDiagnostics(ImmutableArray> GetDiagnostics(ImmutableArray documents) - { - return _implementation.GetDiagnostics(documents); - } - - public ImmutableArray QueueDocumentsForDiagnostics() + public ImmutableArray QueueDocumentsForDiagnostics() { return _implementation.QueueDocumentsForDiagnostics(); } - public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) + public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) { return _implementation.QueueDocumentsForDiagnostics(projectIds); } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs index 7b23396866..d4b7d2c6c2 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs @@ -5,14 +5,19 @@ namespace OmniSharp.Roslyn.CSharp.Services.Diagnostics { public class DocumentDiagnostics { - public DocumentDiagnostics(Document document, ImmutableArray diagnostics) + public DocumentDiagnostics(DocumentId documentId, string documentPath, ProjectId projectId, string projectName, ImmutableArray diagnostics) { + DocumentId = documentId; + DocumentPath = documentPath; + ProjectId = projectId; + ProjectName = projectName; Diagnostics = diagnostics; - Document = document; } - public Document Document { get; } - public Project Project => Document.Project; + public DocumentId DocumentId { get; } + public ProjectId ProjectId { get; } + public string ProjectName { get; } + public string DocumentPath { get; } public ImmutableArray Diagnostics { get; } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs index afaefcf514..2297d04b3f 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs @@ -8,9 +8,8 @@ namespace OmniSharp.Roslyn.CSharp.Workers.Diagnostics public interface ICsDiagnosticWorker { Task> GetDiagnostics(ImmutableArray documentPaths); - Task> GetDiagnostics(ImmutableArray documents); Task> GetAllDiagnosticsAsync(); - ImmutableArray QueueDocumentsForDiagnostics(); - ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectId); + ImmutableArray QueueDocumentsForDiagnostics(); + ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectId); } -} +} \ No newline at end of file diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs index 2da3b59acd..7742c82186 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs @@ -6,19 +6,12 @@ using Microsoft.CodeAnalysis; using Microsoft.Extensions.Logging; using OmniSharp.Roslyn.CSharp.Workers.Diagnostics; -using TestUtility; using Xunit; -using Xunit.Abstractions; namespace OmniSharp.Roslyn.CSharp.Tests { - public class AnalyzerWorkerQueueFacts : AbstractTestFixture + public class AnalyzerWorkerQueueFacts { - public AnalyzerWorkerQueueFacts(ITestOutputHelper output, SharedOmniSharpHostFixture sharedOmniSharpHostFixture) - : base(output, sharedOmniSharpHostFixture) - { - } - private class Logger : ILogger { public IDisposable BeginScope(TState state) @@ -36,7 +29,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except public ImmutableArray RecordedMessages { get; set; } = ImmutableArray.Create(); } - private class TestLoggerFactory : ILoggerFactory + private class LoggerFactory : ILoggerFactory { public Logger Logger { get; } = new Logger(); @@ -60,7 +53,7 @@ public void Dispose() public void WhenItemsAreAddedButThrotlingIsntOverNoWorkShouldBeReturned(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -73,7 +66,7 @@ public void WhenItemsAreAddedButThrotlingIsntOverNoWorkShouldBeReturned(Analyzer public void WhenWorksIsAddedToQueueThenTheyWillBeReturned(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -91,7 +84,7 @@ public void WhenWorksIsAddedToQueueThenTheyWillBeReturned(AnalyzerWorkType workT public void WhenSameItemIsAddedMultipleTimesInRowThenThrottleItemAsOne(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -112,7 +105,7 @@ public void WhenSameItemIsAddedMultipleTimesInRowThenThrottleItemAsOne(AnalyzerW public void WhenForegroundWorkIsAddedThenWaitNextIterationOfItReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -134,7 +127,7 @@ public void WhenForegroundWorkIsAddedThenWaitNextIterationOfItReady() public void WhenForegroundWorkIsUnderAnalysisOutFromQueueThenWaitUntilNextIterationOfItIsReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -156,7 +149,7 @@ public void WhenForegroundWorkIsUnderAnalysisOutFromQueueThenWaitUntilNextIterat public void WhenWorkIsWaitedButTimeoutForWaitIsExceededAllowContinue() { var now = DateTime.UtcNow; - var loggerFactory = new TestLoggerFactory(); + var loggerFactory = new LoggerFactory(); var queue = new AnalyzerWorkQueue(loggerFactory, utcNow: () => now, timeoutForPendingWorkMs: 20); var document = CreateTestDocumentId(); @@ -179,7 +172,7 @@ public async Task WhenMultipleThreadsAreConsumingAnalyzerWorkerQueueItWorksAsExp { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 1000); var parallelQueues = Enumerable.Range(0, 10) @@ -211,7 +204,7 @@ public async Task WhenMultipleThreadsAreConsumingAnalyzerWorkerQueueItWorksAsExp public async Task WhenWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnotherOneToGetReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -239,7 +232,7 @@ public async Task WhenWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnothe [Fact] public void WhenBackgroundWorkIsAdded_DontWaitIt() { - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); @@ -248,15 +241,15 @@ public void WhenBackgroundWorkIsAdded_DontWaitIt() } [Fact] - public void WhenSingleFileIsQueued_ThenPromoteItFromBackgroundQueueToForeground() + public void WhenSingleFileIsPromoted_ThenPromoteItFromBackgroundQueueToForeground() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); - queue.QueueDocumentForeground(document); + queue.TryPromote(document); now = PassOverThrotlingPeriod(now); @@ -264,24 +257,24 @@ public void WhenSingleFileIsQueued_ThenPromoteItFromBackgroundQueueToForeground( } [Fact] - public void WhenFileIsntAtBackgroundQueueAndTriedToBeQueued_ThenQueue() + public void WhenFileIsntAtBackgroundQueueAndTriedToBePromoted_ThenDontDoNothing() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); - queue.QueueDocumentForeground(document); + queue.TryPromote(document); now = PassOverThrotlingPeriod(now); - Assert.Equal(document, queue.TakeWork(AnalyzerWorkType.Foreground).Single()); + Assert.Empty(queue.TakeWork(AnalyzerWorkType.Foreground)); } [Fact] public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); @@ -290,7 +283,7 @@ public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() var activeWork = queue.TakeWork(AnalyzerWorkType.Background); - queue.QueueDocumentForeground(document); + queue.TryPromote(document); now = PassOverThrotlingPeriod(now); @@ -300,12 +293,16 @@ public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() Assert.NotEmpty(activeWork); } - private Document CreateTestDocumentId() + private DocumentId CreateTestDocumentId() { - string fileName = $"{Guid.NewGuid()}"; - var newFile = new TestFile(fileName, ""); - _ = SharedOmniSharpTestHost.AddFilesToWorkspace(newFile).Single(); - return SharedOmniSharpTestHost.Workspace.GetDocument(fileName); + var projectInfo = ProjectInfo.Create( + id: ProjectId.CreateNewId(), + version: VersionStamp.Create(), + name: "testProject", + assemblyName: "AssemblyName", + language: LanguageNames.CSharp); + + return DocumentId.CreateNewId(projectInfo.Id); } } }