From ea8b49492bf237834e047eb58d9a20cd23dfeea5 Mon Sep 17 00:00:00 2001 From: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Date: Mon, 11 Apr 2022 06:21:52 +0300 Subject: [PATCH] Trim unnessasary leading lines when removing usings (#60672) --- .../RemoveUnnecessaryImportsTests.cs | 139 ++++++++++++++++++ .../RemoveUnnecessaryImportsTests.vb | 103 +++++++++++++ ...emoveUnnecessaryImportsService.Rewriter.cs | 30 +++- ...emoveUnnecessaryImportsService.Rewriter.vb | 26 ++++ 4 files changed, 297 insertions(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs index 9955d78a33589..e2f714c21ccc6 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs @@ -1984,5 +1984,144 @@ static void Main(string[] args) LanguageVersion = LanguageVersion.CSharp10, }.RunAsync(); } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + [WorkItem(45866, "https://github.com/dotnet/roslyn/issues/45866")] + public async Task TestUsingGroups_DeleteLeadingBlankLinesIfFirstGroupWasDeleted_SingleUsing() + { + await new VerifyCS.Test + { + TestCode = +@"[|{|IDE0005:using System;|} + +using System.Collections.Generic;|] + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +", + FixedCode = +@"using System.Collections.Generic; + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +" + }.RunAsync(); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + [WorkItem(45866, "https://github.com/dotnet/roslyn/issues/45866")] + public async Task TestUsingGroups_DeleteLeadingBlankLinesIfFirstGroupWasDeleted_MultipleUsings() + { + await new VerifyCS.Test + { + TestCode = +@"[|{|IDE0005:using System; +using System.Threading.Tasks;|} + +using System.Collections.Generic;|] + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +", + FixedCode = +@"using System.Collections.Generic; + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +" + }.RunAsync(); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + [WorkItem(45866, "https://github.com/dotnet/roslyn/issues/45866")] + public async Task TestUsingGroups_NotAllFirstGroupIsDeleted() + { + await new VerifyCS.Test + { + TestCode = +@"[|{|IDE0005:using System;|} +using System.Threading.Tasks; + +using System.Collections.Generic;|] + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + Task task = null; + } +} +", + FixedCode = +@"using System.Threading.Tasks; + +using System.Collections.Generic; + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + Task task = null; + } +} +" + }.RunAsync(); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + [WorkItem(45866, "https://github.com/dotnet/roslyn/issues/45866")] + public async Task TestUsingGroups_AllLastGroupIsDeleted() + { + await new VerifyCS.Test + { + TestCode = +@"[|using System.Collections.Generic; + +{|IDE0005:using System; +using System.Threading.Tasks;|}|] + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +", + FixedCode = +@"using System.Collections.Generic; + +class Program +{ + static void Main(string[] args) + { + var argList = new List(args); + } +} +" + }.RunAsync(); + } } } diff --git a/src/Analyzers/VisualBasic/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.vb b/src/Analyzers/VisualBasic/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.vb index 3c1a0f3a09889..313984f4194a0 100644 --- a/src/Analyzers/VisualBasic/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.vb +++ b/src/Analyzers/VisualBasic/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.vb @@ -1214,5 +1214,108 @@ Imports System Event E() End Class|]") End Function + + + + Public Async Function TestImportGroup_DeleteLeadingBlankLinesIfFirstGroupWasDeleted_SingleImport() As Task + Await TestInRegularAndScript1Async( +"[|Imports System.Threading.Tasks + +Imports System|] + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +", +"Imports System + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +") + End Function + + + + Public Async Function TestImportGroup_DeleteLeadingBlankLinesIfFirstGroupWasDeleted_MultipleImports() As Task + Await TestInRegularAndScript1Async( +"[|Imports System.Threading.Tasks +Imports System.Collections.Generic + +Imports System|] + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +", +"Imports System + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +") + End Function + + + + Public Async Function TestImportGroup_NotAllFirstGroupIsDeleted() As Task + Await TestInRegularAndScript1Async( +"[|Imports System.Threading.Tasks +Imports System.Collections.Generic + +Imports System|] + +Class C + Function Test() + Console.WriteLine() + Dim list As List(Of Integer) = Nothing + End Function +End Class +", +"Imports System.Collections.Generic + +Imports System + +Class C + Function Test() + Console.WriteLine() + Dim list As List(Of Integer) = Nothing + End Function +End Class +") + End Function + + + + Public Async Function TestImportGroup_AllLastGroupIsDeleted() As Task + Await TestInRegularAndScript1Async( +"[|Imports System + +Imports System.Threading.Tasks +Imports System.Collections.Generic|] + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +", +"Imports System + +Class C + Function Test() + Console.WriteLine() + End Function +End Class +") + End Function End Class End Namespace diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs index abb7598f08d4a..58d544ab8a313 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs @@ -9,7 +9,6 @@ using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CSharp.Extensions; -using Microsoft.CodeAnalysis.CSharp.Formatting; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -48,6 +47,8 @@ private static void ProcessUsings( out SyntaxTriviaList finalTrivia) { var currentUsings = new List(usings); + var firstUsingNotBeingRemoved = true; + var passedLeadngTrivia = false; finalTrivia = default; for (var i = 0; i < usings.Count; i++) @@ -83,6 +84,8 @@ private static void ProcessUsings( // want to preserve. currentUsings[nextIndex] = nextUsing.WithLeadingTrivia(leadingTrivia); } + + passedLeadngTrivia = true; } else { @@ -90,6 +93,31 @@ private static void ProcessUsings( } } } + else if (firstUsingNotBeingRemoved) + { + // 1) We only apply this logic for not first using, that is saved: + // =================== + // namespace N; + // + // using System; <- if we save this using, we don't need to cut leading lines + // =================== + // 2) If leading trivia was saved from the previous using, that was removed, + // we don't bother cutting blank lines as well: + // =================== + // namespace N; + // + // using System; <- need to delete this using + // using System.Collections.Generic; <- this using is saved, no need to eat the line, + // otherwise https://github.com/dotnet/roslyn/issues/58972 will happen + if (i > 0 && !passedLeadngTrivia) + { + var currentUsing = currentUsings[i]; + var currentUsingLeadingTrivia = currentUsing.GetLeadingTrivia(); + currentUsings[i] = currentUsing.WithLeadingTrivia(currentUsingLeadingTrivia.WithoutLeadingBlankLines()); + } + + firstUsingNotBeingRemoved = false; + } } finalUsings = currentUsings.WhereNotNull().ToSyntaxList(); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/VisualBasic/LanguageServices/VisualBasicRemoveUnnecessaryImportsService.Rewriter.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/VisualBasic/LanguageServices/VisualBasicRemoveUnnecessaryImportsService.Rewriter.vb index d04c47dba064e..65e9b36a8ac75 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/VisualBasic/LanguageServices/VisualBasicRemoveUnnecessaryImportsService.Rewriter.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/VisualBasic/LanguageServices/VisualBasicRemoveUnnecessaryImportsService.Rewriter.vb @@ -39,6 +39,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports Private Function ProcessImports(compilationUnit As CompilationUnitSyntax) As CompilationUnitSyntax Dim oldImports = compilationUnit.Imports.ToList() + Dim firstImportNotBeingRemoved = True + Dim passedLeadingTrivia = False Dim remainingTrivia As SyntaxTriviaList = Nothing For i = 0 To oldImports.Count - 1 @@ -65,6 +67,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports ' want to preserve. oldImports(nextIndex) = nextImport.WithLeadingTrivia(leadingTrivia) End If + + passedLeadingTrivia = True Else remainingTrivia = leadingTrivia End If @@ -85,6 +89,28 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnnecessaryImports oldImports(index) = previousImport.WithTrailingTrivia(trailingTrivia) End If End If + ElseIf firstImportNotBeingRemoved Then + ' 1) We only apply this logic for Not first using, that is saved: + ' =================== + ' #Const A = 1 + ' + ' Imports System <- if we save this import, we don't need to cut leading lines + ' =================== + ' 2) If leading trivia was saved from the previous import, that was removed, + ' we don't bother cutting blank lines as well: + ' =================== + ' #Const A = 1 + ' + ' Imports System <- need to delete this import + ' Imports System.Collections.Generic <- this import is saved, no need to eat the line, + ' otherwise https://github.com/dotnet/roslyn/issues/58972 will happen + If i > 0 AndAlso Not passedLeadingTrivia Then + Dim currentImport = oldImports(i) + Dim currentImportLeadingTrivia = currentImport.GetLeadingTrivia() + oldImports(i) = currentImport.WithLeadingTrivia(currentImportLeadingTrivia.WithoutLeadingWhitespaceOrEndOfLine()) + End If + + firstImportNotBeingRemoved = False End If Next