Fix code block brace formatting when Roslyn moves the brace up a line

Author: davidwengier

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

@@ -85,31 +85,27 @@ internal static bool IsAttributeDirective(this SyntaxNode node, [NotNullWhen(tru
         return false;
     }
 
-    internal static bool IsCodeDirective(this SyntaxNode node, out SyntaxToken openBraceToken)
+    internal static bool IsCodeDirective(this SyntaxNode node)
     {
         if (IsDirective(node, ComponentCodeDirective.Directive, out var body) &&
             body.CSharpCode is { Children: { Count: > 0 } children } &&
-            children.TryGetOpenBraceToken(out var openBrace))
+            children.TryGetOpenBraceToken(out _))
         {
-            openBraceToken = openBrace;
             return true;
         }
 
-        openBraceToken = default;
         return false;
     }
 
-    internal static bool IsFunctionsDirective(this SyntaxNode node, out SyntaxToken openBraceToken)
+    internal static bool IsFunctionsDirective(this SyntaxNode node)
     {
         if (IsDirective(node, FunctionsDirective.Directive, out var body) &&
             body.CSharpCode is { Children: { Count: > 0 } children } &&
-            children.TryGetOpenBraceToken(out var openBrace))
+            children.TryGetOpenBraceToken(out _))
         {
-            openBraceToken = openBrace;
             return true;
         }
 
-        openBraceToken = default;
         return false;
     }
 

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

@@ -695,14 +695,10 @@ public override LineInfo VisitRazorDirective(RazorDirectiveSyntax node)
                     return VisitTypeParamDirective(typeParam, conditions);
                 }
 
-                if (node.IsCodeDirective(out var openBrace))
+                if (node.IsCodeDirective() ||
+                    node.IsFunctionsDirective())
                 {
-                    return VisitCodeOrFunctionsDirective(openBrace);
-                }
-
-                if (node.IsFunctionsDirective(out var functionsOpenBrace))
-                {
-                    return VisitCodeOrFunctionsDirective(functionsOpenBrace);
+                    return VisitCodeOrFunctionsDirective();
                 }
 
                 // All other directives that have braces are handled here
@@ -721,25 +717,17 @@ body.CSharpCode is CSharpCodeBlockSyntax code &&
                 return EmitCurrentLineAsComment();
             }
 
-            private LineInfo VisitCodeOrFunctionsDirective(RazorSyntaxToken openBrace)
+            private LineInfo VisitCodeOrFunctionsDirective()
             {
-                // If the open brace is on the same line as the directive, then we need to ensure the contents are indented
-                if (GetLineNumber(openBrace) == GetLineNumber(_currentToken))
-                {
-                    // If its an @code or @functions we want to wrap the contents in a class
-                    // so that access modifiers are valid, and will be formatted as appropriate.
-                    _builder.AppendLine("class F");
-                    _builder.AppendLine("{");
-
-                    // Roslyn might move our brace to the previous line, so we might _not_ need to skip it 🤦‍
-                    return CreateLineInfo(skipNextLineIfBrace: true);
-                }
+                // If its an @code or @functions we want to wrap the contents in a class so that access modifiers
+                // on any members declared within it are valid, and will be formatted as appropriate.
+                // We let the users content be the class name, as it will either be "@code" or "@functions", which
+                // are both valid, and it might have an open brace after it, or that might be on the next line,
+                // but if we just let that flow to the generated document, we don't need to do any fancy checking.
+                _builder.Append("class ");
+                _builder.AppendLine(_currentLine.ToString());
 
-                // If the braces are on different lines, then we can do nothing, unless its an @code or @functions
-                // in which case we need to use a class. Note we don't output an open brace, as the next line of
-                // the original file will have one.
-                _builder.AppendLine("class F");
-                return CreateLineInfo();
+                return CreateLineInfo(skipNextLineIfBrace: true);
             }
 
             private LineInfo VisitUsingDirective()

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

@@ -58,6 +58,11 @@ public async Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext con
                 iFormatted++;
             }
 
+            if (iFormatted >= formattedCSharpText.Lines.Count)
+            {
+                break;
+            }
+
             var formattedLine = formattedCSharpText.Lines[iFormatted];
             if (lineInfo.ProcessIndentation &&
                 formattedLine.GetFirstNonWhitespaceOffset() is { } formattedIndentation)
@@ -139,14 +144,26 @@ public async Task<ImmutableArray<TextChange>> ExecuteAsync(FormattingContext con
             }
             else if (lineInfo.SkipNextLineIfBrace)
             {
-                // If the next line is a brace, we skip it, otherwise we don't. This is used to skip the opening brace of a class
+                // If the next line is a brace, we skip it. This is used to skip the opening brace of a class
                 // that we insert, but Roslyn settings might place on the same like as the class declaration.
                 if (iFormatted + 1 < formattedCSharpText.Lines.Count &&
                     formattedCSharpText.Lines[iFormatted + 1] is { Span.Length: > 0 } nextLine &&
                     nextLine.CharAt(0) == '{')
                 {
                     iFormatted++;
                 }
+
+                // On the other hand, we might insert the opening brace of a class, and Roslyn might collapse
+                // it up to the previous line, so we would want to skip the next line in the original document
+                // in that case. Fortunately its illegal to have `@code {\r\n {` in a Razor file, so there can't
+                // be false positives here.
+                if (iOriginal + 1 < changedText.Lines.Count &&
+                    changedText.Lines[iOriginal + 1] is { } nextOriginalLine &&
+                    nextOriginalLine.GetFirstNonWhitespaceOffset() is { } firstChar &&
+                    nextOriginalLine.CharAt(firstChar) == '{')
+                {
+                    iOriginal++;
+                }
             }
         }