Skip to content

Commit

Permalink
New Razor document formatting engine (#11364)
Browse files Browse the repository at this point in the history
Fixes #10402
Fixes #10864

This PR moves from the old document formatting engine, to a new one that
doesn't use the normal Razor compiler generated C# document, and is
therefore able to work regardless of whether the system is configured
for runtime or design time code-gen (ie, FUSE or not). Some details
about how it works can be seen in the doc comments in
`CSharpFormattingPass.CSharpDocumentGenerator.cs`. The existing engine
is still used for On Type Formatting, as well as formatting the results
of code actions, OnAutoInsert, snippet completions, etc. This means
there isn't as much code removal as I'd like, but I'm hoping to follow
up with more PRs in future to address some of this.

Looking at this commit at a time is probably overkill, but would be
workable if you prefer it. If not, there are a couple of spots where the
final git diff won't do you any favours, and I'll comment on them as
appropriate.

As proof of the benefits of the new design, this also fixes
#7933 almost by accident, by which
I mean by design, but without specific action.
Finally, in a meeting about this engine Andrew asked how this engine
would work for lambda attributes, and so I added tests for that, and
also accidentally (ie deliberately) fixes
#9777
  • Loading branch information
davidwengier authored Jan 12, 2025
2 parents f5c6264 + 2e1093f commit 140c23f
Show file tree
Hide file tree
Showing 37 changed files with 2,436 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ internal class DefaultLanguageServerFeatureOptions : LanguageServerFeatureOption
public override bool ForceRuntimeCodeGeneration => false;

public override bool UseRoslynTokenizer => false;

public override bool UseNewFormattingEngine => false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
private readonly bool? _disableRazorLanguageServer;
private readonly bool? _forceRuntimeCodeGeneration;
private readonly bool? _useRoslynTokenizer;
private readonly bool? _useNewFormattingEngine;

public override bool SupportsFileManipulation => _supportsFileManipulation ?? _defaults.SupportsFileManipulation;
public override string CSharpVirtualDocumentSuffix => _csharpVirtualDocumentSuffix ?? DefaultLanguageServerFeatureOptions.DefaultCSharpVirtualDocumentSuffix;
Expand All @@ -39,6 +40,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
public override bool DisableRazorLanguageServer => _disableRazorLanguageServer ?? _defaults.DisableRazorLanguageServer;
public override bool ForceRuntimeCodeGeneration => _forceRuntimeCodeGeneration ?? _defaults.ForceRuntimeCodeGeneration;
public override bool UseRoslynTokenizer => _useRoslynTokenizer ?? _defaults.UseRoslynTokenizer;
public override bool UseNewFormattingEngine => _useNewFormattingEngine ?? _defaults.UseNewFormattingEngine;

public ConfigurableLanguageServerFeatureOptions(string[] args)
{
Expand All @@ -63,6 +65,7 @@ public ConfigurableLanguageServerFeatureOptions(string[] args)
TryProcessBoolOption(nameof(DisableRazorLanguageServer), ref _disableRazorLanguageServer, option, args, i);
TryProcessBoolOption(nameof(ForceRuntimeCodeGeneration), ref _forceRuntimeCodeGeneration, option, args, i);
TryProcessBoolOption(nameof(UseRoslynTokenizer), ref _useRoslynTokenizer, option, args, i);
TryProcessBoolOption(nameof(UseNewFormattingEngine), ref _useNewFormattingEngine, option, args, i);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ public Task<InitializeResult> HandleRequestAsync(InitializeParams request, Razor
if (!s_reportedFeatureFlagState)
{
s_reportedFeatureFlagState = true;
_telemetryReporter.ReportEvent("initialize", Severity.Normal, new
Property(nameof(LanguageServerFeatureOptions.ForceRuntimeCodeGeneration), _options.ForceRuntimeCodeGeneration));
_telemetryReporter.ReportEvent("initialize", Severity.Normal,
new Property(nameof(LanguageServerFeatureOptions.ForceRuntimeCodeGeneration), _options.ForceRuntimeCodeGeneration),
new Property(nameof(LanguageServerFeatureOptions.UseNewFormattingEngine), _options.UseNewFormattingEngine));
}

return Task.FromResult(serverCapabilities);
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 @@ -12,6 +14,19 @@ namespace Microsoft.AspNetCore.Razor.Language.Syntax;

internal static class RazorSyntaxNodeExtensions
{
private static bool IsDirective(SyntaxNode node, DirectiveDescriptor directive, [NotNullWhen(true)] out RazorDirectiveBodySyntax? body)
{
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor, Body: RazorDirectiveBodySyntax directiveBody } &&
descriptor == directive)
{
body = directiveBody;
return true;
}

body = null;
return false;
}

internal static bool IsUsingDirective(this SyntaxNode node, out SyntaxList<SyntaxNode> children)
{
// Using directives are weird, because the directive keyword ("using") is part of the C# statement it represents
Expand All @@ -29,6 +44,66 @@ internal static bool IsUsingDirective(this SyntaxNode node, out SyntaxList<Synta
return false;
}

internal static bool IsConstrainedTypeParamDirective(this SyntaxNode node, [NotNullWhen(true)] out CSharpStatementLiteralSyntax? typeParamNode, [NotNullWhen(true)] out CSharpStatementLiteralSyntax? conditionsNode)
{
// Returns true for "@typeparam T where T : IDisposable", but not "@typeparam T"
if (node is RazorDirectiveSyntax { DirectiveDescriptor: { } descriptor } &&
IsDirective(node, ComponentConstrainedTypeParamDirective.Directive, out var body) &&
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 */, CSharpStatementLiteralSyntax typeParam, _ /* whitespace */, CSharpStatementLiteralSyntax conditions, ..])
{
typeParamNode = typeParam;
conditionsNode = conditions;
return true;
}

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

internal static bool IsAttributeDirective(this SyntaxNode node, [NotNullWhen(true)] out CSharpStatementLiteralSyntax? attributeNode)
{
if (IsDirective(node, AttributeDirective.Directive, out var body) &&
body.CSharpCode.Children is [_, CSharpStatementLiteralSyntax attribute, ..])
{
attributeNode = attribute;
return true;
}

attributeNode = null;
return false;
}

internal static bool IsCodeDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxToken? openBraceToken)
{
if (IsDirective(node, ComponentCodeDirective.Directive, out var body) &&
body.CSharpCode is { Children: { Count: > 0 } children } &&
children.TryGetOpenBraceToken(out var openBrace))
{
openBraceToken = openBrace;
return true;
}

openBraceToken = null;
return false;
}

internal static bool IsFunctionsDirective(this SyntaxNode node, [NotNullWhen(true)] out SyntaxToken? openBraceToken)
{
if (IsDirective(node, FunctionsDirective.Directive, out var body) &&
body.CSharpCode is { Children: { Count: > 0 } children } &&
children.TryGetOpenBraceToken(out var openBrace))
{
openBraceToken = openBrace;
return true;
}

openBraceToken = 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 @@ -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,63 @@ public static string AddIndentationToMethod(string method, int tabSize, bool ins
return AddIndentationToMethod(method, tabSize, insertSpaces, startingIndent);
}

/// <summary>
/// Counts the number of non-whitespace characters in a given span of text.
/// </summary>
/// <param name="text">The source text</param>
/// <param name="start">Inclusive position for where to start counting</param>
/// <param name="endExclusive">Exclusive for where to stop counting</param>
public static int CountNonWhitespaceChars(SourceText text, int start, int endExclusive)
{
var count = 0;
for (var i = start; i < endExclusive; i++)
{
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 currentIndentationWidth = firstNonWhitespaceCharacterPosition - line.Start;
if (insertSpaces)
{
var indentationLevel = currentIndentationWidth / tabSize;
additionalIndentation = new string(' ', currentIndentationWidth % tabSize);
return indentationLevel;
}

// 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

This file was deleted.

Loading

0 comments on commit 140c23f

Please sign in to comment.