Formatting engine changes to allow it to (mostly) run on runtime code-gen

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/RazorSyntaxNodeExtensions.cs

@@ -166,7 +166,9 @@ public static LinePositionSpan GetLinePositionSpan(this SyntaxNode node, RazorSo
             {
                 var previousToken = token.GetPreviousToken(includeWhitespace);
 
-                if (previousToken.Kind != SyntaxKind.Marker || previousToken.Position != foundPosition)
+                if (previousToken is null ||
+                    previousToken.Kind != SyntaxKind.Marker ||
+                    previousToken.Position != foundPosition)
                 {
                     break;
                 }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/CSharpFormatter.cs

@@ -262,11 +262,17 @@ private static (Dictionary<int, IndentationMapData>, SyntaxTree) InitializeInden
 
         using var changes = new PooledArrayBuilder<TextChange>();
 
+        var syntaxTree = CSharpSyntaxTree.ParseText(context.CSharpSourceText, cancellationToken: cancellationToken);
+        var root = syntaxTree.GetRoot(cancellationToken);
+
         var previousMarkerOffset = 0;
         foreach (var projectedDocumentIndex in projectedDocumentLocations)
         {
-            var useMarker = char.IsWhiteSpace(context.CSharpSourceText[projectedDocumentIndex]);
-            if (useMarker)
+            var token = root.FindToken(projectedDocumentIndex, findInsideTrivia: true);
+
+            // We use a marker if the projected location is in trivia, because we can't add annotations to a specific piece of trivia
+            var isInTrivia = projectedDocumentIndex < token.SpanStart || projectedDocumentIndex >= token.Span.End;
+            if (isInTrivia)
             {
                 // We want to add a marker here because the location points to a whitespace
                 // which will not get preserved during formatting.
@@ -299,8 +305,7 @@ private static (Dictionary<int, IndentationMapData>, SyntaxTree) InitializeInden
         }
 
         var changedText = context.CSharpSourceText.WithChanges(changes.ToImmutable());
-        var syntaxTree = CSharpSyntaxTree.ParseText(changedText, cancellationToken: cancellationToken);
-        return (indentationMap, syntaxTree);
+        return (indentationMap, syntaxTree.WithChangedText(changedText));
     }
 
     private static SyntaxNode AttachAnnotations(

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/FormattingVisitor.cs

@@ -408,7 +408,20 @@ public override void VisitMarkupTransition(MarkupTransitionSyntax node)
 
     public override void VisitCSharpStatementLiteral(CSharpStatementLiteralSyntax node)
     {
-        WriteSpan(node, FormattingSpanKind.Code);
+        // Workaround for a quirk of runtime code gen, where an empty marker is inserted before the close brace
+        // of an explicit expression. ie, at the $$ below:
+        //
+        // @{
+        //     something
+        // $$}
+        //
+        // Writing a span for this empty marker will cause the close brace to be incorrectly indented, because its seen as
+        // being "inside" the block.
+        if (node.LiteralTokens is not [{ Kind: SyntaxKind.Marker }])
+        {
+            WriteSpan(node, FormattingSpanKind.Code);
+        }
+
         base.VisitCSharpStatementLiteral(node);
     }
 

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPass.cs

@@ -55,7 +55,7 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
 
         cancellationToken.ThrowIfCancellationRequested();
 
-        var indentationChanges = await AdjustIndentationAsync(changedContext, startLine: 0, endLineInclusive: changedText.Lines.Count - 1, roslynWorkspaceHelper.HostWorkspaceServices, cancellationToken).ConfigureAwait(false);
+        var indentationChanges = await AdjustIndentationAsync(changedContext, startLine: 0, endLineInclusive: changedText.Lines.Count - 1, roslynWorkspaceHelper.HostWorkspaceServices, _logger, cancellationToken).ConfigureAwait(false);
         if (indentationChanges.Length > 0)
         {
             // Apply the edits that modify indentation.
@@ -64,11 +64,33 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
             _logger.LogTestOnly($"After AdjustIndentationAsync:\r\n{changedText}");
         }
 
+        _logger.LogTestOnly($"Source Mappings:\r\n{RenderSourceMappings(context.CodeDocument)}");
+
         _logger.LogTestOnly($"Generated C#:\r\n{context.CSharpSourceText}");
 
         return changedText.GetTextChangesArray(originalText);
     }
 
+    private static string RenderSourceMappings(RazorCodeDocument codeDocument)
+    {
+        var markers = codeDocument.GetCSharpDocument().SourceMappings.SelectMany(mapping =>
+            new[]
+            {
+                (index: mapping.OriginalSpan.AbsoluteIndex, text: "<#" ),
+                (index: mapping.OriginalSpan.AbsoluteIndex + mapping.OriginalSpan.Length, text: "#>"),
+            })
+            .OrderByDescending(mapping => mapping.index)
+            .ThenBy(mapping => mapping.text);
+
+        var output = codeDocument.Source.Text.ToString();
+        foreach (var (index, text) in markers)
+        {
+            output = output.Insert(index, text);
+        }
+
+        return output;
+    }
+
     private async Task<ImmutableArray<TextChange>> FormatCSharpAsync(FormattingContext context, HostWorkspaceServices hostWorkspaceServices, Document csharpDocument, CancellationToken cancellationToken)
     {
         var sourceText = context.SourceText;

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpFormattingPassBase.cs

@@ -15,6 +15,7 @@
 using Microsoft.AspNetCore.Razor.PooledObjects;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Razor.DocumentMapping;
+using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
@@ -37,14 +38,15 @@ public async Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext con
 
     protected abstract Task<ImmutableArray<TextChange>> ExecuteCoreAsync(FormattingContext context, RoslynWorkspaceHelper roslynWorkspaceHelper, ImmutableArray<TextChange> changes, CancellationToken cancellationToken);
 
-    protected async Task<ImmutableArray<TextChange>> AdjustIndentationAsync(FormattingContext context, int startLine, int endLineInclusive, HostWorkspaceServices hostWorkspaceServices, CancellationToken cancellationToken)
+    protected async Task<ImmutableArray<TextChange>> AdjustIndentationAsync(FormattingContext context, int startLine, int endLineInclusive, HostWorkspaceServices hostWorkspaceServices, ILogger logger, CancellationToken cancellationToken)
     {
         // In this method, the goal is to make final adjustments to the indentation of each line.
         // We will take into account the following,
         // 1. The indentation due to nested C# structures
         // 2. The indentation due to Razor and HTML constructs
 
         var text = context.SourceText;
+        var csharpDocument = context.CodeDocument.GetCSharpDocument();
 
         // To help with figuring out the correct indentation, first we will need the indentation
         // that the C# formatter wants to apply in the following locations,
@@ -58,7 +60,7 @@ protected async Task<ImmutableArray<TextChange>> AdjustIndentationAsync(Formatti
 
         // First, collect all the locations at the beginning and end of each source mapping.
         var sourceMappingMap = new Dictionary<int, int>();
-        foreach (var mapping in context.CodeDocument.GetCSharpDocument().SourceMappings)
+        foreach (var mapping in csharpDocument.SourceMappings)
         {
             var mappingSpan = new TextSpan(mapping.OriginalSpan.AbsoluteIndex, mapping.OriginalSpan.Length);
 #if DEBUG
@@ -108,17 +110,58 @@ protected async Task<ImmutableArray<TextChange>> AdjustIndentationAsync(Formatti
             var lineStart = line.GetFirstNonWhitespacePosition() ?? line.Start;
 
             var lineStartSpan = new TextSpan(lineStart, 0);
-            if (!ShouldFormat(context, lineStartSpan, allowImplicitStatements: true))
+            if (!ShouldFormat(context, lineStartSpan, allowImplicitStatements: true, out var owner))
             {
                 // We don't care about this range as this can potentially lead to incorrect scopes.
+                logger.LogTestOnly($"Don't care about line: {line.ToString()}");
                 continue;
             }
 
-            if (DocumentMappingService.TryMapToGeneratedDocumentPosition(context.CodeDocument.GetCSharpDocument(), lineStart, out _, out var projectedLineStart))
+            if (DocumentMappingService.TryMapToGeneratedDocumentPosition(csharpDocument, lineStart, out _, out var projectedLineStart))
             {
                 lineStartMap[lineStart] = projectedLineStart;
                 significantLocations.Add(projectedLineStart);
             }
+            else if (owner is CSharpTransitionSyntax &&
+                owner.Parent is RazorDirectiveSyntax containingDirective &&
+                containingDirective.DirectiveDescriptor.Directive == SectionDirective.Directive.Directive)
+            {
+                // Section directives are a challenge because they have Razor indentation (we want to indent their contents one level)
+                // and their contents will have Html indentation, and the generated code for them is indented (contents are in a lambda)
+                // but they have no C# mapping themselves to rely on. In there is no C# in a section block at all, everything works great
+                // but even simple C# poses a challenge. For example:
+                //
+                // @section Goo {
+                //     @if (true)
+                //     {
+                //          // some C# content
+                //     }
+                // }
+                //
+                // The `if` in the generated code will be indented by virtue of being in a lambda, but with nothing in the @section directive
+                // itself that is mapped, the baseline indentation will be whatever happens to be the nearest C# mapping from outside the block
+                // which is not helpful. To solve this, we artificially introduce a mapping for the start of the section block, which points to
+                // the first C# mapping inside it.
+                if (((RazorDirectiveBodySyntax)containingDirective.Body).CSharpCode.Children is [.., MarkupBlockSyntax block, RazorMetaCodeSyntax /* close brace */])
+                {
+                    var blockSpan = block.Span;
+                    foreach (var mapping in csharpDocument.SourceMappings)
+                    {
+                        if (blockSpan.Contains(mapping.OriginalSpan.AbsoluteIndex))
+                        {
+                            var projectedStartLocation = mapping.GeneratedSpan.AbsoluteIndex;
+                            lineStartMap[blockSpan.Start] = projectedStartLocation;
+                            sourceMappingMap[blockSpan.Start] = projectedStartLocation;
+                            significantLocations.Add(projectedStartLocation);
+                            break;
+                        }
+                    }
+                }
+            }
+            else
+            {
+                logger.LogTestOnly($"Couldn't map line: {line.ToString()}");
+            }
         }
 
         // Now, invoke the C# formatter to obtain the CSharpDesiredIndentation for all significant locations.
@@ -377,7 +420,7 @@ private static bool ShouldFormat(FormattingContext context, TextSpan mappingSpan
             return false;
         }
 
-        if (IsInSectionDirectiveBrace())
+        if (IsInSectionDirectiveOrBrace())
         {
             return false;
         }
@@ -548,41 +591,38 @@ bool IsInTemplateBlock()
             return owner.AncestorsAndSelf().Any(n => n is CSharpTemplateBlockSyntax);
         }
 
-        bool IsInSectionDirectiveBrace()
+        bool IsInSectionDirectiveOrBrace()
         {
             // @section Scripts {
             //     <script></script>
             // }
-            //
-            // Due to how sections are generated (inside a multi-line lambda), we want to exclude the braces
+
+            // In design time there is a source mapping for the section name, but it doesn't appear in runtime, so
+            // we effectively pretend it doesn't exist so the formatting engine can handle both forms.
+            if (owner is CSharpStatementLiteralSyntax literal &&
+                owner.Parent?.Parent?.Parent is RazorDirectiveSyntax directive3 &&
+                directive3.DirectiveDescriptor.Directive == SectionDirective.Directive.Directive)
+            {
+                return true;
+            }
+
+            // Due to how sections are generated (inside a multi-line lambda), we also want to exclude the braces
             // from being formatted, or it will be indented by one level due to the lambda. The rest we don't
             // need to worry about, because the one level indent is actually desirable.
 
-            // Due to the Razor tree being so odd, the checks for open and close are surprisingly different
-
-            // Open brace is a child of the C# code block that is the directive itself
+            // Open brace is the 4th child of the C# code block that is the directive itself
+            // and close brace is the last child
             if (owner is RazorMetaCodeSyntax &&
                 owner.Parent is CSharpCodeBlockSyntax codeBlock &&
                 codeBlock.Children.Count > 3 &&
-                owner == codeBlock.Children[3] &&
+                (owner == codeBlock.Children[3] || owner == codeBlock.Children[^1]) &&
                 // CSharpCodeBlock -> RazorDirectiveBody -> RazorDirective
                 codeBlock.Parent?.Parent is RazorDirectiveSyntax directive2 &&
                 directive2.DirectiveDescriptor.Directive == SectionDirective.Directive.Directive)
             {
                 return true;
             }
 
-            // Close brace is a child of the section content, which is a MarkupBlock
-            if (owner is MarkupTextLiteralSyntax &&
-                owner.Parent is MarkupBlockSyntax block &&
-                owner == block.Children[^1] &&
-                // MarkupBlock -> CSharpCodeBlock -> RazorDirectiveBody -> RazorDirective
-                block.Parent?.Parent?.Parent is RazorDirectiveSyntax directive &&
-                directive.DirectiveDescriptor.Directive == SectionDirective.Directive.Directive)
-            {
-                return true;
-            }
-
             return false;
         }
     }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Formatting/Passes/CSharpOnTypeFormattingPass.cs

@@ -167,7 +167,7 @@ protected async override Task<ImmutableArray<TextChange>> ExecuteCoreAsync(Forma
 
         Debug.Assert(cleanedText.Lines.Count > endLineInclusive, "Invalid range. This is unexpected.");
 
-        var indentationChanges = await AdjustIndentationAsync(changedContext, startLine, endLineInclusive, roslynWorkspaceHelper.HostWorkspaceServices, cancellationToken).ConfigureAwait(false);
+        var indentationChanges = await AdjustIndentationAsync(changedContext, startLine, endLineInclusive, roslynWorkspaceHelper.HostWorkspaceServices, _logger, cancellationToken).ConfigureAwait(false);
         if (indentationChanges.Length > 0)
         {
             // Apply the edits that modify indentation.