diff --git a/src/Analyzers/CSharp/CodeFixes/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs index 1f5780bd27a77..16e771dd63693 100644 --- a/src/Analyzers/CSharp/CodeFixes/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs +++ b/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 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(); + + 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); + } + } + + // Update the token and replace it within its parent. + var node = token.GetRequiredParent(); + editor.ReplaceNode( + node, + node.ReplaceToken(token, token.WithLeadingTrivia(leadingTrivia))); } 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(); + } } } diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveRedundantNullableDirectiveTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveRedundantNullableDirectiveTests.cs index ca2d1ccd34d85..8e9539da0ed53 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveRedundantNullableDirectiveTests.cs +++ b/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 diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveTests.cs index 5d261d2930df2..36c8e57cbb8b8 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryNullableDirective/CSharpRemoveUnnecessaryNullableDirectiveTests.cs @@ -86,7 +86,6 @@ enum EnumName """ // File Header - enum EnumName { First,