Apply the SourceWriter pattern to the STJ source generator. #86526

eiriktsarpalis · 2023-05-19T20:13:49Z

Makes the following changes:

Refactors the Emitter class to use the SourceWriter pattern already used in the ConfigurationBinder source generator. This should minimize the amount indentation-related problems and minimize the amount of intermediate strings being created by the emitter.
Minimizes the amount of generated code by reworking the shared helpers used for resolving runtime converter. Fix Size saving opportunity for System.Text.Json generators with custom converters #84015.

Generated code now looks as follows:

Main context file

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618

namespace System.Text.Json.SourceGeneration.Tests
{
    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Text.Json.SourceGeneration", "42.42.42.42")]
    internal partial class MixedModeContext
    {
        private readonly static global::System.Text.Json.JsonSerializerOptions s_defaultOptions = new()
        {
            DefaultIgnoreCondition = global::System.Text.Json.Serialization.JsonIgnoreCondition.Never,
            IgnoreReadOnlyFields = false,
            IgnoreReadOnlyProperties = false,
            IncludeFields = true,
            WriteIndented = false,
            PropertyNamingPolicy = null
        };

        private static global::System.Text.Json.SourceGeneration.Tests.MixedModeContext? s_defaultContext;
        
        /// <summary>
        /// The default <see cref="global::System.Text.Json.Serialization.JsonSerializerContext"/> associated with a default <see cref="global::System.Text.Json.JsonSerializerOptions"/> instance.
        /// </summary>
        public static global::System.Text.Json.SourceGeneration.Tests.MixedModeContext Default => s_defaultContext ??= new global::System.Text.Json.SourceGeneration.Tests.MixedModeContext(new global::System.Text.Json.JsonSerializerOptions(s_defaultOptions));
        
        /// <summary>
        /// The source-generated options associated with this context.
        /// </summary>
        protected override global::System.Text.Json.JsonSerializerOptions? GeneratedSerializerOptions { get; } = s_defaultOptions;
        
        /// <inheritdoc/>
        public MixedModeContext() : base(null)
        {
        }
        
        /// <inheritdoc/>
        public MixedModeContext(global::System.Text.Json.JsonSerializerOptions options) : base(options)
        {
        }

        private static bool TryGetTypeInfoForRuntimeCustomConverter<TJsonMetadataType>(global::System.Text.Json.JsonSerializerOptions options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<TJsonMetadataType> jsonTypeInfo)
        {
            global::System.Text.Json.Serialization.JsonConverter? converter = GetRuntimeConverterForType(typeof(TJsonMetadataType), options);
            if (converter != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<TJsonMetadataType>(options, converter);
                return true;
            }
        
            jsonTypeInfo = null;
            return false;
        }
        
        private static global::System.Text.Json.Serialization.JsonConverter? GetRuntimeConverterForType(global::System.Type type, global::System.Text.Json.JsonSerializerOptions options)
        {
            for (int i = 0; i < options.Converters.Count; i++)
            {
                global::System.Text.Json.Serialization.JsonConverter? converter = options.Converters[i];
                if (converter?.CanConvert(type) == true)
                {
                    return ExpandConverter(type, converter, options);
                }
            }
        
            return null;
        }
        
        private static global::System.Text.Json.Serialization.JsonConverter ExpandConverter(global::System.Type type, global::System.Text.Json.Serialization.JsonConverter converter, global::System.Text.Json.JsonSerializerOptions options)
        {
            if (converter is global::System.Text.Json.Serialization.JsonConverterFactory factory)
            {
                converter = factory.CreateConverter(type, options);
                if (converter is null || converter is global::System.Text.Json.Serialization.JsonConverterFactory)
                {
                    throw new global::System.InvalidOperationException(string.Format("The converter '{0}' cannot return null or a JsonConverterFactory instance.", factory.GetType()));
                }
            }
        
            if (!converter.CanConvert(type))
            {
                throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), type));
            }
        
            return converter;
        }
    }
}

JsonTypeInfo declaration file

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618

namespace System.Text.Json.SourceGeneration.Tests
{
    internal partial class MixedModeContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>? _HighLowTempsRecord;
        
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> HighLowTempsRecord
        {
            get => _HighLowTempsRecord ??= (global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>)Options.GetTypeInfo(typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord));
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> Create_HighLowTempsRecord(global::System.Text.Json.JsonSerializerOptions options)
        {
            if (!TryGetTypeInfoForRuntimeCustomConverter<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>(options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> jsonTypeInfo))
            {
                var objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>()
                {
                    ObjectCreator = null,
                    ObjectWithParameterizedConstructorCreator = static (args) => new global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord((int)args[0], (int)args[1]),
                    PropertyMetadataInitializer = _ => HighLowTempsRecordPropInit(options),
                    ConstructorParameterMetadataInitializer = HighLowTempsRecordCtorParamInit,
                    NumberHandling = default,
                    SerializeHandler = null
                };
                
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>(options, objectInfo);
            }
        
            jsonTypeInfo.OriginatingResolver = this;
            return jsonTypeInfo;
        }

        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] HighLowTempsRecordPropInit(global::System.Text.Json.JsonSerializerOptions options)
        {
            var properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[3];

            var info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type>()
            {
                IsProperty = true,
                IsPublic = false,
                IsVirtual = true,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = null,
                Setter = null,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "EqualityContract",
                JsonPropertyName = null
            };
            
            properties[0] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Type>(options, info0);

            var info1 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<int>()
            {
                IsProperty = true,
                IsPublic = true,
                IsVirtual = false,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = static (obj) => ((global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord)obj).High,
                Setter = static (obj, value) => throw new global::System.InvalidOperationException("Setting init-only properties is not supported in source generation mode."),
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "High",
                JsonPropertyName = null
            };
            
            properties[1] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<int>(options, info1);

            var info2 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<int>()
            {
                IsProperty = true,
                IsPublic = true,
                IsVirtual = false,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = static (obj) => ((global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord)obj).Low,
                Setter = static (obj, value) => throw new global::System.InvalidOperationException("Setting init-only properties is not supported in source generation mode."),
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "Low",
                JsonPropertyName = null
            };
            
            properties[2] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<int>(options, info2);

            return properties;
        }

        private static global::System.Text.Json.Serialization.Metadata.JsonParameterInfoValues[] HighLowTempsRecordCtorParamInit()
        {
            var parameters = new global::System.Text.Json.Serialization.Metadata.JsonParameterInfoValues[2];

            parameters[0] = new()
            {
                Name = "High",
                ParameterType = typeof(int),
                Position = 0,
                HasDefaultValue = false,
                DefaultValue = default(int)
            };
            
            parameters[1] = new()
            {
                Name = "Low",
                ParameterType = typeof(int),
                Position = 1,
                HasDefaultValue = false,
                DefaultValue = default(int)
            };
            
            return parameters;
        }
    }
}

JsonTypeInfo with design-time custom converter

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618

namespace System.Text.Json.SourceGeneration.Tests
{
    internal partial class MetadataContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter>? _StructWithCustomConverter;
        
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter> StructWithCustomConverter
        {
            get => _StructWithCustomConverter ??= (global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter>)Options.GetTypeInfo(typeof(global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter));
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter> Create_StructWithCustomConverter(global::System.Text.Json.JsonSerializerOptions options)
        {
            if (!TryGetTypeInfoForRuntimeCustomConverter<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter>(options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter> jsonTypeInfo))
            {
                global::System.Text.Json.Serialization.JsonConverter converter = ExpandConverter(typeof(global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter), new global::System.Text.Json.SourceGeneration.Tests.CustomConverter_StructWithCustomConverter(), options);
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.Text.Json.SourceGeneration.Tests.StructWithCustomConverter> (options, converter);
            }
        
            jsonTypeInfo.OriginatingResolver = this;
            return jsonTypeInfo;
        }
    }
}

Contributes to #68353.

ghost · 2023-05-19T20:14:01Z

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the following changes:

Refactors the Emitter class to use the SourceWriter pattern already used in the ConfigurationBinder source generator. This should minimize the amount indentation-related problems and minimize the amount of intermediate strings being created by the emitter.
Minimizes the amount of generated code by reworking the shared helpers used for resolving runtime converter. Fix Size saving opportunity for System.Text.Json generators with custom converters #84015.

Generated code now looks as follows:

Main context file

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618

namespace System.Text.Json.SourceGeneration.Tests
{
    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Text.Json.SourceGeneration", "42.42.42.42")]
    internal partial class MixedModeContext
    {
        private readonly static global::System.Text.Json.JsonSerializerOptions s_defaultOptions = new()
        {
            DefaultIgnoreCondition = global::System.Text.Json.Serialization.JsonIgnoreCondition.Never,
            IgnoreReadOnlyFields = false,
            IgnoreReadOnlyProperties = false,
            IncludeFields = true,
            WriteIndented = false,
            PropertyNamingPolicy = null
        };

        private static global::System.Text.Json.SourceGeneration.Tests.MixedModeContext? s_defaultContext;
        
        /// <summary>
        /// The default <see cref="global::System.Text.Json.Serialization.JsonSerializerContext"/> associated with a default <see cref="global::System.Text.Json.JsonSerializerOptions"/> instance.
        /// </summary>
        public static global::System.Text.Json.SourceGeneration.Tests.MixedModeContext Default => s_defaultContext ??= new global::System.Text.Json.SourceGeneration.Tests.MixedModeContext(new global::System.Text.Json.JsonSerializerOptions(s_defaultOptions));
        
        /// <summary>
        /// The source-generated options associated with this context.
        /// </summary>
        protected override global::System.Text.Json.JsonSerializerOptions? GeneratedSerializerOptions { get; } = s_defaultOptions;
        
        /// <inheritdoc/>
        public MixedModeContext() : base(null)
        {
        }
        
        /// <inheritdoc/>
        public MixedModeContext(global::System.Text.Json.JsonSerializerOptions options) : base(options)
        {
        }

        private bool TryGetTypeInfoForRuntimeCustomConverter<TJsonMetadataType>(global::System.Text.Json.JsonSerializerOptions options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<TJsonMetadataType> jsonTypeInfo)
        {
            foreach (global::System.Text.Json.Serialization.JsonConverter? converter in options.Converters)
            {
                if (converter?.CanConvert(typeof(TJsonMetadataType)) == true)
                {
                    jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<TJsonMetadataType>(options, ExpandConverter(typeof(TJsonMetadataType), converter, options));
                    return true;
                }
            }
        
            jsonTypeInfo = null;
            return false;
        }
        
        private static global::System.Text.Json.Serialization.JsonConverter ExpandConverter(global::System.Type type, global::System.Text.Json.Serialization.JsonConverter converter, global::System.Text.Json.JsonSerializerOptions options)
        {
            if (converter is global::System.Text.Json.Serialization.JsonConverterFactory factory)
            {
                converter = factory.CreateConverter(type, options);
                if (converter is null or global::System.Text.Json.Serialization.JsonConverterFactory)
                {
                    throw new global::System.InvalidOperationException(string.Format("The converter '{0}' cannot return null or a JsonConverterFactory instance.", factory.GetType()));
                }
            }
        
            if (!converter.CanConvert(type))
            {
                throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), type));
            }
        
            return converter;
        }
    }
}

JsonTypeInfo declaration file

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0612, CS0618

namespace System.Text.Json.SourceGeneration.Tests
{
    internal partial class MixedModeContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>? _HighLowTempsRecord;
        
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> HighLowTempsRecord
        {
            get => _HighLowTempsRecord ??= (global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>)Options.GetTypeInfo(typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord));
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> Create_HighLowTempsRecord(global::System.Text.Json.JsonSerializerOptions options)
        {
            if (!TryGetTypeInfoForRuntimeCustomConverter<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>(options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord> jsonTypeInfo))
            {
                var objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>()
                {
                    ObjectCreator = null,
                    ObjectWithParameterizedConstructorCreator = static (args) => new global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord((int)args[0], (int)args[1]),
                    PropertyMetadataInitializer = _ => HighLowTempsRecordPropInit(options),
                    ConstructorParameterMetadataInitializer = HighLowTempsRecordCtorParamInit,
                    NumberHandling = default,
                    SerializeHandler = null
                };
                
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord>(options, objectInfo);
            }
        
            jsonTypeInfo.OriginatingResolver = this;
            return jsonTypeInfo;
        }

        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] HighLowTempsRecordPropInit(global::System.Text.Json.JsonSerializerOptions options)
        {
            var properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[3];

            var info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type>()
            {
                IsProperty = true,
                IsPublic = false,
                IsVirtual = true,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = null,
                Setter = null,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "EqualityContract",
                JsonPropertyName = null
            };
            
            properties[0] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Type>(options, info0);

            var info1 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<int>()
            {
                IsProperty = true,
                IsPublic = true,
                IsVirtual = false,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = static (obj) => ((global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord)obj).High,
                Setter = static (obj, value) => throw new global::System.InvalidOperationException("Setting init-only properties is not supported in source generation mode."),
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "High",
                JsonPropertyName = null
            };
            
            properties[1] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<int>(options, info1);

            var info2 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<int>()
            {
                IsProperty = true,
                IsPublic = true,
                IsVirtual = false,
                DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord),
                Converter = null,
                Getter = static (obj) => ((global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord)obj).Low,
                Setter = static (obj, value) => throw new global::System.InvalidOperationException("Setting init-only properties is not supported in source generation mode."),
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "Low",
                JsonPropertyName = null
            };
            
            properties[2] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<int>(options, info2);

            return properties;
        }

        private static global::System.Text.Json.Serialization.Metadata.JsonParameterInfoValues[] HighLowTempsRecordCtorParamInit()
        {
            var parameters = new global::System.Text.Json.Serialization.Metadata.JsonParameterInfoValues[2];

            parameters[0] = new()
            {
                Name = "High",
                ParameterType = typeof(int),
                Position = 0,
                HasDefaultValue = false,
                DefaultValue = default(int)
            };
            
            parameters[1] = new()
            {
                Name = "Low",
                ParameterType = typeof(int),
                Position = 1,
                HasDefaultValue = false,
                DefaultValue = default(int)
            };
            
            return parameters;
        }
    }
}

Author:	eiriktsarpalis
Assignees:	-
Labels:	`area-System.Text.Json`
Milestone:	-

eiriktsarpalis · 2023-05-19T20:15:25Z

cc @Sergio0694

layomia

LGTM - just a couple questions about the writer.

src/libraries/System.Text.Json/gen/Helpers/SourceWriter.cs

layomia · 2023-05-22T16:06:53Z

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

-                ImmutableEquatableArray<string> declarationList = _currentContext.ContextClassDeclarations;
-                int declarationCount = declarationList.Count;
-                Debug.Assert(declarationCount >= 1);
+                writer.WriteLine("""


Could this method be renamed given that this isn't one line?

Any suggestions? I figured it should follow StringBuilder convention in which "Line" suggests that a newline is inserted at the end of the string, which is happening here.

I'm using WriteBlock in the config generator. My comments about this apply if we can consider this a C# writer. If it's a general source writer, I'm not sure why IndentedStringBuilder isn't sufficient for writing.

If you mean IndentedSourceWriter, the type doesn't support multi-line string literals. It's intended as a direct replacement of that type.

layomia · 2023-05-22T16:09:32Z

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

-                        $"{contextName}.g.cs",
-                        GetRootJsonContextImplementation(),
-                        isRootContextDef: true);
+                    _sourceGenerationContext.AddSource($"{contextName}.g.cs", GetRootJsonContextImplementation(contextGenerationSpec));


nit: first assign to sourceText declared above?

layomia · 2023-05-22T16:16:05Z

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

-                {
-                    valueToWrite = $"{valueToWrite}.ToString()";
-                }
+                writer.WriteLine('{');


Why not let the writer handle brackets and indentation?

There are a few reasons, one I'd prefer if the writer class remained language agnostic, secondly you might want to indent without inserting brackets (e.g. loop containing a single statement), thirdly in some cases it makes sense to make non-trivial changes to indentation, e.g. here:

https://github.com/dotnet/runtime/pull/86526/files#diff-b7da6b90947be2bdebc35f367d9dc2c5bb66dd3be881af8a2d6cbb9c18108a7fR1010-R1012

layomia · 2023-05-22T16:20:02Z

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

+                return writer;
+            }
+
+            private static SourceText CompleteSourceFileAndReturnText(SourceWriter writer)


Could this be a helper on the writer?

It could, although I wanted to make the writer language agnostic (and only concern itself with the responsibility of track indentation).

layomia · 2023-05-22T16:37:17Z

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

+                string metadataInitSource = $$"""
+                    var {{InfoVarName}} = new {{JsonCollectionInfoValuesTypeRef}}<{{typeFQN}}>()
+                    {
+                        {{ObjectCreatorPropName}} = {{FormatDefaultConstructorExpr(typeGenerationSpec)}},


Reckon it's worth it, wrt. saving LOC, to skip props that would be set to default values in the init logic?

e.g. many of these props

var info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type>() { IsProperty = true, IsPublic = false, IsVirtual = true, DeclaringType = typeof(global::System.Text.Json.SourceGeneration.Tests.HighLowTempsRecord), Converter = null, Getter = null, Setter = null, IgnoreCondition = null, HasJsonInclude = false, IsExtensionData = false, NumberHandling = default, PropertyName = "EqualityContract", JsonPropertyName = null };

(btw should we fix #77675 soon?)

I've been thinking about it, one concern is that conditionally emitting each of the assignments would result in harder-to-read emitter code. I've been thinking about approaches such as using Regex to trim Foo = null assignments but it's probably too hacky to consider. Happy to entertain other approaches that make this simple to achieve.

(btw should we fix #77675 soon?)

Absolutely. I've been trying to avoid any functional changes for the scope of these refactoring PRs.

Sergio0694

Very cool! 🙌
Will do another before-after comparison in the Store as well to check the binary size with the changes from this PR as well, it's possible that multiplied by all the types we have this could result in a visible difference (maybe not huge, but still nice) 😄

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

Use the SourceWriter pattern in the STJ SG emitter.

c31367a

dotnet-issue-labeler bot added the area-System.Text.Json label May 19, 2023

eiriktsarpalis requested a review from layomia May 19, 2023 20:13

ghost assigned eiriktsarpalis May 19, 2023

eiriktsarpalis requested review from krwq, stephentoub and tarekgh May 19, 2023 20:14

eiriktsarpalis added this to the 8.0.0 milestone May 19, 2023

eiriktsarpalis added 2 commits May 19, 2023 21:24

Make helper method static.

f3dc06b

Minimize the size of the generic helper method.

abcbe72

This was referenced May 19, 2023

Failed USB connection via port 54050, error 61, in tvOS arm64 Release AllSubsets_Mono #82637

Open

Build ios-arm64 Release AllSubsets_Mono failures dotnet/arcade#13625

Closed

layomia approved these changes May 22, 2023

View reviewed changes

Sergio0694 reviewed May 22, 2023

View reviewed changes

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs Show resolved Hide resolved

eiriktsarpalis merged commit c5cfd7b into dotnet:main May 22, 2023

eiriktsarpalis deleted the refactor/json-emitter branch May 22, 2023 17:30

eiriktsarpalis added the size-reduction Issues impacting final app size primary for size sensitive workloads label May 25, 2023

Sergio0694 mentioned this pull request May 26, 2023

Size saving opportunity for System.Text.Json generators with custom converters #84015

Closed

This was referenced May 30, 2023

Micro-optimizations for STJ generated source #52218

Closed

Json source generator should support JsonConverter on non-nullable and nullable types #66547

Closed

This was referenced Jun 7, 2023

Consider improving the source formatter in the STJ source generator #81260

Closed

Improve formatting of System.Text.Json source generated output #79534

Closed

ghost locked as resolved and limited conversation to collaborators Jun 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply the SourceWriter pattern to the STJ source generator. #86526

Apply the SourceWriter pattern to the STJ source generator. #86526

eiriktsarpalis commented May 19, 2023 •

edited

Loading

ghost commented May 19, 2023

Main context file

JsonTypeInfo declaration file

eiriktsarpalis commented May 19, 2023

layomia left a comment

layomia May 22, 2023

eiriktsarpalis May 22, 2023

layomia May 22, 2023

eiriktsarpalis May 22, 2023

layomia May 22, 2023

layomia May 22, 2023

eiriktsarpalis May 22, 2023

layomia May 22, 2023

eiriktsarpalis May 22, 2023 •

edited

Loading

layomia May 22, 2023

eiriktsarpalis May 22, 2023

Sergio0694 left a comment

Apply the SourceWriter pattern to the STJ source generator. #86526

Apply the SourceWriter pattern to the STJ source generator. #86526

Conversation

eiriktsarpalis commented May 19, 2023 • edited Loading

Main context file

JsonTypeInfo declaration file

JsonTypeInfo with design-time custom converter

ghost commented May 19, 2023

Main context file

JsonTypeInfo declaration file

eiriktsarpalis commented May 19, 2023

layomia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eiriktsarpalis May 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sergio0694 left a comment

Choose a reason for hiding this comment

eiriktsarpalis commented May 19, 2023 •

edited

Loading

eiriktsarpalis May 22, 2023 •

edited

Loading