diff --git a/src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllRequest.cs b/src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllRequest.cs index aef5e65cd4..5180f474c9 100644 --- a/src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllRequest.cs +++ b/src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllRequest.cs @@ -13,5 +13,7 @@ public class RunFixAllRequest : SimpleFileRequest public int Timeout { get; set; } = 3000; public bool WantsAllCodeActionOperations { get; set; } public bool WantsTextChanges { get; set; } + // Nullable for backcompat: null == true, for requests that don't set it + public bool? ApplyChanges { get; set; } } } diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/CodeFixProviderExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/CodeFixProviderExtensions.cs new file mode 100644 index 0000000000..e16b97e8e7 --- /dev/null +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/CodeFixProviderExtensions.cs @@ -0,0 +1,29 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis.CodeFixes; + +namespace OmniSharp.Roslyn.CSharp.Helpers +{ + internal static class CodeFixProviderExtensions + { + // http://sourceroslyn.io/#Microsoft.VisualStudio.LanguageServices.CSharp/LanguageService/CSharpCodeCleanupFixer.cs,d9a375db0f1e430e,references + // CS8019 isn't directly used (via roslyn) but has an analyzer that report different diagnostic based on CS8019 to improve user experience. + private static readonly Dictionary _customDiagVsFixMap = new Dictionary + { + { "CS8019", "RemoveUnnecessaryImportsFixable" } + }; + + // Theres specific filterings between what is shown and what is fixed because of some custom mappings + // between diagnostics and their fixers. We dont want to show text 'RemoveUnnecessaryImportsFixable: ...' + // but instead 'CS8019: ...' where actual fixer is RemoveUnnecessaryImportsFixable behind the scenes. + public static bool HasFixForId(this CodeFixProvider provider, string diagnosticId) + { + return provider.FixableDiagnosticIds.Any(id => id == diagnosticId) && !_customDiagVsFixMap.ContainsKey(diagnosticId) || HasMappedFixAvailable(diagnosticId, provider); + } + + private static bool HasMappedFixAvailable(string diagnosticId, CodeFixProvider provider) + { + return (_customDiagVsFixMap.ContainsKey(diagnosticId) && provider.FixableDiagnosticIds.Any(id => id == _customDiagVsFixMap[diagnosticId])); + } + } +} diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs index 623e81463f..7ba8f29cc1 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.ProjectName, diagnostic: child)) + .SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.Project.Name, 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 2c8ec78556..252f442dc2 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.DocumentPath == fileName) + || x.Document.FilePath == fileName) .DistinctDiagnosticLocationsByProject() .Where(x => x.FileName != null); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/DocumentWithFixProvidersAndMatchingDiagnostics.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/DocumentWithFixProvidersAndMatchingDiagnostics.cs deleted file mode 100644 index ab51ee46d5..0000000000 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/DocumentWithFixProvidersAndMatchingDiagnostics.cs +++ /dev/null @@ -1,102 +0,0 @@ -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using OmniSharp.Roslyn.CSharp.Services.Diagnostics; - -namespace OmniSharp.Roslyn.CSharp.Services.Refactoring -{ - - public class DocumentWithFixProvidersAndMatchingDiagnostics - { - private readonly DocumentDiagnostics _documentDiagnostics; - - // http://source.roslyn.io/#Microsoft.VisualStudio.LanguageServices.CSharp/LanguageService/CSharpCodeCleanupFixer.cs,d9a375db0f1e430e,references - // CS8019 isn't directly used (via roslyn) but has an analyzer that report different diagnostic based on CS8019 to improve user experience. - private static readonly Dictionary _customDiagVsFixMap = new Dictionary - { - { "CS8019", "RemoveUnnecessaryImportsFixable" } - }; - - private DocumentWithFixProvidersAndMatchingDiagnostics(CodeFixProvider provider, DocumentDiagnostics documentDiagnostics) - { - CodeFixProvider = provider; - _documentDiagnostics = documentDiagnostics; - FixAllProvider = provider.GetFixAllProvider(); - } - - public CodeFixProvider CodeFixProvider { get; } - public FixAllProvider FixAllProvider { get; } - public DocumentId DocumentId => _documentDiagnostics.DocumentId; - public ProjectId ProjectId => _documentDiagnostics.ProjectId; - public string DocumentPath => _documentDiagnostics.DocumentPath; - - public IEnumerable<(string id, string messsage)> FixableDiagnostics => _documentDiagnostics.Diagnostics - .Where(x => HasFixToShow(x.Id)) - .Select(x => (x.Id, x.GetMessage())); - - // Theres specific filterings between what is shown and what is fixed because of some custom mappings - // between diagnostics and their fixers. We dont want to show text 'RemoveUnnecessaryImportsFixable: ...' - // but instead 'CS8019: ...' where actual fixer is RemoveUnnecessaryImportsFixable behind the scenes. - private bool HasFixToShow(string diagnosticId) - { - return CodeFixProvider.FixableDiagnosticIds.Any(id => id == diagnosticId) && !_customDiagVsFixMap.ContainsValue(diagnosticId) || HasMappedFixAvailable(diagnosticId); - } - - public bool HasFixForId(string diagnosticId) - { - return CodeFixProvider.FixableDiagnosticIds.Any(id => id == diagnosticId && !_customDiagVsFixMap.ContainsKey(diagnosticId)) || HasMappedFixAvailable(diagnosticId); - } - - private bool HasMappedFixAvailable(string diagnosticId) - { - return (_customDiagVsFixMap.ContainsKey(diagnosticId) && CodeFixProvider.FixableDiagnosticIds.Any(id => id == _customDiagVsFixMap[diagnosticId])); - } - - public static ImmutableArray CreateWithMatchingProviders(ImmutableArray providers, DocumentDiagnostics documentDiagnostics) - { - return - providers - .Select(provider => new DocumentWithFixProvidersAndMatchingDiagnostics(provider, documentDiagnostics)) - .Where(x => x._documentDiagnostics.Diagnostics.Any(d => x.HasFixToShow(d.Id)) || x._documentDiagnostics.Diagnostics.Any(d => x.HasFixForId(d.Id))) - .Where(x => x.FixAllProvider != null) - .ToImmutableArray(); - } - - public async Task<(CodeAction action, ImmutableArray idsToFix)> RegisterCodeFixesOrDefault(Document document) - { - CodeAction action = null; - - var fixableDiagnostics = _documentDiagnostics - .Diagnostics - .Where(x => HasFixForId(x.Id)) - .ToImmutableArray(); - - foreach (var diagnostic in fixableDiagnostics) - { - var context = new CodeFixContext( - document, - diagnostic, - (a, _) => - { - if (action == null) - { - action = a; - } - }, - CancellationToken.None); - - await CodeFixProvider.RegisterCodeFixesAsync(context).ConfigureAwait(false); - } - - if(action == null) - return default; - - return (action, fixableDiagnostics.Select(x => x.Id).Distinct().ToImmutableArray()); - } - } -} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs index 24d516b324..73fe279472 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/GetFixAllCodeActionService.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Collections.Immutable; using System.Composition; using System.Linq; using System.Threading.Tasks; @@ -6,6 +7,7 @@ using Microsoft.Extensions.Logging; using OmniSharp.Abstractions.Models.V1.FixAll; using OmniSharp.Mef; +using OmniSharp.Roslyn.CSharp.Helpers; using OmniSharp.Roslyn.CSharp.Services.Refactoring.V2; using OmniSharp.Roslyn.CSharp.Workers.Diagnostics; using OmniSharp.Services; @@ -28,17 +30,30 @@ CachingCodeFixProviderForProjects codeFixesForProject public override async Task Handle(GetFixAllRequest request) { - var availableFixes = await GetDiagnosticsMappedWithFixAllProviders(request.Scope, request.FileName); + var document = Workspace.GetDocument(request.FileName); + if (document is null) + { + Logger.LogWarning("Could not find document for file {0}", request.FileName); + return new GetFixAllResponse(ImmutableArray.Empty); + } - var distinctDiagnosticsThatCanBeFixed = availableFixes - .SelectMany(x => x.FixableDiagnostics) - .GroupBy(x => x.id) // Distinct isn't good fit here since theres cases where Id has multiple different messages based on location, just show one of them. - .Select(x => x.First()) - .Select(x => new FixAllItem(x.id, x.messsage)) + var allDiagnostics = await GetDiagnosticsAsync(request.Scope, document); + var validFixes = allDiagnostics + .GroupBy(docAndDiag => docAndDiag.Document.Project) + .SelectMany(grouping => + { + var projectFixProviders = GetCodeFixProviders(grouping.Key); + return grouping + .SelectMany(docAndDiag => docAndDiag.Diagnostics) + .Where(diag => projectFixProviders.Any(provider => provider.HasFixForId(diag.Id))); + }) + .GroupBy(diag => diag.Id) + .Select(grouping => grouping.First()) + .Select(x => new FixAllItem(x.Id, x.GetMessage())) .OrderBy(x => x.Id) .ToArray(); - return new GetFixAllResponse(distinctDiagnosticsThatCanBeFixed); + return new GetFixAllResponse(validFixes); } } -} \ No newline at end of file +} diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs index 1bde6dd6a7..50ec4cdb02 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/RunFixAllCodeActionService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -16,6 +17,7 @@ using OmniSharp.Roslyn.CSharp.Workers.Diagnostics; using OmniSharp.Services; using FixAllScope = OmniSharp.Abstractions.Models.V1.FixAll.FixAllScope; +using RoslynFixAllScope = Microsoft.CodeAnalysis.CodeFixes.FixAllScope; namespace OmniSharp.Roslyn.CSharp.Services.Refactoring { @@ -44,76 +46,140 @@ public RunFixAllCodeActionService(ICsDiagnosticWorker diagnosticWorker, public async override Task Handle(RunFixAllRequest request) { - if (request.Scope != FixAllScope.Document && request.FixAllFilter == null) - throw new NotImplementedException($"Only scope '{nameof(FixAllScope.Document)}' is currently supported when filter '{nameof(request.FixAllFilter)}' is not set."); - var solutionBeforeChanges = Workspace.CurrentSolution; + if (!(Workspace.GetDocument(request.FileName) is Document document)) + { + _logger.LogWarning("Requested fix all for document {0} that does not exist!", request.FileName); + return new RunFixAllResponse(); + } - var mappedProvidersWithDiagnostics = await GetDiagnosticsMappedWithFixAllProviders(request.Scope, request.FileName); + var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); + switch (request) + { + case { Scope: FixAllScope.Document, FixAllFilter: null }: + var allDiagnosticsInFile = await GetDiagnosticsAsync(FixAllScope.Document, document); + if (allDiagnosticsInFile.IsDefaultOrEmpty) + { + break; + } - var filteredProvidersWithFix = mappedProvidersWithDiagnostics - .Where(diagWithFix => - { - if (request.FixAllFilter == null) - return true; + if (allDiagnosticsInFile.Length > 1) + { + Debug.Fail("Not expecting to get back more documents than were passed in"); + break; + } - return request.FixAllFilter.Any(x => diagWithFix.HasFixForId(x.Id)); - }); + _logger.LogInformation("Found {0} diagnostics to fix.", allDiagnosticsInFile[0].Diagnostics.Length); + foreach (var diagnostic in allDiagnosticsInFile[0].Diagnostics) + { + document = await FixSpecificDiagnosticIdAsync(document, diagnostic.Id, FixAllScope.Document, CancellationToken.None); + } - var cancellationSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(request.Timeout)); + break; - foreach (var singleFixableProviderWithDocument in filteredProvidersWithFix) - { - try - { - var document = Workspace.CurrentSolution.GetDocument(singleFixableProviderWithDocument.DocumentId); - - var fixer = singleFixableProviderWithDocument.FixAllProvider; + case { FixAllFilter: { } filters }: + foreach (var filter in filters) + { + document = await FixSpecificDiagnosticIdAsync(document, filter.Id, request.Scope, cancellationSource.Token); + } - var (action, fixableDiagnosticIds) = await singleFixableProviderWithDocument.RegisterCodeFixesOrDefault(document); + break; - if (action == null) - continue; + default: + throw new NotImplementedException($"Only scope '{nameof(FixAllScope.Document)}' is currently supported when filter '{nameof(request.FixAllFilter)}' is not set."); + } - var fixAllContext = new FixAllContext( - document, - singleFixableProviderWithDocument.CodeFixProvider, - Microsoft.CodeAnalysis.CodeFixes.FixAllScope.Project, - action.EquivalenceKey, - fixableDiagnosticIds, - _fixAllDiagnosticProvider, - cancellationSource.Token - ); + var solutionAfterChanges = document.Project.Solution; + if (request.ApplyChanges != false) + { + _logger.LogInformation("Applying changes from the fixers."); + Workspace.TryApplyChanges(solutionAfterChanges); + } - var fixes = await singleFixableProviderWithDocument.FixAllProvider.GetFixAsync(fixAllContext); + var changes = await GetFileChangesAsync(document.Project.Solution, solutionBeforeChanges, Path.GetDirectoryName(request.FileName), request.WantsTextChanges, request.WantsAllCodeActionOperations); - if (fixes == null) - continue; + return new RunFixAllResponse + { + Changes = changes.FileChanges + }; + } - var operations = await fixes.GetOperationsAsync(cancellationSource.Token); + private async Task FixSpecificDiagnosticIdAsync(Document document, string diagnosticId, FixAllScope scope, CancellationToken cancellationToken) + { + _logger.LogInformation("Fixing {0}.", diagnosticId); + var originalDoc = document; + var codeFixProvider = GetCodeFixProviderForId(document, diagnosticId); + if (codeFixProvider is null || !(codeFixProvider.GetFixAllProvider() is FixAllProvider fixAllProvider)) + { + _logger.LogInformation("Could not find a codefix provider or a fixall provider for {0}.", diagnosticId); + return originalDoc; + } - foreach (var o in operations) - { - _logger.LogInformation($"Applying operation {o.ToString()} from fix all with fix provider {singleFixableProviderWithDocument.CodeFixProvider} to workspace document {document.FilePath}."); + _logger.LogTrace("Determing if {0} is still present in the document.", diagnosticId); + var (diagnosticDocId, primaryDiagnostic) = await GetDocumentIdAndDiagnosticForGivenId(scope, document, diagnosticId); + cancellationToken.ThrowIfCancellationRequested(); + if (primaryDiagnostic is null) + { + _logger.LogInformation("No diagnostic locations found for {0}.", diagnosticId); + return originalDoc; + } - if (o is ApplyChangesOperation applyChangesOperation) - { - applyChangesOperation.Apply(Workspace, cancellationSource.Token); - } - } - } - catch (Exception ex) + if (document.Id != diagnosticDocId) + { + document = document.Project.Solution.GetDocument(diagnosticDocId); + if (document is null) { - _logger.LogError($"Running fix all action {singleFixableProviderWithDocument} in document {singleFixableProviderWithDocument.DocumentPath} prevented by error: {ex}"); + throw new InvalidOperationException("Could not find the document with the diagnostic in the solution"); } } - var changes = await GetFileChangesAsync(Workspace.CurrentSolution, solutionBeforeChanges, Path.GetDirectoryName(request.FileName), request.WantsTextChanges, request.WantsAllCodeActionOperations); + _logger.LogTrace("{0} is still present in the document. Getting fixes.", diagnosticId); + CodeAction action = null; + var context = new CodeFixContext(document, primaryDiagnostic, + (a, _) => + { + if (action == null) + { + action = a; + } + }, + cancellationToken); + + await codeFixProvider.RegisterCodeFixesAsync(context).ConfigureAwait(false); - return new RunFixAllResponse + var roslynScope = scope switch { - Changes = changes.FileChanges + FixAllScope.Document => RoslynFixAllScope.Document, + FixAllScope.Project => RoslynFixAllScope.Project, + FixAllScope.Solution => RoslynFixAllScope.Solution, + _ => throw new InvalidOperationException() }; + + var fixAllContext = new FixAllContext(document, codeFixProvider, roslynScope, action.EquivalenceKey, ImmutableArray.Create(diagnosticId), _fixAllDiagnosticProvider, cancellationToken); + + _logger.LogTrace("Finding FixAll fix for {0}.", diagnosticId); + var fixes = await fixAllProvider.GetFixAsync(fixAllContext); + if (fixes == null) + { + _logger.LogInformation("FixAll not found for {0}.", diagnosticId); + return originalDoc; + } + + _logger.LogTrace("Getting FixAll operations for {0}.", diagnosticId); + var operations = await fixes.GetOperationsAsync(cancellationToken); + + // Currently, there are no roslyn changes that will result in multiple ApplyChangesOperations + Debug.Assert(operations.OfType().Count() < 2); + if (operations.OfType().FirstOrDefault() is ApplyChangesOperation applyChangesOperation) + { + _logger.LogTrace("Found apply changes operation for {0}.", diagnosticId); + return applyChangesOperation.ChangedSolution.GetDocument(originalDoc.Id); + } + else + { + _logger.LogTrace("No apply changes operation for {0}.", diagnosticId); + return originalDoc; + } } private class FixAllDiagnosticProvider : FixAllContext.DiagnosticProvider @@ -127,13 +193,13 @@ public FixAllDiagnosticProvider(ICsDiagnosticWorker diagnosticWorker) public override async Task> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken) { - var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.Select(x => x.FilePath).ToImmutableArray()); + var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray()); return diagnostics.SelectMany(x => x.Diagnostics); } public override async Task> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken) { - var documentDiagnostics = await _diagnosticWorker.GetDiagnostics(ImmutableArray.Create(document.FilePath)); + var documentDiagnostics = await _diagnosticWorker.GetDiagnostics(ImmutableArray.Create(document)); if (!documentDiagnostics.Any()) return new Diagnostic[] { }; @@ -143,7 +209,7 @@ public override async Task> GetDocumentDiagnosticsAsync( public override async Task> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken) { - var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.Select(x => x.FilePath).ToImmutableArray()); + var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray()); return diagnostics.SelectMany(x => x.Diagnostics); } } diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs index 62622c71b9..71576c0c54 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/BaseCodeActionService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.IO; using System.Linq; using System.Reflection; @@ -16,6 +17,7 @@ using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.V2.CodeActions; +using OmniSharp.Roslyn.CSharp.Helpers; using OmniSharp.Roslyn.CSharp.Services.Diagnostics; using OmniSharp.Roslyn.CSharp.Workers.Diagnostics; using OmniSharp.Roslyn.Utilities; @@ -23,6 +25,8 @@ using OmniSharp.Utilities; using FixAllScope = OmniSharp.Abstractions.Models.V1.FixAll.FixAllScope; +#nullable enable + namespace OmniSharp.Roslyn.CSharp.Services.Refactoring.V2 { public abstract class BaseCodeActionService : IRequestHandler @@ -30,8 +34,8 @@ public abstract class BaseCodeActionService : IRequestHandl protected readonly OmniSharpWorkspace Workspace; protected readonly IEnumerable Providers; protected readonly ILogger Logger; - private readonly ICsDiagnosticWorker diagnostics; - private readonly CachingCodeFixProviderForProjects codeFixesForProject; + private readonly ICsDiagnosticWorker _diagnostics; + private readonly CachingCodeFixProviderForProjects _codeFixesForProject; private readonly MethodInfo _getNestedCodeActions; protected Lazy> OrderedCodeRefactoringProviders; @@ -52,8 +56,8 @@ protected BaseCodeActionService( Workspace = workspace; Providers = providers; Logger = logger; - this.diagnostics = diagnostics; - this.codeFixesForProject = codeFixesForProject; + _diagnostics = diagnostics; + _codeFixesForProject = codeFixesForProject; OrderedCodeRefactoringProviders = new Lazy>(() => GetSortedCodeRefactoringProviders()); // Sadly, the CodeAction.NestedCodeActions property is still internal. @@ -123,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.FilePath)); + var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document)); var groupedBySpan = diagnosticsWithProjects .SelectMany(x => x.Diagnostics) @@ -163,7 +167,7 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl private List GetSortedCodeFixProviders(Document document) { - return ExtensionOrderer.GetOrderedOrUnorderedList(codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id), attribute => attribute.Name).ToList(); + return ExtensionOrderer.GetOrderedOrUnorderedList(_codeFixesForProject.GetAllCodeFixesForProject(document.Project), attribute => attribute.Name).ToList(); } private List GetSortedCodeRefactoringProviders() @@ -211,33 +215,47 @@ private IEnumerable ConvertToAvailableCodeAction(IEnumerabl }); } - // Mapping means: each mapped item has one document that has one code fix provider and it's corresponding diagnostics. - // If same document has multiple codefixers (diagnostics with different fixers) will them be mapped as separate items. - protected async Task> GetDiagnosticsMappedWithFixAllProviders(FixAllScope scope, string fileName) + protected async Task<(DocumentId DocumentId, Diagnostic Diagnostic)> GetDocumentIdAndDiagnosticForGivenId(FixAllScope scope, Document document, string diagnosticId) { - ImmutableArray allDiagnostics = await GetCorrectDiagnosticsInScope(scope, fileName); + var allDocumentDiagnostics = await GetDiagnosticsAsync(scope, document); - var mappedProvidersWithDiagnostics = allDiagnostics - .SelectMany(diagnosticsInDocument => - DocumentWithFixProvidersAndMatchingDiagnostics.CreateWithMatchingProviders(codeFixesForProject.GetAllCodeFixesForProject(diagnosticsInDocument.ProjectId), diagnosticsInDocument)); + foreach (var documentAndDiagnostics in allDocumentDiagnostics) + { + if (documentAndDiagnostics.Diagnostics.FirstOrDefault(d => d.Id == diagnosticId) is Diagnostic diagnostic) + { + return (documentAndDiagnostics.Document.Id, diagnostic); + } + } - return mappedProvidersWithDiagnostics.ToImmutableArray(); + return default; + } + + protected ImmutableArray GetCodeFixProviders(Project project) + { + return _codeFixesForProject.GetAllCodeFixesForProject(project); + } + + protected CodeFixProvider? GetCodeFixProviderForId(Document document, string id) + { + // 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)); } - private async Task> GetCorrectDiagnosticsInScope(FixAllScope scope, string fileName) + protected async Task> GetDiagnosticsAsync(FixAllScope scope, Document document) { switch (scope) { case FixAllScope.Solution: - var documentsInSolution = Workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).Select(x => x.FilePath).ToImmutableArray(); - return await diagnostics.GetDiagnostics(documentsInSolution); + var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).ToImmutableArray(); + return await _diagnostics.GetDiagnostics(documentsInSolution); case FixAllScope.Project: - var documentsInProject = Workspace.GetDocument(fileName).Project.Documents.Select(x => x.FilePath).ToImmutableArray(); - return await diagnostics.GetDiagnostics(documentsInProject); + var documentsInProject = document.Project.Documents.ToImmutableArray(); + return await _diagnostics.GetDiagnostics(documentsInProject); case FixAllScope.Document: - return await diagnostics.GetDiagnostics(ImmutableArray.Create(fileName)); + return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document)); default: - throw new NotImplementedException(); + throw new InvalidOperationException(); } } @@ -253,7 +271,7 @@ private async Task> GetCorrectDiagnosticsInS foreach (var documentId in projectChange.GetAddedDocuments()) { var newDocument = newSolution.GetDocument(documentId); - var text = await newDocument.GetTextAsync(); + var text = await newDocument!.GetTextAsync(); var newFilePath = newDocument.FilePath == null || !Path.IsPathRooted(newDocument.FilePath) ? Path.Combine(directory, newDocument.Name) @@ -307,14 +325,16 @@ private async Task> GetCorrectDiagnosticsInS { var newDocument = newSolution.GetDocument(documentId); var oldDocument = oldSolution.GetDocument(documentId); - var filePath = newDocument.FilePath; + Debug.Assert(oldDocument!.FilePath != null); + Debug.Assert(newDocument!.FilePath != null); + string filePath = newDocument.FilePath!; // file rename if (oldDocument != null && newDocument.Name != oldDocument.Name) { if (wantsAllCodeActionOperations) { - var newFilePath = GetNewFilePath(newDocument.Name, oldDocument.FilePath); + var newFilePath = GetNewFilePath(newDocument.Name, oldDocument.FilePath!); var text = await oldDocument.GetTextAsync(); var temp = solution.RemoveDocument(documentId); solution = temp.AddDocument(DocumentId.CreateNewId(oldDocument.Project.Id, newDocument.Name), newDocument.Name, text, oldDocument.Folders, newFilePath); diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs index ea230f0a16..e3d919f6ef 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Refactoring/V2/CachingCodeFixProviderForProjects.cs @@ -41,16 +41,14 @@ public CachingCodeFixProviderForProjects(ILoggerFactory loggerFactory, OmniSharp }; } - public ImmutableArray GetAllCodeFixesForProject(ProjectId projectId) + public ImmutableArray GetAllCodeFixesForProject(Project project) { - if (_cache.ContainsKey(projectId)) - return _cache[projectId]; - - var project = _workspace.CurrentSolution.GetProject(projectId); + if (_cache.ContainsKey(project.Id)) + return _cache[project.Id]; if (project == null) { - _cache.TryRemove(projectId, out _); + _cache.TryRemove(project.Id, 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 0559544976..6f1e818e83 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 documentIds, AnalyzerWorkType workType) + public void PutWork(IReadOnlyCollection documents, AnalyzerWorkType workType) { lock (_queueLock) { @@ -56,21 +56,21 @@ public void PutWork(IReadOnlyCollection documentIds, AnalyzerWorkTyp if (queue.WorkPendingToken == null) queue.WorkPendingToken = new CancellationTokenSource(); - queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documentIds); + queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documents); } } - 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,15 +107,9 @@ public Task WaitForegroundWorkComplete() .ContinueWith(task => LogTimeouts(task)); } - public bool TryPromote(DocumentId id) + public void QueueDocumentForeground(Document document) { - if (_queues[AnalyzerWorkType.Background].WorkWaitingToExecute.Contains(id) || _queues[AnalyzerWorkType.Background].WorkExecuting.Contains(id)) - { - PutWork(new[] { id }, AnalyzerWorkType.Foreground); - return true; - } - - return false; + PutWork(new[] { document }, AnalyzerWorkType.Foreground); } 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 fc63dc1981..dab71967f1 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorker.cs @@ -145,6 +145,17 @@ 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) @@ -152,7 +163,7 @@ public async Task> GetDiagnostics(ImmutableA var projectName = document.Project.Name; var diagnostics = await GetDiagnosticsForDocument(document, projectName); - results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics)); + results.Add(new DocumentDiagnostics(document, diagnostics)); } return results.ToImmutableArray(); @@ -174,18 +185,18 @@ private static async Task> GetDiagnosticsForDocument( } } - public ImmutableArray QueueDocumentsForDiagnostics() + public ImmutableArray QueueDocumentsForDiagnostics() { - var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents); + var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray()); - return documents.Select(x => x.Id).ToImmutableArray(); + return documents; } - public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) + public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) { - var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents); + var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents).ToImmutableArray(); QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray()); - return documents.Select(x => x.Id).ToImmutableArray(); + return documents; } public Task> GetAllDiagnosticsAsync() diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs index be2d7cbe08..13898530a1 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 ConcurrentDictionary _currentDiagnosticResultLookup = - new ConcurrentDictionary(); + private readonly ConditionalWeakTable _currentDiagnosticResultLookup = + new ConditionalWeakTable(); private readonly ImmutableArray _providers; private readonly DiagnosticEventForwarder _forwarder; private readonly OmniSharpOptions _options; @@ -71,42 +71,68 @@ public void OnWorkspaceInitialized(bool isInitialized) { if (isInitialized) { - var documentIds = QueueDocumentsForDiagnostics(); - _logger.LogInformation($"Solution initialized -> queue all documents for code analysis. Initial document count: {documentIds.Length}."); + var documents = QueueDocumentsForDiagnostics(); + _logger.LogInformation($"Solution initialized -> queue all documents for code analysis. Initial document count: {documents.Length}."); } } public async Task> GetDiagnostics(ImmutableArray documentPaths) { - var documentIds = GetDocumentIdsFromPaths(documentPaths); + var documents = documentPaths + .Select(docPath => _workspace.GetDocument(docPath)) + .Where(x => x != default) + .ToImmutableArray(); + return await GetDiagnosticsByDocument(documents, waitForDocuments: true); + } - return await GetDiagnosticsByDocumentIds(documentIds, waitForDocuments: true); + public async Task> GetDiagnostics(ImmutableArray documents) + { + return await GetDiagnosticsByDocument(documents, waitForDocuments: true); } - private async Task> GetDiagnosticsByDocumentIds(ImmutableArray documentIds, bool waitForDocuments) + private async Task> GetDiagnosticsByDocument(ImmutableArray documents, bool waitForDocuments) { - if (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++) { - foreach (var documentId in documentIds) + if (_currentDiagnosticResultLookup.TryGetValue(documents[i], out var diagnostics)) + { + resultsBuilder[i] = diagnostics; + } + else { - _workQueue.TryPromote(documentId); + _workQueue.QueueDocumentForeground(documents[i]); + foundAll = false; } + } - await _workQueue.WaitForegroundWorkComplete(); + if (foundAll) + { + return resultsBuilder.MoveToImmutable(); } - return documentIds - .Where(x => _currentDiagnosticResultLookup.ContainsKey(x)) - .Select(x => _currentDiagnosticResultLookup[x]) - .ToImmutableArray(); - } + await _workQueue.WaitForegroundWorkComplete(); - private ImmutableArray GetDocumentIdsFromPaths(ImmutableArray documentPaths) - { - return documentPaths - .Select(docPath => _workspace.GetDocumentId(docPath)) - .Where(x => x != default) - .ToImmutableArray(); + 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 resultsBuilder.MoveToImmutable(); } private async Task Worker(AnalyzerWorkType workType) @@ -115,22 +141,19 @@ private async Task Worker(AnalyzerWorkType workType) { try { - var solution = _workspace.CurrentSolution; - var currentWorkGroupedByProjects = _workQueue .TakeWork(workType) - .Select(documentId => (projectId: solution.GetDocument(documentId)?.Project?.Id, documentId)) - .Where(x => x.projectId != null) - .GroupBy(x => x.projectId, x => x.documentId) + .Select(document => (project: document.Project, document)) + .GroupBy(x => x.project, x => x.document) .ToImmutableArray(); foreach (var projectGroup in currentWorkGroupedByProjects) { - var projectPath = solution.GetProject(projectGroup.Key).FilePath; + var projectPath = projectGroup.Key.FilePath; EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Started); - await AnalyzeProject(solution, projectGroup); + await AnalyzeProject(projectGroup); EventIfBackgroundWork(workType, projectPath, ProjectDiagnosticStatus.Ready); } @@ -152,9 +175,9 @@ private void EventIfBackgroundWork(AnalyzerWorkType workType, string projectPath _forwarder.ProjectAnalyzedInBackground(projectPath, status); } - private void QueueForAnalysis(ImmutableArray documentIds, AnalyzerWorkType workType) + private void QueueForAnalysis(ImmutableArray documents, AnalyzerWorkType workType) { - _workQueue.PutWork(documentIds, workType); + _workQueue.PutWork(documents, workType); } private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEvent) @@ -165,20 +188,16 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv case WorkspaceChangeKind.DocumentAdded: case WorkspaceChangeKind.DocumentReloaded: case WorkspaceChangeKind.DocumentInfoChanged: - QueueForAnalysis(ImmutableArray.Create(changeEvent.DocumentId), AnalyzerWorkType.Foreground); + QueueForAnalysis(ImmutableArray.Create(changeEvent.NewSolution.GetDocument(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 projectDocumentIds = _workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.Select(x => x.Id).ToImmutableArray(); - QueueForAnalysis(projectDocumentIds, AnalyzerWorkType.Background); + var projectDocuments = _workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.ToImmutableArray(); + QueueForAnalysis(projectDocuments, AnalyzerWorkType.Background); break; case WorkspaceChangeKind.SolutionAdded: case WorkspaceChangeKind.SolutionChanged: @@ -189,26 +208,20 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv } } - private async Task AnalyzeProject(Solution solution, IGrouping documentsGroupedByProject) + private async Task AnalyzeProject(IGrouping documentsGroupedByProject) { try { - var project = solution.GetProject(documentsGroupedByProject.Key); + var project = documentsGroupedByProject.Key; + ImmutableArray allAnalyzers = GetProjectAnalyzers(project); - var allAnalyzers = _providers - .SelectMany(x => x.CodeDiagnosticAnalyzerProviders) - .Concat(project.AnalyzerReferences.SelectMany(x => 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 = GetWorkspaceAnalyzerOptions(project); - foreach (var documentId in documentsGroupedByProject) + foreach (var document in documentsGroupedByProject) { - var document = project.GetDocument(documentId); - await AnalyzeDocument(project, allAnalyzers, compiled, workspaceAnalyzerOptions, document); + await AnalyzeAndUpdateDocument(project, allAnalyzers, compilation, workspaceAnalyzerOptions, document); } } catch (Exception ex) @@ -217,56 +230,75 @@ private async Task AnalyzeProject(Solution solution, IGrouping allAnalyzers, Compilation compiled, AnalyzerOptions workspaceAnalyzerOptions, Document document) + 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) { try { - // There's real possibility that bug in analyzer causes analysis hang at document. - var perDocumentTimeout = - new CancellationTokenSource(_options.RoslynExtensionsOptions.DocumentAnalysisTimeoutMs); + 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)}"); + } + } - var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token); + 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 diagnostics = ImmutableArray.Empty; + var documentSemanticModel = await document.GetSemanticModelAsync(perDocumentTimeout.Token); - // 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(); - } + var diagnostics = ImmutableArray.Empty; - UpdateCurrentDiagnostics(project, document, diagnostics); + // 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(); } - catch (Exception ex) + else if (allAnalyzers.Any()) // Analyzers cannot be called with empty analyzer list. { - _logger.LogError($"Analysis of document {document.Name} failed or cancelled by timeout: {ex.Message}, analysers: {string.Join(", ", allAnalyzers)}"); + 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(); } + else + { + diagnostics = documentSemanticModel.GetDiagnostics().ToImmutableArray(); + } + + return diagnostics; } private void OnAnalyzerException(Exception ex, DiagnosticAnalyzer analyzer, Diagnostic diagnostic) @@ -277,10 +309,19 @@ private void OnAnalyzerException(Exception ex, DiagnosticAnalyzer analyzer, Diag $"\n exception: {ex.Message}"); } - private void UpdateCurrentDiagnostics(Project project, Document document, ImmutableArray diagnosticsWithAnalyzers) + private void UpdateCurrentDiagnostics(Document document, ImmutableArray diagnosticsWithAnalyzers) { - _currentDiagnosticResultLookup[document.Id] = new DocumentDiagnostics(document.Id, document.FilePath, project.Id, project.Name, diagnosticsWithAnalyzers); - EmitDiagnostics(_currentDiagnosticResultLookup[document.Id]); + 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); } private void EmitDiagnostics(DocumentDiagnostics results) @@ -291,7 +332,7 @@ private void EmitDiagnostics(DocumentDiagnostics results) { new DiagnosticResult { - FileName = results.DocumentPath, QuickFixes = results.Diagnostics + FileName = results.Document.FilePath, QuickFixes = results.Diagnostics .Select(x => x.ToDiagnosticLocation()) .ToList() } @@ -299,26 +340,26 @@ private void EmitDiagnostics(DocumentDiagnostics results) }); } - public ImmutableArray QueueDocumentsForDiagnostics() + public ImmutableArray QueueDocumentsForDiagnostics() { - var documentIds = _workspace.CurrentSolution.Projects.SelectMany(x => x.DocumentIds).ToImmutableArray(); - QueueForAnalysis(documentIds, AnalyzerWorkType.Background); - return documentIds; + var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); + QueueForAnalysis(documents, AnalyzerWorkType.Background); + return documents; } public async Task> GetAllDiagnosticsAsync() { - var allDocumentsIds = _workspace.CurrentSolution.Projects.SelectMany(x => x.DocumentIds).ToImmutableArray(); - return await GetDiagnosticsByDocumentIds(allDocumentsIds, waitForDocuments: false); + var allDocuments = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray(); + return await GetDiagnosticsByDocument(allDocuments, waitForDocuments: false); } - public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) + public ImmutableArray QueueDocumentsForDiagnostics(ImmutableArray projectIds) { - var documentIds = projectIds - .SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents.Select(x => x.Id)) + var documents = projectIds + .SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents) .ToImmutableArray(); - QueueForAnalysis(documentIds, AnalyzerWorkType.Background); - return documentIds; + QueueForAnalysis(documents, AnalyzerWorkType.Background); + return documents; } public void Dispose() diff --git a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs index 9916347c3c..f509c2510b 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CsharpDiagnosticWorkerComposer.cs @@ -77,12 +77,17 @@ public Task> GetDiagnostics(ImmutableArray QueueDocumentsForDiagnostics() + public Task> GetDiagnostics(ImmutableArray documents) + { + return _implementation.GetDiagnostics(documents); + } + + 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 d4b7d2c6c2..7b23396866 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/DocumentDiagnostics.cs @@ -5,19 +5,14 @@ namespace OmniSharp.Roslyn.CSharp.Services.Diagnostics { public class DocumentDiagnostics { - public DocumentDiagnostics(DocumentId documentId, string documentPath, ProjectId projectId, string projectName, ImmutableArray diagnostics) + public DocumentDiagnostics(Document document, ImmutableArray diagnostics) { - DocumentId = documentId; - DocumentPath = documentPath; - ProjectId = projectId; - ProjectName = projectName; Diagnostics = diagnostics; + Document = document; } - public DocumentId DocumentId { get; } - public ProjectId ProjectId { get; } - public string ProjectName { get; } - public string DocumentPath { get; } + public Document Document { get; } + public Project Project => Document.Project; 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 2297d04b3f..afaefcf514 100644 --- a/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs +++ b/src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/ICsDiagnosticWorker.cs @@ -8,8 +8,9 @@ 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 7742c82186..2da3b59acd 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/AnalyzerWorkerQueueFacts.cs @@ -6,12 +6,19 @@ 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 + public class AnalyzerWorkerQueueFacts : AbstractTestFixture { + public AnalyzerWorkerQueueFacts(ITestOutputHelper output, SharedOmniSharpHostFixture sharedOmniSharpHostFixture) + : base(output, sharedOmniSharpHostFixture) + { + } + private class Logger : ILogger { public IDisposable BeginScope(TState state) @@ -29,7 +36,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except public ImmutableArray RecordedMessages { get; set; } = ImmutableArray.Create(); } - private class LoggerFactory : ILoggerFactory + private class TestLoggerFactory : ILoggerFactory { public Logger Logger { get; } = new Logger(); @@ -53,7 +60,7 @@ public void Dispose() public void WhenItemsAreAddedButThrotlingIsntOverNoWorkShouldBeReturned(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -66,7 +73,7 @@ public void WhenItemsAreAddedButThrotlingIsntOverNoWorkShouldBeReturned(Analyzer public void WhenWorksIsAddedToQueueThenTheyWillBeReturned(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -84,7 +91,7 @@ public void WhenWorksIsAddedToQueueThenTheyWillBeReturned(AnalyzerWorkType workT public void WhenSameItemIsAddedMultipleTimesInRowThenThrottleItemAsOne(AnalyzerWorkType workType) { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, workType); @@ -105,7 +112,7 @@ public void WhenSameItemIsAddedMultipleTimesInRowThenThrottleItemAsOne(AnalyzerW public void WhenForegroundWorkIsAddedThenWaitNextIterationOfItReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -127,7 +134,7 @@ public void WhenForegroundWorkIsAddedThenWaitNextIterationOfItReady() public void WhenForegroundWorkIsUnderAnalysisOutFromQueueThenWaitUntilNextIterationOfItIsReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 500); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -149,7 +156,7 @@ public void WhenForegroundWorkIsUnderAnalysisOutFromQueueThenWaitUntilNextIterat public void WhenWorkIsWaitedButTimeoutForWaitIsExceededAllowContinue() { var now = DateTime.UtcNow; - var loggerFactory = new LoggerFactory(); + var loggerFactory = new TestLoggerFactory(); var queue = new AnalyzerWorkQueue(loggerFactory, utcNow: () => now, timeoutForPendingWorkMs: 20); var document = CreateTestDocumentId(); @@ -172,7 +179,7 @@ public async Task WhenMultipleThreadsAreConsumingAnalyzerWorkerQueueItWorksAsExp { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 1000); var parallelQueues = Enumerable.Range(0, 10) @@ -204,7 +211,7 @@ public async Task WhenMultipleThreadsAreConsumingAnalyzerWorkerQueueItWorksAsExp public async Task WhenWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnotherOneToGetReady() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Foreground); @@ -232,7 +239,7 @@ public async Task WhenWorkIsAddedAgainWhenPreviousIsAnalysing_ThenDontWaitAnothe [Fact] public void WhenBackgroundWorkIsAdded_DontWaitIt() { - var queue = new AnalyzerWorkQueue(new LoggerFactory(), timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); @@ -241,15 +248,15 @@ public void WhenBackgroundWorkIsAdded_DontWaitIt() } [Fact] - public void WhenSingleFileIsPromoted_ThenPromoteItFromBackgroundQueueToForeground() + public void WhenSingleFileIsQueued_ThenPromoteItFromBackgroundQueueToForeground() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); - queue.TryPromote(document); + queue.QueueDocumentForeground(document); now = PassOverThrotlingPeriod(now); @@ -257,24 +264,24 @@ public void WhenSingleFileIsPromoted_ThenPromoteItFromBackgroundQueueToForegroun } [Fact] - public void WhenFileIsntAtBackgroundQueueAndTriedToBePromoted_ThenDontDoNothing() + public void WhenFileIsntAtBackgroundQueueAndTriedToBeQueued_ThenQueue() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); - queue.TryPromote(document); + queue.QueueDocumentForeground(document); now = PassOverThrotlingPeriod(now); - Assert.Empty(queue.TakeWork(AnalyzerWorkType.Foreground)); + Assert.Equal(document, queue.TakeWork(AnalyzerWorkType.Foreground).Single()); } [Fact] public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() { var now = DateTime.UtcNow; - var queue = new AnalyzerWorkQueue(new LoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); + var queue = new AnalyzerWorkQueue(new TestLoggerFactory(), utcNow: () => now, timeoutForPendingWorkMs: 10*1000); var document = CreateTestDocumentId(); queue.PutWork(new[] { document }, AnalyzerWorkType.Background); @@ -283,7 +290,7 @@ public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() var activeWork = queue.TakeWork(AnalyzerWorkType.Background); - queue.TryPromote(document); + queue.QueueDocumentForeground(document); now = PassOverThrotlingPeriod(now); @@ -293,16 +300,12 @@ public void WhenFileIsProcessingInBackgroundQueue_ThenPromoteItAsForeground() Assert.NotEmpty(activeWork); } - private DocumentId CreateTestDocumentId() + private Document CreateTestDocumentId() { - var projectInfo = ProjectInfo.Create( - id: ProjectId.CreateNewId(), - version: VersionStamp.Create(), - name: "testProject", - assemblyName: "AssemblyName", - language: LanguageNames.CSharp); - - return DocumentId.CreateNewId(projectInfo.Id); + string fileName = $"{Guid.NewGuid()}"; + var newFile = new TestFile(fileName, ""); + _ = SharedOmniSharpTestHost.AddFilesToWorkspace(newFile).Single(); + return SharedOmniSharpTestHost.Workspace.GetDocument(fileName); } } } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs index 2d7bb81940..545132bed9 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FixAllFacts.cs @@ -47,7 +47,7 @@ public async Task WhenFileContainsFixableIssuesWithAnalyzersEnabled_ThenFixThemA }); string textAfterFix = await GetContentOfDocumentFromWorkspace(host, testFilePath); - AssertUtils.AssertIgnoringIndent(textAfterFix, expectedText); + AssertUtils.AssertIgnoringIndent(expectedText, textAfterFix); var internalClassChange = response.Changes.OfType().Single().Changes.Single(x => x.NewText == "internal "); @@ -70,23 +70,16 @@ public async Task WhenFixAllItemsAreDefinedByFilter_ThenFixOnlyFilteredItems() { using (var host = GetHost(true)) { - var originalText = - @" - class C{} - "; + var originalText = " class C{}"; // If filtering isn't set, this should also add 'internal' etc which // should not appear now as result. - var expectedText = - @" - class C { } - "; var testFilePath = CreateTestProjectWithDocument(host, originalText); var handler = host.GetRequestHandler(OmniSharpEndpoints.RunFixAll); - await handler.Handle(new RunFixAllRequest + var response = await handler.Handle(new RunFixAllRequest { Scope = FixAllScope.Document, FileName = testFilePath, @@ -97,14 +90,16 @@ await handler.Handle(new RunFixAllRequest string textAfterFix = await GetContentOfDocumentFromWorkspace(host, testFilePath); - AssertUtils.AssertIgnoringIndent(expectedText, textAfterFix); + Assert.Equal("class C { }", textAfterFix); + + Assert.Equal("class C { ", ((ModifiedFileResponse)response.Changes.Single()).Changes.Single().NewText); } } [Theory] [InlineData(FixAllScope.Document)] [InlineData(FixAllScope.Project)] - public async Task WhenFixAllIsScopedToDocumentAndProject_ThenOnlyFixInScopeInsteadOfEverything(FixAllScope scope) + public async Task WhenFixAllIsScopedToDocumentAndProject_ThenOnlyFixInScopeInsteadOfEverything_DifferentProjects(FixAllScope scope) { using (var host = GetHost(true)) { @@ -141,12 +136,106 @@ await handler.Handle(new RunFixAllRequest } } - [Fact(Skip = @"Fails on windows only inside roslyn -System.ArgumentOutOfRangeException -Specified argument was out of the range of valid values. -Parameter name: start -... -")] + [Fact] + public async Task WhenFixAllIsScopedToDocumentAndProject_ThenOnlyFixInScopeInsteadOfEverything_DifferentDocuments() + { + const string UnformattedString = @" +partial class C {} +"; + + const string FormattedString = @" +internal partial class C {} +"; + + const string File1 = "file1.cs"; + const string File2 = "file2.cs"; + + using var host = GetHost(true); + var projectId = host.AddFilesToWorkspace(new TestFile(File1, UnformattedString), new TestFile(File2, UnformattedString)).First(); + var project = host.Workspace.CurrentSolution.GetProject(projectId); + + var handler = host.GetRequestHandler(OmniSharpEndpoints.RunFixAll); + + await handler.Handle(new RunFixAllRequest + { + Scope = FixAllScope.Document, + FileName = File1, + FixAllFilter = new[] { new FixAllItem("IDE0040", "Fix formatting") }, + WantsTextChanges = true, + WantsAllCodeActionOperations = true + }); + + var file1Content = await GetContentOfDocumentFromWorkspace(host, File1); + Assert.Equal(FormattedString, file1Content); + + var file2Content = await GetContentOfDocumentFromWorkspace(host, File2); + Assert.Equal(UnformattedString, file2Content); + } + + [Fact] + public async Task WhenFixAllIsScopedToSolution_ThenFixInSolution() + { + + const string UnformattedString = @" +partial class C {} +"; + + const string FormattedString = @" +internal partial class C {} +"; + + using var host = GetHost(true); + var file1 = CreateTestProjectWithDocument(host, UnformattedString); + var file2 = CreateTestProjectWithDocument(host, UnformattedString); + + var handler = host.GetRequestHandler(OmniSharpEndpoints.RunFixAll); + + await handler.Handle(new RunFixAllRequest + { + Scope = FixAllScope.Solution, + FileName = file1, + FixAllFilter = new[] { new FixAllItem("IDE0040", "Fix formatting") }, + WantsTextChanges = true, + WantsAllCodeActionOperations = true + }); + + var file1Content = await GetContentOfDocumentFromWorkspace(host, file1); + Assert.Equal(FormattedString, file1Content); + + var file2Content = await GetContentOfDocumentFromWorkspace(host, file2); + Assert.Equal(FormattedString, file2Content); + } + + [Fact] + public async Task IntersectingFixAllLocations_ApplyCorrectly() + { + using var host = GetHost(true); + + var file = CreateTestProjectWithDocument(host, @" +class C +{ + object o = 1; +}"); + + var handler = host.GetRequestHandler(OmniSharpEndpoints.RunFixAll); + + await handler.Handle(new RunFixAllRequest + { + Scope = FixAllScope.Document, + FileName = file, + WantsTextChanges = true, + WantsAllCodeActionOperations = true + }); + + var file1Content = await GetContentOfDocumentFromWorkspace(host, file); + Assert.Equal(@" +internal class C +{ + private readonly object o = 1; +}", file1Content); + } + + [Fact] // This is specifically tested because has custom mapping logic in it. public async Task WhenTextContainsUnusedImports_ThenTheyCanBeAutomaticallyFixed() { @@ -295,6 +384,32 @@ await Assert.ThrowsAsync(async () => await handler.Hand } } + [Fact] + public async Task WhenApplyChangesIsFalseChangesAreNotApplied() + { + const string OriginalContent = "class C { }"; + const string ExpectedResponseContent = "internal "; + using var host = GetHost(roslynAnalyzersEnabled: true); + var file = CreateTestProjectWithDocument(host, OriginalContent); + + var handler = host.GetRequestHandler(OmniSharpEndpoints.RunFixAll); + + var response = await handler.Handle(new RunFixAllRequest + { + Scope = FixAllScope.Document, + FileName = file, + WantsTextChanges = true, + WantsAllCodeActionOperations = true, + FixAllFilter = null, // This means: try fix everything. + ApplyChanges = false + }); + + string textAfterFix = await GetContentOfDocumentFromWorkspace(host, file); + Assert.Equal(OriginalContent, textAfterFix); + + Assert.Equal(ExpectedResponseContent, ((ModifiedFileResponse)response.Changes.Single()).Changes.Single().NewText); + } + private OmniSharpTestHost GetHost(bool roslynAnalyzersEnabled) { return OmniSharpTestHost.Create(