Skip to content

Commit

Permalink
Clean up code actions code (#10582)
Browse files Browse the repository at this point in the history 
This PR is a style-only clean up of our code actions handlers and
related code. Definitely could have gone further (more collection
pooling, returning concrete types etc.) but I wanted to try to have it
serve as an indication for what we consider our current idiomatic coding
style* and to avoid conflicts with the extract to component work.



<sub>* Current style is subject to change at any time, and are not well
documented. To quote a great Australian film, [it's just the
vibe](https://www.youtube.com/watch?v=nMuh33BMZYY).</sub>
  • Loading branch information
@davidwengier
davidwengier authored Jul 9, 2024
2 parents f746561 + ce9be80 commit 63e225d
Show file tree
Hide file tree
Showing 29 changed files with 158 additions and 399 deletions.
2 changes: 1 addition & 1 deletion ...icrosoft.AspNetCore.Razor.LanguageServer/CodeActions/AddUsingsCodeActionProviderHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static async Task<TextEdit[]> GetUsingStatementEditsAsync(RazorCodeDocume
edits.AddRange(workspaceEdit.DocumentChanges!.Value.First.First().Edits);
}

return edits.ToArray();
return [.. edits];
}

private static async Task<IEnumerable<string>> FindUsingDirectiveStringsAsync(SourceText originalCSharpText, CancellationToken cancellationToken)
Expand Down
6 changes: 1 addition & 5 deletions .../Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/CSharpCodeActionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

internal abstract class CSharpCodeActionResolver : BaseDelegatedCodeActionResolver
internal abstract class CSharpCodeActionResolver(IClientConnection clientConnection) : BaseDelegatedCodeActionResolver(clientConnection)
{
public CSharpCodeActionResolver(IClientConnection clientConnection)
: base(clientConnection)
{
}
}
39 changes: 6 additions & 33 deletions ...oft.AspNetCore.Razor.LanguageServer/CodeActions/CSharp/DefaultCSharpCodeActionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

internal sealed class DefaultCSharpCodeActionResolver : CSharpCodeActionResolver
internal sealed class DefaultCSharpCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorFormattingService razorFormattingService) : CSharpCodeActionResolver(clientConnection)
{
// Usually when we need to format code, we utilize the formatting options provided
// by the platform. However, we aren't provided such options in the case of code actions
Expand All @@ -33,28 +36,8 @@ internal sealed class DefaultCSharpCodeActionResolver : CSharpCodeActionResolver
},
};

private readonly IDocumentContextFactory _documentContextFactory;
private readonly IRazorFormattingService _razorFormattingService;

public DefaultCSharpCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorFormattingService razorFormattingService)
: base(clientConnection)
{
if (documentContextFactory is null)
{
throw new ArgumentNullException(nameof(documentContextFactory));
}

if (razorFormattingService is null)
{
throw new ArgumentNullException(nameof(razorFormattingService));
}

_documentContextFactory = documentContextFactory;
_razorFormattingService = razorFormattingService;
}
private readonly IDocumentContextFactory _documentContextFactory = documentContextFactory;
private readonly IRazorFormattingService _razorFormattingService = razorFormattingService;

public override string Action => LanguageServerConstants.CodeActions.Default;

Expand All @@ -63,16 +46,6 @@ public async override Task<CodeAction> ResolveAsync(
CodeAction codeAction,
CancellationToken cancellationToken)
{
if (csharpParams is null)
{
throw new ArgumentNullException(nameof(csharpParams));
}

if (codeAction is null)
{
throw new ArgumentNullException(nameof(codeAction));
}

if (!_documentContextFactory.TryCreateForOpenDocument(csharpParams.RazorFileIdentifier, out var documentContext))
{
return codeAction;
Expand Down
14 changes: 2 additions & 12 deletions ...AspNetCore.Razor.LanguageServer/CodeActions/CSharp/TypeAccessibilityCodeActionProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,12 @@ internal sealed class TypeAccessibilityCodeActionProvider : ICSharpCodeActionPro
IEnumerable<RazorVSInternalCodeAction> codeActions,
CancellationToken cancellationToken)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}

if (codeActions is null)
{
throw new ArgumentNullException(nameof(codeActions));
}

if (context.Request?.Context?.Diagnostics is null)
{
return SpecializedTasks.AsNullable(SpecializedTasks.EmptyReadOnlyList<RazorVSInternalCodeAction>());
}

if (codeActions is null || !codeActions.Any())
if (!codeActions.Any())
{
return SpecializedTasks.AsNullable(SpecializedTasks.EmptyReadOnlyList<RazorVSInternalCodeAction>());
}
Expand Down Expand Up @@ -276,7 +266,7 @@ private static RazorVSInternalCodeAction CreateFQNCodeAction(
var fqnWorkspaceEditDocumentChange = new TextDocumentEdit()
{
TextDocument = codeDocumentIdentifier,
Edits = new[] { fqnTextEdit },
Edits = [fqnTextEdit],
};

var fqnWorkspaceEdit = new WorkspaceEdit()
Expand Down
31 changes: 7 additions & 24 deletions ...e.Razor.LanguageServer/CodeActions/CSharp/UnformattedRemappingCSharpCodeActionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,13 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;
/// <summary>
/// Resolves and remaps the code action, without running formatting passes.
/// </summary>
internal sealed class UnformattedRemappingCSharpCodeActionResolver : CSharpCodeActionResolver
internal sealed class UnformattedRemappingCSharpCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorDocumentMappingService documentMappingService) : CSharpCodeActionResolver(clientConnection)
{
private readonly IDocumentContextFactory _documentContextFactory;
private readonly IRazorDocumentMappingService _documentMappingService;

public UnformattedRemappingCSharpCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorDocumentMappingService documentMappingService)
: base(clientConnection)
{
_documentContextFactory = documentContextFactory ?? throw new ArgumentNullException(nameof(documentContextFactory));
_documentMappingService = documentMappingService ?? throw new ArgumentNullException(nameof(documentMappingService));
}
private readonly IDocumentContextFactory _documentContextFactory = documentContextFactory;
private readonly IRazorDocumentMappingService _documentMappingService = documentMappingService;

public override string Action => LanguageServerConstants.CodeActions.UnformattedRemap;

Expand All @@ -42,16 +35,6 @@ public async override Task<CodeAction> ResolveAsync(
CodeAction codeAction,
CancellationToken cancellationToken)
{
if (csharpParams is null)
{
throw new ArgumentNullException(nameof(csharpParams));
}

if (codeAction is null)
{
throw new ArgumentNullException(nameof(codeAction));
}

cancellationToken.ThrowIfCancellationRequested();

if (!_documentContextFactory.TryCreateForOpenDocument(csharpParams.RazorFileIdentifier, out var documentContext))
Expand Down Expand Up @@ -112,7 +95,7 @@ public async override Task<CodeAction> ResolveAsync(
new TextDocumentEdit()
{
TextDocument = codeDocumentIdentifier,
Edits = new[] { textEdit },
Edits = [textEdit],
}
},
};
Expand Down
19 changes: 7 additions & 12 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ internal sealed class CodeActionEndpoint(

public async Task<SumType<Command, CodeAction>[]?> HandleRequestAsync(VSCodeActionParams request, RazorRequestContext requestContext, CancellationToken cancellationToken)
{
if (request is null)
{
throw new ArgumentNullException(nameof(request));
}

var documentContext = requestContext.DocumentContext;
if (documentContext is null)
{
Expand Down Expand Up @@ -127,12 +122,12 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V

serverCapabilities.CodeActionProvider = new CodeActionOptions
{
CodeActionKinds = new[]
{
CodeActionKinds =
[
CodeActionKind.RefactorExtract,
CodeActionKind.QuickFix,
CodeActionKind.Refactor
},
],
ResolveProvider = true,
};
}
Expand Down Expand Up @@ -186,13 +181,13 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeAc
// No point delegating if we're in a Razor context
if (languageKind == RazorLanguageKind.Razor)
{
return ImmutableArray<RazorVSInternalCodeAction>.Empty;
return [];
}

var codeActions = await GetCodeActionsFromLanguageServerAsync(languageKind, documentContext, context, correlationId, cancellationToken).ConfigureAwait(false);
if (codeActions is not [_, ..])
{
return ImmutableArray<RazorVSInternalCodeAction>.Empty;
return [];
}

IEnumerable<ICodeActionProvider> providers;
Expand Down Expand Up @@ -268,7 +263,7 @@ internal async Task<RazorVSInternalCodeAction[]> GetCodeActionsFromLanguageServe
// For C# we have to map the ranges to the generated document
if (!_documentMappingService.TryMapToGeneratedDocumentRange(context.CodeDocument.GetCSharpDocument(), context.Request.Range, out var projectedRange))
{
return Array.Empty<RazorVSInternalCodeAction>();
return [];
}

var newContext = context.Request.Context;
Expand Down Expand Up @@ -301,7 +296,7 @@ internal async Task<RazorVSInternalCodeAction[]> GetCodeActionsFromLanguageServe
{
_telemetryReporter?.ReportFault(e, "Error getting code actions from delegate language server for {languageKind}", languageKind);
_logger.LogError(e, $"Error getting code actions from delegate language server for {languageKind}");
return Array.Empty<RazorVSInternalCodeAction>();
return [];
}
}

Expand Down
53 changes: 9 additions & 44 deletions ...or/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionResolveEndpoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,63 +17,28 @@
namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

[RazorLanguageServerEndpoint(Methods.CodeActionResolveName)]
internal sealed class CodeActionResolveEndpoint : IRazorDocumentlessRequestHandler<CodeAction, CodeAction>
internal sealed class CodeActionResolveEndpoint(
IEnumerable<IRazorCodeActionResolver> razorCodeActionResolvers,
IEnumerable<CSharpCodeActionResolver> csharpCodeActionResolvers,
IEnumerable<HtmlCodeActionResolver> htmlCodeActionResolvers,
ILoggerFactory loggerFactory) : IRazorDocumentlessRequestHandler<CodeAction, CodeAction>
{
private readonly ImmutableDictionary<string, IRazorCodeActionResolver> _razorCodeActionResolvers;
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _csharpCodeActionResolvers;
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _htmlCodeActionResolvers;
private readonly ILogger _logger;
private readonly ImmutableDictionary<string, IRazorCodeActionResolver> _razorCodeActionResolvers = CreateResolverMap(razorCodeActionResolvers);
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _csharpCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(csharpCodeActionResolvers);
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _htmlCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(htmlCodeActionResolvers);
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<CodeActionResolveEndpoint>();

public bool MutatesSolutionState => false;

public CodeActionResolveEndpoint(
IEnumerable<IRazorCodeActionResolver> razorCodeActionResolvers,
IEnumerable<CSharpCodeActionResolver> csharpCodeActionResolvers,
IEnumerable<HtmlCodeActionResolver> htmlCodeActionResolvers,
ILoggerFactory loggerFactory)
{
if (razorCodeActionResolvers is null)
{
throw new ArgumentNullException(nameof(razorCodeActionResolvers));
}

if (htmlCodeActionResolvers is null)
{
throw new ArgumentNullException(nameof(htmlCodeActionResolvers));
}

if (csharpCodeActionResolvers is null)
{
throw new ArgumentNullException(nameof(csharpCodeActionResolvers));
}

if (loggerFactory is null)
{
throw new ArgumentNullException(nameof(loggerFactory));
}

_logger = loggerFactory.GetOrCreateLogger<CodeActionResolveEndpoint>();

_razorCodeActionResolvers = CreateResolverMap(razorCodeActionResolvers);
_csharpCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(csharpCodeActionResolvers);
_htmlCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(htmlCodeActionResolvers);
}

public async Task<CodeAction> HandleRequestAsync(CodeAction request, RazorRequestContext requestContext, CancellationToken cancellationToken)
{
if (request is null)
{
throw new ArgumentNullException(nameof(request));
}

if (request.Data is not JsonElement paramsObj)
{
_logger.LogError($"Invalid CodeAction Received '{request.Title}'.");
return request;
}

var resolutionParams = paramsObj.Deserialize<RazorCodeActionResolutionParams>();

if (resolutionParams is null)
{
throw new ArgumentOutOfRangeException($"request.Data should be convertible to {nameof(RazorCodeActionResolutionParams)}");
Expand Down
9 changes: 2 additions & 7 deletions ...crosoft.AspNetCore.Razor.LanguageServer/CodeActions/Html/DefaultHtmlCodeActionProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,9 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

internal sealed class DefaultHtmlCodeActionProvider : IHtmlCodeActionProvider
internal sealed class DefaultHtmlCodeActionProvider(IRazorDocumentMappingService documentMappingService) : IHtmlCodeActionProvider
{
private readonly IRazorDocumentMappingService _documentMappingService;

public DefaultHtmlCodeActionProvider(IRazorDocumentMappingService documentMappingService)
{
_documentMappingService = documentMappingService;
}
private readonly IRazorDocumentMappingService _documentMappingService = documentMappingService;

public async Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(
RazorCodeActionContext context,
Expand Down
39 changes: 6 additions & 33 deletions ...crosoft.AspNetCore.Razor.LanguageServer/CodeActions/Html/DefaultHtmlCodeActionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,13 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

internal sealed class DefaultHtmlCodeActionResolver : HtmlCodeActionResolver
internal sealed class DefaultHtmlCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorDocumentMappingService documentMappingService) : HtmlCodeActionResolver(clientConnection)
{
private readonly IDocumentContextFactory _documentContextFactory;
private readonly IRazorDocumentMappingService _documentMappingService;

public DefaultHtmlCodeActionResolver(
IDocumentContextFactory documentContextFactory,
IClientConnection clientConnection,
IRazorDocumentMappingService documentMappingService)
: base(clientConnection)
{
if (documentContextFactory is null)
{
throw new ArgumentNullException(nameof(documentContextFactory));
}

if (documentMappingService is null)
{
throw new ArgumentNullException(nameof(documentMappingService));
}

_documentContextFactory = documentContextFactory;
_documentMappingService = documentMappingService;
}
private readonly IDocumentContextFactory _documentContextFactory = documentContextFactory;
private readonly IRazorDocumentMappingService _documentMappingService = documentMappingService;

public override string Action => LanguageServerConstants.CodeActions.Default;

Expand All @@ -45,16 +28,6 @@ public async override Task<CodeAction> ResolveAsync(
CodeAction codeAction,
CancellationToken cancellationToken)
{
if (resolveParams is null)
{
throw new ArgumentNullException(nameof(resolveParams));
}

if (codeAction is null)
{
throw new ArgumentNullException(nameof(codeAction));
}

if (!_documentContextFactory.TryCreateForOpenDocument(resolveParams.RazorFileIdentifier, out var documentContext))
{
return codeAction;
Expand Down
6 changes: 1 addition & 5 deletions .../src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/Html/HtmlCodeActionResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

internal abstract class HtmlCodeActionResolver : BaseDelegatedCodeActionResolver
internal abstract class HtmlCodeActionResolver(IClientConnection clientConnection) : BaseDelegatedCodeActionResolver(clientConnection)
{
public HtmlCodeActionResolver(IClientConnection clientConnection)
: base(clientConnection)
{
}
}
2 changes: 2 additions & 0 deletions ...Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/Models/AddUsingsCodeActionParams.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ internal sealed class AddUsingsCodeActionParams
{
[JsonPropertyName("uri")]
public required Uri Uri { get; set; }

[JsonPropertyName("namespace")]
public required string Namespace { get; set; }

[JsonPropertyName("additionalEdit")]
public TextDocumentEdit? AdditionalEdit { get; set; }
}
Loading

0 comments on commit 63e225d

Please sign in to comment.