Merge pull request #1895 from sharwell/fix-1818

Fix bugs in handling of System using directives

Author: sharwell

StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1208UnitTests.cs

@@ -44,6 +44,20 @@ class A
             await this.VerifyCSharpDiagnosticAsync(usingsInNamespaceDeclaration, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        public async Task TestSystemUsingDirectivesWithEscapeSequenceAsync()
+        {
+            string usingsInNamespaceDeclaration = @"namespace Test
+{
+    using @System;
+    using System.Diagnostics;
+    using \u0053ystem.IO;
+    using System.Threading;
+}";
+
+            await this.VerifyCSharpDiagnosticAsync(usingsInNamespaceDeclaration, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         [Fact]
         public async Task TestWhenSystemUsingDirectivesAreNotOnTopInCompilationAsync()
         {
@@ -160,9 +174,7 @@ class A
 
             DiagnosticResult[] expected =
             {
-                this.CSharpDiagnostic().WithLocation("Test3.cs", 2, 1).WithArguments("global::System.IO", "global::AnotherNamespace"),
                 this.CSharpDiagnostic().WithLocation("Test3.cs", 8, 5).WithArguments("System.Threading", "Xyz"),
-                this.CSharpDiagnostic().WithLocation("Test3.cs", 9, 5).WithArguments("global::System", "Xyz")
             };
 
             await this.VerifyCSharpDiagnosticAsync(sources, expected, CancellationToken.None).ConfigureAwait(false);
@@ -336,6 +348,31 @@ public async Task TestPreprocessorDirectivesAsync()
             await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// This is a regression test for DotNetAnalyzers/StyleCopAnalyzers#1818.
+        /// </summary>
+        /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+        [Fact]
+        public async Task TestValidUsingDirectivesWithGlobalAliasAsync()
+        {
+            var testCode = @"
+namespace Foo
+{
+    extern alias corlib;
+    using System;
+    using System.Threading;
+    using corlib::System;
+    using Foo;
+    using global::Foo;
+    using global::System;
+    using global::System.IO;
+    using global::System.Linq;
+    using Microsoft;
+}";
+
+            await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
+        }
+
         /// <inheritdoc/>
         protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
         {

StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1210UnitTests.cs

@@ -182,6 +182,7 @@ public async Task TestInvalidOrderedUsingDirectivesWithNamespaceAliasQualifierAs
 using global::System;
 using global::Foo;
 using Foo;
+using Microsoft;
 
 namespace Foo
 {
@@ -200,6 +201,7 @@ namespace Foo
     using global::System;
     using global::System.IO;
     using global::System.Linq;
+    using Microsoft;
 }";
 
             DiagnosticResult[] expected =

StyleCop.Analyzers/StyleCop.Analyzers/Helpers/UsingDirectiveSyntaxHelpers.cs

@@ -21,7 +21,7 @@ internal static class UsingDirectiveSyntaxHelpers
         /// </summary>
         /// <param name="usingDirective">The <see cref="UsingDirectiveSyntax"/> that will be checked.</param>
         /// <returns>Return true if the <see cref="UsingDirectiveSyntax"/>is system using directive, otherwise false.</returns>
-        internal static bool IsSystemUsingDirective(this UsingDirectiveSyntax usingDirective) => string.Equals(SystemUsingDirectiveIdentifier, GetFirstIdentifierInUsingDirective(usingDirective)?.Text, StringComparison.Ordinal);
+        internal static bool IsSystemUsingDirective(this UsingDirectiveSyntax usingDirective) => string.Equals(SystemUsingDirectiveIdentifier, GetFirstIdentifierInUsingDirective(usingDirective)?.ValueText, StringComparison.Ordinal);
 
         /// <summary>
         /// Check if <see cref="UsingDirectiveSyntax"/> is preceded by a preprocessor directive.

StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1208SystemUsingDirectivesMustBePlacedBeforeOtherUsingDirectives.cs

@@ -98,7 +98,7 @@ private static void ProcessUsingsAndReportDiagnostic(SyntaxList<UsingDirectiveSy
                     continue;
                 }
 
-                if (usingDirective.IsSystemUsingDirective())
+                if (usingDirective.IsSystemUsingDirective() && !usingDirective.HasNamespaceAliasQualifier())
                 {
                     if (systemUsingDirectivesShouldBeBeforeThisName != null)
                     {
@@ -108,7 +108,9 @@ private static void ProcessUsingsAndReportDiagnostic(SyntaxList<UsingDirectiveSy
 
                     var previousUsing = usings[i - 1];
 
-                    if (!previousUsing.IsSystemUsingDirective() || previousUsing.StaticKeyword.Kind() != SyntaxKind.None)
+                    if (!previousUsing.IsSystemUsingDirective()
+                        || previousUsing.HasNamespaceAliasQualifier()
+                        || previousUsing.StaticKeyword.Kind() != SyntaxKind.None)
                     {
                         systemUsingDirectivesShouldBeBeforeThisName = previousUsing.Name.ToNormalizedString();
                         context.ReportDiagnostic(Diagnostic.Create(Descriptor, usingDirective.GetLocation(), usingDirective.Name.ToNormalizedString(), systemUsingDirectivesShouldBeBeforeThisName));

documentation/KnownChanges.md

@@ -151,6 +151,12 @@ var a = new[] { 1, 2, 3 }.ToArray();
 
 ## Ordering Rules
 
+### SA1208
+
+StyleCop Analyzers only considers using directives to be "System" using directives if they are not alias-qualified,
+while StyleCop Classic ignored the alias. For example, `using global::System;` would be not be considered a System using
+directive by StyleCop Analyzers, but it would be considered a System using directive by StyleCop Classic.
+
 ### SA1214
 
 StyleCop Classic only reports SA1214 for violations involving static fields. In StyleCop Analyzers, SA1214 and SA1215