Rework SymbolId handling of generics

1. Use ConstructedFrom instead of OriginalDefinition to handle partially
   constructed types.
2. Always encode type parameter references using `n or ``n, since they can
   only refer to type parameters within the current symbol being encoded.
3. When *parsing* SymbolIds, *do* set the type parameter context when
   parsing types.  That way we can handle self-constructed types.  However,
   we don't want to reset the context when we're parsing out the type of a
   parameter/return type/etc, because the signature *can't* refer to the
   type parameters of *that* simple.  So, now we keep track of when the
   parser has finished parsing the top level type, and only reset the
   type parameter context while we're at the top level.
4. Add test cases for a bunch of partially constructed symbols.

(While I was here, I noticed that a GetMembers() call could be simplified to
GetTypeMembers() and a type check/cast elimiated, so I did so.  The diff
looks better with whitespace ignored because of that).

src/Workspaces/Core/Portable/SymbolId/SymbolId.cs

@@ -47,7 +47,7 @@ public static string CreateId(ISymbol symbol)
             }
 
             var builder = new StringBuilder();
-            var generator = new Generator(builder, typeParameterContext: null);
+            var generator = new Generator(builder);
             generator.Visit(symbol);
             return builder.ToString();
         }
@@ -202,12 +202,10 @@ private static string DecodePropertyName(string name, string language)
         private struct Generator
         {
             private readonly StringBuilder _builder;
-            private readonly ISymbol _typeParameterContext;
 
-            public Generator(StringBuilder builder, ISymbol typeParameterContext)
+            public Generator(StringBuilder builder)
             {
                 _builder = builder;
-                _typeParameterContext = typeParameterContext;
             }
 
             public bool Visit(ISymbol symbol)
@@ -363,7 +361,7 @@ private void EncodeGenericMethodInfo(IMethodSymbol symbol)
             {
                 if (symbol.IsGenericMethod)
                 {
-                    if (object.ReferenceEquals(symbol.OriginalDefinition, symbol))
+                    if (Equals(symbol, symbol.ConstructedFrom))
                     {
                         if (symbol.TypeParameters.Length > 0)
                         {
@@ -382,7 +380,7 @@ private void EncodeGenericMethodInfo(IMethodSymbol symbol)
                                 _builder.Append(",");
                             }
 
-                            new Generator(_builder, symbol.ConstructedFrom).Visit(symbol.TypeArguments[i]);
+                            Visit(symbol.TypeArguments[i]);
                         }
 
                         _builder.Append("}");
@@ -404,7 +402,7 @@ private void EncodeParameters(ImmutableArray<IParameterSymbol> parameters)
                         }
 
                         var p = parameters[i];
-                        new Generator(_builder, p.ContainingSymbol).Visit(p.Type);
+                        Visit(p.Type);
                         if (p.RefKind != RefKind.None)
                         {
                             _builder.Append("@");
@@ -467,7 +465,7 @@ private void EncodeGenericTypeInfo(INamedTypeSymbol symbol)
             {
                 if (symbol.IsGenericType)
                 {
-                    if (Equals(symbol.OriginalDefinition, symbol))
+                    if (Equals(symbol, symbol.ConstructedFrom))
                     {
                         _builder.Append("`");
                         _builder.Append(symbol.TypeParameters.Length);
@@ -485,15 +483,8 @@ private void EncodeGenericTypeInfo(INamedTypeSymbol symbol)
 
                             // If we already have a type parameter context (say because we're encoding parameters of a generic method), then use it.
                             // Otherwise, use the current type as the context, so that it's type parameters resolve
-                            if (_typeParameterContext == null)
-                            {
-                                new Generator(_builder, symbol).Visit(symbol.TypeArguments[i]);
-                            }
-                            else
-                            {
                             this.Visit(symbol.TypeArguments[i]);
                         }
-                        }
 
                         _builder.Append("}");
                     }
@@ -539,15 +530,6 @@ private bool VisitPointerType(IPointerTypeSymbol symbol)
 
             private bool VisitTypeParameter(ITypeParameterSymbol symbol)
             {
-                if (!IsInScope(symbol))
-                {
-                    // reference to type parameter not in scope, make explicit scope reference
-                    if (this.Visit(symbol.ContainingSymbol))
-                    {
-                        _builder.Append(":");
-                    }
-                }
-
                 if (symbol.DeclaringMethod != null)
                 {
                     _builder.Append("``");
@@ -565,22 +547,6 @@ private bool VisitTypeParameter(ITypeParameterSymbol symbol)
                 return true;
             }
 
-            private bool IsInScope(ITypeParameterSymbol typeParameterSymbol)
-            {
-                // determine if the type parameter is declared in scope defined by the typeParameterContext symbol
-                var typeParameterDeclarer = typeParameterSymbol.ContainingSymbol;
-
-                for (var scope = _typeParameterContext; scope != null; scope = scope.ContainingSymbol)
-                {
-                    if (Equals(scope, typeParameterDeclarer))
-                    {
-                        return true;
-                    }
-                }
-
-                return false;
-            }
-
             private bool VisitLocal(ILocalSymbol symbol)
             {
                 if (this.Visit(symbol.ContainingSymbol))
@@ -806,6 +772,7 @@ private struct Parser
             private readonly string _id;
             private readonly Compilation _compilation;
             private ISymbol _typeParameterContext;
+            private bool _inSignature;
             private int _index;
 
             private Parser(string id, int index, Compilation compilation, ISymbol typeParameterContext)
@@ -814,6 +781,7 @@ private Parser(string id, int index, Compilation compilation, ISymbol typeParame
                 _compilation = compilation;
                 _typeParameterContext = typeParameterContext;
                 _index = index;
+                _inSignature = false;
             }
 
             public static bool Parse(string id, Compilation compilation, List<ISymbol> results)
@@ -882,18 +850,22 @@ private void ParseNamedSymbol(List<ISymbol> results)
                         switch (prefix)
                         {
                             case MethodPrefix:
+                                _inSignature = true;
                                 GetMatchingMethods(containers, name, results);
                                 break;
                             case NamedTypePrefix:
                                 GetMatchingTypes(containers, name, results);
                                 break;
                             case PropertyPrefix:
+                                _inSignature = true;
                                 GetMatchingProperties(containers, name, results);
                                 break;
                             case EventPrefix:
+                                _inSignature = true;
                                 GetMatchingEvents(containers, name, results);
                                 break;
                             case FieldPrefix:
+                                _inSignature = true;
                                 GetMatchingFields(containers, name, results);
                                 break;
                             case NamespacePrefix:
@@ -1258,25 +1230,27 @@ private void GetMatchingTypes(IReadOnlyList<ISymbol> containers, string name, Li
 
             private void GetMatchingTypes(INamespaceOrTypeSymbol container, string name, List<ISymbol> results)
             {
+                var originalContext = _typeParameterContext;
                 var typeArguments = s_symbolListPool.Allocate();
                 try
                 {
                     var startIndex = _index;
                     var endIndex = _index;
 
-                    var members = container.GetMembers(name);
+                    var members = container.GetTypeMembers(name);
 
                     foreach (var symbol in members)
                     {
-                        if (symbol.Kind == SymbolKind.NamedType)
-                        {
-                            var namedType = (INamedTypeSymbol)symbol;
+                        var namedType = symbol;
                         _index = startIndex;
 
                         // has type arguments?
                         if (PeekNextChar() == '{')
                         {
-                                //_typeParameterContext = namedType;
+                            if (!_inSignature)
+                            {
+                                _typeParameterContext = namedType;
+                            }
 
                             typeArguments.Clear();
                             ParseTypeArguments(typeArguments);
@@ -1307,13 +1281,13 @@ private void GetMatchingTypes(INamespaceOrTypeSymbol container, string name, Lis
                         endIndex = _index;
                         results.Add(namedType);
                     }
-                    }
 
                     _index = endIndex;
                 }
                 finally
                 {
                     s_symbolListPool.ClearAndFree(typeArguments);
+                    _typeParameterContext = originalContext;
                 }
             }
 

src/Workspaces/CoreTest/SymbolIdTests.cs

@@ -446,8 +446,58 @@ public class B<TInner>
             var outer = GetDeclaredSymbols(compilation).OfType<INamedTypeSymbol>().First(s => s.Name == "A");
             var constructed = outer.Construct(compilation.GetSpecialType(SpecialType.System_String));
             var inner = constructed.GetTypeMembers().Single();
-            var id = SymbolId.CreateId(inner);
-            Assert.Equal("T:A{N:System.T:String}.T:B{`1}", id);
+            TestRoundTrip(inner, compilation, "T:A{N:System.T:String}.T:B`1");
+        }
+
+        [Fact, WorkItem(235912, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=235912&_a=edit")]
+        public void TestNestedGenericType1()
+        {
+            var source = @"
+using System.Collections.Generic;
+
+public class A<T1>
+{
+    public class B<T2>
+    {
+        void M<T3>(T1 t1, T2, T3 t3, List<int> l1, List<T3> l2) { }
+    }
+}";
+            var compilation = GetCompilation(source);
+            var tree = compilation.SyntaxTrees.First();
+            var model = compilation.GetSemanticModel(tree);
+
+            var a = GetDeclaredSymbols(compilation).OfType<INamedTypeSymbol>().Single(s => s.Name == "A");
+            var a_b = a.GetTypeMembers().Single();
+            var a_b_m = a_b.GetMembers().Single(s => s.Name == "M");
+
+            TestRoundTrip(a, compilation, "T:A`1");
+            TestRoundTrip(a_b, compilation, "T:A`1.T:B`1");
+            TestRoundTrip(a_b_m, compilation, "T:A`1.T:B`1.M:M`1(`0,`1,``0,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{``0})");
+
+            var a_string = a.Construct(compilation.GetSpecialType(SpecialType.System_String));
+            var a_string_b = a_string.GetTypeMembers().Single();
+            var a_string_b_m = a_string_b.GetMembers().Single(s => s.Name == "M");
+            TestRoundTrip(a_string, compilation, "T:A{N:System.T:String}");
+            TestRoundTrip(a_string_b, compilation, "T:A{N:System.T:String}.T:B`1");
+            TestRoundTrip(a_string_b_m, compilation, "T:A{N:System.T:String}.T:B`1.M:M`1(N:System.T:String,`1,``0,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{``0})");
+
+            var a_string_b_int = a_string_b.Construct(compilation.GetSpecialType(SpecialType.System_Int32));
+            var a_string_b_int_m = a_string_b_int.GetMembers().Single(s => s.Name == "M");
+            TestRoundTrip(a_string_b_int, compilation, "T:A{N:System.T:String}.T:B{N:System.T:Int32}");
+            TestRoundTrip(a_string_b_int_m, compilation, "T:A{N:System.T:String}.T:B{N:System.T:Int32}.M:M`1(N:System.T:String,N:System.T:Int32,``0,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{``0})");
+
+            var a_string_b_int_m_datetime = ((IMethodSymbol)a_string_b_int_m).Construct(compilation.GetSpecialType(SpecialType.System_DateTime));
+            TestRoundTrip(a_string_b_int_m_datetime, compilation, "T:A{N:System.T:String}.T:B{N:System.T:Int32}.M:M{N:System.T:DateTime}(N:System.T:String,N:System.T:Int32,N:System.T:DateTime,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{N:System.T:DateTime})");
+
+            var a_b_int = a_b.Construct(compilation.GetSpecialType(SpecialType.System_Int32));
+            var a_b_int_m = a_b_int.GetMembers().Single(s => s.Name == "M");
+            var a_b_int_m_datetime = ((IMethodSymbol)a_b_int_m).Construct(compilation.GetSpecialType(SpecialType.System_DateTime));
+            TestRoundTrip(a_b_int, compilation, "T:A`1.T:B{N:System.T:Int32}");
+            TestRoundTrip(a_b_int_m, compilation, "T:A`1.T:B{N:System.T:Int32}.M:M`1(`0,N:System.T:Int32,``0,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{``0})");
+            TestRoundTrip(a_b_int_m_datetime, compilation, "T:A`1.T:B{N:System.T:Int32}.M:M{N:System.T:DateTime}(`0,N:System.T:Int32,N:System.T:DateTime,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{N:System.T:DateTime})");
+
+            var a_b_m_datetime = ((IMethodSymbol)a_b_m).Construct(compilation.GetSpecialType(SpecialType.System_DateTime));
+            TestRoundTrip(a_b_m_datetime, compilation, "T:A`1.T:B`1.M:M{N:System.T:DateTime}(`0,`1,N:System.T:DateTime,N:System.N:Collections.N:Generic.T:List{N:System.T:Int32},N:System.N:Collections.N:Generic.T:List{N:System.T:DateTime})");
         }
 
         [Fact, WorkItem(11193, "https://github.com/dotnet/roslyn/issues/11193")]
@@ -500,14 +550,19 @@ private void TestRoundTrip(IEnumerable<ISymbol> symbols, Compilation compilation
         {
             foreach (var symbol in symbols)
             {
-                TestRoundTrip(symbol, compilation, fnId);
+                TestRoundTrip(symbol, compilation, fnId: fnId);
             }
         }
 
-        private void TestRoundTrip(ISymbol symbol, Compilation compilation, Func<ISymbol, object> fnId = null)
+        private void TestRoundTrip(ISymbol symbol, Compilation compilation, string expectedId = null, Func<ISymbol, object> fnId = null)
         {
             var id = SymbolId.CreateId(symbol);
             Assert.NotNull(id);
+            if (expectedId != null)
+            {
+                Assert.Equal(expectedId, id);
+            }
+
             var found = SymbolId.GetFirstSymbolForId(id, compilation);
             Assert.NotNull(found);