From 8c6bc98299952dcc346dbca273e914538543391f Mon Sep 17 00:00:00 2001
From: CyrusNajmabadi <cyrusn@microsoft.com>
Date: Fri, 5 Aug 2016 14:40:19 -0400
Subject: [PATCH] Fix crash in symbolkeywriting.

---
 .../SymbolId/SymbolKeyCompilationsTests.cs    | 33 +++++++++++++++++
 .../CSharpCompletionCommandHandlerTests.vb    | 25 ++++++++++++-
 .../SymbolId/SymbolKey.SymbolKeyWriter.cs     | 37 ++++++++++++++++++-
 3 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/src/EditorFeatures/CSharpTest/SymbolId/SymbolKeyCompilationsTests.cs b/src/EditorFeatures/CSharpTest/SymbolId/SymbolKeyCompilationsTests.cs
index c45982ecad6..228488ef6e1 100644
--- a/src/EditorFeatures/CSharpTest/SymbolId/SymbolKeyCompilationsTests.cs
+++ b/src/EditorFeatures/CSharpTest/SymbolId/SymbolKeyCompilationsTests.cs
@@ -176,6 +176,39 @@ class C<T> : I<T>, I
             Assert.Equal(indexer2, ResolveSymbol(indexer2, compilation, SymbolKeyComparison.None));
         }
 
+        [Fact]
+        public void RecursiveReferenceToConstructedGeneric()
+        {
+            var src1 =
+@"using System.Collections.Generic;
+
+class C
+{
+    public void M<Z>(List<Z> list)
+    {
+        var v = list.Add(default(Z));
+    }
+}";
+
+            var comp1 = CreateCompilationWithMscorlib(src1);
+            var comp2 = CreateCompilationWithMscorlib(src1);
+
+            var symbols1 = GetSourceSymbols(comp1, includeLocal: true).ToList();
+            var symbols2 = GetSourceSymbols(comp1, includeLocal: true).ToList();
+
+            // First, make sure that all the symbols in this file resolve properly 
+            // to themselves.
+            ResolveAndVerifySymbolList(symbols1, symbols2, comp1);
+
+            // Now do this for the members of types we see.  We want this 
+            // so we hit things like the members of the constructed type
+            // List<Z>
+            var members1 = symbols1.OfType<INamespaceOrTypeSymbol>().SelectMany(n => n.GetMembers()).ToList();
+            var members2 = symbols2.OfType<INamespaceOrTypeSymbol>().SelectMany(n => n.GetMembers()).ToList();
+
+            ResolveAndVerifySymbolList(members1, members2, comp1);
+        }
+
         #endregion
 
         #region "Change to symbol"
diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb
index e5302f1364b..efe06e5c98b 100644
--- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb
+++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb
@@ -1682,5 +1682,28 @@ class C
             End Using
 
         End Function
+
+        <WpfFact, Trait(Traits.Feature, Traits.Features.Completion)>
+        Public Async Function TestRecursiveGenericSymbolKey() As Task
+            Using state = TestState.CreateCSharpTestState(
+                           <Document><![CDATA[
+using System.Collections.Generic;
+
+class Program
+{
+    static void ReplaceInList<T>(List<T> list, T oldItem, T newItem)
+    {
+        $$
+    }
+}]]></Document>, extraExportedTypes:={GetType(CSharpEditorFormattingService)}.ToList())
+
+                state.SendTypeChars("list")
+                state.SendTypeChars(".")
+                Await state.AssertCompletionSession()
+                state.SendTypeChars("Add")
+
+                Await state.AssertSelectedCompletionItem("Add", description:="void List<T>.Add(T item)")
+            End Using
+        End Function
     End Class
-End Namespace
+End Namespace
\ No newline at end of file
diff --git a/src/Workspaces/Core/Portable/SymbolId/SymbolKey.SymbolKeyWriter.cs b/src/Workspaces/Core/Portable/SymbolId/SymbolKey.SymbolKeyWriter.cs
index e35e5e7b2d3..2bde1379917 100644
--- a/src/Workspaces/Core/Portable/SymbolId/SymbolKey.SymbolKeyWriter.cs
+++ b/src/Workspaces/Core/Portable/SymbolId/SymbolKey.SymbolKeyWriter.cs
@@ -160,13 +160,46 @@ private void WriteSymbolKey(ISymbol symbol, bool first)
 
                 StartKey();
                 symbol.Accept(this);
-                WriteInteger(id);
 
                 if (!shouldWriteOrdinal)
                 {
-                    _symbolToId.Add(symbol, id);
+                    // Note: it is possible in some situations to hit the same symbol 
+                    // multiple times.  For example, if you have:
+                    //
+                    //      Foo<Z>(List<Z> list)
+                    //
+                    // If we start with the symbol for "list" then we'll see the following
+                    // chain of symbols hit:
+                    //
+                    //      List<Z>     
+                    //          Z
+                    //              Foo<Z>(List<Z>)
+                    //                  List<Z>
+                    //
+                    // The recursion is prevented because when we hit 'Foo' we mark that
+                    // we're writing out a signature.  And, in signature mode we only write
+                    // out the ordinal for 'Z' without recursing.  However, even though
+                    // we prevent the recursion, we still hit List<Z> twice.  After writing
+                    // the innermost one out, we'll give it a reference ID.  When we
+                    // then hit the outermost one, we want to just reuse that one.
+                    int existingId;
+                    if (_symbolToId.TryGetValue(symbol, out existingId))
+                    {
+                        // While we recursed, we already hit this symbol.  Use its ID as our
+                        // ID.
+                        id = existingId;
+                    }
+                    else
+                    {
+                        // Haven't hit this symbol before, write out its fresh ID.
+                        _symbolToId.Add(symbol, id);
+                    }
                 }
 
+                // Now write out the ID for this symbol so that any future hits of it can 
+                // write out a reference to it instead.
+                WriteInteger(id);
+
                 EndKey();
             }
 
-- 
GitLab