-
Notifications
You must be signed in to change notification settings - Fork 212
Implement "Extract to Foo.razor.css" code action #11989
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
Changes from all commits
5da9c12
184300a
1628829
17eda37
4c2e17e
06ef015
6199e76
4dd2d93
292b41a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Text.Json.Serialization; | ||
|
||
namespace Microsoft.CodeAnalysis.Razor.CodeActions.Models; | ||
|
||
internal sealed class ExtractToCssCodeActionParams | ||
{ | ||
[JsonPropertyName("extractStart")] | ||
public int ExtractStart { get; set; } | ||
|
||
[JsonPropertyName("extractEnd")] | ||
public int ExtractEnd { get; set; } | ||
|
||
[JsonPropertyName("removeStart")] | ||
public int RemoveStart { get; set; } | ||
|
||
[JsonPropertyName("removeEnd")] | ||
public int RemoveEnd { get; set; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.AspNetCore.Razor.Language; | ||
using Microsoft.AspNetCore.Razor.Language.Syntax; | ||
using Microsoft.AspNetCore.Razor.Threading; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.Razor.CodeActions.Models; | ||
using Microsoft.CodeAnalysis.Razor.CodeActions.Razor; | ||
using Microsoft.CodeAnalysis.Razor.Logging; | ||
using Microsoft.CodeAnalysis.Razor.Protocol; | ||
|
||
namespace Microsoft.CodeAnalysis.Razor.CodeActions; | ||
|
||
internal class ExtractToCssCodeActionProvider(ILoggerFactory loggerFactory) : IRazorCodeActionProvider | ||
{ | ||
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<ExtractToCssCodeActionProvider>(); | ||
|
||
public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeActionContext context, CancellationToken cancellationToken) | ||
{ | ||
if (!context.SupportsFileCreation) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
if (!context.CodeDocument.FileKind.IsComponent()) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
if (!context.CodeDocument.TryGetSyntaxRoot(out var root)) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
var owner = root.FindInnermostNode(context.StartAbsoluteIndex); | ||
if (owner is null) | ||
{ | ||
_logger.LogWarning("Owner should never be null."); | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
// If we're inside an element, move to the start tag so the following checks work as expected | ||
if (owner is MarkupTextLiteralSyntax { Parent: MarkupElementSyntax { StartTag: { } startTag } }) | ||
{ | ||
owner = startTag; | ||
} | ||
|
||
// We have to be in a style tag (or inside it, but we'll have moved to the parent if so, above) | ||
if (owner is not (MarkupStartTagSyntax { Name.Content: "style" } or MarkupEndTagSyntax { Name.Content: "style" })) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
// If there is any C# or Razor in the style tag, we can't offer, so it has to be one big text literal. | ||
if (owner.Parent is not MarkupElementSyntax { Body: [MarkupTextLiteralSyntax textLiteral] } markupElement || | ||
textLiteral.ChildNodes().Any()) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
if (textLiteral.LiteralTokens.All(static t => t.IsWhitespace())) | ||
{ | ||
// If the text literal is all whitespace, we don't want to offer the action. | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
// If there are diagnostics, we can't trust the tree to be what we expect. | ||
if (markupElement.GetDiagnostics().Any(static d => d.Severity == RazorDiagnosticSeverity.Error)) | ||
{ | ||
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>(); | ||
} | ||
|
||
var actionParams = new ExtractToCssCodeActionParams() | ||
{ | ||
ExtractStart = textLiteral.Span.Start, | ||
ExtractEnd = textLiteral.Span.End, | ||
RemoveStart = markupElement.Span.Start, | ||
RemoveEnd = markupElement.Span.End | ||
}; | ||
|
||
var resolutionParams = new RazorCodeActionResolutionParams() | ||
{ | ||
TextDocument = context.Request.TextDocument, | ||
Action = LanguageServerConstants.CodeActions.ExtractToCss, | ||
Language = RazorLanguageKind.Razor, | ||
DelegatedDocumentUri = context.DelegatedDocumentUri, | ||
Data = actionParams, | ||
}; | ||
|
||
var razorFileName = Path.GetFileName(context.Request.TextDocument.DocumentUri.GetAbsoluteOrUNCPath()); | ||
var codeAction = RazorCodeActionFactory.CreateExtractToCss(razorFileName, resolutionParams); | ||
return Task.FromResult<ImmutableArray<RazorVSInternalCodeAction>>([codeAction]); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Buffers; | ||
using System.IO; | ||
using System.Text.Json; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.AspNetCore.Razor.PooledObjects; | ||
using Microsoft.CodeAnalysis.Razor.CodeActions.Models; | ||
using Microsoft.CodeAnalysis.Razor.Formatting; | ||
using Microsoft.CodeAnalysis.Razor.ProjectSystem; | ||
using Microsoft.CodeAnalysis.Razor.Protocol; | ||
using Microsoft.CodeAnalysis.Razor.Utilities; | ||
using Microsoft.CodeAnalysis.Razor.Workspaces; | ||
using Microsoft.CodeAnalysis.Text; | ||
|
||
namespace Microsoft.CodeAnalysis.Razor.CodeActions; | ||
|
||
internal class ExtractToCssCodeActionResolver( | ||
LanguageServerFeatureOptions languageServerFeatureOptions, | ||
IFileSystem fileSystem) : IRazorCodeActionResolver | ||
{ | ||
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions; | ||
private readonly IFileSystem _fileSystem = fileSystem; | ||
|
||
public string Action => LanguageServerConstants.CodeActions.ExtractToCss; | ||
|
||
public async Task<WorkspaceEdit?> ResolveAsync(DocumentContext documentContext, JsonElement data, RazorFormattingOptions options, CancellationToken cancellationToken) | ||
{ | ||
var actionParams = data.Deserialize<ExtractToCssCodeActionParams>(); | ||
if (actionParams is null) | ||
{ | ||
return null; | ||
} | ||
|
||
var codeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false); | ||
|
||
var cssFilePath = $"{FilePathNormalizer.Normalize(documentContext.Uri.GetAbsoluteOrUNCPath())}.css"; | ||
var cssFileUri = LspFactory.CreateFilePathUri(cssFilePath, _languageServerFeatureOptions); | ||
|
||
var text = await documentContext.GetSourceTextAsync(cancellationToken).ConfigureAwait(false); | ||
|
||
var cssContent = text.GetSubTextString(new TextSpan(actionParams.ExtractStart, actionParams.ExtractEnd - actionParams.ExtractStart)).Trim(); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var removeRange = codeDocument.Source.Text.GetRange(actionParams.RemoveStart, actionParams.RemoveEnd); | ||
|
||
var codeDocumentIdentifier = new OptionalVersionedTextDocumentIdentifier { DocumentUri = new(documentContext.Uri) }; | ||
var cssDocumentIdentifier = new OptionalVersionedTextDocumentIdentifier { DocumentUri = new(cssFileUri) }; | ||
|
||
using var changes = new PooledArrayBuilder<SumType<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>>(capacity: 3); | ||
|
||
// First, an edit to remove the script tag and its contents. | ||
changes.Add(new TextDocumentEdit | ||
{ | ||
TextDocument = codeDocumentIdentifier, | ||
Edits = [LspFactory.CreateTextEdit(removeRange, string.Empty)] | ||
}); | ||
|
||
if (_fileSystem.FileExists(cssFilePath)) | ||
{ | ||
// CSS file already exists, insert the content at the end. | ||
GetLastLineNumberAndLength(cssFilePath, out var lastLineNumber, out var lastLineLength); | ||
|
||
changes.Add(new TextDocumentEdit | ||
{ | ||
TextDocument = cssDocumentIdentifier, | ||
Edits = [LspFactory.CreateTextEdit( | ||
position: (lastLineNumber, lastLineLength), | ||
newText: lastLineNumber == 0 && lastLineLength == 0 | ||
? cssContent | ||
: Environment.NewLine + Environment.NewLine + cssContent)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should make no difference. My understanding is the concat will lower to a I'm sure Todd or Dustin will correct me if I'm wrong :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, the compiler doesn't optimize this, since It's correct that this'll just be a |
||
}); | ||
} | ||
else | ||
{ | ||
// No CSS file, create it and fill it in | ||
changes.Add(new CreateFile { DocumentUri = cssDocumentIdentifier.DocumentUri }); | ||
changes.Add(new TextDocumentEdit | ||
{ | ||
TextDocument = cssDocumentIdentifier, | ||
Edits = [LspFactory.CreateTextEdit(position: (0, 0), cssContent)] | ||
}); | ||
} | ||
|
||
return new WorkspaceEdit | ||
{ | ||
DocumentChanges = changes.ToArray(), | ||
}; | ||
} | ||
|
||
private void GetLastLineNumberAndLength(string cssFilePath, out int lastLineNumber, out int lastLineLength) | ||
{ | ||
using var stream = _fileSystem.OpenReadStream(cssFilePath); | ||
GetLastLineNumberAndLength(stream, bufferSize: 4096, out lastLineNumber, out lastLineLength); | ||
} | ||
|
||
private static void GetLastLineNumberAndLength(Stream stream, int bufferSize, out int lastLineNumber, out int lastLineLength) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could, but also feels pretty niche to me so I'm not sure |
||
{ | ||
lastLineNumber = 0; | ||
lastLineLength = 0; | ||
|
||
using var _ = ArrayPool<char>.Shared.GetPooledArray(bufferSize, out var buffer); | ||
using var reader = new StreamReader(stream); | ||
|
||
var currLineLength = 0; | ||
var currLineNumber = 0; | ||
|
||
int charsRead; | ||
while ((charsRead = reader.Read(buffer, 0, buffer.Length)) > 0) | ||
{ | ||
var chunk = buffer.AsSpan(0, charsRead); | ||
while (true) | ||
{ | ||
// Since we're only concerned with the last line length, we don't need to worry about \r\n. Strictly speaking, | ||
// we're incorrectly counting the \r in the line length, but since the last line can't end with a \n (since that | ||
// starts a new line) it doesn't actually change the output of the method. | ||
var index = chunk.IndexOf('\n'); | ||
if (index == -1) | ||
{ | ||
currLineLength += chunk.Length; | ||
break; | ||
} | ||
|
||
currLineNumber++; | ||
currLineLength = 0; | ||
chunk = chunk[(index + 1)..]; | ||
} | ||
} | ||
|
||
lastLineNumber = currLineNumber; | ||
lastLineLength = currLineLength; | ||
} | ||
|
||
internal readonly struct TestAccessor | ||
{ | ||
public static void GetLastLineNumberAndLength(Stream stream, int bufferSize, out int lastLineNumber, out int lastLineLength) | ||
{ | ||
ExtractToCssCodeActionResolver.GetLastLineNumberAndLength(stream, bufferSize, out lastLineNumber, out lastLineLength); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 Broader question: Should we just be using
TextSpan
for "start/end" pairs of properties like these? After all, we generally assign them withTextSpan.Start
andTextSpan.End
. If you agree, that'd probably be better in a separate change since here are several "params" classes with a similar shape.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be JSON serializable so we'd have to use Range, which means it's probably a wash to me, but I don't mind it. One day I'll beat these provider and resolver classes into a shape I like, and have things spread across less files, so maybe then? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to ask the question. I'd forgotten that
[DataMember]
isn't supported by System.Text.Json, but I'm glad you're keeping track of those details. 😄It does seem like it'd be pretty easy to add
RazorTextSpan
that is JSON-serializable and implicitly convertible to/fromTextSpan
, if you fancy that idea.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot about that type!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize we already had. FWIW, it looks like that type should probably be a record struct rather than a sealed record to avoid heap allocations. Then, it's just a matter of making it implicitly convertible to/from TextSpan it should be super easy to use.