Skip to content

Commit

Permalink
Fix all providers support (#1581)
Browse files Browse the repository at this point in the history
* Something to begin with fix all providers.

* Basic routine for fix all dummy version.

* Added api etc for fixall.

* Initial version that should return changes.

* For some reason file changes are not serialized correctly, fix routine seems to work.

* Created more revelant service files.

* Refactored some logig down to base class.

* Added missing attribute.

* Fixed invalid import for importingConstructor.

* Refactored code a bit.

* Added expected result after everything is fixed.

* Moved assert as utility.

* Moved fix all abstractions to correct location.

* Added tests and support for simple get api for available fixes.

* Moved certain logic down to fixallprovider.

* Refactored diagnostic to expose more context internally for fix all.

* Minor regression fixes.

* Refactoring and added correct scoping for tests.

* Now fix all providers works as intended.

* Refactoring.

* Refactoring.

* Removed need for workaround on tessts.

* Refactored initialazation logig with analyzer workers.

* Testfix.

* Added scoping for fix all run service.

* Improved test to contain asserts for changes.

* Refactored tests.

* Todo marker test for unused import fix (doesn't pass yet).

* Added support for action filtering.

* Tweaks.

* Fix for diagnostic blocks.

* Fixed bug.

* Added logging for executed fix all actions.

* Hardcoded trick to get RCSxxxx work

* Refactoring and now remove unnecessary imports is working as intended

* Fixes for 'fix everything'

* Updates to work with unused usings

* Small tweaks for fixing

* Refactoring

* Improved initialization logig and fixed few tests

* Refactoring for services and base class

* Some test fixes

* Additional test fixes

* Blocked fix everything in project and solution scope in first version

* Fixed asyncronity issue

* Tweaks and fix for hanging ci test runs

* Added comment to improve readibility

* Testfix

* Fixed typo

* Tweak for fix all action sorting and grouping

* Removed unused method

* Renamame for events to keep backward compatibility and timeout update

* Testing to use existing routines from code actions with fix all actions

* Refactoring logig down to base classes and test fix

* Added support for get all text and wants changes

* Correctly pass wantsTextChanges and wantscodeactions to fix all service

* Added missing Wants* flags to tests

* Buildfix

* Removed unused method

* Ignore casing on refactoring asserts

* Review fixes

* Added AssertIgnoringIndentAndNewlines and removed unused code

* fixed compliation error

* added skip for single failing test

Co-authored-by: David Driscoll <[email protected]>
Co-authored-by: Martin Björkström <[email protected]>
Co-authored-by: Filip W <[email protected]>
  • Loading branch information
4 people authored Jul 30, 2020
1 parent 1d552c5 commit f7c6f88
Show file tree
Hide file tree
Showing 33 changed files with 1,045 additions and 287 deletions.
15 changes: 15 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/FixAllItem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
namespace OmniSharp.Abstractions.Models.V1.FixAll
{
public class FixAllItem
{
public FixAllItem() {}
public FixAllItem(string id, string message)
{
Id = id;
Message = message;
}

public string Id { get; set; }
public string Message { get; set; }
}
}
11 changes: 11 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/GetFixAllRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using OmniSharp.Mef;
using OmniSharp.Models;

namespace OmniSharp.Abstractions.Models.V1.FixAll
{
[OmniSharpEndpoint(OmniSharpEndpoints.GetFixAll, typeof(GetFixAllRequest), typeof(GetFixAllResponse))]
public class GetFixAllRequest: SimpleFileRequest
{
public FixAllScope Scope { get; set; } = FixAllScope.Document;
}
}
14 changes: 14 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/GetFixAllResponse.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System.Collections.Generic;

namespace OmniSharp.Abstractions.Models.V1.FixAll
{
public class GetFixAllResponse
{
public GetFixAllResponse(IEnumerable<FixAllItem> fixableItems)
{
Items = fixableItems;
}

public IEnumerable<FixAllItem> Items { get; set; }
}
}
17 changes: 17 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using OmniSharp.Mef;
using OmniSharp.Models;

namespace OmniSharp.Abstractions.Models.V1.FixAll
{
[OmniSharpEndpoint(OmniSharpEndpoints.RunFixAll, typeof(RunFixAllRequest), typeof(RunFixAllResponse))]
public class RunFixAllRequest : SimpleFileRequest
{
public FixAllScope Scope { get; set; } = FixAllScope.Document;

// If this is null -> filter not set -> try to fix all issues in current defined scope.
public FixAllItem[] FixAllFilter { get; set; }
public int Timeout { get; set; } = 3000;
public bool WantsAllCodeActionOperations { get; set; }
public bool WantsTextChanges { get; set; }
}
}
17 changes: 17 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllResponse.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System.Collections.Generic;
using OmniSharp.Models;

namespace OmniSharp.Abstractions.Models.V1.FixAll
{
public class RunFixAllResponse : IAggregateResponse
{
public RunFixAllResponse()
{
Changes = new List<FileOperationResponse>();
}

public IEnumerable<FileOperationResponse> Changes { get; set; }

public IAggregateResponse Merge(IAggregateResponse response) { return response; }
}
}
9 changes: 9 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/FixAll/RunFixAllScope.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace OmniSharp.Abstractions.Models.V1.FixAll
{
public enum FixAllScope
{
Document = 0,
Project = 1,
Solution = 2
}
}
3 changes: 2 additions & 1 deletion src/OmniSharp.Abstractions/OmniSharpEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public static class OmniSharpEndpoints
public const string WorkspaceInformation = "/projects";
public const string ProjectInformation = "/project";
public const string FixUsings = "/fixusings";

public const string RunFixAll = "/runfixall";
public const string GetFixAll = "/getfixall";
public const string CheckAliveStatus = "/checkalivestatus";
public const string CheckReadyStatus = "/checkreadystatus";
public const string StopServer = "/stopserver";
Expand Down
9 changes: 5 additions & 4 deletions src/OmniSharp.Roslyn.CSharp/Helpers/DiagnosticExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
using Microsoft.CodeAnalysis;
using OmniSharp.Models.Diagnostics;
using OmniSharp.Roslyn.CSharp.Services.Diagnostics;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;

namespace OmniSharp.Helpers
{
internal static class DiagnosticExtensions
{
private static readonly ImmutableHashSet<string> _tagFilter =
ImmutableHashSet.Create<string>("Unnecessary");
ImmutableHashSet.Create("Unnecessary");

internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnostic)
{
Expand All @@ -32,9 +32,10 @@ internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnost
};
}

internal static IEnumerable<DiagnosticLocation> DistinctDiagnosticLocationsByProject(this IEnumerable<(string projectName, Diagnostic diagnostic)> diagnostics)
internal static IEnumerable<DiagnosticLocation> DistinctDiagnosticLocationsByProject(this IEnumerable<DocumentDiagnostics> documentDiagnostic)
{
return diagnostics
return documentDiagnostic
.SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.ProjectName, diagnostic: child))
.Select(x => new
{
location = x.diagnostic.ToDiagnosticLocation(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading.Tasks;
Expand All @@ -9,7 +8,6 @@
using OmniSharp.Mef;
using OmniSharp.Models;
using OmniSharp.Models.CodeCheck;
using OmniSharp.Models.Diagnostics;
using OmniSharp.Options;
using OmniSharp.Roslyn.CSharp.Workers.Diagnostics;

Expand Down Expand Up @@ -45,11 +43,11 @@ public async Task<QuickFixResponse> Handle(CodeCheckRequest request)
return GetResponseFromDiagnostics(diagnostics, request.FileName);
}

private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray<(string projectName, Diagnostic diagnostic)> diagnostics, string fileName)
private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray<DocumentDiagnostics> diagnostics, string fileName)
{
var diagnosticLocations = diagnostics
.Where(x => (string.IsNullOrEmpty(fileName)
|| x.diagnostic.Location.GetLineSpan().Path == fileName))
.Where(x => string.IsNullOrEmpty(fileName)
|| x.DocumentPath == fileName)
.DistinctDiagnosticLocationsByProject()
.Where(x => x.FileName != null);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
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<string, string> _customDiagVsFixMap = new Dictionary<string, string>
{
{ "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<DocumentWithFixProvidersAndMatchingDiagnostics> CreateWithMatchingProviders(ImmutableArray<CodeFixProvider> 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<string> 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());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System.Collections.Generic;
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.Services.Refactoring.V2;
using OmniSharp.Roslyn.CSharp.Workers.Diagnostics;
using OmniSharp.Services;

namespace OmniSharp.Roslyn.CSharp.Services.Refactoring
{
[OmniSharpHandler(OmniSharpEndpoints.GetFixAll, LanguageNames.CSharp)]
public class GetFixAllCodeActionService : BaseCodeActionService<GetFixAllRequest, GetFixAllResponse>
{
[ImportingConstructor]
public GetFixAllCodeActionService(
OmniSharpWorkspace workspace,
[ImportMany] IEnumerable<ICodeActionProvider> providers,
ILoggerFactory loggerFactory,
ICsDiagnosticWorker diagnostics,
CachingCodeFixProviderForProjects codeFixesForProject
) : base(workspace, providers, loggerFactory.CreateLogger<GetFixAllCodeActionService>(), diagnostics, codeFixesForProject)
{
}

public override async Task<GetFixAllResponse> Handle(GetFixAllRequest request)
{
var availableFixes = await GetDiagnosticsMappedWithFixAllProviders(request.Scope, request.FileName);

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))
.OrderBy(x => x.Id)
.ToArray();

return new GetFixAllResponse(distinctDiagnosticsThatCanBeFixed);
}
}
}
Loading

0 comments on commit f7c6f88

Please sign in to comment.