Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass per-document options when organizing imports or usings #17347

Merged
merged 3 commits into from
Feb 24, 2017
Merged

Pass per-document options when organizing imports or usings #17347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a8875e
Pass per-document options when organizing imports or usings
jmarolf Feb 24, 2017
5cb1063
small test case changes
jmarolf Feb 24, 2017
f533a9b
fixing up some PR feedback from cyrus
jmarolf Feb 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 41 additions & 38 deletions src/EditorFeatures/CSharpTest/Organizing/OrganizeUsingsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.OrganizeImports;
using Roslyn.Test.Utilities;
using Xunit;
Expand All @@ -12,28 +14,29 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Organizing
{
public class OrganizeUsingsTests
{
protected async Task CheckAsync(string initial, string final, bool specialCaseSystem, CSharpParseOptions options = null)
protected async Task CheckAsync(string initial, string final, bool placeSystemNamespaceFirst = false, CSharpParseOptions options = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i hate that this is optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make it required. Just saw a lot of tests passing in random true/false values even when it didn't affect the test. false is the default in VS. It seemed reasonable that only tests that were explicitly testing this behavior should change it. should parse options be non-optional as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm ok with this being optional now :)

{
using (var workspace = await TestWorkspace.CreateCSharpAsync(initial))
{
var document = workspace.CurrentSolution.GetDocument(workspace.Documents.First().Id);
var newRoot = await (await OrganizeImportsService.OrganizeImportsAsync(document, specialCaseSystem)).GetSyntaxRootAsync();
workspace.Options = workspace.Options.WithChangedOption(new OptionKey(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language), placeSystemNamespaceFirst);
var newRoot = await (await OrganizeImportsService.OrganizeImportsAsync(document)).GetSyntaxRootAsync();
Assert.Equal(final.NormalizeLineEndings(), newRoot.ToFullString());
}
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
public async Task EmptyFile()
{
await CheckAsync(string.Empty, string.Empty, true);
await CheckAsync(string.Empty, string.Empty);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
public async Task SingleUsingStatement()
{
var initial = @"using A;";
var final = initial;
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -52,7 +55,7 @@ public async Task AliasesAtBottom()
using D = E;
";

await CheckAsync(initial, final, false);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -77,7 +80,7 @@ public async Task UsingStaticsBetweenUsingsAndAliases()
using D = E;
";

await CheckAsync(initial, final, false);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -162,7 +165,7 @@ namespace N5
using N;
}
}";
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -180,7 +183,7 @@ public async Task SpecialCaseSystem()
using M1;
using M2;
";
await CheckAsync(initial, final, true);
await CheckAsync(initial, final, placeSystemNamespaceFirst: true);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -202,7 +205,7 @@ public async Task SpecialCaseSystemWithUsingStatic()
using static System.BitConverter;
using static Microsoft.Win32.Registry;
";
await CheckAsync(initial, final, true);
await CheckAsync(initial, final, placeSystemNamespaceFirst: true);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -221,7 +224,7 @@ public async Task DoNotSpecialCaseSystem()
using System.Linq;
";

await CheckAsync(initial, final, false);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -242,7 +245,7 @@ public async Task DoNotSpecialCaseSystemWithUsingStatics()
using System.Linq;
using static Microsoft.Win32.Registry;
using static System.BitConverter;";
await CheckAsync(initial, final, false);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -276,7 +279,7 @@ namespace U { }
namespace V.W { }
namespace X.Y.Z { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -302,7 +305,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -328,7 +331,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -354,7 +357,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[WorkItem(2480, "https://github.com/dotnet/roslyn/issues/2480")]
Expand All @@ -379,7 +382,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[WorkItem(2480, "https://github.com/dotnet/roslyn/issues/2480")]
Expand All @@ -404,7 +407,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[WorkItem(2480, "https://github.com/dotnet/roslyn/issues/2480")]
Expand All @@ -429,7 +432,7 @@ namespace B { }";
namespace A { }
namespace B { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[WorkItem(2480, "https://github.com/dotnet/roslyn/issues/2480")]
Expand All @@ -454,7 +457,7 @@ public async Task CommentsNotAtTheStartOfTheFile1()
using System.Text;
}";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[WorkItem(2480, "https://github.com/dotnet/roslyn/issues/2480")]
Expand All @@ -481,7 +484,7 @@ public async Task CommentsNotAtTheStartOfTheFile2()
using System.Text;
}";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -502,7 +505,7 @@ namespace C { }
namespace D { }";

var final = initial;
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -582,7 +585,7 @@ struct T { }
}
}";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -594,7 +597,7 @@ public async Task DuplicateUsings()

var final = initial;

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -614,7 +617,7 @@ public async Task InlineComments()
/*00*/using/*01*/D/*02*/;/*03*/
/*16*/";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -628,7 +631,7 @@ public async Task AllOnOneLine()
using B;
using C; ";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -655,7 +658,7 @@ class Class1
{
}";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -670,7 +673,7 @@ public async Task NestedRegionBlock()

var final = initial;

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -687,7 +690,7 @@ public async Task MultipleRegionBlocks()

var final = initial;

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -709,7 +712,7 @@ class D { }";

class D { }";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -729,7 +732,7 @@ public async Task InsideIfEndIfBlock()
using C;
#endif";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -746,7 +749,7 @@ public async Task IfEndIfBlockAbove()
using E;";

var final = initial;
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -766,7 +769,7 @@ public async Task IfEndIfBlockMiddle()
using G;";

var final = initial;
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand All @@ -783,7 +786,7 @@ public async Task IfEndIfBlockBelow()
#endif";

var final = initial;
await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -822,7 +825,7 @@ public async Task Korean()
using 하;
";

await CheckAsync(initial, final, true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -851,7 +854,7 @@ public async Task DoNotSpecialCaseSystem1()
using SystemZ;
";

await CheckAsync(initial, final, specialCaseSystem: false);
await CheckAsync(initial, final, placeSystemNamespaceFirst: false);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -894,7 +897,7 @@ public async Task DoNotSpecialCaseSystem2()
using Z = System.Int32;
";

await CheckAsync(initial, final, specialCaseSystem: false);
await CheckAsync(initial, final, placeSystemNamespaceFirst: false);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -987,7 +990,7 @@ public async Task CaseSensitivity1()

// If Kana is sensitive あ != ア, if Kana is insensitive あ == ア.
// If Width is sensitiveア != ｱ, if Width is insensitive ア == ｱ.";
await CheckAsync(initial, final, specialCaseSystem: true);
await CheckAsync(initial, final);
}

[Fact, Trait(Traits.Feature, Traits.Features.Organizing)]
Expand Down Expand Up @@ -1022,7 +1025,7 @@ public async Task CaseSensitivity2()
using ああ;
";

await CheckAsync(initial, final, specialCaseSystem: true);
await CheckAsync(initial, final);
}
}
}
4 changes: 1 addition & 3 deletions src/EditorFeatures/Core/Implementation/Organizing/OrganizeDocumentCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System;
using System.ComponentModel.Composition;
using System.Threading;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Editor.Commands;
using Microsoft.CodeAnalysis.Editor.Host;
using Microsoft.CodeAnalysis.Editor.Shared;
Expand All @@ -12,7 +11,6 @@
using Microsoft.CodeAnalysis.Organizing;
using Microsoft.CodeAnalysis.RemoveUnnecessaryImports;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Options;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Text;
using Roslyn.Utilities;
Expand Down Expand Up @@ -123,7 +121,7 @@ private void SortAndRemoveUnusedImports(ITextBuffer subjectBuffer, CancellationT
if (document != null)
{
var newDocument = document.GetLanguageService<IRemoveUnnecessaryImportsService>().RemoveUnnecessaryImportsAsync(document, cancellationToken).WaitAndGetResult(cancellationToken);
newDocument = OrganizeImportsService.OrganizeImportsAsync(newDocument, subjectBuffer.GetFeatureOnOffOption(GenerationOptions.PlaceSystemNamespaceFirst), cancellationToken).WaitAndGetResult(cancellationToken);
newDocument = OrganizeImportsService.OrganizeImportsAsync(newDocument, cancellationToken).WaitAndGetResult(cancellationToken);
if (document != newDocument)
{
ApplyTextChange(document, newDocument);
Expand Down
Loading