Merge pull request #46357 from CyrusNajmabadi/genericErrorType

Fix roundtripping of generic error types.

Author: CyrusNajmabadi

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.ErrorTypeSymbolKey.cs

@@ -30,8 +30,11 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
                         break;
                 }
 
+                var isConstructed = !symbol.Equals(symbol.ConstructedFrom);
                 visitor.WriteInteger(symbol.Arity);
-                if (!symbol.Equals(symbol.ConstructedFrom))
+                visitor.WriteBoolean(isConstructed);
+
+                if (isConstructed)
                 {
                     visitor.WriteSymbolKeyArray(symbol.TypeArguments);
                 }
@@ -63,6 +66,7 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader, out string fai
 
                 var containingSymbolResolution = ResolveContainer(reader, out var containingSymbolFailureReason);
                 var arity = reader.ReadInteger();
+                var isConstructed = reader.ReadBoolean();
 
                 using var typeArguments = reader.ReadSymbolKeyArray<ITypeSymbol>(out var typeArgumentsFailureReason);
 
@@ -86,19 +90,17 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader, out string fai
 
                 using var result = PooledArrayBuilder<INamedTypeSymbol>.GetInstance();
 
-                var typeArgumentsArray = arity > 0 ? typeArguments.Builder.ToArray() : null;
+                var typeArgumentsArray = isConstructed ? typeArguments.Builder.ToArray() : null;
                 foreach (var container in containingSymbolResolution.OfType<INamespaceOrTypeSymbol>())
                 {
-                    result.AddIfNotNull(Construct(
-                        reader, container, name, arity, typeArgumentsArray));
+                    var originalType = reader.Compilation.CreateErrorTypeSymbol(container, name, arity);
+                    var errorType = isConstructed ? originalType.Construct(typeArgumentsArray) : originalType;
+                    result.AddIfNotNull(errorType);
                 }
 
                 // Always ensure at least one error type was created.
                 if (result.Count == 0)
-                {
-                    result.AddIfNotNull(Construct(
-                        reader, container: null, name, arity, typeArgumentsArray));
-                }
+                    result.AddIfNotNull(reader.Compilation.CreateErrorTypeSymbol(container: null, name, arity));
 
                 return CreateResolution(result, $"({nameof(ErrorTypeSymbolKey)} failed)", out failureReason);
             }
@@ -131,12 +133,6 @@ private static SymbolKeyResolution ResolveContainer(SymbolKeyReader reader, out
 
                 throw ExceptionUtilities.UnexpectedValue(type);
             }
-
-            private static INamedTypeSymbol Construct(SymbolKeyReader reader, INamespaceOrTypeSymbol container, string name, int arity, ITypeSymbol[] typeArguments)
-            {
-                var result = reader.Compilation.CreateErrorTypeSymbol(container, name, arity);
-                return typeArguments != null ? result.Construct(typeArguments) : result;
-            }
         }
     }
 }

src/Workspaces/CoreTest/SymbolKeyTests.cs

@@ -1016,6 +1016,52 @@ class C
             TestRoundTrip(fields, comp);
         }
 
+        [Fact]
+        public void TestGenericErrorType()
+        {
+            var source1 = @"
+public class C
+{
+    public Goo<X> G() { }
+}";
+
+            // 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 tree = compilation1.SyntaxTrees.Single();
+            var root = tree.GetRoot();
+            var node = root.DescendantNodes().OfType<CSharp.Syntax.GenericNameSyntax>().Single();
+
+            var semanticModel = compilation1.GetSemanticModel(tree);
+            var symbol = semanticModel.GetTypeInfo(node).Type;
+
+            {
+                // Ensure we don't crash getting these symbol keys.
+                var id = SymbolKey.CreateString(symbol);
+                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, compilation1).GetAnySymbol();
+                Assert.NotNull(found);
+
+                Assert.Equal(symbol.Name, found.Name);
+                Assert.Equal(symbol.Kind, found.Kind);
+            }
+
+            {
+                // Ensure we don't crash getting these symbol keys.
+                var id = SymbolKey.CreateString(symbol.OriginalDefinition);
+                Assert.NotNull(id);
+
+                var found = SymbolKey.ResolveString(id, compilation1).GetAnySymbol();
+                Assert.NotNull(found);
+
+                Assert.Equal(symbol.Name, found.Name);
+                Assert.Equal(symbol.Kind, found.Kind);
+            }
+        }
+
         private static void TestRoundTrip(IEnumerable<ISymbol> symbols, Compilation compilation, Func<ISymbol, object> fnId = null)
         {
             foreach (var symbol in symbols)