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

New Razor document formatting engine #11364

Merged
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
81264b1
Add another test.
davidwengier Dec 20, 2024
e13da44
Add some useful directive extension methods
davidwengier Dec 28, 2024
abf6e27
Add a helper method to calculate the indent level from an existing line
davidwengier Dec 28, 2024
54ca90b
Fix tests that use tabs, or a custom tab size
davidwengier Dec 28, 2024
36efbe9
Add or expand a few formatting tests
davidwengier Dec 28, 2024
98d3a2a
Add some helper utiltiies with basic test coverageMore hlper methods
davidwengier Dec 29, 2024
dd87d34
Create a simple Html formatting pass
davidwengier Dec 29, 2024
b60fa37
Add a new C# formatting pass
davidwengier Dec 29, 2024
4d78e67
Expose Roslyn workspace helper
davidwengier Dec 29, 2024
ef1c95e
Dodgily hook up the new formatting engine
davidwengier Dec 29, 2024
cf487c9
Slight tweaks to the RazorFormattingPass to make it not break the new…
davidwengier Dec 29, 2024
c2a3723
Tweaks to formatting logic for explicit expressions
davidwengier Dec 30, 2024
de4ab65
Format implicit or explicit expressions no matter where they are on a…
davidwengier Dec 30, 2024
10b692c
Add tests and coverage for the quirks of single line empty explicit s…
davidwengier Dec 30, 2024
8e0514f
Little bit of code cleanup and sharing
davidwengier Dec 31, 2024
f6b00ef
Unskip a few tests
davidwengier Dec 31, 2024
bcde950
Update expected output to match the new formatter
davidwengier Dec 31, 2024
5b3e819
Update expected output to match the new engine behaviour
davidwengier Dec 31, 2024
1736268
Fix start position calculation to allow for whitespace differences be…
davidwengier Jan 1, 2025
ebde05b
PR feedbacl
davidwengier Jan 3, 2025
74136b2
Fix build errors
davidwengier Jan 3, 2025
6d53a14
Fix failing test
davidwengier Jan 3, 2025
6bf1b19
Add tests for lambda parameters, and relax an if statement so they're…
davidwengier Jan 3, 2025
c503970
Language server and cohost test runners have different using directiv…
davidwengier Jan 4, 2025
5cdd419
Remove ability to opt in to the old formatting engine
davidwengier Jan 7, 2025
766b9a5
Remove old document formatting bits
davidwengier Jan 7, 2025
7449200
Move new formatting engine, so there is only one
davidwengier Jan 7, 2025
161f6f4
Remove ability to force design time code-gen in FUSE
davidwengier Jan 7, 2025
1620344
SyntaxNodeExtensions related PR feedback
davidwengier Jan 9, 2025
5e61c87
Formatting utilities PR feedback
davidwengier Jan 9, 2025
431a0fe
CSharpDocumentGenerator PR feedback and clean up
davidwengier Jan 9, 2025
9524144
Whitespace🤦‍♂️
davidwengier Jan 9, 2025
d1fa870
...
davidwengier Jan 9, 2025
3fd0ab2
Merge remote-tracking branch 'upstream/main' into OneFormattingEngine…
davidwengier Jan 10, 2025
162ad30
Add feature flag to allow opting in to the new formatting engine
davidwengier Jan 10, 2025
be8725b
Put back the old formatting engine, but opt to use the new one in code
davidwengier Jan 10, 2025
beabfce
Revert "Remove ability to force design time code-gen in FUSE"
davidwengier Jan 10, 2025
2e3adc7
Run all (most) tests with the new engine, and the existing one
davidwengier Jan 10, 2025
2e1093f
PR Feedback
davidwengier Jan 12, 2025
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
3 changes: 0 additions & 3 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@
<ExcludeFromSourceOnlyBuild>true</ExcludeFromSourceOnlyBuild>

<DefaultNetFxTargetFramework>net472</DefaultNetFxTargetFramework>

<!-- Uncomment this line to run formatting on runtime code-gen if FUSE is turned on -->
<!-- <DefineConstants>$(DefineConstants);FORMAT_FUSE</DefineConstants> -->
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<!--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Microsoft.AspNetCore.Razor.Language.Components;
using Microsoft.AspNetCore.Razor.Language.Extensions;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
Expand All @@ -29,6 +31,69 @@ internal static bool IsUsingDirective(this SyntaxNode node, out SyntaxList<Synta
return false;
}

internal static bool IsConstrainedTypeParamDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? typeParamNode, [NotNullWhen(true)] out SyntaxNode? conditionsNode)
{
// Returns true for "@typeparam T where T : IDisposable", but not "@typeparam T"
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor, Body: RazorDirectiveBodySyntax body } &&
descriptor.Directive == ComponentConstrainedTypeParamDirective.Directive.Directive &&
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
descriptor.Tokens.Any(t => t.Name == ComponentResources.TypeParamDirective_Constraint_Name) &&
// Children is the " T where T : IDisposable" part of the directive
body.CSharpCode.Children is [_ /* whitespace */, { } typeParam, _ /* whitespace */, { } conditions, ..])
{
typeParamNode = typeParam;
conditionsNode = conditions;
return true;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
}

typeParamNode = null;
conditionsNode = null;
return false;
}

internal static bool IsAttributeDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? attributeNode)
{
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor, Body: RazorDirectiveBodySyntax body } &&
descriptor.Directive == AttributeDirective.Directive.Directive &&
body.CSharpCode.Children is [_, { } attribute, ..])
{
attributeNode = attribute;
return true;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
}

attributeNode = null;
return false;
}

internal static bool IsCodeDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? openBraceNode)
{
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor, Body: RazorDirectiveBodySyntax body } &&
descriptor.Directive == ComponentCodeDirective.Directive.Directive &&
body.CSharpCode is { Children: { Count: > 0 } children } &&
children.TryGetOpenBraceToken(out var openBrace))
{
openBraceNode = openBrace;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

openBraceNode = null;
return false;
}

internal static bool IsFunctionsDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? openBraceNode)
{
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor, Body: RazorDirectiveBodySyntax body } &&
descriptor.Directive == FunctionsDirective.Directive.Directive &&
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
body.CSharpCode is { Children: { Count: > 0 } children } &&
children.TryGetOpenBraceToken(out var openBrace))
{
openBraceNode = openBrace;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

openBraceNode = null;
return false;
}

/// <summary>
/// Walks up the tree through the <paramref name="owner"/>'s parents to find the outermost node that starts at the same position.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,19 @@ public static string GetSubTextString(this SourceText text, TextSpan span)
}

public static bool NonWhitespaceContentEquals(this SourceText text, SourceText other)
=> NonWhitespaceContentEquals(text, other, 0, text.Length, 0, other.Length);

public static bool NonWhitespaceContentEquals(
this SourceText text,
SourceText other,
int textStart,
int textEnd,
int otherStart,
int otherEnd)
{
var i = 0;
var j = 0;
while (i < text.Length && j < other.Length)
var i = textStart;
var j = otherStart;
while (i < textEnd && j < otherEnd)
{
if (char.IsWhiteSpace(text[i]))
{
Expand All @@ -85,17 +94,17 @@ public static bool NonWhitespaceContentEquals(this SourceText text, SourceText o
j++;
}

while (i < text.Length && char.IsWhiteSpace(text[i]))
while (i < textEnd && char.IsWhiteSpace(text[i]))
{
i++;
}

while (j < other.Length && char.IsWhiteSpace(other[j]))
while (j < otherEnd && char.IsWhiteSpace(other[j]))
{
j++;
}

return i == text.Length && j == other.Length;
return i == textEnd && j == otherEnd;
}

public static bool TryGetFirstNonWhitespaceOffset(this SourceText text, out int offset)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,21 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.Razor.Formatting;

internal sealed class CSharpFormatter(IDocumentMappingService documentMappingService)
internal sealed class CSharpFormatter
{
private const string MarkerId = "RazorMarker";

private readonly IDocumentMappingService _documentMappingService = documentMappingService;

public async Task<ImmutableArray<TextChange>> FormatAsync(HostWorkspaceServices hostWorkspaceServices, Document csharpDocument, FormattingContext context, LinePositionSpan spanToFormat, CancellationToken cancellationToken)
{
if (!_documentMappingService.TryMapToGeneratedDocumentRange(context.CodeDocument.GetCSharpDocument(), spanToFormat, out var projectedSpan))
{
return [];
}

var edits = await GetFormattingEditsAsync(hostWorkspaceServices, csharpDocument, projectedSpan, context.Options.ToIndentationOptions(), cancellationToken).ConfigureAwait(false);
var mappedEdits = MapEditsToHostDocument(context.CodeDocument, edits);
return mappedEdits;
}

public static async Task<IReadOnlyDictionary<int, int>> GetCSharpIndentationAsync(
FormattingContext context,
HashSet<int> projectedDocumentLocations,
Expand All @@ -51,24 +35,6 @@ public static async Task<IReadOnlyDictionary<int, int>> GetCSharpIndentationAsyn
return indentations;
}

private ImmutableArray<TextChange> MapEditsToHostDocument(RazorCodeDocument codeDocument, ImmutableArray<TextChange> csharpEdits)
{
var actualEdits = _documentMappingService.GetHostDocumentEdits(codeDocument.GetCSharpDocument(), csharpEdits);

return actualEdits.ToImmutableArray();
}

private static async Task<ImmutableArray<TextChange>> GetFormattingEditsAsync(HostWorkspaceServices hostWorkspaceServices, Document csharpDocument, LinePositionSpan projectedSpan, RazorIndentationOptions indentationOptions, CancellationToken cancellationToken)
{
var root = await csharpDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var csharpSourceText = await csharpDocument.GetTextAsync(cancellationToken).ConfigureAwait(false);
var spanToFormat = csharpSourceText.GetTextSpan(projectedSpan);
Assumes.NotNull(root);

var changes = RazorCSharpFormattingInteractionService.GetFormattedTextChanges(hostWorkspaceServices, root, spanToFormat, indentationOptions, cancellationToken);
return changes.ToImmutableArray();
}

private static async Task<Dictionary<int, int>> GetCSharpIndentationCoreAsync(FormattingContext context, ImmutableArray<int> projectedDocumentLocations, HostWorkspaceServices hostWorkspaceServices, CancellationToken cancellationToken)
{
// No point calling the C# formatting if we won't be interested in any of its work anyway
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;

#if !FORMAT_FUSE
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
#endif

namespace Microsoft.CodeAnalysis.Razor.Formatting;

internal sealed class FormattingContext
Expand Down Expand Up @@ -228,14 +224,7 @@ public bool TryGetFormattingSpan(int absoluteIndex, [NotNullWhen(true)] out Form
public async Task<FormattingContext> WithTextAsync(SourceText changedText, CancellationToken cancellationToken)
{
var changedSnapshot = OriginalSnapshot.WithText(changedText);

#if !FORMAT_FUSE
// Formatting always uses design time document
var generator = (IDesignTimeCodeGenerator)changedSnapshot;
var codeDocument = await generator.GenerateDesignTimeOutputAsync(cancellationToken).ConfigureAwait(false);
#else
var codeDocument = await changedSnapshot.GetGeneratedOutputAsync(cancellationToken).ConfigureAwait(false);
#endif

DEBUG_ValidateComponents(CodeDocument, codeDocument);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.AspNetCore.Razor;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -120,6 +122,57 @@ public static string AddIndentationToMethod(string method, int tabSize, bool ins
return AddIndentationToMethod(method, tabSize, insertSpaces, startingIndent);
}

public static int CountNonWhitespaceChars(SourceText text, int start, int end)
{
var count = 0;
for (var i = start; i < end; i++)
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
{
if (!char.IsWhiteSpace(text[i]))
{
count++;
}
}

return count;
}

public static int GetIndentationLevel(TextLine line, int firstNonWhitespaceCharacterPosition, bool insertSpaces, int tabSize, out string additionalIndentation)
{
if (firstNonWhitespaceCharacterPosition > line.End)
{
throw new ArgumentOutOfRangeException(nameof(firstNonWhitespaceCharacterPosition), "The first non-whitespace character position must be within the line.");
}

// For spaces, the actual indentation needs to be divided by the tab size to get the level, and additional is the remainder
var indentation = firstNonWhitespaceCharacterPosition - line.Start;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
if (insertSpaces)
{
var indent = indentation / tabSize;
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
additionalIndentation = new string(' ', indentation % tabSize);
return indent;
}

// For tabs, we just count the tabs, and additional is any spaces at the end.
var tabCount = 0;
var text = line.Text.AssumeNotNull();
for (var i = line.Start; i < firstNonWhitespaceCharacterPosition; i++)
{
if (text[i] == '\t')
{
tabCount++;
}
else
{
Debug.Assert(text[i] == ' ');
additionalIndentation = text.GetSubTextString(TextSpan.FromBounds(i, firstNonWhitespaceCharacterPosition));
return tabCount;
}
}

additionalIndentation = "";
return tabCount;
}

/// <summary>
/// Given a <paramref name="indentation"/> amount of characters, generate a string representing the configured indentation.
/// </summary>
Expand Down
Loading
Loading