Merge pull request #17735 from CyrusNajmabadi/offerSystemResultsFirst

Prefer results from 'System' namespaces in Add-Using.

src/EditorFeatures/CSharpTest/Diagnostics/AddUsing/AddUsingTests.cs

@@ -4405,6 +4405,96 @@ List<int> Method()
 }");
         }
 
+        [WorkItem(15025, "https://github.com/dotnet/roslyn/issues/15025")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddImport)]
+        public async Task TestPreferSystemNamespaceFirst()
+        {
+            await TestInRegularAndScriptAsync(
+@"
+namespace Microsoft
+{
+    public class SomeClass { }
+}
+
+namespace System
+{
+    public class SomeClass { }
+}
+
+namespace N
+{
+    class Class
+    {
+        [|SomeClass|] c;
+    }
+}",
+@"
+using System;
+
+namespace Microsoft
+{
+    public class SomeClass { }
+}
+
+namespace System
+{
+    public class SomeClass { }
+}
+
+namespace N
+{
+    class Class
+    {
+        SomeClass c;
+    }
+}");
+        }
+
+        [WorkItem(15025, "https://github.com/dotnet/roslyn/issues/15025")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddImport)]
+        public async Task TestPreferSystemNamespaceFirst2()
+        {
+            await TestInRegularAndScriptAsync(
+@"
+namespace Microsoft
+{
+    public class SomeClass { }
+}
+
+namespace System
+{
+    public class SomeClass { }
+}
+
+namespace N
+{
+    class Class
+    {
+        [|SomeClass|] c;
+    }
+}",
+@"
+using Microsoft;
+
+namespace Microsoft
+{
+    public class SomeClass { }
+}
+
+namespace System
+{
+    public class SomeClass { }
+}
+
+namespace N
+{
+    class Class
+    {
+        SomeClass c;
+    }
+}", index: 1);
+        }
+
         public partial class AddUsingTestsWithAddImportDiagnosticProvider : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
         {
             internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)

src/EditorFeatures/CSharpTest/Diagnostics/FullyQualify/FullyQualifyTests.cs

@@ -1116,7 +1116,8 @@ public class Program
 {
     static void M()
     {
-        [|Xaml|] }
+        [|Xaml|]
+    }
 }",
 @"namespace MS.Internal.Xaml
 {
@@ -1136,7 +1137,8 @@ public class Program
 {
     static void M()
     {
-        System.Xaml }
+        System.Xaml
+    }
 }");
 
             await TestInRegularAndScriptAsync(
@@ -1158,7 +1160,8 @@ public class Program
 {
     static void M()
     {
-        [|Xaml|] }
+        [|Xaml|]
+    }
 }",
 @"namespace MS.Internal.Xaml
 {
@@ -1178,8 +1181,9 @@ public class Program
 {
     static void M()
     {
-        MS.Internal.Xaml }
-}");
+        MS.Internal.Xaml
+    }
+}", index: 1);
         }
 
         [WorkItem(11071, "https://github.com/dotnet/roslyn/issues/11071")]

src/EditorFeatures/Test2/Diagnostics/AddImport/AddImportCrossLanguageTests.vb

@@ -219,7 +219,7 @@ Namespace VBAssembly1
 End Namespace
                 </text>.Value.Trim()
 
-            Await TestAsync(input, expected, codeActionIndex:=0)
+            Await TestAsync(input, expected, codeActionIndex:=1)
         End Function
 
         <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddImport)>

src/Features/Core/Portable/AddImport/References/Reference.cs

@@ -71,7 +71,8 @@ public int CompareTo(Document document, Reference other)
                 // If the weights are the same and no names changed, just order 
                 // them based on the namespace we're adding an import for.
                 return INamespaceOrTypeSymbolExtensions.CompareNameParts(
-                    this.SearchResult.NameParts, other.SearchResult.NameParts);
+                    this.SearchResult.NameParts, other.SearchResult.NameParts,
+                    placeSystemNamespaceFirst: true);
             }
 
             public override bool Equals(object obj)

src/Features/Core/Portable/CodeFixes/Qualify/AbstractFullyQualifyCodeFixProvider.SymbolResult.cs

@@ -23,24 +23,16 @@ public SymbolResult(INamespaceOrTypeSymbol symbol, int weight)
             }
 
             public override bool Equals(object obj)
-            {
-                return Equals((SymbolResult)obj);
-            }
+                => Equals((SymbolResult)obj);
 
             public bool Equals(SymbolResult other)
-            {
-                return Equals(Symbol, other.Symbol);
-            }
+                => Equals(Symbol, other.Symbol);
 
             public override int GetHashCode()
-            {
-                return Symbol.GetHashCode();
-            }
+                => Symbol.GetHashCode();
 
             public SymbolResult WithSymbol(INamespaceOrTypeSymbol other)
-            {
-                return new SymbolResult(other, Weight);
-            }
+                => new SymbolResult(other, Weight);
 
             public int CompareTo(SymbolResult other)
             {
@@ -54,7 +46,7 @@ public int CompareTo(SymbolResult other)
                 }
 
                 return INamespaceOrTypeSymbolExtensions.CompareNameParts(
-                    this.NameParts, other.NameParts);
+                    this.NameParts, other.NameParts, placeSystemNamespaceFirst: true);
             }
         }
     }

src/Workspaces/CSharp/Portable/Utilities/TokenComparer.cs

@@ -8,8 +8,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Utilities
 {
     internal class TokenComparer : IComparer<SyntaxToken>
     {
-        private const string SystemNamespace = "System";
-
         public static readonly IComparer<SyntaxToken> NormalInstance = new TokenComparer(specialCaseSystem: false);
         public static readonly IComparer<SyntaxToken> SystemFirstInstance = new TokenComparer(specialCaseSystem: true);
 
@@ -20,19 +18,14 @@ private TokenComparer(bool specialCaseSystem)
             _specialCaseSystem = specialCaseSystem;
         }
 
-        private static bool IsSystem(string s)
-        {
-            return s == SystemNamespace;
-        }
-
         public int Compare(SyntaxToken x, SyntaxToken y)
         {
             if (_specialCaseSystem &&
                 x.GetPreviousToken(includeSkipped: true).IsKind(SyntaxKind.UsingKeyword, SyntaxKind.StaticKeyword) &&
                 y.GetPreviousToken(includeSkipped: true).IsKind(SyntaxKind.UsingKeyword, SyntaxKind.StaticKeyword))
             {
-                var token1IsSystem = IsSystem(x.ValueText);
-                var token2IsSystem = IsSystem(y.ValueText);
+                var token1IsSystem = x.ValueText == nameof(System);
+                var token2IsSystem = y.ValueText == nameof(System);
 
                 if (token1IsSystem && !token2IsSystem)
                 {
@@ -55,8 +48,8 @@ private int CompareWorker(SyntaxToken x, SyntaxToken y)
             }
 
             // By using 'ValueText' we get the value that is normalized.  i.e.
-            // @class will be 'class', and unicode escapes will be converted
-            // to actual unicode.  This allows sorting to work properly across
+            // @class will be 'class', and Unicode escapes will be converted
+            // to actual Unicode.  This allows sorting to work properly across
             // tokens that have different source representations, but which
             // mean the same thing.
             var string1 = x.ValueText;
@@ -78,4 +71,4 @@ private int CompareWorker(SyntaxToken x, SyntaxToken y)
                 CompareOptions.IgnoreNonSpace | CompareOptions.IgnoreWidth);
         }
     }
-}
+}
\ No newline at end of file

src/Workspaces/Core/Portable/Shared/Extensions/INamespaceOrTypeSymbolExtensions.cs

@@ -23,8 +23,6 @@ internal static partial class INamespaceOrTypeSymbolExtensions
         private static readonly SymbolDisplayFormat s_shortNameFormat = new SymbolDisplayFormat(
             miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes | SymbolDisplayMiscellaneousOptions.ExpandNullable);
 
-        public static readonly Comparison<INamespaceOrTypeSymbol> CompareNamespaceOrTypeSymbols = CompareTo;
-
         public static string GetShortName(this INamespaceOrTypeSymbol symbol)
         {
             return symbol.ToDisplayString(s_shortNameFormat);
@@ -37,24 +35,34 @@ public static IEnumerable<IPropertySymbol> GetIndexers(this INamespaceOrTypeSymb
                 : symbol.GetMembers(WellKnownMemberNames.Indexer).OfType<IPropertySymbol>().Where(p => p.IsIndexer);
         }
 
-        public static int CompareTo(this INamespaceOrTypeSymbol n1, INamespaceOrTypeSymbol n2)
-        {
-            var names1 = s_namespaceOrTypeToNameMap.GetValue(n1, s_getNamePartsCallBack);
-            var names2 = s_namespaceOrTypeToNameMap.GetValue(n2, s_getNamePartsCallBack);
-
-            return CompareNameParts(names1, names2);
-        }
-
         public static IReadOnlyList<string> GetNameParts(this INamespaceOrTypeSymbol symbol)
-        {
-            return s_namespaceOrTypeToNameMap.GetValue(symbol, s_getNamePartsCallBack);
-        }
+            => s_namespaceOrTypeToNameMap.GetValue(symbol, s_getNamePartsCallBack);
 
-        public static int CompareNameParts(IReadOnlyList<string> names1, IReadOnlyList<string> names2)
+        public static int CompareNameParts(
+            IReadOnlyList<string> names1, IReadOnlyList<string> names2, 
+            bool placeSystemNamespaceFirst)
         {
             for (var i = 0; i < Math.Min(names1.Count, names2.Count); i++)
             {
-                var comp = names1[i].CompareTo(names2[i]);
+                var name1 = names1[i];
+                var name2 = names2[i];
+
+                if (i == 0 && placeSystemNamespaceFirst)
+                {
+                    var name1IsSystem = name1 == nameof(System);
+                    var name2IsSystem = name2 == nameof(System);
+
+                    if (name1IsSystem && !name2IsSystem)
+                    {
+                        return -1;
+                    }
+                    else if (!name1IsSystem && name2IsSystem)
+                    {
+                        return 1;
+                    }
+                }
+
+                var comp = name1.CompareTo(name2);
                 if (comp != 0)
                 {
                     return comp;