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

Including @using for Out-of-Scope Razor Component References #10651

Merged
merged 19 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,27 @@
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Text.Json.Serialization;

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;

internal sealed class ExtractToNewComponentCodeActionParams
// NOTE: As mentioned before, these have changed in future PRs, where much of the Provider logic was moved to the resolver.
// The last three properties are not used in the current implementation.
internal sealed class ExtractToComponentCodeActionParams
{
[JsonPropertyName("uri")]
public required Uri Uri { get; set; }

[JsonPropertyName("extractStart")]
public int ExtractStart { get; set; }

[JsonPropertyName("extractEnd")]
public int ExtractEnd { get; set; }

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

[JsonPropertyName("usingDirectives")]
public required List<string> usingDirectives { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,33 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using ICSharpCode.Decompiler.CSharp.Syntax;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Components;
using Microsoft.AspNetCore.Razor.Language.Extensions;
using Microsoft.AspNetCore.Razor.Language.Intermediate;
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.AspNetCore.Razor.Threading;
using Microsoft.AspNetCore.Razor.Utilities;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Range = Microsoft.VisualStudio.LanguageServer.Protocol.Range;

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Razor;

internal sealed class ExtractToNewComponentCodeActionProvider(ILoggerFactory loggerFactory) : IRazorCodeActionProvider
internal sealed class ExtractToComponentCodeActionProvider(ILoggerFactory loggerFactory) : IRazorCodeActionProvider
{
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<ExtractToNewComponentCodeActionProvider>();
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<ExtractToComponentCodeActionProvider>();

public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeActionContext context, CancellationToken cancellationToken)
{
Expand All @@ -51,31 +56,51 @@ public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeAct
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

var (startElementNode, endElementNode) = GetStartAndEndElements(context, syntaxTree, _logger);

// Make sure the selection starts on an element tag
var (startElementNode, endElementNode) = GetStartAndEndElements(context, syntaxTree, _logger);
if (startElementNode is null)
{
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

if (endElementNode is null)
{
endElementNode = startElementNode;
}

if (!TryGetNamespace(context.CodeDocument, out var @namespace))
{
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

var actionParams = CreateInitialActionParams(context, startElementNode, @namespace);
var actionParams = CreateInitialActionParams(context, startElementNode, @namespace);

ProcessSelection(startElementNode, endElementNode, actionParams);

var utilityScanRoot = FindNearestCommonAncestor(startElementNode, endElementNode) ?? startElementNode;

// The new component usings are going to be a subset of the usings in the source razor file
var usingsInFile = syntaxTree.Root.DescendantNodes()
.OfType<CSharpStatementLiteralSyntax>()
.Where(literal => literal.SerializedValue.Contains("using"))
.Select(literal => literal.SerializedValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close but this is still wrong because this will be true for statements where a variable has using in the name (and worth adding a test for that case). I think this is correct but I'm coding in github so 🤷

Suggested change
.OfType<CSharpStatementLiteralSyntax>()
.Where(literal => literal.SerializedValue.Contains("using"))
.Select(literal => literal.SerializedValue)
.OfType<CSharpStatementLiteralSyntax>()
.Where(statement => statement.LiteralTokens.FirstOrDefault() is SyntaxToken { Kind: SyntaxKind.Keyword } token && CSharpTokenizer.GetTokenKeyword(token) == CSharpSyntaxKind.UsingKeyword))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, you're right

.Aggregate(new StringBuilder(), (sb, line) => sb.AppendLine(line))
.ToString();

AddUsingDirectivesInRange(utilityScanRoot,
usingsInFile,
actionParams.ExtractStart,
actionParams.ExtractEnd,
actionParams);

var resolutionParams = new RazorCodeActionResolutionParams()
{
Action = LanguageServerConstants.CodeActions.ExtractToNewComponentAction,
Language = LanguageServerConstants.CodeActions.Languages.Razor,
Data = actionParams,
};

var codeAction = RazorCodeActionFactory.CreateExtractToNewComponent(resolutionParams);
var codeAction = RazorCodeActionFactory.CreateExtractToComponent(resolutionParams);
return Task.FromResult<ImmutableArray<RazorVSInternalCodeAction>>([codeAction]);
}

Expand All @@ -95,6 +120,7 @@ private static (MarkupElementSyntax? Start, MarkupElementSyntax? End) GetStartAn
}

var endElementNode = GetEndElementNode(context, syntaxTree);

return (startElementNode, endElementNode);
}

Expand Down Expand Up @@ -135,14 +161,15 @@ private static bool IsInsideProperHtmlContent(RazorCodeActionContext context, Ma
return endOwner.FirstAncestorOrSelf<MarkupElementSyntax>();
}

private static ExtractToNewComponentCodeActionParams CreateInitialActionParams(RazorCodeActionContext context, MarkupElementSyntax startElementNode, string @namespace)
private static ExtractToComponentCodeActionParams CreateInitialActionParams(RazorCodeActionContext context, MarkupElementSyntax startElementNode, string @namespace)
{
return new ExtractToNewComponentCodeActionParams
return new ExtractToComponentCodeActionParams
{
Uri = context.Request.TextDocument.Uri,
ExtractStart = startElementNode.Span.Start,
ExtractEnd = startElementNode.Span.End,
Namespace = @namespace
Namespace = @namespace,
usingDirectives = []
};
}

Expand All @@ -152,7 +179,7 @@ private static ExtractToNewComponentCodeActionParams CreateInitialActionParams(R
/// <param name="startElementNode">The starting element of the selection.</param>
/// <param name="endElementNode">The ending element of the selection, if it exists.</param>
/// <param name="actionParams">The parameters for the extraction action, which will be updated.</param>
private static void ProcessSelection(MarkupElementSyntax startElementNode, MarkupElementSyntax? endElementNode, ExtractToNewComponentCodeActionParams actionParams)
private static void ProcessSelection(MarkupElementSyntax startElementNode, MarkupElementSyntax? endElementNode, ExtractToComponentCodeActionParams actionParams)
{
// If there's no end element, we can't process a multi-point selection
if (endElementNode is null)
Expand Down Expand Up @@ -207,7 +234,7 @@ private static (SyntaxNode? Start, SyntaxNode? End) FindContainingSiblingPair(Sy
{
// Find the lowest common ancestor of both nodes
var nearestCommonAncestor = FindNearestCommonAncestor(startNode, endNode);
if (nearestCommonAncestor == null)
if (nearestCommonAncestor is null)
{
return (null, null);
}
Expand All @@ -223,7 +250,7 @@ private static (SyntaxNode? Start, SyntaxNode? End) FindContainingSiblingPair(Sy
{
var childSpan = child.Span;

if (startContainingNode == null && childSpan.Contains(startSpan))
if (startContainingNode is null && childSpan.Contains(startSpan))
{
startContainingNode = child;
if (endContainingNode is not null)
Expand All @@ -245,7 +272,10 @@ private static (SyntaxNode? Start, SyntaxNode? End) FindContainingSiblingPair(Sy
{
var current = node1;

while (current.Kind == SyntaxKind.MarkupElement && current is not null)
while (current is MarkupElementSyntax or
MarkupTagHelperAttributeSyntax or
MarkupBlockSyntax &&
current is not null)
{
if (current.Span.Contains(node2.Span))
{
Expand All @@ -257,4 +287,35 @@ private static (SyntaxNode? Start, SyntaxNode? End) FindContainingSiblingPair(Sy

return null;
}

private static void AddUsingDirectivesInRange(SyntaxNode root, string usingsInFile, int extractStart, int extractEnd, ExtractToComponentCodeActionParams actionParams)
{
var components = new HashSet<string>();
var extractSpan = new TextSpan(extractStart, extractEnd - extractStart);

foreach (var node in root.DescendantNodes().Where(node => extractSpan.Contains(node.Span)))
{
if (node is MarkupTagHelperElementSyntax { TagHelperInfo: { } tagHelperInfo })
{
AddUsingFromTagHelperInfo(tagHelperInfo, components, usingsInFile, actionParams);
}
}
}

private static void AddUsingFromTagHelperInfo(TagHelperInfo tagHelperInfo, HashSet<string> components, string usingsInImportsFile, ExtractToComponentCodeActionParams actionParams)
{
foreach (var descriptor in tagHelperInfo.BindingResult.Descriptors)
{
if (descriptor is null)
{
continue;
}

var typeNamespace = descriptor.GetTypeNamespace();
if (components.Add(typeNamespace) && usingsInImportsFile.Contains(typeNamespace))
{
actionParams.usingDirectives.Add($"@using {typeNamespace}");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Razor;

internal sealed class ExtractToNewComponentCodeActionResolver(
internal sealed class ExtractToComponentCodeActionResolver
(
Comment on lines +30 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
internal sealed class ExtractToComponentCodeActionResolver
(
internal sealed class ExtractToComponentCodeActionResolver(

IDocumentContextFactory documentContextFactory,
LanguageServerFeatureOptions languageServerFeatureOptions) : IRazorCodeActionResolver
{
Expand All @@ -44,7 +45,7 @@ internal sealed class ExtractToNewComponentCodeActionResolver(
return null;
}

var actionParams = JsonSerializer.Deserialize<ExtractToNewComponentCodeActionParams>(data.GetRawText());
var actionParams = JsonSerializer.Deserialize<ExtractToComponentCodeActionParams>(data.GetRawText());
if (actionParams is null)
{
return null;
Expand Down Expand Up @@ -90,7 +91,15 @@ internal sealed class ExtractToNewComponentCodeActionResolver(
}

var componentName = Path.GetFileNameWithoutExtension(componentPath);
var newComponentContent = text.GetSubTextString(new TextSpan(actionParams.ExtractStart, actionParams.ExtractEnd - actionParams.ExtractStart)).Trim();
var newComponentContent = string.Empty;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use a StringBuilder, not just concatenate strings, and use a pooled one at that.


newComponentContent += string.Join(Environment.NewLine, actionParams.usingDirectives);
if (actionParams.usingDirectives.Count > 0)
{
newComponentContent += Environment.NewLine + Environment.NewLine; // Ensure there's a newline after the dependencies if any exist.
}

newComponentContent += text.GetSubTextString(new CodeAnalysis.Text.TextSpan(actionParams.ExtractStart, actionParams.ExtractEnd - actionParams.ExtractStart)).Trim();

var start = componentDocument.Source.Text.Lines.GetLinePosition(actionParams.ExtractStart);
var end = componentDocument.Source.Text.Lines.GetLinePosition(actionParams.ExtractEnd);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal static class RazorCodeActionFactory
private readonly static Guid s_fullyQualifyComponentTelemetryId = new("3d9abe36-7d10-4e08-8c18-ad88baa9a923");
private readonly static Guid s_createComponentFromTagTelemetryId = new("a28e0baa-a4d5-4953-a817-1db586035841");
private readonly static Guid s_createExtractToCodeBehindTelemetryId = new("f63167f7-fdc6-450f-8b7b-b240892f4a27");
private readonly static Guid s_createExtractToNewComponentTelemetryId = new("af67b0a3-f84b-4808-97a7-b53e85b22c64");
private readonly static Guid s_createExtractToComponentTelemetryId = new("af67b0a3-f84b-4808-97a7-b53e85b22c64");
private readonly static Guid s_generateMethodTelemetryId = new("c14fa003-c752-45fc-bb29-3a123ae5ecef");
private readonly static Guid s_generateAsyncMethodTelemetryId = new("9058ca47-98e2-4f11-bf7c-a16a444dd939");

Expand Down Expand Up @@ -68,15 +68,15 @@ public static RazorVSInternalCodeAction CreateExtractToCodeBehind(RazorCodeActio
return codeAction;
}

public static RazorVSInternalCodeAction CreateExtractToNewComponent(RazorCodeActionResolutionParams resolutionParams)
public static RazorVSInternalCodeAction CreateExtractToComponent(RazorCodeActionResolutionParams resolutionParams)
{
var title = SR.ExtractTo_NewComponent_Title;
var title = SR.ExtractTo_Component_Title;
var data = JsonSerializer.SerializeToElement(resolutionParams);
var codeAction = new RazorVSInternalCodeAction()
{
Title = title,
Data = data,
TelemetryId = s_createExtractToNewComponentTelemetryId,
TelemetryId = s_createExtractToComponentTelemetryId,
};
return codeAction;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ public static void AddCodeActionsServices(this IServiceCollection services)
// Razor Code actions
services.AddSingleton<IRazorCodeActionProvider, ExtractToCodeBehindCodeActionProvider>();
services.AddSingleton<IRazorCodeActionResolver, ExtractToCodeBehindCodeActionResolver>();
services.AddSingleton<IRazorCodeActionProvider, ExtractToNewComponentCodeActionProvider>();
services.AddSingleton<IRazorCodeActionResolver ,ExtractToNewComponentCodeActionResolver>();
services.AddSingleton<IRazorCodeActionProvider, ExtractToComponentCodeActionProvider>();
services.AddSingleton<IRazorCodeActionResolver ,ExtractToComponentCodeActionResolver>();
services.AddSingleton<IRazorCodeActionProvider, ComponentAccessibilityCodeActionProvider>();
services.AddSingleton<IRazorCodeActionResolver, CreateComponentCodeActionResolver>();
services.AddSingleton<IRazorCodeActionResolver, AddUsingsCodeActionResolver>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
<data name="Statement" xml:space="preserve">
<value>statement</value>
</data>
<data name="ExtractTo_NewComponent_Title" xml:space="preserve">
<data name="ExtractTo_Component_Title" xml:space="preserve">
<value>Extract element to new component</value>
</data>
</root>

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading