Skip to content

Commit 40c163a

Browse files
vweijstersvweijsters
committed
Updated UsingCodeFixProvider to support required group separation
1 parent 182cede commit 40c163a

3 files changed

Lines changed: 185 additions & 35 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.cs‎

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@ namespace StyleCop.Analyzers.OrderingRules
2525
[Shared]
2626
internal sealed class UsingCodeFixProvider : CodeFixProvider
2727
{
28+
private const string SystemUsingDirectiveIdentifier = nameof(System);
29+
2830
private static readonly List<UsingDirectiveSyntax> EmptyUsingsList = new List<UsingDirectiveSyntax>();
2931
private static readonly SyntaxAnnotation UsingCodeFixAnnotation = new SyntaxAnnotation(nameof(UsingCodeFixAnnotation));
32+
private static readonly SymbolDisplayFormat FullNamespaceDisplayFormat = SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted);
3033

3134
/// <inheritdoc/>
3235
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
@@ -287,7 +290,7 @@ private static SyntaxNode StripMultipleBlankLines(SyntaxNode syntaxRoot)
287290
var nextToken = usingDirective.SemicolonToken.GetNextToken(true);
288291

289292
// start at -1 to compensate for the always present end-of-line.
290-
var count = -1;
293+
var trailingCount = -1;
291294

292295
// count the blanks lines at the end of the using statement.
293296
foreach (var trivia in usingDirective.SemicolonToken.TrailingTrivia.Reverse())
@@ -297,23 +300,39 @@ private static SyntaxNode StripMultipleBlankLines(SyntaxNode syntaxRoot)
297300
break;
298301
}
299302

300-
count++;
303+
trailingCount++;
301304
}
302305

303306
// count the blank lines at the start of the next token
307+
var leadingCount = 0;
308+
304309
foreach (var trivia in nextToken.LeadingTrivia)
305310
{
306311
if (!trivia.IsKind(SyntaxKind.EndOfLineTrivia))
307312
{
308313
break;
309314
}
310315

311-
count++;
316+
leadingCount++;
312317
}
313318

314-
if (count > 1)
319+
if ((trailingCount + leadingCount) > 1)
315320
{
316-
replaceMap[nextToken] = nextToken.WithLeadingTrivia(nextToken.LeadingTrivia.Skip(count - 1));
321+
var totalStripCount = trailingCount + leadingCount - 1;
322+
323+
if (trailingCount > 0)
324+
{
325+
var trailingStripCount = Math.Min(totalStripCount, trailingCount);
326+
327+
var trailingTrivia = usingDirective.SemicolonToken.TrailingTrivia;
328+
replaceMap[usingDirective.SemicolonToken] = usingDirective.SemicolonToken.WithTrailingTrivia(trailingTrivia.Take(trailingTrivia.Count - trailingStripCount));
329+
totalStripCount -= trailingStripCount;
330+
}
331+
332+
if (totalStripCount > 0)
333+
{
334+
replaceMap[nextToken] = nextToken.WithLeadingTrivia(nextToken.LeadingTrivia.Skip(totalStripCount));
335+
}
317336
}
318337
}
319338

@@ -488,6 +507,12 @@ private class UsingsHelper
488507
private readonly ImmutableArray<SyntaxTrivia> fileHeader;
489508
private readonly DirectiveSpan conditionalDirectiveTree;
490509
private readonly bool separateSystemDirectives;
510+
private readonly bool insertBlankLinesBetweenGroups;
511+
512+
private Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> systemUsings = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
513+
private Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> namespaceUsings = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
514+
private Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> aliases = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
515+
private Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> staticImports = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
491516

492517
public UsingsHelper(StyleCopSettings settings, SemanticModel semanticModel, CompilationUnitSyntax compilationUnit, ImmutableArray<SyntaxTrivia> fileHeader)
493518
{
@@ -497,17 +522,12 @@ public UsingsHelper(StyleCopSettings settings, SemanticModel semanticModel, Comp
497522

498523
this.conditionalDirectiveTree = DirectiveSpan.BuildConditionalDirectiveTree(compilationUnit);
499524
this.separateSystemDirectives = settings.OrderingRules.SystemUsingDirectivesFirst;
525+
this.insertBlankLinesBetweenGroups = settings.OrderingRules.UseBlankLinesBetweenUsingGroups == OptionSetting.Require;
500526

501527
this.ProcessUsingDirectives(compilationUnit.Usings);
502528
this.ProcessMembers(compilationUnit.Members);
503529
}
504530

505-
public Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> NamespaceUsings { get; } = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
506-
507-
public Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> Aliases { get; } = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
508-
509-
public Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>> StaticImports { get; } = new Dictionary<DirectiveSpan, List<UsingDirectiveSyntax>>();
510-
511531
public DirectiveSpan RootSpan
512532
{
513533
get { return this.conditionalDirectiveTree; }
@@ -518,17 +538,22 @@ public List<UsingDirectiveSyntax> GetContainedUsings(DirectiveSpan directiveSpan
518538
List<UsingDirectiveSyntax> result = new List<UsingDirectiveSyntax>();
519539
List<UsingDirectiveSyntax> usingsList;
520540

521-
if (this.NamespaceUsings.TryGetValue(directiveSpan, out usingsList))
541+
if (this.systemUsings.TryGetValue(directiveSpan, out usingsList))
522542
{
523543
result.AddRange(usingsList);
524544
}
525545

526-
if (this.Aliases.TryGetValue(directiveSpan, out usingsList))
546+
if (this.namespaceUsings.TryGetValue(directiveSpan, out usingsList))
527547
{
528548
result.AddRange(usingsList);
529549
}
530550

531-
if (this.StaticImports.TryGetValue(directiveSpan, out usingsList))
551+
if (this.aliases.TryGetValue(directiveSpan, out usingsList))
552+
{
553+
result.AddRange(usingsList);
554+
}
555+
556+
if (this.staticImports.TryGetValue(directiveSpan, out usingsList))
532557
{
533558
result.AddRange(usingsList);
534559
}
@@ -541,9 +566,10 @@ public SyntaxList<UsingDirectiveSyntax> GenerateGroupedUsings(DirectiveSpan dire
541566
var usingList = new List<UsingDirectiveSyntax>();
542567
List<SyntaxTrivia> triviaToMove = new List<SyntaxTrivia>();
543568

544-
usingList.AddRange(this.GenerateUsings(this.NamespaceUsings, directiveSpan, indentation, triviaToMove, qualifyNames));
545-
usingList.AddRange(this.GenerateUsings(this.StaticImports, directiveSpan, indentation, triviaToMove, qualifyNames));
546-
usingList.AddRange(this.GenerateUsings(this.Aliases, directiveSpan, indentation, triviaToMove, qualifyNames));
569+
usingList.AddRange(this.GenerateUsings(this.systemUsings, directiveSpan, indentation, triviaToMove, qualifyNames));
570+
usingList.AddRange(this.GenerateUsings(this.namespaceUsings, directiveSpan, indentation, triviaToMove, qualifyNames));
571+
usingList.AddRange(this.GenerateUsings(this.staticImports, directiveSpan, indentation, triviaToMove, qualifyNames));
572+
usingList.AddRange(this.GenerateUsings(this.aliases, directiveSpan, indentation, triviaToMove, qualifyNames));
547573

548574
if (triviaToMove.Count > 0)
549575
{
@@ -565,9 +591,10 @@ public SyntaxList<UsingDirectiveSyntax> GenerateGroupedUsings(List<UsingDirectiv
565591
var usingList = new List<UsingDirectiveSyntax>();
566592
List<SyntaxTrivia> triviaToMove = new List<SyntaxTrivia>();
567593

568-
usingList.AddRange(this.GenerateUsings(this.NamespaceUsings, usingsList, indentation, triviaToMove, qualifyNames));
569-
usingList.AddRange(this.GenerateUsings(this.StaticImports, usingsList, indentation, triviaToMove, qualifyNames));
570-
usingList.AddRange(this.GenerateUsings(this.Aliases, usingsList, indentation, triviaToMove, qualifyNames));
594+
usingList.AddRange(this.GenerateUsings(this.systemUsings, usingsList, indentation, triviaToMove, qualifyNames));
595+
usingList.AddRange(this.GenerateUsings(this.namespaceUsings, usingsList, indentation, triviaToMove, qualifyNames));
596+
usingList.AddRange(this.GenerateUsings(this.staticImports, usingsList, indentation, triviaToMove, qualifyNames));
597+
usingList.AddRange(this.GenerateUsings(this.aliases, usingsList, indentation, triviaToMove, qualifyNames));
571598

572599
if (triviaToMove.Count > 0)
573600
{
@@ -693,6 +720,13 @@ private List<UsingDirectiveSyntax> GenerateUsings(List<UsingDirectiveSyntax> usi
693720

694721
result.Sort(this.CompareUsings);
695722

723+
if (this.insertBlankLinesBetweenGroups)
724+
{
725+
var last = result[result.Count - 1];
726+
727+
result[result.Count - 1] = last.WithTrailingTrivia(last.GetTrailingTrivia().Add(SyntaxFactory.CarriageReturnLineFeed));
728+
}
729+
696730
return result;
697731
}
698732

@@ -789,27 +823,29 @@ private int CompareUsings(UsingDirectiveSyntax left, UsingDirectiveSyntax right)
789823
return CultureInfo.InvariantCulture.CompareInfo.Compare(left.Alias.Name.Identifier.ValueText, right.Alias.Name.Identifier.ValueText, CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace | CompareOptions.IgnoreWidth);
790824
}
791825

792-
bool leftIsSystem = this.IsSeparatedSystemUsing(left);
793-
bool rightIsSystem = this.IsSeparatedSystemUsing(right);
794-
if (leftIsSystem != rightIsSystem)
795-
{
796-
return leftIsSystem ? -1 : 1;
797-
}
798-
799826
return CultureInfo.InvariantCulture.CompareInfo.Compare(left.Name.ToNormalizedString(), right.Name.ToNormalizedString(), CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace | CompareOptions.IgnoreWidth);
800827
}
801828

802829
private bool IsSeparatedSystemUsing(UsingDirectiveSyntax syntax)
803830
{
804-
if (!this.separateSystemDirectives)
831+
if (!this.separateSystemDirectives
832+
|| (syntax.Alias != null)
833+
|| syntax.StaticKeyword.IsKind(SyntaxKind.StaticKeyword)
834+
|| syntax.HasNamespaceAliasQualifier())
805835
{
806836
return false;
807837
}
808838

809-
return syntax.Alias == null
810-
&& syntax.IsSystemUsingDirective()
811-
&& !syntax.HasNamespaceAliasQualifier()
812-
&& syntax.StaticKeyword.IsKind(SyntaxKind.None);
839+
var namespaceSymbol = this.semanticModel.GetSymbolInfo(syntax.Name).Symbol as INamespaceSymbol;
840+
if (namespaceSymbol == null)
841+
{
842+
return false;
843+
}
844+
845+
var namespaceTypeName = namespaceSymbol.ToDisplayString(FullNamespaceDisplayFormat);
846+
var firstPart = namespaceTypeName.Split('.')[0];
847+
848+
return string.Equals(SystemUsingDirectiveIdentifier, firstPart, StringComparison.Ordinal);
813849
}
814850

815851
private void ProcessMembers(SyntaxList<MemberDeclarationSyntax> members)
@@ -829,15 +865,19 @@ private void ProcessUsingDirectives(SyntaxList<UsingDirectiveSyntax> usingDirect
829865

830866
if (usingDirective.Alias != null)
831867
{
832-
this.AddUsingDirective(this.Aliases, usingDirective, owningSpan);
868+
this.AddUsingDirective(this.aliases, usingDirective, owningSpan);
833869
}
834870
else if (!usingDirective.StaticKeyword.IsKind(SyntaxKind.None))
835871
{
836-
this.AddUsingDirective(this.StaticImports, usingDirective, owningSpan);
872+
this.AddUsingDirective(this.staticImports, usingDirective, owningSpan);
873+
}
874+
else if (this.IsSeparatedSystemUsing(usingDirective))
875+
{
876+
this.AddUsingDirective(this.systemUsings, usingDirective, owningSpan);
837877
}
838878
else
839879
{
840-
this.AddUsingDirective(this.NamespaceUsings, usingDirective, owningSpan);
880+
this.AddUsingDirective(this.namespaceUsings, usingDirective, owningSpan);
841881
}
842882
}
843883
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/UsingCodeFixProviderGroupSeparationUnitTests.cs‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Test.OrderingRules
5+
{
6+
using System.Collections.Generic;
7+
using System.Threading;
8+
using System.Threading.Tasks;
9+
using Microsoft.CodeAnalysis.CodeFixes;
10+
using Microsoft.CodeAnalysis.Diagnostics;
11+
using StyleCop.Analyzers.OrderingRules;
12+
using StyleCop.Analyzers.Settings.ObjectModel;
13+
using TestHelper;
14+
using Xunit;
15+
16+
/// <summary>
17+
/// Unit tests for the <see cref="UsingCodeFixProvider"/> for the special case where
18+
/// <see cref="OrderingSettings.UseBlankLinesBetweenUsingGroups"/> is <see cref="OptionSetting.Require"/>.
19+
/// </summary>
20+
public class UsingCodeFixProviderGroupSeparationUnitTests : CodeFixVerifier
21+
{
22+
private bool? systemUsingDirectivesFirst;
23+
24+
/// <summary>
25+
/// Verifies that the code fix will properly reorder using statements.
26+
/// </summary>
27+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
28+
[Fact]
29+
public async Task VerifyUsingReorderingAsync()
30+
{
31+
this.systemUsingDirectivesFirst = true;
32+
33+
var testCode = @"using Microsoft.CodeAnalysis;
34+
using SystemAction = System.Action;
35+
using static System.Math;
36+
using System;
37+
using static System.String;
38+
using MyFunc = System.Func<int,bool>;
39+
using System.Collections.Generic;
40+
using System.Collections;
41+
42+
namespace Foo
43+
{
44+
public class Bar
45+
{
46+
}
47+
}
48+
";
49+
50+
var fixedTestCode = @"namespace Foo
51+
{
52+
using System;
53+
using System.Collections;
54+
using System.Collections.Generic;
55+
56+
using Microsoft.CodeAnalysis;
57+
58+
using static System.Math;
59+
using static System.String;
60+
61+
using MyFunc = System.Func<int,bool>;
62+
using SystemAction = System.Action;
63+
64+
public class Bar
65+
{
66+
}
67+
}
68+
";
69+
70+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
71+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
72+
}
73+
74+
/// <inheritdoc/>
75+
protected override string GetSettings()
76+
{
77+
var systemUsingDirectivesFirst = this.systemUsingDirectivesFirst ?? true;
78+
79+
return $@"
80+
{{
81+
""settings"": {{
82+
""orderingRules"": {{
83+
""systemUsingDirectivesFirst"" : {systemUsingDirectivesFirst.ToString().ToLowerInvariant()},
84+
""useBlankLinesBetweenUsingGroups"": ""require""
85+
}}
86+
}}
87+
}}
88+
";
89+
}
90+
91+
/// <inheritdoc/>
92+
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
93+
{
94+
yield return new SA1200UsingDirectivesMustBePlacedCorrectly();
95+
yield return new SA1208SystemUsingDirectivesMustBePlacedBeforeOtherUsingDirectives();
96+
yield return new SA1209UsingAliasDirectivesMustBePlacedAfterOtherUsingDirectives();
97+
yield return new SA1210UsingDirectivesMustBeOrderedAlphabeticallyByNamespace();
98+
yield return new SA1211UsingAliasDirectivesMustBeOrderedAlphabeticallyByAliasName();
99+
yield return new SA1216UsingStaticDirectivesMustBePlacedAtTheCorrectLocation();
100+
yield return new SA1217UsingStaticDirectivesMustBeOrderedAlphabetically();
101+
}
102+
103+
/// <inheritdoc/>
104+
protected override CodeFixProvider GetCSharpCodeFixProvider()
105+
{
106+
return new UsingCodeFixProvider();
107+
}
108+
}
109+
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@
293293
<Compile Include="OrderingRules\SA1216UnitTests.cs" />
294294
<Compile Include="OrderingRules\SA1217UnitTests.cs" />
295295
<Compile Include="OrderingRules\UsingCodeFixProviderCombinedSystemDirectivesUnitTests.cs" />
296+
<Compile Include="OrderingRules\UsingCodeFixProviderGroupSeparationUnitTests.cs" />
296297
<Compile Include="OrderingRules\UsingCodeFixProviderUnitTests.cs" />
297298
<Compile Include="Properties\AssemblyInfo.cs" />
298299
<Compile Include="PublicApiTests.cs" />

0 commit comments

Comments
 (0)