Remove at most 1 end of line preceding redundant nullable directives

src/Analyzers/CSharp/CodeFixes/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs

@@ -5,28 +5,28 @@
 using System;
 using System.Collections.Immutable;
 using System.Composition;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Extensions;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Shared.Extensions;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.CodeFixes.RemoveUnnecessaryNullableDirective
 {
     [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.RemoveUnnecessaryNullableDirective)]
     [Shared]
-    internal sealed class CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider
+    [method: ImportingConstructor]
+    [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+    internal sealed class CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider()
         : SyntaxEditorBasedCodeFixProvider
     {
-        [ImportingConstructor]
-        [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-        public CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider()
-        {
-        }
-
         public override ImmutableArray<string> FixableDiagnosticIds
             => [
                 IDEDiagnosticIds.RemoveRedundantNullableDirectiveDiagnosticId,
@@ -53,13 +53,63 @@ protected override Task FixAllAsync(
             CodeActionOptionsProvider fallbackOptions,
             CancellationToken cancellationToken)
         {
-            foreach (var diagnostic in diagnostics)
+            // We first group the nullable directives by the token they are attached This allows to replace each token
+            // separately even if they have multiple nullable directives.
+            var nullableDirectives = diagnostics
+                .Select(d => d.Location.FindNode(findInsideTrivia: true, getInnermostNodeForTie: true, cancellationToken))
+                .OfType<NullableDirectiveTriviaSyntax>();
+
+            foreach (var (token, directives) in nullableDirectives.GroupBy(d => d.ParentTrivia.Token))
             {
-                var nullableDirective = diagnostic.Location.FindNode(findInsideTrivia: true, getInnermostNodeForTie: true, cancellationToken);
-                editor.RemoveNode(nullableDirective, SyntaxRemoveOptions.KeepNoTrivia);
+                var leadingTrivia = token.LeadingTrivia;
+                var nullableDirectiveIndices = nullableDirectives
+                    .Select(x => leadingTrivia.IndexOf(x.ParentTrivia));
+
+                // Walk backwards through the nullable directives on the token.  That way our indices stay correct as we
+                // are removing later directives.
+                foreach (var index in nullableDirectiveIndices.OrderByDescending(x => x))
+                {
+                    // Remove the directive itself.
+                    leadingTrivia = leadingTrivia.RemoveAt(index);
+
+                    // If we have a blank line both before and after the directive, then remove the one that follows to
+                    // keep the code clean.
+                    if (HasPrecedingBlankLine(leadingTrivia, index - 1) &&
+                        HasFollowingBlankLine(leadingTrivia, index))
+                    {
+                        // Delete optional following whitespace.
+                        if (leadingTrivia[index].IsWhitespace())
+                            leadingTrivia = leadingTrivia.RemoveAt(index);
+
+                        // Then the following blank line.
+                        leadingTrivia = leadingTrivia.RemoveAt(index);
+                    }
+                }
+
+                // We replace the leading trivias of the token the nullable directive was attached,
+                // then replace the token on the node and finally, replace the node with its new token.
+                var newToken = token.WithLeadingTrivia(leadingTrivia);
+                var node = token.GetRequiredParent();
+                editor.ReplaceNode(node, node.ReplaceToken(token, newToken));
             }
 
             return Task.CompletedTask;
         }
+
+        private static bool HasPrecedingBlankLine(SyntaxTriviaList leadingTrivia, int index)
+        {
+            if (index >= 0 && leadingTrivia[index].IsWhitespace())
+                index--;
+
+            return index >= 0 && leadingTrivia[index].IsEndOfLine();
+        }
+
+        private static bool HasFollowingBlankLine(SyntaxTriviaList leadingTrivia, int index)
+        {
+            if (index < leadingTrivia.Count && leadingTrivia[index].IsWhitespace())
+                index++;
+
+            return index < leadingTrivia.Count && leadingTrivia[index].IsEndOfLine();
+        }
     }
 }

src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveRedundantNullableDirectiveTests.cs

@@ -182,13 +182,97 @@ class Program
                 """
                 // File Header
 
-
                 class Program
                 {
                 }
                 """);
         }
 
+        [Fact]
+        public async Task TestRedundantDirectiveBetweenUsingAndNamespace()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveBetweenUsingAndNamespace2()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveBetweenUsingAndNamespace3()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+
+                [|#nullable enable|]
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
         [Fact]
         public async Task TestRedundantDirectiveWithNamespaceAndDerivedType()
         {
@@ -219,6 +303,155 @@ class ProgramException : Exception
                 """);
         }
 
+        [Fact]
+        public async Task TestRedundantDirectiveMultiple1()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+
+                [|#nullable enable|]
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveMultiple2()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+
+                [|#nullable enable|]
+
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveMultiple3()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+                [|#nullable enable|]
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveMultiple4()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+                [|#nullable enable|]
+
+                [|#nullable enable|]
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
+        [Fact]
+        public async Task TestRedundantDirectiveMultiple5()
+        {
+            await VerifyCodeFixAsync(
+                NullableContextOptions.Enable,
+                """
+                using System;
+
+                [|#nullable enable|]
+                [|#nullable enable|]
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """,
+                """
+                using System;
+
+                namespace MyNamespace
+                {
+                    class MyClass
+                    {
+                    }
+                }
+                """);
+        }
+
         private static string GetDisableDirectiveContext(NullableContextOptions options)
         {
             return options switch

src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveTests.cs

@@ -86,7 +86,6 @@ enum EnumName
                 """
                 // File Header
 
-
                 enum EnumName
                 {
                     First,