Skip to content

Commit

Permalink
Merge pull request #73458 from CyrusNajmabadi/arrayAllocs
Browse files Browse the repository at this point in the history
Remove array allocations for formatting rules.
  • Loading branch information
CyrusNajmabadi authored May 14, 2024
2 parents 74093fd + 0782259 commit 9b6dd87
Show file tree
Hide file tree
Showing 41 changed files with 149 additions and 127 deletions.
15 changes: 8 additions & 7 deletions src/Analyzers/Core/Analyzers/Formatting/FormatterHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using Microsoft.CodeAnalysis.Formatting.Rules;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -30,10 +31,10 @@ internal static IEnumerable<AbstractFormattingRule> GetDefaultFormattingRules(IS
/// <param name="cancellationToken">An optional cancellation token.</param>
/// <returns>The formatted tree's root node.</returns>
public static SyntaxNode Format(SyntaxNode node, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, CancellationToken cancellationToken)
=> Format(node, [node.FullSpan], syntaxFormattingService, options, rules: null, cancellationToken: cancellationToken);
=> Format(node, [node.FullSpan], syntaxFormattingService, options, rules: default, cancellationToken: cancellationToken);

public static SyntaxNode Format(SyntaxNode node, TextSpan spanToFormat, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, CancellationToken cancellationToken)
=> Format(node, [spanToFormat], syntaxFormattingService, options, rules: null, cancellationToken: cancellationToken);
=> Format(node, [spanToFormat], syntaxFormattingService, options, rules: default, cancellationToken: cancellationToken);

/// <summary>
/// Formats the whitespace of a syntax tree.
Expand All @@ -44,15 +45,15 @@ public static SyntaxNode Format(SyntaxNode node, TextSpan spanToFormat, ISyntaxF
/// <param name="cancellationToken">An optional cancellation token.</param>
/// <returns>The formatted tree's root node.</returns>
public static SyntaxNode Format(SyntaxNode node, SyntaxAnnotation annotation, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule>? rules, CancellationToken cancellationToken)
=> Format(node, GetAnnotatedSpans(node, annotation), syntaxFormattingService, options, rules, cancellationToken: cancellationToken);
=> Format(node, GetAnnotatedSpans(node, annotation), syntaxFormattingService, options, rules.AsImmutableOrNull(), cancellationToken: cancellationToken);

internal static SyntaxNode Format(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule>? rules, CancellationToken cancellationToken)
internal static SyntaxNode Format(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, ImmutableArray<AbstractFormattingRule> rules, CancellationToken cancellationToken)
=> GetFormattingResult(node, spans, syntaxFormattingService, options, rules, cancellationToken).GetFormattedRoot(cancellationToken);

internal static IList<TextChange> GetFormattedTextChanges(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule>? rules, CancellationToken cancellationToken)
internal static IList<TextChange> GetFormattedTextChanges(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, ImmutableArray<AbstractFormattingRule> rules, CancellationToken cancellationToken)
=> GetFormattingResult(node, spans, syntaxFormattingService, options, rules, cancellationToken).GetTextChanges(cancellationToken);

internal static IFormattingResult GetFormattingResult(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule>? rules, CancellationToken cancellationToken)
internal static IFormattingResult GetFormattingResult(SyntaxNode node, IEnumerable<TextSpan> spans, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, ImmutableArray<AbstractFormattingRule> rules, CancellationToken cancellationToken)
=> syntaxFormattingService.GetFormattingResult(node, spans, options, rules, cancellationToken);

/// <summary>
Expand All @@ -63,5 +64,5 @@ internal static IFormattingResult GetFormattingResult(SyntaxNode node, IEnumerab
/// <param name="cancellationToken">An optional cancellation token.</param>
/// <returns>The changes necessary to format the tree.</returns>
public static IList<TextChange> GetFormattedTextChanges(SyntaxNode node, ISyntaxFormatting syntaxFormattingService, SyntaxFormattingOptions options, CancellationToken cancellationToken)
=> GetFormattedTextChanges(node, [node.FullSpan], syntaxFormattingService, options, rules: null, cancellationToken: cancellationToken);
=> GetFormattedTextChanges(node, [node.FullSpan], syntaxFormattingService, options, rules: default, cancellationToken: cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal static void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context, Format
var root = tree.GetRoot(cancellationToken);
var span = context.FilterSpan.HasValue ? context.FilterSpan.GetValueOrDefault() : root.FullSpan;
var spans = SpecializedCollections.SingletonEnumerable(span);
var formattingChanges = Formatter.GetFormattedTextChanges(root, spans, formattingProvider, options, rules: null, cancellationToken);
var formattingChanges = Formatter.GetFormattedTextChanges(root, spans, formattingProvider, options, rules: default, cancellationToken);

// formattingChanges could include changes that impact a larger section of the original document than
// necessary. Before reporting diagnostics, process the changes to minimize the span of individual
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,10 +850,10 @@ private async Task<SyntaxNode> AdjustLocalDeclarationsAsync(
// Run formatter prior to invoking IMoveDeclarationNearReferenceService.
#if CODE_STYLE
var provider = GetSyntaxFormatting();
rootWithTrackedNodes = FormatterHelper.Format(rootWithTrackedNodes, originalDeclStatementsToMoveOrRemove.Select(s => s.Span), provider, options, rules: null, cancellationToken);
rootWithTrackedNodes = FormatterHelper.Format(rootWithTrackedNodes, originalDeclStatementsToMoveOrRemove.Select(s => s.Span), provider, options, rules: default, cancellationToken);
#else
var provider = document.Project.Solution.Services;
rootWithTrackedNodes = Formatter.Format(rootWithTrackedNodes, originalDeclStatementsToMoveOrRemove.Select(s => s.Span), provider, options, rules: null, cancellationToken);
rootWithTrackedNodes = Formatter.Format(rootWithTrackedNodes, originalDeclStatementsToMoveOrRemove.Select(s => s.Span), provider, options, rules: default, cancellationToken);
#endif

document = document.WithSyntaxRoot(rootWithTrackedNodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ await FixOneAsync(
// formatted to explicitly format things. Note: all we will format is the new
// conditional expression as that's the only node that has the appropriate
// annotation on it.
var rules = new List<AbstractFormattingRule> { GetMultiLineFormattingRule() };
var rules = ImmutableArray.Create(GetMultiLineFormattingRule());

#if CODE_STYLE
var provider = GetSyntaxFormatting();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ protected override IList<TextChange> FormatBasedOnEndToken(ParsedDocument docume
root,
[CommonFormattingHelpers.GetFormattingSpan(root, span.Value)],
options,
rules: null,
rules: default,
cancellationToken).GetTextChanges(cancellationToken);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ public void M()
var document = workspace.CurrentSolution.Projects.Single().Documents.Single();
var syntaxRoot = await document.GetRequiredSyntaxRootAsync(CancellationToken.None);
var options = CSharpSyntaxFormattingOptions.Default;
var node = Formatter.Format(syntaxRoot, spans, workspace.Services.SolutionServices, options, rules: null, CancellationToken.None);
var node = Formatter.Format(syntaxRoot, spans, workspace.Services.SolutionServices, options, rules: default, CancellationToken.None);
Assert.Equal(expected, node.ToFullString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private void ApplyEdits(Document document, ITextView textView, ITextBuffer subje

var formattingOptions = subjectBuffer.GetSyntaxFormattingOptions(_editorOptionsService, document.Project.Services, explicitFormat: false);
var formattingSpans = trackingSnapshotSpans.Select(change => CommonFormattingHelpers.GetFormattingSpan(newRoot, change.Span.ToTextSpan()));
var formattedChanges = Formatter.GetFormattedTextChanges(newRoot, formattingSpans, document.Project.Solution.Services, formattingOptions, rules: null, cancellationToken);
var formattedChanges = Formatter.GetFormattedTextChanges(newRoot, formattingSpans, document.Project.Solution.Services, formattingOptions, rules: default, cancellationToken);

subjectBuffer.ApplyChanges(formattedChanges);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,14 @@ private protected async Task AssertFormatAsync(string expected, string code, IEn
? formattingService.GetFormattingOptions(options, fallbackOptions: null)
: formattingService.DefaultOptions;

var rules = formattingRuleProvider.CreateRule(documentSyntax, 0).Concat(Formatter.GetDefaultFormattingRules(document));
ImmutableArray<AbstractFormattingRule> rules = [formattingRuleProvider.CreateRule(documentSyntax, 0), .. Formatter.GetDefaultFormattingRules(document)];
AssertFormat(workspace, expected, formattingOptions, rules, clonedBuffer, documentSyntax.Root, spans);

// format with node and transform
AssertFormatWithTransformation(workspace, expected, formattingOptions, rules, documentSyntax.Root, spans);
}

internal void AssertFormatWithTransformation(Workspace workspace, string expected, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule> rules, SyntaxNode root, IEnumerable<TextSpan> spans)
internal void AssertFormatWithTransformation(Workspace workspace, string expected, SyntaxFormattingOptions options, ImmutableArray<AbstractFormattingRule> rules, SyntaxNode root, IEnumerable<TextSpan> spans)
{
var newRootNode = Formatter.Format(root, spans, workspace.Services.SolutionServices, options, rules, CancellationToken.None);

Expand All @@ -224,7 +224,7 @@ internal void AssertFormatWithTransformation(Workspace workspace, string expecte
Assert.True(newRootNodeFromString.IsEquivalentTo(newRootNode));
}

internal void AssertFormat(Workspace workspace, string expected, SyntaxFormattingOptions options, IEnumerable<AbstractFormattingRule> rules, ITextBuffer clonedBuffer, SyntaxNode root, IEnumerable<TextSpan> spans)
internal void AssertFormat(Workspace workspace, string expected, SyntaxFormattingOptions options, ImmutableArray<AbstractFormattingRule> rules, ITextBuffer clonedBuffer, SyntaxNode root, IEnumerable<TextSpan> spans)
{
var result = Formatter.GetFormattedTextChanges(root, spans, workspace.Services.SolutionServices, options, rules, CancellationToken.None);
var actual = ApplyResultAndGetFormattedText(clonedBuffer, result);
Expand Down
11 changes: 8 additions & 3 deletions src/EditorFeatures/VisualBasic/LineCommit/CommitFormatter.vb
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit
oldTree As SyntaxTree,
newDirtySpan As SnapshotSpan,
newTree As SyntaxTree,
cancellationToken As CancellationToken) As IEnumerable(Of AbstractFormattingRule)
cancellationToken As CancellationToken) As ImmutableArray(Of AbstractFormattingRule)

' if the span we are going to format is same as the span that got changed, don't bother to do anything special.
' just do full format of the span.
Expand Down Expand Up @@ -212,12 +212,17 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit
' for now, do very simple checking. basically, we see whether we get same number of indent operation for the give span. alternative, but little bit
' more expensive and complex, we can actually calculate indentation right after the span, and see whether that is changed. not sure whether that much granularity
' is needed.
Dim coreRules = Formatter.GetDefaultFormattingRules(languageServices)

If GetNumberOfIndentOperations(languageServices, options, oldTree, oldDirtySpan, cancellationToken) =
GetNumberOfIndentOperations(languageServices, options, newTree, newDirtySpan, cancellationToken) Then
Return (New NoAnchorFormatterRule()).Concat(Formatter.GetDefaultFormattingRules(languageServices))
Dim result = New FixedSizeArrayBuilder(Of AbstractFormattingRule)(coreRules.Length + 1)
result.Add(New NoAnchorFormatterRule())
result.AddRange(coreRules)
Return result.MoveToImmutable()
End If

Return Formatter.GetDefaultFormattingRules(languageServices)
Return coreRules
End Function

Private Shared Function GetNumberOfIndentOperations(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports System.Collections.Immutable
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Editor.UnitTests
Expand Down Expand Up @@ -66,7 +67,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Formatting
workspace.Documents.First(Function(d) d.SelectedSpans.Any()).SelectedSpans,
workspace.Services.SolutionServices,
options,
rules,
rules.ToImmutableArray(),
CancellationToken.None)

AssertResult(expected, clonedBuffer, changes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -888,8 +888,8 @@ public override async Task<ImmutableArray<ISymbol>> DetermineCascadedSymbolsFrom
return convertedMethodGroups;
}

protected override IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document)
=> Formatter.GetDefaultFormattingRules(document).Concat(new ChangeSignatureFormattingRule());
protected override ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document)
=> [.. Formatter.GetDefaultFormattingRules(document), new ChangeSignatureFormattingRule()];

protected override TArgumentSyntax AddNameToArgument<TArgumentSyntax>(TArgumentSyntax newArgument, string name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static async Task<Document> FormatDocumentAsync(Document document, Syntax
document,
[node.FullSpan],
options,
CSharpDecompiledSourceFormattingRule.Instance.Concat(Formatter.GetDefaultFormattingRules(document)),
[CSharpDecompiledSourceFormattingRule.Instance, .. Formatter.GetDefaultFormattingRules(document)],
cancellationToken).ConfigureAwait(false);

return formattedDoc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ protected override async Task<Document> AddAssemblyInfoRegionAsync(Document docu
return document.WithSyntaxRoot(newRoot);
}

protected override IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document)
=> s_memberSeparationRule.Concat(Formatter.GetDefaultFormattingRules(document));
protected override ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document)
=> [s_memberSeparationRule, .. Formatter.GetDefaultFormattingRules(document)];

protected override async Task<Document> ConvertDocCommentsToRegularCommentsAsync(Document document, IDocumentationCommentFormattingService docCommentFormattingService, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand Down Expand Up @@ -84,13 +85,8 @@ protected override async Task<SyntaxNode> UpdatePropertyAsync(
return updatedProperty.WithTrailingTrivia(trailingTrivia).WithAdditionalAnnotations(SpecializedFormattingAnnotation);
}

protected override IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document)
{
var rules = new List<AbstractFormattingRule> { new SingleLinePropertyFormattingRule() };
rules.AddRange(Formatter.GetDefaultFormattingRules(document));

return rules;
}
protected override ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document)
=> [new SingleLinePropertyFormattingRule(), .. Formatter.GetDefaultFormattingRules(document)];

private class SingleLinePropertyFormattingRule : AbstractFormattingRule
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public abstract Task<SyntaxNode> ChangeSignatureAsync(
LineFormattingOptionsProvider fallbackOptions,
CancellationToken cancellationToken);

protected abstract IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document);
protected abstract ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document);

protected abstract T TransferLeadingWhitespaceTrivia<T>(T newArgument, SyntaxNode oldArgument) where T : SyntaxNode;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ protected abstract bool TryWrapWithUnchecked(

public async Task<Document> FormatDocumentAsync(Document document, SyntaxFormattingOptions options, CancellationToken cancellationToken)
{
var rules = new List<AbstractFormattingRule> { new FormatLargeBinaryExpressionRule(document.GetRequiredLanguageService<ISyntaxFactsService>()) };
rules.AddRange(Formatter.GetDefaultFormattingRules(document));

var formatBinaryRule = new FormatLargeBinaryExpressionRule(document.GetRequiredLanguageService<ISyntaxFactsService>());
var formattedDocument = await Formatter.FormatAsync(
document, s_specializedFormattingAnnotation,
options, rules, cancellationToken).ConfigureAwait(false);
options,
[formatBinaryRule, .. Formatter.GetDefaultFormattingRules(document)],
cancellationToken).ConfigureAwait(false);
return formattedDocument;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public async Task<Document> AddSourceToAsync(
/// <summary>
/// provide formatting rules to be used when formatting MAS file
/// </summary>
protected abstract IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document);
protected abstract ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document);

/// <summary>
/// Prepends a region directive at the top of the document with a name containing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public sealed override ImmutableArray<string> FixableDiagnosticIds
protected abstract TPropertyDeclaration GetPropertyDeclaration(SyntaxNode node);
protected abstract SyntaxNode GetNodeToRemove(TVariableDeclarator declarator);

protected abstract IEnumerable<AbstractFormattingRule> GetFormattingRules(Document document);
protected abstract ImmutableArray<AbstractFormattingRule> GetFormattingRules(Document document);

protected abstract Task<SyntaxNode> UpdatePropertyAsync(
Document propertyDocument, Compilation compilation, IFieldSymbol fieldSymbol, IPropertySymbol propertySymbol,
Expand Down Expand Up @@ -281,10 +281,8 @@ private static bool CanEditDocument(
private async Task<SyntaxNode> FormatAsync(SyntaxNode newRoot, Document document, CodeCleanupOptionsProvider fallbackOptions, CancellationToken cancellationToken)
{
var formattingRules = GetFormattingRules(document);
if (formattingRules == null)
{
if (formattingRules.IsDefault)
return newRoot;
}

var options = await document.GetSyntaxFormattingOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false);
return Formatter.Format(newRoot, SpecializedFormattingAnnotation, document.Project.Solution.Services, options, formattingRules, cancellationToken);
Expand Down
Loading

0 comments on commit 9b6dd87

Please sign in to comment.