Update 'sort usings' to always preserve the new-lines that were present in the document (#76332)

Author: CyrusNajmabadi

src/EditorFeatures/Core/Organizing/OrganizeDocumentCommandHandler.cs

@@ -35,7 +35,7 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.Organizing;
 [Name(PredefinedCommandHandlerNames.OrganizeDocument)]
 [method: ImportingConstructor]
 [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
-internal class OrganizeDocumentCommandHandler(
+internal sealed class OrganizeDocumentCommandHandler(
     IThreadingContext threadingContext,
     IAsynchronousOperationListenerProvider listenerProvider) :
     ICommandHandler<OrganizeDocumentCommandArgs>,

src/Workspaces/CSharp/Portable/OrganizeImports/CSharpOrganizeImportsService.Rewriter.cs

@@ -15,28 +15,21 @@ namespace Microsoft.CodeAnalysis.CSharp.OrganizeImports;
 
 internal partial class CSharpOrganizeImportsService
 {
-    private sealed class Rewriter : CSharpSyntaxRewriter
+    private sealed class Rewriter(OrganizeImportsOptions options) : CSharpSyntaxRewriter
     {
-        private readonly bool _placeSystemNamespaceFirst;
-        private readonly bool _separateGroups;
-        private readonly SyntaxTrivia _newLineTrivia;
+        private readonly bool _placeSystemNamespaceFirst = options.PlaceSystemNamespaceFirst;
+        private readonly bool _separateGroups = options.SeparateImportDirectiveGroups;
+        private readonly SyntaxTrivia _fallbackTrivia = CSharpSyntaxGeneratorInternal.Instance.EndOfLine(options.NewLine);
 
         public readonly IList<TextChange> TextChanges = [];
 
-        public Rewriter(OrganizeImportsOptions options)
-        {
-            _placeSystemNamespaceFirst = options.PlaceSystemNamespaceFirst;
-            _separateGroups = options.SeparateImportDirectiveGroups;
-            _newLineTrivia = CSharpSyntaxGeneratorInternal.Instance.EndOfLine(options.NewLine);
-        }
-
         public override SyntaxNode VisitCompilationUnit(CompilationUnitSyntax node)
         {
             node = (CompilationUnitSyntax)base.VisitCompilationUnit(node)!;
             UsingsAndExternAliasesOrganizer.Organize(
                 node.Externs, node.Usings,
                 _placeSystemNamespaceFirst, _separateGroups,
-                _newLineTrivia,
+                _fallbackTrivia,
                 out var organizedExternAliasList, out var organizedUsingList);
 
             var result = node.WithExterns(organizedExternAliasList).WithUsings(organizedUsingList);
@@ -61,7 +54,7 @@ private BaseNamespaceDeclarationSyntax VisitBaseNamespaceDeclaration(BaseNamespa
             UsingsAndExternAliasesOrganizer.Organize(
                 node.Externs, node.Usings,
                 _placeSystemNamespaceFirst, _separateGroups,
-                _newLineTrivia,
+                _fallbackTrivia,
                 out var organizedExternAliasList, out var organizedUsingList);
 
             var result = node.WithExterns(organizedExternAliasList).WithUsings(organizedUsingList);

src/Workspaces/CSharp/Portable/OrganizeImports/CSharpOrganizeImportsService.cs

@@ -2,32 +2,28 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Composition;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.OrganizeImports;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.OrganizeImports;
 
 [ExportLanguageService(typeof(IOrganizeImportsService), LanguageNames.CSharp), Shared]
-internal partial class CSharpOrganizeImportsService : IOrganizeImportsService
+[method: ImportingConstructor]
+[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+internal sealed partial class CSharpOrganizeImportsService() : IOrganizeImportsService
 {
-    [ImportingConstructor]
-    [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-    public CSharpOrganizeImportsService()
-    {
-    }
-
     public async Task<Document> OrganizeImportsAsync(Document document, OrganizeImportsOptions options, CancellationToken cancellationToken)
     {
         var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
         var rewriter = new Rewriter(options);
         var newRoot = rewriter.Visit(root);
+        Contract.ThrowIfNull(newRoot);
 
         return document.WithSyntaxRoot(newRoot);
     }

src/Workspaces/CSharpTest/OrganizeImports/OrganizeUsingsTests.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.OrganizeImports;
@@ -16,10 +17,11 @@ namespace Microsoft.CodeAnalysis.CSharp.Workspaces.UnitTests.OrganizeImports;
 
 [UseExportProvider]
 [Trait(Traits.Feature, Traits.Features.Organizing)]
-public class OrganizeUsingsTests
+public sealed class OrganizeUsingsTests
 {
-    protected static async Task CheckAsync(
-        string initial, string final,
+    private static async Task CheckAsync(
+        string initial,
+        string final,
         bool placeSystemNamespaceFirst = false,
         bool separateImportGroups = false,
         string? endOfLine = null)
@@ -76,6 +78,32 @@ public async Task AliasesAtBottom()
         await CheckAsync(initial, final);
     }
 
+    [Theory, WorkItem("https://github.com/dotnet/roslyn/issues/44136")]
+    [InlineData("\n")]
+    [InlineData("\r\n")]
+    public async Task PreserveExistingEndOfLine(string fallbackEndOfLine)
+    {
+        var initial = "using A = B;\nusing C;\nusing D = E;\nusing F;\n";
+
+        var final = "using C;\nusing F;\nusing A = B;\nusing D = E;\n";
+
+        using var workspace = new AdhocWorkspace();
+        var project = workspace.CurrentSolution.AddProject("Project", "Project.dll", LanguageNames.CSharp);
+        var document = project.AddDocument("Document", initial);
+
+        var options = new OrganizeImportsOptions()
+        {
+            PlaceSystemNamespaceFirst = false,
+            SeparateImportDirectiveGroups = false,
+            NewLine = fallbackEndOfLine,
+        };
+
+        var organizeImportsService = document.GetRequiredLanguageService<IOrganizeImportsService>();
+        var newDocument = await organizeImportsService.OrganizeImportsAsync(document, options, CancellationToken.None);
+        var newRoot = await newDocument.GetRequiredSyntaxRootAsync(default);
+        Assert.Equal(final, newRoot.ToFullString());
+    }
+
     [Fact]
     public async Task UsingStaticsBetweenUsingsAndAliases()
     {

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Utilities/UsingsAndExternAliasesOrganizer.cs

@@ -16,11 +16,20 @@ internal static partial class UsingsAndExternAliasesOrganizer
     public static void Organize(
         SyntaxList<ExternAliasDirectiveSyntax> externAliasList,
         SyntaxList<UsingDirectiveSyntax> usingList,
-        bool placeSystemNamespaceFirst, bool separateGroups,
-        SyntaxTrivia newLineTrivia,
+        bool placeSystemNamespaceFirst,
+        bool separateGroups,
+        SyntaxTrivia fallbackTrivia,
         out SyntaxList<ExternAliasDirectiveSyntax> organizedExternAliasList,
         out SyntaxList<UsingDirectiveSyntax> organizedUsingList)
     {
+        // Attempt to use an existing newline trivia from the existing usings/externs.  If we can't find any use what
+        // the caller passed in.
+        var newLineTrivia = ((IEnumerable<SyntaxNode>)externAliasList)
+            .Concat(usingList)
+            .Select(n => n.GetTrailingTrivia().FirstOrNull(t => t.Kind() == SyntaxKind.EndOfLineTrivia))
+            .Where(t => t != null)
+            .FirstOrDefault() ?? fallbackTrivia;
+
         OrganizeWorker(
             externAliasList, usingList, placeSystemNamespaceFirst,
             newLineTrivia,