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.

Sign up for GitHub

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

Update wrapping refactoring provider to be more async #75588

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Update wrapping refactoring provider to be more async #75588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 24 additions & 23 deletions src/Features/Core/Portable/Wrapping/AbstractCodeActionComputer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ protected abstract class AbstractCodeActionComputer<TWrapper> : ICodeActionCompu

protected readonly Document OriginalDocument;
protected readonly SourceText OriginalSourceText;
protected readonly CancellationToken CancellationToken;
protected readonly SyntaxWrappingOptions Options;

protected readonly SyntaxTriviaList NewLineTrivia;
Expand All @@ -65,13 +64,11 @@ public AbstractCodeActionComputer(
TWrapper service,
Document document,
SourceText originalSourceText,
SyntaxWrappingOptions options,
CancellationToken cancellationToken)
SyntaxWrappingOptions options)
{
Wrapper = service;
OriginalDocument = document;
OriginalSourceText = originalSourceText;
CancellationToken = cancellationToken;
Options = options;

var generator = SyntaxGenerator.GetGenerator(document);
Expand All @@ -80,12 +77,13 @@ public AbstractCodeActionComputer(
SingleWhitespaceTrivia = new SyntaxTriviaList(generator.Whitespace(" "));
}

protected abstract Task<ImmutableArray<WrappingGroup>> ComputeWrappingGroupsAsync();
protected abstract Task<ImmutableArray<WrappingGroup>> ComputeWrappingGroupsAsync(CancellationToken cancellationToken);

protected string GetSmartIndentationAfter(SyntaxNodeOrToken nodeOrToken)
=> GetIndentationAfter(nodeOrToken, FormattingOptions2.IndentStyle.Smart);
protected Task<string> GetSmartIndentationAfterAsync(SyntaxNodeOrToken nodeOrToken, CancellationToken cancellationToken)
=> GetIndentationAfterAsync(nodeOrToken, FormattingOptions2.IndentStyle.Smart, cancellationToken);

protected string GetIndentationAfter(SyntaxNodeOrToken nodeOrToken, FormattingOptions2.IndentStyle indentStyle)
protected async Task<string> GetIndentationAfterAsync(
SyntaxNodeOrToken nodeOrToken, FormattingOptions2.IndentStyle indentStyle, CancellationToken cancellationToken)
{
var newLine = Options.FormattingOptions.NewLine;
var newSourceText = OriginalSourceText.WithChanges(
Expand All @@ -100,12 +98,12 @@ protected string GetIndentationAfter(SyntaxNodeOrToken nodeOrToken, FormattingOp
var originalLineNumber = newSourceText.Lines.GetLineFromPosition(nodeOrToken.Span.End).LineNumber;

// TODO: should be async https://github.com/dotnet/roslyn/issues/61998
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be removed?

var newParsedDocument = ParsedDocument.CreateSynchronously(newDocument, CancellationToken);
var newParsedDocument = await ParsedDocument.CreateAsync(newDocument, cancellationToken).ConfigureAwait(false);

var desiredIndentation = indentationService.GetIndentation(
newParsedDocument, originalLineNumber + 1,
indentationOptions,
CancellationToken);
cancellationToken);

return desiredIndentation.GetIndentationString(newSourceText, Options.FormattingOptions.UseTabs, Options.FormattingOptions.TabSize);
}
Expand All @@ -119,10 +117,10 @@ protected string GetIndentationAfter(SyntaxNodeOrToken nodeOrToken, FormattingOp
/// 3. A previous code action was created that already had the same effect.
/// </summary>
protected async Task<WrapItemsAction?> TryCreateCodeActionAsync(
ImmutableArray<Edit> edits, string parentTitle, string title)
ImmutableArray<Edit> edits, string parentTitle, string title, CancellationToken cancellationToken)
{
// First, rewrite the tree with the edits provided.
var (root, rewrittenRoot, spanToFormat) = await RewriteTreeAsync(edits).ConfigureAwait(false);
var (root, rewrittenRoot, spanToFormat) = await RewriteTreeAsync(edits, cancellationToken).ConfigureAwait(false);
if (rewrittenRoot == null)
{
// Couldn't rewrite for some reason. No code action to create.
Expand All @@ -131,8 +129,8 @@ protected string GetIndentationAfter(SyntaxNodeOrToken nodeOrToken, FormattingOp

// Now, format the part of the tree that we edited. This will ensure we properly
// respect the user preferences around things like comma/operator spacing.
var formattedDocument = await FormatDocumentAsync(rewrittenRoot, spanToFormat).ConfigureAwait(false);
var formattedRoot = await formattedDocument.GetRequiredSyntaxRootAsync(CancellationToken).ConfigureAwait(false);
var formattedDocument = await FormatDocumentAsync(rewrittenRoot, spanToFormat, cancellationToken).ConfigureAwait(false);
var formattedRoot = await formattedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

// Now, check if this new formatted tree matches our starting tree, or any of the
// trees we've already created for our other code actions. If so, we don't want to
Expand Down Expand Up @@ -161,15 +159,17 @@ protected string GetIndentationAfter(SyntaxNodeOrToken nodeOrToken, FormattingOp
return new WrapItemsAction(title, parentTitle, (_, _) => Task.FromResult(formattedDocument));
}

private async Task<Document> FormatDocumentAsync(SyntaxNode rewrittenRoot, TextSpan spanToFormat)
private async Task<Document> FormatDocumentAsync(
SyntaxNode rewrittenRoot, TextSpan spanToFormat, CancellationToken cancellationToken)
{
var newDocument = OriginalDocument.WithSyntaxRoot(rewrittenRoot);
var formattedDocument = await Formatter.FormatAsync(
newDocument, spanToFormat, Options.FormattingOptions, CancellationToken).ConfigureAwait(false);
newDocument, spanToFormat, Options.FormattingOptions, cancellationToken).ConfigureAwait(false);
return formattedDocument;
}

private async Task<(SyntaxNode root, SyntaxNode rewrittenRoot, TextSpan spanToFormat)> RewriteTreeAsync(ImmutableArray<Edit> edits)
private async Task<(SyntaxNode root, SyntaxNode rewrittenRoot, TextSpan spanToFormat)> RewriteTreeAsync(
ImmutableArray<Edit> edits, CancellationToken cancellationToken)
{
using var _1 = PooledDictionary<SyntaxToken, SyntaxTriviaList>.GetInstance(out var leftTokenToTrailingTrivia);
using var _2 = PooledDictionary<SyntaxToken, SyntaxTriviaList>.GetInstance(out var rightTokenToLeadingTrivia);
Expand Down Expand Up @@ -200,7 +200,7 @@ private async Task<Document> FormatDocumentAsync(SyntaxNode rewrittenRoot, TextS
}

return await RewriteTreeAsync(
leftTokenToTrailingTrivia, rightTokenToLeadingTrivia).ConfigureAwait(false);
leftTokenToTrailingTrivia, rightTokenToLeadingTrivia, cancellationToken).ConfigureAwait(false);
}

private static bool IsSafeToRemove(string text)
Expand All @@ -219,9 +219,10 @@ private static bool IsSafeToRemove(string text)

private async Task<(SyntaxNode root, SyntaxNode rewrittenRoot, TextSpan spanToFormat)> RewriteTreeAsync(
Dictionary<SyntaxToken, SyntaxTriviaList> leftTokenToTrailingTrivia,
Dictionary<SyntaxToken, SyntaxTriviaList> rightTokenToLeadingTrivia)
Dictionary<SyntaxToken, SyntaxTriviaList> rightTokenToLeadingTrivia,
CancellationToken cancellationToken)
{
var root = await OriginalDocument.GetRequiredSyntaxRootAsync(CancellationToken).ConfigureAwait(false);
var root = await OriginalDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var tokens = leftTokenToTrailingTrivia.Keys.Concat(rightTokenToLeadingTrivia.Keys).Distinct().ToImmutableArray();

// Find the closest node that contains all the tokens we're editing. That's the
Expand Down Expand Up @@ -266,12 +267,12 @@ private static bool IsSafeToRemove(string text)
return (root, rewrittenRoot, trackedNode.Span);
}

public async Task<ImmutableArray<CodeAction>> GetTopLevelCodeActionsAsync()
public async Task<ImmutableArray<CodeAction>> GetTopLevelCodeActionsAsync(CancellationToken cancellationToken)
{
try
{
// Ask subclass to produce whole nested list of wrapping code actions
var wrappingGroups = await ComputeWrappingGroupsAsync().ConfigureAwait(false);
var wrappingGroups = await ComputeWrappingGroupsAsync(cancellationToken).ConfigureAwait(false);

using var result = TemporaryArray<CodeAction>.Empty;
foreach (var group in wrappingGroups)
Expand Down Expand Up @@ -306,7 +307,7 @@ public async Task<ImmutableArray<CodeAction>> GetTopLevelCodeActionsAsync()
// both the top level items and the nested items are ordered appropriate.
return WrapItemsAction.SortActionsByMostRecentlyUsed(result.ToImmutableAndClear());
}
catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex, CancellationToken, ErrorSeverity.Diagnostic))
catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex, cancellationToken, ErrorSeverity.Diagnostic))
{
throw;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
if (computer == null)
continue;

var actions = await computer.GetTopLevelCodeActionsAsync().ConfigureAwait(false);
var actions = await computer.GetTopLevelCodeActionsAsync(cancellationToken).ConfigureAwait(false);
if (actions.IsDefaultOrEmpty)
continue;

Expand Down
3 changes: 1 addition & 2 deletions src/Features/Core/Portable/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ protected AbstractBinaryExpressionWrapper(

var sourceText = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
return new BinaryExpressionCodeActionComputer(
this, document, sourceText, options, binaryExpr,
exprsAndOperators, cancellationToken);
this, document, sourceText, options, binaryExpr, exprsAndOperators);
}

private ImmutableArray<SyntaxNodeOrToken> GetExpressionsAndOperators(
Expand Down
37 changes: 20 additions & 17 deletions src/Features/Core/Portable/Wrapping/BinaryExpression/BinaryExpressionCodeActionComputer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Wrapping.BinaryExpression;

Expand Down Expand Up @@ -40,17 +41,16 @@ private sealed class BinaryExpressionCodeActionComputer :
/// The indent trivia to insert if we are trying to simply smart-indent all wrapped
/// parts of the expression.
/// </summary>
private readonly SyntaxTriviaList _smartIndentTrivia;
private readonly AsyncLazy<SyntaxTriviaList> _smartIndentTrivia;

public BinaryExpressionCodeActionComputer(
AbstractBinaryExpressionWrapper<TBinaryExpressionSyntax> service,
Document document,
SourceText originalSourceText,
SyntaxWrappingOptions options,
TBinaryExpressionSyntax binaryExpression,
ImmutableArray<SyntaxNodeOrToken> exprsAndOperators,
CancellationToken cancellationToken)
: base(service, document, originalSourceText, options, cancellationToken)
ImmutableArray<SyntaxNodeOrToken> exprsAndOperators)
: base(service, document, originalSourceText, options)
{
_exprsAndOperators = exprsAndOperators;

Expand All @@ -62,30 +62,33 @@ public BinaryExpressionCodeActionComputer(
OriginalSourceText.GetOffset(binaryExpression.Span.Start)
.CreateIndentationString(options.FormattingOptions.UseTabs, options.FormattingOptions.TabSize)));

_smartIndentTrivia = new SyntaxTriviaList(generator.Whitespace(
GetSmartIndentationAfter(_exprsAndOperators[1])));
_smartIndentTrivia = AsyncLazy.Create(async cancellationToken => new SyntaxTriviaList(generator.Whitespace(
await GetSmartIndentationAfterAsync(_exprsAndOperators[1], cancellationToken).ConfigureAwait(false))));
}

protected override async Task<ImmutableArray<WrappingGroup>> ComputeWrappingGroupsAsync()
protected override async Task<ImmutableArray<WrappingGroup>> ComputeWrappingGroupsAsync(CancellationToken cancellationToken)
=> [new WrappingGroup(
isInlinable: true,
[
await GetWrapCodeActionAsync(align: false).ConfigureAwait(false),
await GetWrapCodeActionAsync(align: true).ConfigureAwait(false),
await GetUnwrapCodeActionAsync().ConfigureAwait(false),
await GetWrapCodeActionAsync(align: false, cancellationToken).ConfigureAwait(false),
await GetWrapCodeActionAsync(align: true, cancellationToken).ConfigureAwait(false),
await GetUnwrapCodeActionAsync(cancellationToken).ConfigureAwait(false),
])];

private Task<WrapItemsAction> GetWrapCodeActionAsync(bool align)
=> TryCreateCodeActionAsync(GetWrapEdits(align), FeaturesResources.Wrapping,
align ? FeaturesResources.Wrap_and_align_expression : FeaturesResources.Wrap_expression);
private async Task<WrapItemsAction> GetWrapCodeActionAsync(bool align, CancellationToken cancellationToken)
=> await TryCreateCodeActionAsync(await GetWrapEditsAsync(align, cancellationToken).ConfigureAwait(false), FeaturesResources.Wrapping,
align ? FeaturesResources.Wrap_and_align_expression : FeaturesResources.Wrap_expression,
cancellationToken).ConfigureAwait(false);

private Task<WrapItemsAction> GetUnwrapCodeActionAsync()
=> TryCreateCodeActionAsync(GetUnwrapEdits(), FeaturesResources.Wrapping, FeaturesResources.Unwrap_expression);
private Task<WrapItemsAction> GetUnwrapCodeActionAsync(CancellationToken cancellationToken)
=> TryCreateCodeActionAsync(GetUnwrapEdits(), FeaturesResources.Wrapping, FeaturesResources.Unwrap_expression, cancellationToken);

private ImmutableArray<Edit> GetWrapEdits(bool align)
private async Task<ImmutableArray<Edit>> GetWrapEditsAsync(bool align, CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<Edit>.GetInstance(out var result);
var indentationTrivia = align ? _indentAndAlignTrivia : _smartIndentTrivia;
var indentationTrivia = align
? _indentAndAlignTrivia
: await _smartIndentTrivia.GetValueAsync(cancellationToken).ConfigureAwait(false);

for (var i = 1; i < _exprsAndOperators.Length; i += 2)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Features/Core/Portable/Wrapping/ChainedExpression/AbstractChainedExpressionWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ protected AbstractChainedExpressionWrapper(
// the set of wrapping options to provide.
var sourceText = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
return new CallExpressionCodeActionComputer(
this, document, sourceText, options, chunks, cancellationToken);
this, document, sourceText, options, chunks);
}

private ImmutableArray<ImmutableArray<SyntaxNodeOrToken>> GetChainChunks(SyntaxNode node)
Expand Down
Loading
Loading