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
Changes from 1 commit
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 @@ -75,16 +75,19 @@ public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeAct

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

var path = FilePathNormalizer.Normalize(actionParams.Uri.GetAbsoluteOrUNCPath());
var directoryName = Path.GetDirectoryName(path).AssumeNotNull();
var importsBuilder = new StringBuilder();
var usingsInImportsFile = GetUsingsInImportsFile(directoryName, importsBuilder);

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 = context.SourceText.ToString()
Copy link
Contributor

Choose a reason for hiding this comment

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

In general in tooling we don't want to read the sourcetext directly to get syntax. It's better to use the syntax tree. In this case it will be a CSharpStatementLiteral where there's a keyword of using in it

Copy link
Contributor

Choose a reason for hiding this comment

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

the other thing is that the text in the using may not be the full namespace. Since usings are put inside the class they can be relative. So if you have a component MyApp.MyComponents.Header and a component MyApp.HomePage with @using MyComponents then Header is valid to use

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

perfect, I should have checked if there was an extension or helper

.Split('\n')
.Where(line => line.TrimStart().StartsWith("@using"))
.Aggregate(new StringBuilder(), (sb, line) => sb.AppendLine(line))
.ToString();

AddUsingDirectivesInRange(utilityScanRoot,
usingsInImportsFile,
usingsInFile,
actionParams.ExtractStart,
actionParams.ExtractEnd,
actionParams);
Expand Down Expand Up @@ -169,29 +172,6 @@ private static ExtractToComponentCodeActionParams CreateInitialActionParams(Razo
};
}

// When adding usings, we must check for the first _Imports.razor file in the current directory or any parent directories.
// In the new component, only add usings that are not already present in the _Imports.razor file.
public static string GetUsingsInImportsFile(string currentDirectory, StringBuilder importsBuilder)
{
var importsFile = Path.Combine(currentDirectory, "_Imports.razor");
if (File.Exists(importsFile))
{
return File.ReadAllLines(importsFile)
.Where(line => line.TrimStart().StartsWith("@using"))
.Select(line => line.Trim())
.Aggregate(importsBuilder, (sb, line) => sb.AppendLine(line))
.ToString();
}

var parentDirectory = Path.GetDirectoryName(currentDirectory);
if (parentDirectory is not null && Directory.Exists(parentDirectory))
{
return GetUsingsInImportsFile(parentDirectory, importsBuilder);
}

return string.Empty;
}

/// <summary>
/// Processes a multi-point selection to determine the correct range for extraction.
/// </summary>
Expand Down Expand Up @@ -307,7 +287,7 @@ MarkupTagHelperAttributeSyntax or
return null;
}

private static void AddUsingDirectivesInRange(SyntaxNode root, string usingsInImportsFile, int extractStart, int extractEnd, ExtractToComponentCodeActionParams actionParams)
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);
Expand All @@ -316,7 +296,7 @@ private static void AddUsingDirectivesInRange(SyntaxNode root, string usingsInIm
{
if (node is MarkupTagHelperElementSyntax { TagHelperInfo: { } tagHelperInfo })
{
AddUsingFromTagHelperInfo(tagHelperInfo, components, usingsInImportsFile, actionParams);
AddUsingFromTagHelperInfo(tagHelperInfo, components, usingsInFile, actionParams);
}
}
}
Expand All @@ -331,13 +311,9 @@ private static void AddUsingFromTagHelperInfo(TagHelperInfo tagHelperInfo, HashS
}

var typeNamespace = descriptor.GetTypeNamespace();
if (components.Add(typeNamespace))
if (components.Add(typeNamespace) && usingsInImportsFile.Contains(typeNamespace))
{
var usingDirective = $"@using {typeNamespace}";
if (!usingsInImportsFile.Contains(usingDirective))
{
actionParams.usingDirectives.Add(usingDirective);
}
actionParams.usingDirectives.Add($"@using {typeNamespace}");
}
}
}
Expand Down