Improve Generics support in System.Text.Json.SourceGeneration (#71619)

* Improve Generics support in System.Text.Json.SourceGeneration * Apply suggestions * Apply suggestions 2 * unify first element check * apply code styles
dotnet · Jul 12, 2022 · 0cd5fe5 · 0cd5fe5
1 parent cf83273
commit 0cd5fe5
Show file tree

Hide file tree

Showing 8 changed files with 324 additions and 42 deletions.
diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -1101,6 +1101,11 @@ private string GetRootJsonContextImplementation()
             {
                 string contextTypeRef = _currentContext.ContextTypeRef;
                 string contextTypeName = _currentContext.ContextType.Name;
+                int backTickIndex = contextTypeName.IndexOf('`');
+                if (backTickIndex != -1)
+                {
+                    contextTypeName = contextTypeName.Substring(0, backTickIndex);
+                }
 
                 StringBuilder sb = new();
 

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
@@ -466,7 +466,7 @@ private static bool TryGetClassDeclarationList(INamedTypeSymbol typeSymbol, [Not
                         }
 
                         declarationElements[tokenCount] = "class";
-                        declarationElements[tokenCount + 1] = currentSymbol.Name;
+                        declarationElements[tokenCount + 1] = GetClassDeclarationName(currentSymbol);
 
                         (classDeclarationList ??= new List<string>()).Add(string.Join(" ", declarationElements));
                     }
@@ -478,6 +478,38 @@ private static bool TryGetClassDeclarationList(INamedTypeSymbol typeSymbol, [Not
                 return true;
             }
 
+            private static string GetClassDeclarationName(INamedTypeSymbol typeSymbol)
+            {
+                if (typeSymbol.TypeArguments.Length == 0)
+                {
+                    return typeSymbol.Name;
+                }
+
+                StringBuilder sb = new StringBuilder();
+
+                sb.Append(typeSymbol.Name);
+                sb.Append('<');
+
+                bool first = true;
+                foreach (ITypeSymbol typeArg in typeSymbol.TypeArguments)
+                {
+                    if (!first)
+                    {
+                        sb.Append(", ");
+                    }
+                    else
+                    {
+                        first = false;
+                    }
+
+                    sb.Append(typeArg.Name);
+                }
+
+                sb.Append('>');
+
+                return sb.ToString();
+            }
+
             private TypeGenerationSpec? GetRootSerializableType(
                 SemanticModel compilationSemanticModel,
                 AttributeSyntax attributeSyntax,

diff --git a/src/libraries/System.Text.Json/gen/Reflection/TypeExtensions.cs b/src/libraries/System.Text.Json/gen/Reflection/TypeExtensions.cs
@@ -1,9 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 
 namespace System.Text.Json.Reflection
 {
@@ -16,43 +14,61 @@ public static string GetCompilableName(this Type type)
                 return GetCompilableName(type.GetElementType()) + "[]";
             }
 
-            string compilableName;
-
-            if (!type.IsGenericType)
+            if (type.IsGenericParameter)
             {
-                compilableName = type.FullName;
+                return type.Name;
             }
-            else
-            {
-                StringBuilder sb = new();
 
-                string fullName = type.FullName;
-                int backTickIndex = fullName.IndexOf('`');
+            StringBuilder sb = new();
 
-                string baseName = fullName.Substring(0, backTickIndex);
+            sb.Append("global::");
 
-                sb.Append(baseName);
+            string @namespace = type.Namespace;
+            if (!string.IsNullOrEmpty(@namespace) && @namespace != JsonConstants.GlobalNamespaceValue)
+            {
+                sb.Append(@namespace);
+                sb.Append('.');
+            }
 
-                sb.Append('<');
+            int argumentIndex = 0;
+            AppendTypeChain(sb, type, type.GetGenericArguments(), ref argumentIndex);
 
-                Type[] genericArgs = type.GetGenericArguments();
-                int genericArgCount = genericArgs.Length;
-                List<string> genericArgNames = new(genericArgCount);
+            return sb.ToString();
 
-                for (int i = 0; i < genericArgCount; i++)
+            static void AppendTypeChain(StringBuilder sb, Type type, Type[] genericArguments, ref int argumentIndex)
+            {
+                Type declaringType = type.DeclaringType;
+                if (declaringType != null)
                 {
-                    genericArgNames.Add(GetCompilableName(genericArgs[i]));
+                    AppendTypeChain(sb, declaringType, genericArguments, ref argumentIndex);
+                    sb.Append('.');
                 }
+                int backTickIndex = type.Name.IndexOf('`');
+                if (backTickIndex == -1)
+                {
+                    sb.Append(type.Name);
+                }
+                else
+                {
+                    sb.Append(type.Name, 0, backTickIndex);
+
+                    sb.Append('<');
 
-                sb.Append(string.Join(", ", genericArgNames));
+                    int startIndex = argumentIndex;
+                    argumentIndex = type.GetGenericArguments().Length;
+                    for (int i = startIndex; i < argumentIndex; i++)
+                    {
+                        if (i != startIndex)
+                        {
+                            sb.Append(", ");
+                        }
 
-                sb.Append('>');
+                        sb.Append(GetCompilableName(genericArguments[i]));
+                    }
 
-                compilableName = sb.ToString();
+                    sb.Append('>');
+                }
             }
-
-            compilableName = compilableName.Replace("+", ".");
-            return "global::" + compilableName;
         }
 
         public static string GetTypeInfoPropertyName(this Type type)

diff --git a/src/libraries/System.Text.Json/gen/Reflection/TypeWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/TypeWrapper.cs
@@ -40,7 +40,7 @@ public override string AssemblyQualifiedName
         {
             get
             {
-                if (_assemblyQualifiedName == null)
+                if (_assemblyQualifiedName == null && !IsGenericParameter)
                 {
                     StringBuilder sb = new();
 
@@ -111,13 +111,15 @@ public override string AssemblyQualifiedName
 
         public override Type BaseType => _typeSymbol.BaseType!.AsType(_metadataLoadContext);
 
+        public override Type DeclaringType => _typeSymbol.ContainingType?.ConstructedFrom.AsType(_metadataLoadContext);
+
         private string? _fullName;
 
         public override string FullName
         {
             get
             {
-                if (_fullName == null)
+                if (_fullName == null && !IsGenericParameter)
                 {
                     StringBuilder sb = new();
 
@@ -133,24 +135,32 @@ public override string FullName
                     }
                     else
                     {
-                        sb.Append(Name);
-
-                        for (ISymbol currentSymbol = _typeSymbol.ContainingSymbol; currentSymbol != null && currentSymbol.Kind != SymbolKind.Namespace; currentSymbol = currentSymbol.ContainingSymbol)
-                        {
-                            sb.Insert(0, $"{currentSymbol.Name}+");
-                        }
-
                         if (!string.IsNullOrWhiteSpace(Namespace) && Namespace != JsonConstants.GlobalNamespaceValue)
                         {
-                            sb.Insert(0, $"{Namespace}.");
+                            sb.Append(Namespace);
+                            sb.Append('.');
                         }
 
-                        if (this.IsGenericType && !ContainsGenericParameters)
+                        AppendContainingTypes(sb, _typeSymbol);
+
+                        sb.Append(Name);
+
+                        if (IsGenericType && !ContainsGenericParameters)
                         {
                             sb.Append('[');
 
+                            bool first = true;
                             foreach (Type genericArg in GetGenericArguments())
                             {
+                                if (!first)
+                                {
+                                    sb.Append(',');
+                                }
+                                else
+                                {
+                                    first = false;
+                                }
+
                                 sb.Append('[');
                                 sb.Append(genericArg.AssemblyQualifiedName);
                                 sb.Append(']');
@@ -164,6 +174,16 @@ public override string FullName
                 }
 
                 return _fullName;
+
+                static void AppendContainingTypes(StringBuilder sb, ITypeSymbol typeSymbol)
+                {
+                    if (typeSymbol.ContainingType != null)
+                    {
+                        AppendContainingTypes(sb, typeSymbol.ContainingType);
+                        sb.Append(typeSymbol.ContainingType.MetadataName);
+                        sb.Append('+');
+                    }
+                }
             }
         }
 
@@ -207,20 +227,55 @@ public override bool IsEnum
 
         public override bool IsGenericType => _namedTypeSymbol?.IsGenericType == true;
 
-        public override bool ContainsGenericParameters => _namedTypeSymbol?.IsUnboundGenericType == true;
+        public override bool ContainsGenericParameters
+        {
+            get
+            {
+                if (IsGenericParameter)
+                {
+                    return true;
+                }
 
-        public override bool IsGenericTypeDefinition => base.IsGenericTypeDefinition;
+                for (INamedTypeSymbol currentSymbol = _namedTypeSymbol; currentSymbol != null; currentSymbol = currentSymbol.ContainingType)
+                {
+                    if (currentSymbol.TypeArguments.Any(arg => arg.TypeKind == TypeKind.TypeParameter))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
+        }
+
+        public override bool IsGenericTypeDefinition => IsGenericType && SymbolEqualityComparer.Default.Equals(_namedTypeSymbol, _namedTypeSymbol.ConstructedFrom);
+
+        public override bool IsGenericParameter => _typeSymbol.TypeKind == TypeKind.TypeParameter;
 
         public INamespaceSymbol GetNamespaceSymbol => _typeSymbol.ContainingNamespace;
 
         public override Type[] GetGenericArguments()
         {
-            var args = new List<Type>();
-            foreach (ITypeSymbol item in _namedTypeSymbol.TypeArguments)
+            if (!IsGenericType)
             {
-                args.Add(item.AsType(_metadataLoadContext));
+                return EmptyTypes;
             }
+
+            var args = new List<Type>();
+            AddTypeArguments(args, _namedTypeSymbol, _metadataLoadContext);
             return args.ToArray();
+
+            static void AddTypeArguments(List<Type> args, INamedTypeSymbol typeSymbol, MetadataLoadContextInternal metadataLoadContext)
+            {
+                if (typeSymbol.ContainingType != null)
+                {
+                    AddTypeArguments(args, typeSymbol.ContainingType, metadataLoadContext);
+                }
+                foreach (ITypeSymbol item in typeSymbol.TypeArguments)
+                {
+                    args.Add(item.AsType(metadataLoadContext));
+                }
+            }
         }
 
         public override Type GetGenericTypeDefinition()

diff --git a/...ibraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/ContextClasses.cs b/...ibraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/ContextClasses.cs
@@ -118,4 +118,23 @@ internal partial class DictionaryTypeContext : JsonSerializerContext { }
 
     [JsonSerializable(typeof(JsonMessage))]
     public partial class PublicContext : JsonSerializerContext { }
+
+    [JsonSerializable(typeof(JsonMessage))]
+    public partial class GenericContext<T> : JsonSerializerContext { }
+
+    public partial class ContextGenericContainer<T>
+    {
+        [JsonSerializable(typeof(JsonMessage))]
+        public partial class NestedInGenericContainerContext : JsonSerializerContext { }
+    }
+
+    [JsonSerializable(typeof(MyContainingClass.MyNestedClass.MyNestedNestedClass))]
+    [JsonSerializable(typeof(MyContainingClass.MyNestedClass.MyNestedNestedGenericClass<int>))]
+    [JsonSerializable(typeof(MyContainingClass.MyNestedGenericClass<int>.MyNestedGenericNestedClass))]
+    [JsonSerializable(typeof(MyContainingClass.MyNestedGenericClass<int>.MyNestedGenericNestedGenericClass<int>))]
+    [JsonSerializable(typeof(MyContainingGenericClass<int>.MyNestedClass.MyNestedNestedClass))]
+    [JsonSerializable(typeof(MyContainingGenericClass<int>.MyNestedClass.MyNestedNestedGenericClass<int>))]
+    [JsonSerializable(typeof(MyContainingGenericClass<int>.MyNestedGenericClass<int>.MyNestedGenericNestedClass))]
+    [JsonSerializable(typeof(MyContainingGenericClass<int>.MyNestedGenericClass<int>.MyNestedGenericNestedGenericClass<int>))]
+    internal partial class NestedGenericTypesContext : JsonSerializerContext { }
 }
diff --git a/...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs
@@ -21,6 +21,14 @@ public static void VariousNestingAndVisibilityLevelsAreSupported()
             Assert.NotNull(NestedPublicContext.NestedProtectedInternalClass.Default);
         }
 
+        [Fact]
+        public static void VariousGenericsAreSupported()
+        {
+            Assert.NotNull(GenericContext<object>.Default);
+            Assert.NotNull(ContextGenericContainer<object>.NestedInGenericContainerContext.Default);
+            Assert.NotNull(NestedGenericTypesContext.Default);
+        }
+
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/63802", TargetFrameworkMonikers.NetFramework)]
         public static void Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresent()

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.cs
@@ -193,4 +193,32 @@ public class DerivedClass : PolymorphicClass
             public bool Boolean { get; set; }
         }
     }
+
+    public class MyContainingClass
+    {
+        public class MyNestedClass
+        {
+            public class MyNestedNestedClass { }
+            public class MyNestedNestedGenericClass<T1> { }
+        }
+        public class MyNestedGenericClass<T1>
+        {
+            public class MyNestedGenericNestedClass { }
+            public class MyNestedGenericNestedGenericClass<T2> { }
+        }
+    }
+
+    public class MyContainingGenericClass<T>
+    {
+        public class MyNestedClass
+        {
+            public class MyNestedNestedClass { }
+            public class MyNestedNestedGenericClass<T1> { }
+        }
+        public class MyNestedGenericClass<T1>
+        {
+            public class MyNestedGenericNestedClass { }
+            public class MyNestedGenericNestedGenericClass<T2> { }
+        }
+    }
 }