Merge pull request #2796 from vweijsters/fix-2163

sharwell · web-flow · commit c2dfa48b48da · 2018-11-15T08:00:12.000-06:00
Added system first support for static usings
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.UsingsSorter.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.UsingsSorter.cs
@@ -35,6 +35,7 @@ private class UsingsSorter
             private readonly Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>> systemUsings = new Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>>();
             private readonly Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>> namespaceUsings = new Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>>();
             private readonly Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>> aliases = new Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>>();
+            private readonly Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>> systemStaticImports = new Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>>();
             private readonly Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>> staticImports = new Dictionary<TreeTextSpan, List<UsingDirectiveSyntax>>();
 
             public UsingsSorter(StyleCopSettings settings, SemanticModel semanticModel, CompilationUnitSyntax compilationUnit, ImmutableArray<SyntaxTrivia> fileHeader)
@@ -76,6 +77,11 @@ public List<UsingDirectiveSyntax> GetContainedUsings(TreeTextSpan directiveSpan)
                     result.AddRange(usingsList);
                 }
 
+                if (this.systemStaticImports.TryGetValue(directiveSpan, out usingsList))
+                {
+                    result.AddRange(usingsList);
+                }
+
                 if (this.staticImports.TryGetValue(directiveSpan, out usingsList))
                 {
                     result.AddRange(usingsList);
@@ -91,6 +97,7 @@ public SyntaxList<UsingDirectiveSyntax> GenerateGroupedUsings(TreeTextSpan direc
 
                 usingList.AddRange(this.GenerateUsings(this.systemUsings, directiveSpan, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.namespaceUsings, directiveSpan, indentation, triviaToMove, qualifyNames));
+                usingList.AddRange(this.GenerateUsings(this.systemStaticImports, directiveSpan, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.staticImports, directiveSpan, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.aliases, directiveSpan, indentation, triviaToMove, qualifyNames));
 
@@ -116,6 +123,7 @@ public SyntaxList<UsingDirectiveSyntax> GenerateGroupedUsings(List<UsingDirectiv
 
                 usingList.AddRange(this.GenerateUsings(this.systemUsings, usingsList, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.namespaceUsings, usingsList, indentation, triviaToMove, qualifyNames));
+                usingList.AddRange(this.GenerateUsings(this.systemStaticImports, usingsList, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.staticImports, usingsList, indentation, triviaToMove, qualifyNames));
                 usingList.AddRange(this.GenerateUsings(this.aliases, usingsList, indentation, triviaToMove, qualifyNames));
 
@@ -419,22 +427,35 @@ private int CompareUsings(UsingDirectiveSyntax left, UsingDirectiveSyntax right)
                 return NameSyntaxHelpers.Compare(left.Name, right.Name);
             }
 
+            private bool IsSeparatedStaticSystemUsing(UsingDirectiveSyntax syntax)
+            {
+                if (!this.separateSystemDirectives)
+                {
+                    return false;
+                }
+
+                return this.StartsWithSystemUsingDirectiveIdentifier(syntax.Name);
+            }
+
             private bool IsSeparatedSystemUsing(UsingDirectiveSyntax syntax)
             {
                 if (!this.separateSystemDirectives
-                    || (syntax.Alias != null)
-                    || syntax.StaticKeyword.IsKind(SyntaxKind.StaticKeyword)
                     || syntax.HasNamespaceAliasQualifier())
                 {
                     return false;
                 }
 
-                if (!(this.semanticModel.GetSymbolInfo(syntax.Name).Symbol is INamespaceSymbol namespaceSymbol))
+                return this.StartsWithSystemUsingDirectiveIdentifier(syntax.Name);
+            }
+
+            private bool StartsWithSystemUsingDirectiveIdentifier(NameSyntax name)
+            {
+                if (!(this.semanticModel.GetSymbolInfo(name).Symbol is INamespaceOrTypeSymbol namespaceOrTypeSymbol))
                 {
                     return false;
                 }
 
-                var namespaceTypeName = namespaceSymbol.ToDisplayString(FullNamespaceDisplayFormat);
+                var namespaceTypeName = namespaceOrTypeSymbol.ToDisplayString(FullNamespaceDisplayFormat);
                 var firstPart = namespaceTypeName.Split('.')[0];
 
                 return string.Equals(SystemUsingDirectiveIdentifier, firstPart, StringComparison.Ordinal);
@@ -459,9 +480,16 @@ private void ProcessUsingDirectives(SyntaxList<UsingDirectiveSyntax> usingDirect
                     {
                         this.AddUsingDirective(this.aliases, usingDirective, containingSpan);
                     }
-                    else if (!usingDirective.StaticKeyword.IsKind(SyntaxKind.None))
+                    else if (usingDirective.StaticKeyword.IsKind(SyntaxKind.StaticKeyword))
                     {
-                        this.AddUsingDirective(this.staticImports, usingDirective, containingSpan);
+                        if (this.IsSeparatedStaticSystemUsing(usingDirective))
+                        {
+                            this.AddUsingDirective(this.systemStaticImports, usingDirective, containingSpan);
+                        }
+                        else
+                        {
+                            this.AddUsingDirective(this.staticImports, usingDirective, containingSpan);
+                        }
                     }
                     else if (this.IsSeparatedSystemUsing(usingDirective))
                     {
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/UsingCodeFixProvider.cs
@@ -29,7 +29,9 @@ internal sealed partial class UsingCodeFixProvider : CodeFixProvider
 
         private static readonly List<UsingDirectiveSyntax> EmptyUsingsList = new List<UsingDirectiveSyntax>();
         private static readonly SyntaxAnnotation UsingCodeFixAnnotation = new SyntaxAnnotation(nameof(UsingCodeFixAnnotation));
-        private static readonly SymbolDisplayFormat FullNamespaceDisplayFormat = SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted);
+        private static readonly SymbolDisplayFormat FullNamespaceDisplayFormat = SymbolDisplayFormat.FullyQualifiedFormat
+            .WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)
+            .WithMiscellaneousOptions(SymbolDisplayMiscellaneousOptions.EscapeKeywordIdentifiers);
 
         /// <inheritdoc/>
         public override ImmutableArray<string> FixableDiagnosticIds { get; } =
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1217UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1217UnitTests.cs
@@ -7,17 +7,36 @@ namespace StyleCop.Analyzers.Test.OrderingRules
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis.Testing;
     using StyleCop.Analyzers.OrderingRules;
-    using TestHelper;
+    using StyleCop.Analyzers.Test.Verifiers;
     using Xunit;
-    using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
-        StyleCop.Analyzers.OrderingRules.SA1217UsingStaticDirectivesMustBeOrderedAlphabetically,
-        StyleCop.Analyzers.OrderingRules.UsingCodeFixProvider>;
 
     /// <summary>
     /// Unit tests for <see cref="SA1217UsingStaticDirectivesMustBeOrderedAlphabetically"/>.
     /// </summary>
     public class SA1217UnitTests
     {
+        private const string TestSettings = @"
+{
+  ""settings"": {
+    ""orderingRules"": {
+      ""systemUsingDirectivesFirst"": true
+    }
+  }
+}
+";
+
+        private const string TestSettingsNoSystemDirectivesFirst = @"
+{
+  ""settings"": {
+    ""orderingRules"": {
+      ""systemUsingDirectivesFirst"": false
+    }
+  }
+}
+";
+
+        private bool useSystemUsingDirectivesFirst;
+
         /// <summary>
         /// Verifies that the analyzer will not produce diagnostics for correctly ordered using directives inside a namespace.
         /// </summary>
@@ -34,7 +53,7 @@ public async Task TestValidUsingDirectivesInNamespaceAsync()
 }
 ";
 
-            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -61,7 +80,7 @@ namespace Bar
 }
 ";
 
-            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -81,7 +100,7 @@ public class Foo
 }
 ";
 
-            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -131,7 +150,7 @@ namespace Bar
                 Diagnostic().WithLocation(11, 5).WithArguments("System.Math", "System.Array"),
             };
 
-            await VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -150,7 +169,7 @@ public async Task TestValidUsingDirectivesWithInlineCommentsAsync()
 }
 ";
 
-            await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -178,9 +197,12 @@ public async Task TestInvalidUsingDirectivesWithGlobalPrefixAsync()
 }
 ";
 
-            var expectedDiagnostic = Diagnostic().WithLocation(5, 5).WithArguments("System.Math", "global::System.Array");
+            DiagnosticResult[] expectedDiagnostic =
+            {
+                Diagnostic().WithLocation(5, 5).WithArguments("System.Math", "global::System.Array"),
+            };
 
-            await VerifyCSharpFixAsync(testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+            await this.VerifyCSharpFixAsync(testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
         }
 
         /// <summary>
@@ -191,8 +213,8 @@ public async Task TestInvalidUsingDirectivesWithGlobalPrefixAsync()
         public async Task TestPreprocessorDirectivesAsync()
         {
             var testCode = @"
-using System;
 using Microsoft.Win32;
+using System;
 using MyList = System.Collections.Generic.List<int>;
 using static System.Tuple;
 
@@ -205,8 +227,8 @@ public async Task TestPreprocessorDirectivesAsync()
 #endif";
 
             var fixedTestCode = @"
-using System;
 using Microsoft.Win32;
+using System;
 using static System.Tuple;
 using MyList = System.Collections.Generic.List<int>;
 
@@ -219,9 +241,136 @@ public async Task TestPreprocessorDirectivesAsync()
 #endif";
 
             // else block is skipped
-            var expected = Diagnostic().WithLocation(8, 1).WithArguments("System.String", "System.Math");
+            DiagnosticResult[] expectedDiagnostic =
+            {
+                Diagnostic().WithLocation(8, 1).WithArguments("System.String", "System.Math"),
+            };
+
+            await this.VerifyCSharpFixAsync(testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verify that the systemUsingDirectivesFirst setting is honored correctly.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(2163, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2163")]
+        public async Task VerifySystemUsingDirectivesFirstAsync()
+        {
+            this.useSystemUsingDirectivesFirst = true;
+
+            var testCode = @"
+using static MyNamespace.TestClass;
+using static System.Math;
+
+namespace MyNamespace
+{
+    public static class TestClass
+    {
+        public static void TestMethod()
+        {
+        }
+    }
+}
+";
+
+            var fixedTestCode = @"
+using static System.Math;
+using static MyNamespace.TestClass;
+
+namespace MyNamespace
+{
+    public static class TestClass
+    {
+        public static void TestMethod()
+        {
+        }
+    }
+}
+";
+
+            DiagnosticResult[] expectedDiagnostic =
+            {
+                Diagnostic().WithLocation(2, 1).WithArguments("MyNamespace.TestClass", "System.Math"),
+            };
+
+            await this.VerifyCSharpFixAsync(testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Verify that the systemUsingDirectivesFirst setting is honored correctly when using multiple static system usings.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+        [Fact]
+        [WorkItem(2163, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2163")]
+        public async Task VerifyMultipleStaticSystemUsingDirectivesAsync()
+        {
+            this.useSystemUsingDirectivesFirst = true;
+
+            var testCode = @"
+using static System.Math;
+using static System.Activator;
+
+namespace MyNamespace
+{
+    public static class TestClass
+    {
+        public static void TestMethod()
+        {
+        }
+    }
+}
+";
+
+            var fixedTestCode = @"
+using static System.Activator;
+using static System.Math;
+
+namespace MyNamespace
+{
+    public static class TestClass
+    {
+        public static void TestMethod()
+        {
+        }
+    }
+}
+";
+
+            DiagnosticResult[] expectedDiagnostic =
+            {
+                Diagnostic().WithLocation(2, 1).WithArguments("System.Math", "System.Activator"),
+            };
+
+            await this.VerifyCSharpFixAsync(testCode, expectedDiagnostic, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+        }
+
+        private static DiagnosticResult Diagnostic()
+            => StyleCopCodeFixVerifier<SA1217UsingStaticDirectivesMustBeOrderedAlphabetically, UsingCodeFixProvider>.Diagnostic();
+
+        private Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken)
+        {
+            var test = new StyleCopCodeFixVerifier<SA1217UsingStaticDirectivesMustBeOrderedAlphabetically, UsingCodeFixProvider>.CSharpTest
+            {
+                TestCode = source,
+                Settings = this.useSystemUsingDirectivesFirst ? TestSettings : TestSettingsNoSystemDirectivesFirst,
+            };
+
+            test.ExpectedDiagnostics.AddRange(expected);
+            return test.RunAsync(cancellationToken);
+        }
+
+        private Task VerifyCSharpFixAsync(string source, DiagnosticResult[] expected, string fixedSource, CancellationToken cancellationToken)
+        {
+            var test = new StyleCopCodeFixVerifier<SA1217UsingStaticDirectivesMustBeOrderedAlphabetically, UsingCodeFixProvider>.CSharpTest
+            {
+                TestCode = source,
+                FixedCode = fixedSource,
+                Settings = this.useSystemUsingDirectivesFirst ? TestSettings : TestSettingsNoSystemDirectivesFirst,
+            };
 
-            await VerifyCSharpFixAsync(testCode, expected, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
+            test.ExpectedDiagnostics.AddRange(expected);
+            return test.RunAsync(cancellationToken);
         }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1217UsingStaticDirectivesMustBeOrderedAlphabetically.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1217UsingStaticDirectivesMustBeOrderedAlphabetically.cs
