Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple FixAll from the workspace #1962

Merged
merged 11 commits into from
Oct 8, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
29 changes: 29 additions & 0 deletions src/OmniSharp.Roslyn.CSharp/Helpers/CodeFixProviderExtensions.cs
Original file line number Diff line number Diff line change
@@ -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<string, string> _customDiagVsFixMap = new Dictionary<string, string>
{
{ "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]));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnost
internal static IEnumerable<DiagnosticLocation> DistinctDiagnosticLocationsByProject(this IEnumerable<DocumentDiagnostics> 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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray<Docume
{
var diagnosticLocations = diagnostics
.Where(x => string.IsNullOrEmpty(fileName)
|| x.DocumentPath == fileName)
|| x.Document.FilePath == fileName)
.DistinctDiagnosticLocationsByProject()
.Where(x => x.FileName != null);

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
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;
Expand All @@ -28,17 +30,30 @@ CachingCodeFixProviderForProjects codeFixesForProject

public override async Task<GetFixAllResponse> 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<FixAllItem>.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);
}
}
}
}
Loading