Workaround Roslyn formatting bug

Due to dotnet/roslyn#50129, `FormatAsync` will give us changes past the location that we wanted if there were changes to be made on the current line. This can lead to a frustrating typing experience now that we listen for `\n` in vscode to trigger formatting. I've implemented a simple workaround for this while the bug is fixed upstream.

src/OmniSharp.Roslyn.CSharp/Workers/Formatting/FormattingWorker.cs

@@ -36,7 +36,7 @@ public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattingC
                 Debug.Assert(targetLine.Text != null);
                 if (!string.IsNullOrWhiteSpace(targetLine.Text!.ToString(targetLine.Span)))
                 {
-                    return await GetFormattingChanges(document, targetLine.Start, targetLine.End, omnisharpOptions, loggerFactory);
+                    return await GetFormattingChanges(document, targetLine.Start, targetLine.End, omnisharpOptions, loggerFactory, trimExtendingChanges: true);
                 }
             }
             else if (character == '}' || character == ';')
@@ -47,7 +47,7 @@ public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattingC
                 var node = FindFormatTarget(root!, position);
                 if (node != null)
                 {
-                    return await GetFormattingChanges(document, node.FullSpan.Start, node.FullSpan.End, omnisharpOptions, loggerFactory);
+                    return await GetFormattingChanges(document, node.FullSpan.Start, node.FullSpan.End, omnisharpOptions, loggerFactory, trimExtendingChanges: false);
                 }
             }
 
@@ -91,10 +91,10 @@ public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattingC
             return null;
         }
 
-        public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattingChanges(Document document, int start, int end, OmniSharpOptions omnisharpOptions, ILoggerFactory loggerFactory)
+        public static async Task<IEnumerable<LinePositionSpanTextChange>> GetFormattingChanges(Document document, int start, int end, OmniSharpOptions omnisharpOptions, ILoggerFactory loggerFactory, bool trimExtendingChanges = false)
         {
             var newDocument = await FormatDocument(document, omnisharpOptions, loggerFactory, TextSpan.FromBounds(start, end));
-            return await TextChanges.GetAsync(newDocument, document);
+            return await TextChanges.GetAsync(newDocument, document, trimExtendingChanges ? end : (int?)null);
         }
 
         public static async Task<string> GetFormattedText(Document document, OmniSharpOptions omnisharpOptions, ILoggerFactory loggerFactory)

src/OmniSharp.Roslyn/Utilities/TextChangeHelper.cs

@@ -9,20 +9,20 @@ namespace OmniSharp.Roslyn.Utilities
 {
     internal static class TextChanges
     {
-        public static async Task<IEnumerable<LinePositionSpanTextChange>> GetAsync(Document document, Document oldDocument)
+        public static async Task<IEnumerable<LinePositionSpanTextChange>> GetAsync(Document document, Document oldDocument, int? originalEnd = null)
         {
             var changes = await document.GetTextChangesAsync(oldDocument);
             var oldText = await oldDocument.GetTextAsync();
 
-            return Convert(oldText, changes);
+            return Convert(oldText, changes, originalEnd);
         }
 
         public static IEnumerable<LinePositionSpanTextChange> Convert(SourceText oldText, params TextChange[] changes)
         {
             return Convert(oldText, (IEnumerable<TextChange>)changes);
         }
 
-        public static IEnumerable<LinePositionSpanTextChange> Convert(SourceText oldText, IEnumerable<TextChange> changes)
+        public static IEnumerable<LinePositionSpanTextChange> Convert(SourceText oldText, IEnumerable<TextChange> changes, int? originalEnd = null)
         {
             return changes
                 .OrderByDescending(change => change.Span)
@@ -35,6 +35,32 @@ public static IEnumerable<LinePositionSpanTextChange> Convert(SourceText oldText
 
                     if (newText.Length > 0)
                     {
+                        // Due to https://github.com/dotnet/roslyn/issues/50129, we might get a text change
+                        // that extends further than the requested end bound. This doesn't matter for every
+                        // scenario, but is annoying when hitting enter and having a trailing space on the previous
+                        // line, as that will end up removing the newly-added indentation on the current line.
+                        // For the cases that matters, originalEnd will be not null, and we reduce the end of the span
+                        // to be that location.
+                        if (originalEnd is int oEnd && span.Start < oEnd && span.End > oEnd)
+                        {
+                            // Since the new change is beyond the requested line, it's also going to end in either a
+                            // \r\n or \n, which replaces the newline on the current line. To avoid that newline
+                            // becoming an extra blank line, we extend the span an addition 2 or 1 characters to replace
+                            // that point.
+                            int newLength = span.Length - (span.End - oEnd);
+                            if (newText.EndsWith("\r\n"))
+                            {
+                                newLength += 2;                                
+                            }
+                            else if (newText.EndsWith("\n"))
+                            {
+                                newLength += 1;
+                            }
+
+                            span = new TextSpan(span.Start, newLength);
+                        }
+
+
                         // Roslyn computes text changes on character arrays. So it might happen that a
                         // change starts inbetween \r\n which is OK when you are offset-based but a problem
                         // when you are line,column-based. This code extends text edits which just overlap
@@ -67,7 +93,7 @@ public static IEnumerable<LinePositionSpanTextChange> Convert(SourceText oldText
                         EndLine = linePositionSpan.End.Line,
                         EndColumn = linePositionSpan.End.Character
                     };
-                });
+                }).ToList();
         }
     }
 }

tests/OmniSharp.Roslyn.CSharp.Tests/FormatAfterKeystrokeTests.cs

@@ -360,6 +360,82 @@ public void M()
     {
         ///
 $$
+    }
+}");
+        }
+
+        [Theory]
+        [InlineData("file.cs")]
+        [InlineData("file.csx")]
+        public async Task LeadingSpacesNotRemovedOnNewline_ExtraWhitespaceOnPreviousLine(string fileName)
+        {
+            await VerifyChange(fileName, "\n",
+@"class C
+{
+    public void M()
+    {
+        // There is a space after the ; here
+        var i = 1; 
+        $$
+    }
+}",
+@"class C
+{
+    public void M()
+    {
+        // There is a space after the ; here
+        var i = 1;
+        
+    }
+}");
+        }
+
+        [Theory]
+        [InlineData("file.cs")]
+        [InlineData("file.csx")]
+        public async Task LeadingSpacesNotRemovedOnNewline_FormattingChangesOnPreviousLine(string fileName)
+        {
+            await VerifyChange(fileName, "\n",
+@"class C
+{
+    public void M()
+    {
+        var i =1;
+        $$
+    }
+}",
+@"class C
+{
+    public void M()
+    {
+        var i = 1;
+        
+    }
+}");
+        }
+
+        [Theory]
+        [InlineData("file.cs")]
+        [InlineData("file.csx")]
+        public async Task LeadingSpacesNotRemovedOnNewline_NewlinesWithSpaceAfter(string fileName)
+        {
+            await VerifyChange(fileName, "\n",
+@"class C
+{
+    public void M()
+    {
+        var i =1;
+        $$
+        
+    }
+}",
+@"class C
+{
+    public void M()
+    {
+        var i = 1;
+        
+        
     }
 }");
         }