From 5cc02a9ecb933c60b9bb87eb0425ed039d0d5769 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 13 Oct 2020 19:42:24 -0700 Subject: [PATCH] Decouple FixAll from the worker by introducing bypass methods to analyze a specific document directly and skip the cache. --- .../Helpers/DiagnosticExtensions.cs | 2 +- .../Services/Diagnostics/CodeCheckService.cs | 2 +- .../Refactoring/GetFixAllCodeActionService.cs | 2 +- .../Refactoring/RunFixAllCodeActionService.cs | 19 ++----- .../Refactoring/V2/BaseCodeActionService.cs | 16 +++--- .../Diagnostics/CSharpDiagnosticWorker.cs | 19 +++++++ .../CSharpDiagnosticWorkerWithAnalyzers.cs | 53 ++++++++++++++++--- .../CsharpDiagnosticWorkerComposer.cs | 10 ++++ .../Diagnostics/ICsDiagnosticWorker.cs | 6 ++- 9 files changed, 93 insertions(+), 36 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs index 7ba8f29cc1..623e81463f 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs @@ -35,7 +35,7 @@ internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnost internal static IEnumerable DistinctDiagnosticLocationsByProject(this IEnumerable documentDiagnostic) { return documentDiagnostic - .SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.Project.Name, diagnostic: child)) + .SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.ProjectName, diagnostic: child)) .Select(x => new { location = x.diagnostic.ToDiagnosticLocation(), diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs index 252f442dc2..2c8ec78556 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Diagnostics/CodeCheckService.cs @@ -47,7 +47,7 @@ private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray string.IsNullOrEmpty(fileName) - || x.Document.FilePath == fileName) + || x.DocumentPath == fileName) .DistinctDiagnosticLocationsByProject() .Where(x => x.FileName != null); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs index 73fe279472..8c16173561 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs @@ -39,7 +39,7 @@ public override async Task Handle(GetFixAllRequest request) var allDiagnostics = await GetDiagnosticsAsync(request.Scope, document); var validFixes = allDiagnostics - .GroupBy(docAndDiag => docAndDiag.Document.Project) + .GroupBy(docAndDiag => docAndDiag.ProjectId) .SelectMany(grouping => { var projectFixProviders = GetCodeFixProviders(grouping.Key); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs index 50ec4cdb02..7b2c168cb6 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs @@ -192,26 +192,13 @@ public FixAllDiagnosticProvider(ICsDiagnosticWorker diagnosticWorker) } public override async Task> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken) - { - var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray()); - return diagnostics.SelectMany(x => x.Diagnostics); - } + => await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken); public override async Task> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken) - { - var documentDiagnostics = await _diagnosticWorker.GetDiagnostics(ImmutableArray.Create(document)); - - if (!documentDiagnostics.Any()) - return new Diagnostic[] { }; - - return documentDiagnostics.First().Diagnostics; - } + => await _diagnosticWorker.AnalyzeDocumentAsync(document, cancellationToken); public override async Task> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken) - { - var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray()); - return diagnostics.SelectMany(x => x.Diagnostics); - } + => await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken); } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 71576c0c54..428829b6bd 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -127,7 +127,7 @@ private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText) private async Task CollectCodeFixesActions(Document document, TextSpan span, List codeActions) { - var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document)); + var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath)); var groupedBySpan = diagnosticsWithProjects .SelectMany(x => x.Diagnostics) @@ -167,7 +167,7 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl private List GetSortedCodeFixProviders(Document document) { - return ExtensionOrderer.GetOrderedOrUnorderedList(_codeFixesForProject.GetAllCodeFixesForProject(document.Project), attribute => attribute.Name).ToList(); + return ExtensionOrderer.GetOrderedOrUnorderedList(_codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id), attribute => attribute.Name).ToList(); } private List GetSortedCodeRefactoringProviders() @@ -223,14 +223,14 @@ private IEnumerable ConvertToAvailableCodeAction(IEnumerabl { if (documentAndDiagnostics.Diagnostics.FirstOrDefault(d => d.Id == diagnosticId) is Diagnostic diagnostic) { - return (documentAndDiagnostics.Document.Id, diagnostic); + return (documentAndDiagnostics.DocumentId, diagnostic); } } return default; } - protected ImmutableArray GetCodeFixProviders(Project project) + protected ImmutableArray GetCodeFixProviders(ProjectId project) { return _codeFixesForProject.GetAllCodeFixesForProject(project); } @@ -239,7 +239,7 @@ protected ImmutableArray GetCodeFixProviders(Project project) { // If Roslyn ever comes up with a UI for selecting what provider the user prefers, we might consider replicating. // https://github.com/dotnet/roslyn/issues/27066 - return _codeFixesForProject.GetAllCodeFixesForProject(document.Project).FirstOrDefault(provider => provider.HasFixForId(id)); + return _codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id).FirstOrDefault(provider => provider.HasFixForId(id)); } protected async Task> GetDiagnosticsAsync(FixAllScope scope, Document document) @@ -247,13 +247,13 @@ protected async Task> GetDiagnosticsAsync(Fi switch (scope) { case FixAllScope.Solution: - var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).ToImmutableArray(); + var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).Select(d => d.FilePath).ToImmutableArray(); return await _diagnostics.GetDiagnostics(documentsInSolution); case FixAllScope.Project: - var documentsInProject = document.Project.Documents.ToImmutableArray(); + var documentsInProject = document.Project.Documents.Select(d => d.FilePath).ToImmutableArray(); return await _diagnostics.GetDiagnostics(documentsInProject); case FixAllScope.Document: - return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document)); + return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath)); default: throw new InvalidOperationException(); } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs index fc63dc1981..1299720e24 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs @@ -7,6 +7,7 @@ using System.Reactive.Concurrency; using System.Reactive.Linq; using System.Reactive.Subjects; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.Extensions.Logging; @@ -201,5 +202,23 @@ public void Dispose() _workspace.DocumentClosed -= OnDocumentOpened; _disposable.Dispose(); } + + public async Task> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + return await GetDiagnosticsForDocument(document, document.Project.Name); + } + + public async Task> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken) + { + var diagnostics = new List(); + foreach (var document in project.Documents) + { + cancellationToken.ThrowIfCancellationRequested(); + diagnostics.AddRange(await GetDiagnosticsForDocument(document, project.Name)); + } + + return diagnostics; + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs index be2d7cbe08..faf31451b8 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs @@ -189,6 +189,42 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv } } + public async Task> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken) + { + Project project = document.Project; + var allAnalyzers = _providers + .SelectMany(x => x.CodeDiagnosticAnalyzerProviders) + .Concat(project.AnalyzerReferences.SelectMany(x => x.GetAnalyzers(project.Language))) + .ToImmutableArray(); + + var compilation = await project.GetCompilationAsync(); + var workspaceAnalyzerOptions = (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution }); + + cancellationToken.ThrowIfCancellationRequested(); + return await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document); + } + + public async Task> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken) + { + var diagnostics = new List(); + var allAnalyzers = _providers + .SelectMany(x => x.CodeDiagnosticAnalyzerProviders) + .Concat(project.AnalyzerReferences.SelectMany(x => x.GetAnalyzers(project.Language))) + .ToImmutableArray(); + + var compilation = await project.GetCompilationAsync(); + var workspaceAnalyzerOptions = (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution }); + + + foreach (var document in project.Documents) + { + cancellationToken.ThrowIfCancellationRequested(); + diagnostics.AddRange(await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document)); + } + + return diagnostics; + } + private async Task AnalyzeProject(Solution solution, IGrouping documentsGroupedByProject) { try @@ -200,15 +236,15 @@ private async Task AnalyzeProject(Solution solution, IGrouping x.GetAnalyzers(project.Language))) .ToImmutableArray(); - var compiled = await project - .GetCompilationAsync(); + var compilation = await project.GetCompilationAsync(); - var workspaceAnalyzerOptions = (AnalyzerOptions) _workspaceAnalyzerOptionsConstructor.Invoke(new object[] {project.AnalyzerOptions, project.Solution}); + var workspaceAnalyzerOptions = (AnalyzerOptions)_workspaceAnalyzerOptionsConstructor.Invoke(new object[] { project.AnalyzerOptions, project.Solution }); foreach (var documentId in documentsGroupedByProject) { var document = project.GetDocument(documentId); - await AnalyzeDocument(project, allAnalyzers, compiled, workspaceAnalyzerOptions, document); + var diagnostics = await AnalyzeDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document); + UpdateCurrentDiagnostics(project, document, diagnostics); } } catch (Exception ex) @@ -217,7 +253,7 @@ private async Task AnalyzeProject(Solution solution, IGrouping allAnalyzers, Compilation compiled, AnalyzerOptions workspaceAnalyzerOptions, Document document) + private async Task> AnalyzeDocument(Project project, ImmutableArray allAnalyzers, Compilation compilation, AnalyzerOptions workspaceAnalyzerOptions, Document document) { try { @@ -238,7 +274,7 @@ private async Task AnalyzeDocument(Project project, ImmutableArray.Empty; } } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs index 9916347c3c..6b95507530 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs @@ -92,5 +92,15 @@ public void Dispose() if (_implementation is IDisposable disposable) disposable.Dispose(); _onChange.Dispose(); } + + public Task> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken) + { + return _implementation.AnalyzeDocumentAsync(document, cancellationToken); + } + + public Task> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken) + { + return _implementation.AnalyzeProjectsAsync(project, cancellationToken); + } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs index 2297d04b3f..c7ff0c5339 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs @@ -1,4 +1,6 @@ +using System.Collections.Generic; using System.Collections.Immutable; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using OmniSharp.Roslyn.CSharp.Services.Diagnostics; @@ -9,7 +11,9 @@ public interface ICsDiagnosticWorker { Task> GetDiagnostics(ImmutableArray documentPaths); Task> GetAllDiagnosticsAsync(); + Task> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken); + Task> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken); ImmutableArray QueueDocumentsForDiagnostics(); ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectId); } -} \ No newline at end of file +}