From bdb3e5fd97f747fb66535337d2666190697cee7f Mon Sep 17 00:00:00 2001
From: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>
Date: Mon, 1 Jun 2020 15:55:29 -0700
Subject: [PATCH] properly walk in reverse

---
 .../SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs | 21 +++++---
 src/Workspaces/CoreTest/SymbolKeyTests.cs     | 54 ++++++++++++++++++-
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs
index 59ed5529402..6240d9439b2 100644
--- a/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs
+++ b/src/Workspaces/Core/Portable/SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs
@@ -23,7 +23,7 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
                         break;
                     case INamespaceSymbol parentNamespace:
                         visitor.WriteInteger(1);
-                        visitor.WriteStringArray(GetContainingNamespaceNames(parentNamespace));
+                        visitor.WriteStringArray(GetContainingNamespaceNamesInReverse(parentNamespace));
                         break;
                     default:
                         // writing out `null` here is technically unnecessary.  However, it makes it easier to
@@ -45,7 +45,11 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
                 }
             }
 
-            private static ImmutableArray<string> GetContainingNamespaceNames(INamespaceSymbol namespaceSymbol)
+            /// <summary>
+            /// For a symbol like <c>System.Collections.Generic.IEnumerable</c>, this would produce <c>"Generic",
+            /// "Collections", "System"</c>
+            /// </summary>
+            private static ImmutableArray<string> GetContainingNamespaceNamesInReverse(INamespaceSymbol namespaceSymbol)
             {
                 using var _ = ArrayBuilder<string>.GetInstance(out var builder);
                 while (namespaceSymbol != null && namespaceSymbol.Name != "")
@@ -96,13 +100,16 @@ private static SymbolKeyResolution ResolveContainer(SymbolKeyReader reader)
                     case 0:
                         return reader.ReadSymbolKey();
                     case 1:
-                        var namespaceNames = reader.ReadStringArray();
-                        var currentNamespace = reader.Compilation.GlobalNamespace;
+                        using (var namespaceNames = reader.ReadStringArray())
+                        {
+                            var currentNamespace = reader.Compilation.GlobalNamespace;
 
-                        foreach (var name in namespaceNames)
-                            currentNamespace = reader.Compilation.CreateErrorNamespaceSymbol(currentNamespace, name);
+                            // have to walk the namespaces in reverse because that's how we encoded them.
+                            for (var i = namespaceNames.Count - 1; i >= 0; i--)
+                                currentNamespace = reader.Compilation.CreateErrorNamespaceSymbol(currentNamespace, namespaceNames[i]);
 
-                        return new SymbolKeyResolution(currentNamespace);
+                            return new SymbolKeyResolution(currentNamespace);
+                        }
                     case 2:
                         return reader.ReadSymbolKey();
                     default:
diff --git a/src/Workspaces/CoreTest/SymbolKeyTests.cs b/src/Workspaces/CoreTest/SymbolKeyTests.cs
index f941da449e9..a1afd30e4f9 100644
--- a/src/Workspaces/CoreTest/SymbolKeyTests.cs
+++ b/src/Workspaces/CoreTest/SymbolKeyTests.cs
@@ -793,8 +793,8 @@ class C
 {
     int i { get; }
 }";
-            // Tuples store locations along with them.  But we can only recover those locations
-            // if we're re-resolving into a compilation with the same files.
+
+            // We don't add metadata references, so even `int` will be an error type.
             var compilation1 = GetCompilation(source, LanguageNames.CSharp, "File1.cs", Array.Empty<MetadataReference>());
             var compilation2 = GetCompilation(source, LanguageNames.CSharp, "File2.cs", Array.Empty<MetadataReference>());
 
@@ -821,6 +821,56 @@ class C
             Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
         }
 
+        [Fact, WorkItem(14365, "https://github.com/dotnet/roslyn/issues/14365")]
+        public void TestErrorTypeInNestedNamespace()
+        {
+            var source1 = @"
+public class C
+{
+    public System.Collections.IEnumerable I { get; }
+}";
+
+            var source2 = @"
+class X
+{
+    void M()
+    {
+        new C().I;
+    }
+}";
+
+            // We don't add metadata to the second compilation, so even `System.Collections.IEnumerable` will be an
+            // error type.
+            var compilation1 = GetCompilation(source1, LanguageNames.CSharp, "File1.cs");
+            var compilation2 = GetCompilation(source2, LanguageNames.CSharp, "File2.cs",
+                new[] { compilation1.ToMetadataReference() });
+
+            var symbol = (IPropertySymbol)GetAllSymbols(
+                compilation2.GetSemanticModel(compilation2.SyntaxTrees.Single()),
+                n => n is CSharp.Syntax.MemberAccessExpressionSyntax).Single();
+
+            var propType = symbol.Type;
+            Assert.True(propType.Kind == SymbolKind.ErrorType);
+            Assert.Equal("Collections", propType.ContainingNamespace.Name);
+            Assert.Equal("System", propType.ContainingNamespace.ContainingNamespace.Name);
+
+            // Ensure we don't crash getting these symbol keys.
+            var id = SymbolKey.CreateString(propType);
+            Assert.NotNull(id);
+
+            // Validate that if the client does ask to resolve locations that we
+            // do not crash if those locations cannot be found.
+            var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol();
+            Assert.NotNull(found);
+
+            Assert.Equal(propType.Name, found.Name);
+            Assert.Equal(propType.Kind, found.Kind);
+            Assert.Equal(propType.ContainingNamespace.Name, found.ContainingNamespace.Name);
+
+            var method = found as IErrorTypeSymbol;
+            Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
+        }
+
         [Fact]
         public void TestFunctionPointerTypeSymbols()
         {
-- 
GitLab