Fix formatting issue with code block opening braces

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++;
+                }
             }
         }
 

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/DocumentFormattingTest.cs

@@ -11,7 +11,7 @@
 using Xunit.Abstractions;
 
 #if COHOSTING
-namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
+namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost.Formatting;
 #else
 namespace Microsoft.AspNetCore.Razor.LanguageServer.Formatting;
 #endif
@@ -72,6 +72,168 @@ public void Bar() {
             });
     }
 
+    [FormattingTestFact]
+    public async Task RoslynFormatBracesAsKandR_CodeBlockBraceOnNextLine()
+    {
+        await RunFormattingTestAsync(
+            input: """
+                <h1>count is @counter</h1>
+
+                @code
+                {
+                private int counter;
+
+                class Goo
+                {
+                    public void Bar()
+                    {
+                        counter++;
+                    }
+                }
+                }
+                """,
+            expected: """
+                <h1>count is @counter</h1>
+
+                @code
+                {
+                    private int counter;
+
+                    class Goo {
+                        public void Bar() {
+                            counter++;
+                        }
+                    }
+                }
+                """,
+            formattingOptionsOverride: RazorCSharpSyntaxFormattingOptions.Default with
+            {
+                NewLines = RazorNewLinePlacement.None
+            });
+    }
+
+    [FormattingTestFact]
+    public async Task RoslynFormatBracesAsKandR_NoRazorOrHtml()
+    {
+        await RunFormattingTestAsync(
+            input: """
+                @code {
+                private bool IconMenuActive { get; set; } = false;
+                protected void ToggleIconMenu(bool iconMenuActive)
+                {
+                IconMenuActive = iconMenuActive;
+                }
+                }
+                """,
+            expected: """
+                @code {
+                    private bool IconMenuActive { get; set; } = false;
+                    protected void ToggleIconMenu(bool iconMenuActive)
+                    {
+                        IconMenuActive = iconMenuActive;
+                    }
+                }
+                """,
+            formattingOptionsOverride: RazorCSharpSyntaxFormattingOptions.Default with
+            {
+                NewLines = RazorNewLinePlacement.BeforeOpenBraceInMethods
+            });
+    }
+
+    [FormattingTestFact]
+    public async Task RoslynFormatBracesAsKandR_CodeBlockBraceOnNextLine_NoRazorOrHtml()
+    {
+        await RunFormattingTestAsync(
+            input: """
+                @code
+                {
+                private bool IconMenuActive { get; set; } = false;
+                protected void ToggleIconMenu(bool iconMenuActive)
+                {
+                IconMenuActive = iconMenuActive;
+                }
+                }
+                """,
+            expected: """
+                @code
+                {
+                    private bool IconMenuActive { get; set; } = false;
+                    protected void ToggleIconMenu(bool iconMenuActive)
+                    {
+                        IconMenuActive = iconMenuActive;
+                    }
+                }
+                """,
+            formattingOptionsOverride: RazorCSharpSyntaxFormattingOptions.Default with
+            {
+                NewLines = RazorNewLinePlacement.BeforeOpenBraceInMethods
+            });
+    }
+
+    [FormattingTestFact]
+    public async Task RoslynFormatBracesAsKandR_CodeBlockBraceIndented_NoRazorOrHtml()
+    {
+        await RunFormattingTestAsync(
+            input: """
+                @code
+                    {
+                private bool IconMenuActive { get; set; } = false;
+                protected void ToggleIconMenu(bool iconMenuActive)
+                {
+                IconMenuActive = iconMenuActive;
+                }
+                }
+                """,
+            expected: """
+                @code
+                {
+                    private bool IconMenuActive { get; set; } = false;
+                    protected void ToggleIconMenu(bool iconMenuActive)
+                    {
+                        IconMenuActive = iconMenuActive;
+                    }
+                }
+                """,
+            formattingOptionsOverride: RazorCSharpSyntaxFormattingOptions.Default with
+            {
+                NewLines = RazorNewLinePlacement.BeforeOpenBraceInMethods
+            });
+    }
+
+    [FormattingTestFact]
+    public async Task RoslynFormatBracesAsKandR_CodeBlockBraceIndented_InsideHtml()
+    {
+        await RunFormattingTestAsync(
+            input: """
+                <div>
+                @code
+                            {
+                private bool IconMenuActive { get; set; } = false;
+                protected void ToggleIconMenu(bool iconMenuActive)
+                {
+                IconMenuActive = iconMenuActive;
+                }
+                }
+                </div>
+                """,
+            expected: """
+                <div>
+                    @code
+                    {
+                        private bool IconMenuActive { get; set; } = false;
+                        protected void ToggleIconMenu(bool iconMenuActive)
+                        {
+                            IconMenuActive = iconMenuActive;
+                        }
+                    }
+                </div>
+                """,
+            formattingOptionsOverride: RazorCSharpSyntaxFormattingOptions.Default with
+            {
+                NewLines = RazorNewLinePlacement.BeforeOpenBraceInMethods
+            });
+    }
+
     [FormattingTestFact]
     public async Task PropertyShrunkToOneLine()
     {
@@ -6422,8 +6584,8 @@ public Task NestedExplicitExpression3()
     [FormattingTestFact]
     [WorkItem("https://github.com/dotnet/razor/issues/11873")]
     public Task NestedExplicitExpression4()
-    => RunFormattingTestAsync(
-        input: """
+        => RunFormattingTestAsync(
+            input: """
                 @if (true)
                 {
                     <span>
@@ -6442,7 +6604,7 @@ public Task NestedExplicitExpression4()
                     </span>
                 }
                 """,
-        expected: """
+            expected: """
                 @if (true)
                 {
                     <span>

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting_NetFx/OnTypeFormattingTest.cs

@@ -9,7 +9,7 @@
 using Xunit.Abstractions;
 
 #if COHOSTING
-namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
+namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost.Formatting;
 #else
 namespace Microsoft.AspNetCore.Razor.LanguageServer.Formatting;
 #endif

src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostEndpointTestBase.cs

@@ -19,6 +19,7 @@
 using Microsoft.CodeAnalysis.Razor.Remote;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.CodeAnalysis.Remote.Razor;
+using Microsoft.CodeAnalysis.Remote.Razor.Logging;
 using Microsoft.CodeAnalysis.Remote.Razor.SemanticTokens;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.NET.Sdk.Razor.SourceGenerators;
@@ -69,6 +70,9 @@ protected override async Task InitializeAsync()
 
         AddDisposable(_exportProvider);
 
+        var remoteLogger = _exportProvider.GetExportedValue<RemoteLoggerFactory>();
+        remoteLogger.SetTargetLoggerFactory(LoggerFactory);
+
         _remoteServiceInvoker = new TestRemoteServiceInvoker(JoinableTaskContext, _exportProvider, LoggerFactory);
         AddDisposable(_remoteServiceInvoker);