Skip to content

Commit

Permalink
Fix custom JsonConverterFactories not working with transitive type/pr…
Browse files Browse the repository at this point in the history 
…operty declarations. (dotnet#62643)

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
  • Loading branch information
@github-actions @eiriktsarpalis
github-actions[bot] and eiriktsarpalis committed Jan 10, 2022
1 parent 04a5836 commit 7780d28
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 50 deletions.
24 changes: 12 additions & 12 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ private sealed partial class Emitter

private readonly HashSet<string> _emittedPropertyFileNames = new();

private bool _generateGetConverterMethodForTypes = false;
private bool _generateGetConverterMethodForProperties = false;

public Emitter(in JsonSourceGenerationContext sourceGenerationContext, SourceGenerationSpec generationSpec)
{
_sourceGenerationContext = sourceGenerationContext;
Expand All @@ -109,24 +112,20 @@ public void Emit()
foreach (ContextGenerationSpec contextGenerationSpec in _generationSpec.ContextGenerationSpecList)
{
_currentContext = contextGenerationSpec;

bool generateGetConverterMethodForTypes = false;
bool generateGetConverterMethodForProperties = false;
_generateGetConverterMethodForTypes = false;
_generateGetConverterMethodForProperties = false;

foreach (TypeGenerationSpec typeGenerationSpec in _currentContext.RootSerializableTypes)
{
GenerateTypeInfo(typeGenerationSpec);

generateGetConverterMethodForTypes |= typeGenerationSpec.HasTypeFactoryConverter;
generateGetConverterMethodForProperties |= typeGenerationSpec.HasPropertyFactoryConverters;
}

string contextName = _currentContext.ContextType.Name;

// Add root context implementation.
AddSource(
$"{contextName}.g.cs",
GetRootJsonContextImplementation(generateGetConverterMethodForTypes, generateGetConverterMethodForProperties),
GetRootJsonContextImplementation(),
isRootContextDef: true);

// Add GetJsonTypeInfo override implementation.
Expand Down Expand Up @@ -302,6 +301,9 @@ private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
Location location = typeGenerationSpec.AttributeLocation ?? _currentContext.Location;
_sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(DuplicateTypeName, location, new string[] { typeGenerationSpec.TypeInfoPropertyName }));
}

_generateGetConverterMethodForTypes |= typeGenerationSpec.HasTypeFactoryConverter;
_generateGetConverterMethodForProperties |= typeGenerationSpec.HasPropertyFactoryConverters;
}

private string GenerateForTypeWithKnownConverter(TypeGenerationSpec typeMetadata)
Expand Down Expand Up @@ -1145,9 +1147,7 @@ private string WrapWithCheckForCustomConverter(string source, string typeCompila
{IndentSource(source, numIndentations: 1)}
}}";

private string GetRootJsonContextImplementation(
bool generateGetConverterMethodForTypes,
bool generateGetConverterMethodForProperties)
private string GetRootJsonContextImplementation()
{
string contextTypeRef = _currentContext.ContextTypeRef;
string contextTypeName = _currentContext.ContextType.Name;
Expand All @@ -1171,12 +1171,12 @@ private string GetRootJsonContextImplementation(
{GetFetchLogicForRuntimeSpecifiedCustomConverter()}");

if (generateGetConverterMethodForProperties)
if (_generateGetConverterMethodForProperties)
{
sb.Append(GetFetchLogicForGetCustomConverter_PropertiesWithFactories());
}

if (generateGetConverterMethodForProperties || generateGetConverterMethodForTypes)
if (_generateGetConverterMethodForProperties || _generateGetConverterMethodForTypes)
{
sb.Append(GetFetchLogicForGetCustomConverter_TypesWithFactories());
}
Expand Down
4 changes: 2 additions & 2 deletions ...ibraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/ContextClasses.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public interface ITestContext
public JsonTypeInfo<StructWithCustomConverterFactory> StructWithCustomConverterFactory { get; }
public JsonTypeInfo<ClassWithCustomConverterProperty> ClassWithCustomConverterProperty { get; }
public JsonTypeInfo<StructWithCustomConverterProperty> StructWithCustomConverterProperty { get; }
public JsonTypeInfo<ClassWithCustomConverterPropertyFactory> ClassWithCustomConverterPropertyFactory { get; }
public JsonTypeInfo<StructWithCustomConverterPropertyFactory> StructWithCustomConverterPropertyFactory { get; }
public JsonTypeInfo<ClassWithCustomConverterFactoryProperty> ClassWithCustomConverterFactoryProperty { get; }
public JsonTypeInfo<StructWithCustomConverterFactoryProperty> StructWithCustomConverterFactoryProperty { get; }
public JsonTypeInfo<ClassWithBadCustomConverter> ClassWithBadCustomConverter { get; }
public JsonTypeInfo<StructWithBadCustomConverter> StructWithBadCustomConverter { get; }
}
Expand Down
40 changes: 40 additions & 0 deletions ...tem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +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.Reflection;
using System.Text.Json.Serialization;
using Microsoft.DotNet.RemoteExecutor;
Expand Down Expand Up @@ -89,5 +90,44 @@ internal record Person(string FirstName, string LastName);
internal partial class PersonJsonContext : JsonSerializerContext
{
}

// Regression test for https://github.com/dotnet/runtime/issues/62079
[Fact]
public static void SupportsPropertiesWithCustomConverterFactory()
{
var value = new ClassWithCustomConverterFactoryProperty { MyEnum = Serialization.Tests.SampleEnum.MinZero };
string json = JsonSerializer.Serialize(value, SingleClassWithCustomConverterFactoryPropertyContext.Default.ClassWithCustomConverterFactoryProperty);
Assert.Equal(@"{""MyEnum"":""MinZero""}", json);
}

public class ParentClass
{
public ClassWithCustomConverterFactoryProperty? Child { get; set; }
}

[JsonSerializable(typeof(ParentClass))]
internal partial class SingleClassWithCustomConverterFactoryPropertyContext : JsonSerializerContext
{
}

// Regression test for https://github.com/dotnet/runtime/issues/61860
[Fact]
public static void SupportsGenericParameterWithCustomConverterFactory()
{
var value = new List<TestEnum> { TestEnum.Cee };
string json = JsonSerializer.Serialize(value, GenericParameterWithCustomConverterFactoryContext.Default.ListTestEnum);
Assert.Equal(@"[""Cee""]", json);
}

[JsonConverter(typeof(JsonStringEnumConverter))]
public enum TestEnum
{
Aye, Bee, Cee
}

[JsonSerializable(typeof(List<TestEnum>))]
internal partial class GenericParameterWithCustomConverterFactoryContext : JsonSerializerContext
{
}
}
}
8 changes: 4 additions & 4 deletions ...son/tests/System.Text.Json.SourceGeneration.Tests/MetadataAndSerializationContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterFactory))]
[JsonSerializable(typeof(ClassWithCustomConverterProperty))]
[JsonSerializable(typeof(StructWithCustomConverterProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterPropertyFactory))]
[JsonSerializable(typeof(StructWithCustomConverterPropertyFactory))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
internal partial class MetadataAndSerializationContext : JsonSerializerContext, ITestContext
Expand Down Expand Up @@ -81,8 +81,8 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverterFactory);
Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverterProperty);
Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverterProperty);
Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverterPropertyFactory);
Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverterPropertyFactory);
Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverterFactoryProperty);
Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverterFactoryProperty);
Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.ClassWithBadCustomConverter);
Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.StructWithBadCustomConverter);
}
Expand Down
16 changes: 8 additions & 8 deletions ...es/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MetadataContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterFactory), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterPropertyFactory), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverterPropertyFactory), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
internal partial class MetadataWithPerTypeAttributeContext : JsonSerializerContext, ITestContext
Expand Down Expand Up @@ -79,8 +79,8 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MetadataWithPerTypeAttributeContext.Default.StructWithCustomConverterFactory.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ClassWithCustomConverterProperty.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.StructWithCustomConverterProperty.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ClassWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.StructWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.ClassWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Null(MetadataWithPerTypeAttributeContext.Default.StructWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MetadataWithPerTypeAttributeContext.Default.ClassWithBadCustomConverter.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MetadataWithPerTypeAttributeContext.Default.StructWithBadCustomConverter.SerializeHandler);
}
Expand Down Expand Up @@ -116,8 +116,8 @@ public override void EnsureFastPathGeneratedAsExpected()
[JsonSerializable(typeof(StructWithCustomConverterFactory))]
[JsonSerializable(typeof(ClassWithCustomConverterProperty))]
[JsonSerializable(typeof(StructWithCustomConverterProperty))]
[JsonSerializable(typeof(ClassWithCustomConverterPropertyFactory))]
[JsonSerializable(typeof(StructWithCustomConverterPropertyFactory))]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty))]
[JsonSerializable(typeof(ClassWithBadCustomConverter))]
[JsonSerializable(typeof(StructWithBadCustomConverter))]
internal partial class MetadataContext : JsonSerializerContext, ITestContext
Expand Down Expand Up @@ -183,8 +183,8 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MetadataContext.Default.StructWithCustomConverterFactory.SerializeHandler);
Assert.Null(MetadataContext.Default.ClassWithCustomConverterProperty.SerializeHandler);
Assert.Null(MetadataContext.Default.StructWithCustomConverterProperty.SerializeHandler);
Assert.Null(MetadataContext.Default.ClassWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MetadataContext.Default.StructWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MetadataContext.Default.ClassWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Null(MetadataContext.Default.StructWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.ClassWithBadCustomConverter.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.StructWithBadCustomConverter.SerializeHandler);
}
Expand Down
8 changes: 4 additions & 4 deletions ...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MixedModeContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace System.Text.Json.SourceGeneration.Tests
[JsonSerializable(typeof(StructWithCustomConverterFactory), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterPropertyFactory), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverterPropertyFactory), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithCustomConverterFactoryProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
internal partial class MixedModeContext : JsonSerializerContext, ITestContext
Expand Down Expand Up @@ -81,8 +81,8 @@ public override void EnsureFastPathGeneratedAsExpected()
Assert.Null(MixedModeContext.Default.StructWithCustomConverterFactory.SerializeHandler);
Assert.Null(MixedModeContext.Default.ClassWithCustomConverterProperty.SerializeHandler);
Assert.Null(MixedModeContext.Default.StructWithCustomConverterProperty.SerializeHandler);
Assert.Null(MixedModeContext.Default.ClassWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MixedModeContext.Default.StructWithCustomConverterPropertyFactory.SerializeHandler);
Assert.Null(MixedModeContext.Default.ClassWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Null(MixedModeContext.Default.StructWithCustomConverterFactoryProperty.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.ClassWithBadCustomConverter.SerializeHandler);
Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.StructWithBadCustomConverter.SerializeHandler);
}
Expand Down
20 changes: 10 additions & 10 deletions ...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/RealWorldContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,28 +246,28 @@ public virtual void RoundTripWithCustomPropertyConverterFactory_Class()
{
const string Json = "{\"MyEnum\":\"One\"}";

ClassWithCustomConverterPropertyFactory obj = new()
ClassWithCustomConverterFactoryProperty obj = new()
{
MyEnum = SampleEnum.One
};

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterPropertyFactory));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterFactoryProperty));
}
else
{
string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterPropertyFactory);
string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterFactoryProperty);
Assert.Equal(Json, json);
}

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterPropertyFactory));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterFactoryProperty));
}
else
{
obj = JsonSerializer.Deserialize(Json, DefaultContext.ClassWithCustomConverterPropertyFactory);
obj = JsonSerializer.Deserialize(Json, DefaultContext.ClassWithCustomConverterFactoryProperty);
Assert.Equal(SampleEnum.One, obj.MyEnum);
}
}
Expand All @@ -277,28 +277,28 @@ public virtual void RoundTripWithCustomPropertyConverterFactory_Struct()
{
const string Json = "{\"MyEnum\":\"One\"}";

StructWithCustomConverterPropertyFactory obj = new()
StructWithCustomConverterFactoryProperty obj = new()
{
MyEnum = SampleEnum.One
};

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterPropertyFactory));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterFactoryProperty));
}
else
{
string json = JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterPropertyFactory);
string json = JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterFactoryProperty);
Assert.Equal(Json, json);
}

if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
{
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterPropertyFactory));
Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterFactoryProperty));
}
else
{
obj = JsonSerializer.Deserialize(Json, DefaultContext.StructWithCustomConverterPropertyFactory);
obj = JsonSerializer.Deserialize(Json, DefaultContext.StructWithCustomConverterFactoryProperty);
Assert.Equal(SampleEnum.One, obj.MyEnum);
}
}
Expand Down
Loading

0 comments on commit 7780d28

Please sign in to comment.