Skip to content

Commit

Permalink
Merge pull request #57365 from sharwell/organize-newline
Browse files Browse the repository at this point in the history
Use correct line endings when sorting imports
  • Loading branch information
sharwell authored Oct 26, 2021
2 parents 23f8bf5 + 5aa2afe commit adf1456
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ private class Rewriter : CSharpSyntaxRewriter
{
private readonly bool _placeSystemNamespaceFirst;
private readonly bool _separateGroups;
private readonly SyntaxTrivia _newLineTrivia;

public readonly IList<TextChange> TextChanges = new List<TextChange>();

public Rewriter(bool placeSystemNamespaceFirst,
bool separateGroups)
bool separateGroups,
SyntaxTrivia newLineTrivia)
{
_placeSystemNamespaceFirst = placeSystemNamespaceFirst;
_separateGroups = separateGroups;
_newLineTrivia = newLineTrivia;
}

public override SyntaxNode VisitCompilationUnit(CompilationUnitSyntax node)
Expand All @@ -33,6 +36,7 @@ public override SyntaxNode VisitCompilationUnit(CompilationUnitSyntax node)
UsingsAndExternAliasesOrganizer.Organize(
node.Externs, node.Usings,
_placeSystemNamespaceFirst, _separateGroups,
_newLineTrivia,
out var organizedExternAliasList, out var organizedUsingList);

var result = node.WithExterns(organizedExternAliasList).WithUsings(organizedUsingList);
Expand All @@ -57,6 +61,7 @@ private BaseNamespaceDeclarationSyntax VisitBaseNamespaceDeclaration(BaseNamespa
UsingsAndExternAliasesOrganizer.Organize(
node.Externs, node.Usings,
_placeSystemNamespaceFirst, _separateGroups,
_newLineTrivia,
out var organizedExternAliasList, out var organizedUsingList);

var result = node.WithExterns(organizedExternAliasList).WithUsings(organizedUsingList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.CodeGeneration;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.OrganizeImports;

Expand All @@ -30,8 +32,9 @@ public async Task<Document> OrganizeImportsAsync(Document document, Cancellation

var placeSystemNamespaceFirst = options.GetOption(GenerationOptions.PlaceSystemNamespaceFirst);
var blankLineBetweenGroups = options.GetOption(GenerationOptions.SeparateImportDirectiveGroups);
var newLineTrivia = CSharpSyntaxGeneratorInternal.Instance.EndOfLine(options.GetOption(FormattingOptions2.NewLine));

var rewriter = new Rewriter(placeSystemNamespaceFirst, blankLineBetweenGroups);
var rewriter = new Rewriter(placeSystemNamespaceFirst, blankLineBetweenGroups, newLineTrivia);
var newRoot = rewriter.Visit(root);

return document.WithSyntaxRoot(newRoot);
Expand Down
51 changes: 48 additions & 3 deletions src/Workspaces/CSharpTest/OrganizeImports/OrganizeUsingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editing;
Expand All @@ -20,18 +21,23 @@ public class OrganizeUsingsTests
protected static async Task CheckAsync(
string initial, string final,
bool placeSystemNamespaceFirst = false,
bool separateImportGroups = false)
bool separateImportGroups = false,
string? endOfLine = null)
{
using var workspace = new AdhocWorkspace();
var project = workspace.CurrentSolution.AddProject("Project", "Project.dll", LanguageNames.CSharp);
var document = project.AddDocument("Document", initial.NormalizeLineEndings());
var document = project.AddDocument("Document", initial.ReplaceLineEndings(endOfLine ?? Environment.NewLine));

var newOptions = workspace.Options.WithChangedOption(new OptionKey(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language), placeSystemNamespaceFirst);
newOptions = newOptions.WithChangedOption(new OptionKey(GenerationOptions.SeparateImportDirectiveGroups, document.Project.Language), separateImportGroups);

if (endOfLine is not null)
newOptions = newOptions.WithChangedOption(new OptionKey(FormattingOptions2.NewLine, document.Project.Language), endOfLine);

document = document.WithSolutionOptions(newOptions);

var newRoot = await (await Formatter.OrganizeImportsAsync(document, CancellationToken.None)).GetRequiredSyntaxRootAsync(default);
Assert.Equal(final.NormalizeLineEndings(), newRoot.ToFullString());
Assert.Equal(final.ReplaceLineEndings(endOfLine ?? Environment.NewLine), newRoot.ToFullString());
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -1194,5 +1200,44 @@ public async Task TestGrouping2()

await CheckAsync(initial, final, placeSystemNamespaceFirst: true, separateImportGroups: true);
}

[WorkItem(20988, "https://github.com/dotnet/roslyn/issues/19306")]
[Theory, Trait(Traits.Feature, Traits.Features.Organizing)]
[InlineData("\n")]
[InlineData("\r\n")]
public async Task TestGrouping3(string endOfLine)
{
var initial =
@"// Banner
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;
using IntList = System.Collections.Generic.List<int>;
using static System.Console;";

var final =
@"// Banner
using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;
using static System.Console;
using IntList = System.Collections.Generic.List<int>;
";

await CheckAsync(initial, final, placeSystemNamespaceFirst: true, separateImportGroups: true, endOfLine: endOfLine);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ namespace Microsoft.CodeAnalysis.CSharp.Utilities
{
internal static partial class UsingsAndExternAliasesOrganizer
{
private static readonly SyntaxTrivia s_newLine = SyntaxFactory.CarriageReturnLineFeed;

public static void Organize(
SyntaxList<ExternAliasDirectiveSyntax> externAliasList,
SyntaxList<UsingDirectiveSyntax> usingList,
bool placeSystemNamespaceFirst, bool separateGroups,
SyntaxTrivia newLineTrivia,
out SyntaxList<ExternAliasDirectiveSyntax> organizedExternAliasList,
out SyntaxList<UsingDirectiveSyntax> organizedUsingList)
{
OrganizeWorker(
externAliasList, usingList, placeSystemNamespaceFirst,
newLineTrivia,
out organizedExternAliasList, out organizedUsingList);

if (separateGroups)
Expand All @@ -34,7 +34,7 @@ public static void Organize(

if (!firstUsing.GetLeadingTrivia().Any(t => t.IsEndOfLine()))
{
var newFirstUsing = firstUsing.WithPrependedLeadingTrivia(s_newLine);
var newFirstUsing = firstUsing.WithPrependedLeadingTrivia(newLineTrivia);
organizedUsingList = organizedUsingList.Replace(firstUsing, newFirstUsing);
}
}
Expand All @@ -47,7 +47,7 @@ public static void Organize(
if (NeedsGrouping(lastUsing, currentUsing) &&
!currentUsing.GetLeadingTrivia().Any(t => t.IsEndOfLine()))
{
var newCurrentUsing = currentUsing.WithPrependedLeadingTrivia(s_newLine);
var newCurrentUsing = currentUsing.WithPrependedLeadingTrivia(newLineTrivia);
organizedUsingList = organizedUsingList.Replace(currentUsing, newCurrentUsing);
}
}
Expand Down Expand Up @@ -94,6 +94,7 @@ private static void OrganizeWorker(
SyntaxList<ExternAliasDirectiveSyntax> externAliasList,
SyntaxList<UsingDirectiveSyntax> usingList,
bool placeSystemNamespaceFirst,
SyntaxTrivia newLineTrivia,
out SyntaxList<ExternAliasDirectiveSyntax> organizedExternAliasList,
out SyntaxList<UsingDirectiveSyntax> organizedUsingList)
{
Expand Down Expand Up @@ -123,7 +124,7 @@ private static void OrganizeWorker(
if (!finalList.SequenceEqual(initialList))
{
// Make sure newlines are correct between nodes.
EnsureNewLines(finalList);
EnsureNewLines(finalList, newLineTrivia);

// Reattach the banner.
finalList[0] = finalList[0].WithPrependedLeadingTrivia(leadingTrivia);
Expand All @@ -144,7 +145,7 @@ private static void OrganizeWorker(
organizedUsingList = usingList;
}

private static void EnsureNewLines(IList<SyntaxNode> list)
private static void EnsureNewLines(IList<SyntaxNode> list, SyntaxTrivia newLineTrivia)
{
// First, make sure that every node (except the last one) ends with
// a newline.
Expand All @@ -155,7 +156,7 @@ private static void EnsureNewLines(IList<SyntaxNode> list)

if (!trailingTrivia.Any() || trailingTrivia.Last().Kind() != SyntaxKind.EndOfLineTrivia)
{
list[i] = node.WithTrailingTrivia(trailingTrivia.Concat(s_newLine));
list[i] = node.WithTrailingTrivia(trailingTrivia.Concat(newLineTrivia));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
Partial Friend Class ImportsOrganizer
Private Shared ReadOnly s_newLine As SyntaxTrivia = SyntaxFactory.CarriageReturnLineFeed

Public Shared Function Organize([imports] As SyntaxList(Of ImportsStatementSyntax),
placeSystemNamespaceFirst As Boolean,
separateGroups As Boolean) As SyntaxList(Of ImportsStatementSyntax)
separateGroups As Boolean,
newLineTrivia As SyntaxTrivia) As SyntaxList(Of ImportsStatementSyntax)

[imports] = OrganizeWorker([imports], placeSystemNamespaceFirst)
[imports] = OrganizeWorker([imports], placeSystemNamespaceFirst, newLineTrivia)

If separateGroups Then
For i = 1 To [imports].Count - 1
Expand All @@ -25,7 +24,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
If NeedsGrouping(lastImport, currentImport) AndAlso
Not currentImport.GetLeadingTrivia().Any(Function(t) t.IsEndOfLine()) Then
[imports] = [imports].Replace(
currentImport, currentImport.WithPrependedLeadingTrivia(s_newLine))
currentImport, currentImport.WithPrependedLeadingTrivia(newLineTrivia))
End If
Next
End If
Expand Down Expand Up @@ -71,7 +70,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
End Function

Public Shared Function OrganizeWorker([imports] As SyntaxList(Of ImportsStatementSyntax),
placeSystemNamespaceFirst As Boolean) As SyntaxList(Of ImportsStatementSyntax)
placeSystemNamespaceFirst As Boolean,
newLineTrivia As SyntaxTrivia) As SyntaxList(Of ImportsStatementSyntax)
If [imports].Count > 1 Then
Dim initialList = New List(Of ImportsStatementSyntax)([imports])
If Not [imports].SpansPreprocessorDirective() Then
Expand All @@ -88,7 +88,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
' not, then we don't need to make any changes to the file.
If Not finalList.SequenceEqual(initialList) Then
'' Make sure newlines are correct between nodes.
EnsureNewLines(finalList)
EnsureNewLines(finalList, newLineTrivia)

' Reattach the banner.
finalList(0) = finalList(0).WithLeadingTrivia(leadingTrivia.Concat(finalList(0).GetLeadingTrivia()).ToSyntaxTriviaList())
Expand All @@ -101,9 +101,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Utilities
Return [imports]
End Function

Private Shared Sub EnsureNewLines(list As List(Of ImportsStatementSyntax))
Private Shared Sub EnsureNewLines(list As List(Of ImportsStatementSyntax), newLineTrivia As SyntaxTrivia)
Dim endOfLine = GetExistingEndOfLineTrivia(list)
endOfLine = If(endOfLine.Kind = SyntaxKind.None, s_newLine, endOfLine)
endOfLine = If(endOfLine.Kind = SyntaxKind.None, newLineTrivia, endOfLine)

For i = 0 To list.Count - 1
If Not list(i).GetTrailingTrivia().Any(SyntaxKind.EndOfLineTrivia) Then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,20 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.OrganizeImports

Private ReadOnly _placeSystemNamespaceFirst As Boolean
Private ReadOnly _separateGroups As Boolean
Private ReadOnly _newLineTrivia As SyntaxTrivia

Public ReadOnly TextChanges As IList(Of TextChange) = New List(Of TextChange)()

Public Sub New(placeSystemNamespaceFirst As Boolean, separateGroups As Boolean)
Public Sub New(placeSystemNamespaceFirst As Boolean, separateGroups As Boolean, newLineTrivia As SyntaxTrivia)
_placeSystemNamespaceFirst = placeSystemNamespaceFirst
_separateGroups = separateGroups
_newLineTrivia = newLineTrivia
End Sub

Public Overrides Function VisitCompilationUnit(node As CompilationUnitSyntax) As SyntaxNode
node = DirectCast(MyBase.VisitCompilationUnit(node), CompilationUnitSyntax)
Dim organizedImports = ImportsOrganizer.Organize(
node.Imports, _placeSystemNamespaceFirst, _separateGroups)
node.Imports, _placeSystemNamespaceFirst, _separateGroups, _newLineTrivia)

Dim result = node.WithImports(organizedImports)
If result IsNot node Then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
Imports System.Composition
Imports System.Threading
Imports Microsoft.CodeAnalysis.Editing
Imports Microsoft.CodeAnalysis.Formatting
Imports Microsoft.CodeAnalysis.Host.Mef
Imports Microsoft.CodeAnalysis.OrganizeImports
Imports Microsoft.CodeAnalysis.VisualBasic.CodeGeneration

Namespace Microsoft.CodeAnalysis.VisualBasic.OrganizeImports
<ExportLanguageService(GetType(IOrganizeImportsService), LanguageNames.VisualBasic), [Shared]>
Expand All @@ -25,8 +27,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.OrganizeImports

Dim placeSystemNamespaceFirst = options.GetOption(GenerationOptions.PlaceSystemNamespaceFirst)
Dim separateGroups = options.GetOption(GenerationOptions.SeparateImportDirectiveGroups)
Dim newLineTrivia = VisualBasicSyntaxGeneratorInternal.Instance.EndOfLine(options.GetOption(FormattingOptions2.NewLine))

Dim rewriter = New Rewriter(placeSystemNamespaceFirst, separateGroups)
Dim rewriter = New Rewriter(placeSystemNamespaceFirst, separateGroups, newLineTrivia)
Dim newRoot = rewriter.Visit(root)
Return document.WithSyntaxRoot(newRoot)
End Function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,23 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Workspaces.UnitTests.OrganizeImport
Public Class OrganizeImportsTests
Private Shared Async Function CheckAsync(initial As XElement, final As XElement,
Optional placeSystemNamespaceFirst As Boolean = False,
Optional separateImportGroups As Boolean = False) As Task
Optional separateImportGroups As Boolean = False,
Optional endOfLine As String = Nothing) As Task
Using workspace = New AdhocWorkspace()
Dim project = workspace.CurrentSolution.AddProject("Project", "Project.dll", LanguageNames.VisualBasic)
Dim document = project.AddDocument("Document", SourceText.From(initial.Value.NormalizeLineEndings()))
Dim document = project.AddDocument("Document", SourceText.From(initial.Value.ReplaceLineEndings(If(endOfLine, Environment.NewLine))))

Dim options = workspace.Options.WithChangedOption(New OptionKey(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language), placeSystemNamespaceFirst)
options = options.WithChangedOption(New OptionKey(GenerationOptions.SeparateImportDirectiveGroups, document.Project.Language), separateImportGroups)

If endOfLine IsNot Nothing Then
options = options.WithChangedOption(New OptionKey(FormattingOptions2.NewLine, document.Project.Language), endOfLine)
End If

document = document.WithSolutionOptions(options)

Dim newRoot = Await (Await Formatter.OrganizeImportsAsync(document, CancellationToken.None)).GetSyntaxRootAsync()
Assert.Equal(final.Value.NormalizeLineEndings(), newRoot.ToFullString())
Assert.Equal(final.Value.ReplaceLineEndings(If(endOfLine, Environment.NewLine)), newRoot.ToFullString())
End Using
End Function

Expand Down Expand Up @@ -826,6 +832,48 @@ Imports <xmlns:zz="http://NextNamespace">
Await CheckAsync(initial, final, placeSystemNamespaceFirst:=True, separateImportGroups:=True)
End Function

<WorkItem(19306, "https://github.com/dotnet/roslyn/issues/19306")>
<Theory, Trait(Traits.Feature, Traits.Features.Organizing)>
<InlineData(vbLf)>
<InlineData(vbCrLf)>
Public Async Function TestGrouping3(endOfLine As String) As Task
Dim initial =
<content><![CDATA[' Banner

Imports Microsoft.CodeAnalysis.CSharp.Extensions
Imports Microsoft.CodeAnalysis.CSharp.Syntax
Imports System.Collections.Generic
Imports System.Linq
Imports Microsoft.CodeAnalysis.Shared.Extensions
Imports <xmlns:ab="http://NewNamespace">
Imports <xmlns="http://DefaultNamespace">
Imports Roslyn.Utilities
Imports IntList = System.Collections.Generic.List(Of Integer)
Imports <xmlns:zz="http://NextNamespace">
]]></content>

Dim final =
<content><![CDATA[' Banner

Imports System.Collections.Generic
Imports System.Linq

Imports Microsoft.CodeAnalysis.CSharp.Extensions
Imports Microsoft.CodeAnalysis.CSharp.Syntax
Imports Microsoft.CodeAnalysis.Shared.Extensions

Imports Roslyn.Utilities

Imports IntList = System.Collections.Generic.List(Of Integer)

Imports <xmlns:ab="http://NewNamespace">
Imports <xmlns="http://DefaultNamespace">
Imports <xmlns:zz="http://NextNamespace">
]]></content>

Await CheckAsync(initial, final, placeSystemNamespaceFirst:=True, separateImportGroups:=True, endOfLine:=endOfLine)
End Function

<WorkItem(36984, "https://github.com/dotnet/roslyn/issues/36984")>
<Fact, Trait(Traits.Feature, Traits.Features.Organizing)>
Public Async Function TestGroupingWithFormat() As Task
Expand Down

0 comments on commit adf1456

Please sign in to comment.