Skip to content

Commit

Permalink
Fix serialization for records which have no properties of their own (#…
Browse files Browse the repository at this point in the history
…8289)

* Fix serialization for records which have no properties of their own
  • Loading branch information
ReubenBond authored Jan 30, 2023
1 parent 3dc77d9 commit 62b0b27
Show file tree
Hide file tree
Showing 8 changed files with 388 additions and 173 deletions.
28 changes: 15 additions & 13 deletions src/Orleans.CodeGenerator/CodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@ private MetadataModel GenerateMetadataModel(CancellationToken cancellationToken)
else
{
// Regular type
var supportsPrimaryConstructorParameters = ShouldSupportPrimaryConstructorParameters(symbol);
var includePrimaryConstructorParameters = IncludePrimaryConstructorParameters(symbol);
var constructorParameters = ImmutableArray<IParameterSymbol>.Empty;
if (supportsPrimaryConstructorParameters)
if (includePrimaryConstructorParameters)
{
if (symbol.IsRecord)
{
Expand Down Expand Up @@ -251,7 +251,7 @@ private MetadataModel GenerateMetadataModel(CancellationToken cancellationToken)
throw new OrleansGeneratorDiagnosticAnalysisException(CanNotGenerateImplicitFieldIdsDiagnostic.CreateDiagnostic(symbol, fieldIdAssignmentHelper.FailureReason));
}

var typeDescription = new SerializableTypeDescription(semanticModel, symbol, supportsPrimaryConstructorParameters && constructorParameters.Length > 0, GetDataMembers(fieldIdAssignmentHelper), LibraryTypes);
var typeDescription = new SerializableTypeDescription(semanticModel, symbol, includePrimaryConstructorParameters, GetDataMembers(fieldIdAssignmentHelper), LibraryTypes);
metadataModel.SerializableTypes.Add(typeDescription);
}
}
Expand Down Expand Up @@ -358,9 +358,9 @@ bool ShouldGenerateSerializer(INamedTypeSymbol t)
return false;
}

bool ShouldSupportPrimaryConstructorParameters(INamedTypeSymbol t)
bool IncludePrimaryConstructorParameters(INamedTypeSymbol t)
{
static bool TestGenerateSerializerAttribute(INamedTypeSymbol t, INamedTypeSymbol at)
static bool? TestGenerateSerializerAttribute(INamedTypeSymbol t, INamedTypeSymbol at)
{
var attribute = t.GetAttribute(at);
if (attribute != null)
Expand All @@ -369,31 +369,33 @@ static bool TestGenerateSerializerAttribute(INamedTypeSymbol t, INamedTypeSymbol
{
if (namedArgument.Key == "IncludePrimaryConstructorParameters")
{
if (namedArgument.Value.Kind == TypedConstantKind.Primitive && namedArgument.Value.Value is bool b && b == false)
if (namedArgument.Value.Kind == TypedConstantKind.Primitive && namedArgument.Value.Value is bool b)
{
return false;
return b;
}
}
}
}

return true;
// If there is no such named argument, return null so that other attributes have a chance to apply and defaults can be applied.
return null;
}

if (!TestGenerateSerializerAttribute(t, LibraryTypes.GenerateSerializerAttribute))
if (TestGenerateSerializerAttribute(t, LibraryTypes.GenerateSerializerAttribute) is bool result)
{
return false;
return result;
}

foreach (var attr in _generateSerializerAttributes)
{
if (!TestGenerateSerializerAttribute(t, attr))
if (TestGenerateSerializerAttribute(t, attr) is bool res)
{
return false;
return res;
}
}

return true;
// Default to true for records, false otherwise.
return t.IsRecord;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public GeneratedInvokerDescription(
public TypeSyntax TypeSyntax => _typeSyntax ??= CreateTypeSyntax();
public TypeSyntax OpenTypeSyntax => _openTypeSyntax ??= CreateOpenTypeSyntax();
public bool HasComplexBaseType => BaseType is { SpecialType: not SpecialType.System_Object };
public bool SupportsPrimaryConstructorParameters => false;
public bool IncludePrimaryConstructorParameters => false;
public INamedTypeSymbol BaseType { get; }
public TypeSyntax BaseTypeSyntax => _baseTypeSyntax ??= BaseType.ToTypeSyntax(_methodDescription.TypeParameterSubstitutions);
public string Namespace => GeneratedNamespace;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ internal interface ISerializableTypeDescription
Accessibility Accessibility { get; }
TypeSyntax TypeSyntax { get; }
bool HasComplexBaseType { get; }
bool SupportsPrimaryConstructorParameters { get; }
bool IncludePrimaryConstructorParameters { get; }
INamedTypeSymbol BaseType { get; }
TypeSyntax BaseTypeSyntax { get; }
string Namespace { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal class SerializableTypeDescription : ISerializableTypeDescription
public SerializableTypeDescription(SemanticModel semanticModel, INamedTypeSymbol type, bool supportsPrimaryConstructorParameters, IEnumerable<IMemberDescription> members, LibraryTypes libraryTypes)
{
Type = type;
SupportsPrimaryConstructorParameters = supportsPrimaryConstructorParameters;
IncludePrimaryConstructorParameters = supportsPrimaryConstructorParameters;
Members = members.ToList();
SemanticModel = semanticModel;
_libraryTypes = libraryTypes;
Expand Down Expand Up @@ -114,7 +114,7 @@ static string GetTypeParameterName(HashSet<string> names, ITypeParameterSymbol t

public bool HasComplexBaseType => !IsValueType && BaseType is { SpecialType: not SpecialType.System_Object };

public bool SupportsPrimaryConstructorParameters { get; }
public bool IncludePrimaryConstructorParameters { get; }

public INamedTypeSymbol BaseType => _baseType ??= GetEffectiveBaseType();

Expand Down
32 changes: 20 additions & 12 deletions src/Orleans.CodeGenerator/SerializerGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ private static MemberDeclarationSyntax GenerateSerializeMethod(
.WithInitializer(EqualsValueClause(LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))))))));
}

if (type.SupportsPrimaryConstructorParameters)
if (type.IncludePrimaryConstructorParameters)
{
AddSerializationMembers(type, serializerFields, members.Where(m => m.IsPrimaryConstructorParameter), libraryTypes, writerParam, instanceParam, previousFieldIdVar, body);
body.Add(ExpressionStatement(InvocationExpression(writerParam.Member("WriteEndBase"), ArgumentList())));
Expand Down Expand Up @@ -480,8 +480,8 @@ private static MemberDeclarationSyntax GenerateDeserializeMethod(
AddSerializationCallbacks(type, instanceParam, "OnDeserializing", body);

int emptyBodyCount;
var normalMembers = members.FindAll(m => !m.IsPrimaryConstructorParameter);
if (members.Count == 0 || normalMembers.Count == 0 && !type.SupportsPrimaryConstructorParameters)
var nonCtorMembers = type.IncludePrimaryConstructorParameters ? members.FindAll(static m => !m.IsPrimaryConstructorParameter) : members;
if ((members.Count == 0 || nonCtorMembers.Count == 0) && !type.IncludePrimaryConstructorParameters)
{
// C#: reader.ConsumeEndBaseOrEndObject();
body.Add(ExpressionStatement(InvocationExpression(readerParam.Member("ConsumeEndBaseOrEndObject"))));
Expand All @@ -490,10 +490,13 @@ private static MemberDeclarationSyntax GenerateDeserializeMethod(
else
{
// C#: uint id = 0;
body.Add(LocalDeclarationStatement(
VariableDeclaration(
PredefinedType(Token(SyntaxKind.UIntKeyword)),
SingletonSeparatedList(VariableDeclarator(idVar.Identifier, null, EqualsValueClause(LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))))))));
if (members.Count > 0)
{
body.Add(LocalDeclarationStatement(
VariableDeclaration(
PredefinedType(Token(SyntaxKind.UIntKeyword)),
SingletonSeparatedList(VariableDeclarator(idVar.Identifier, null, EqualsValueClause(LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))))))));
}

// C#: Field header = default;
body.Add(LocalDeclarationStatement(
Expand All @@ -503,15 +506,20 @@ private static MemberDeclarationSyntax GenerateDeserializeMethod(

emptyBodyCount = 2;

if (type.SupportsPrimaryConstructorParameters)
if (type.IncludePrimaryConstructorParameters)
{
body.Add(GetDeserializerLoop(members.FindAll(m => m.IsPrimaryConstructorParameter)));
body.Add(ExpressionStatement(AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, idVar, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0)))));
body.Add(IfStatement(headerVar.Member("IsEndBaseFields"), GetDeserializerLoop(normalMembers)));
var constructorParameterMembers = members.FindAll(m => m.IsPrimaryConstructorParameter);
body.Add(GetDeserializerLoop(constructorParameterMembers));
if (members.Count > 0)
{
body.Add(ExpressionStatement(AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, idVar, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0)))));
}

body.Add(IfStatement(headerVar.Member("IsEndBaseFields"), GetDeserializerLoop(nonCtorMembers)));
}
else
{
body.Add(GetDeserializerLoop(normalMembers));
body.Add(GetDeserializerLoop(nonCtorMembers));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Orleans.Serialization.Abstractions/Annotations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ public sealed class GenerateSerializerAttribute : Attribute
{
/// <summary>
/// Get or sets if primary constructor parameters should automatically be included as Serializable fields.
/// Defaults to <see langword="true"/> for record types.
/// Defaults to <see langword="true"/> for <see langword="record"/> types, <see langword="false"/> otherwise.
/// </summary>
public bool IncludePrimaryConstructorParameters { get; init; } = true;
public bool IncludePrimaryConstructorParameters { get; init; }

/// <summary>
/// Get or sets when Orleans should auto-assign field ids. The default behavior is to not auto-assign field ids.
Expand Down
142 changes: 0 additions & 142 deletions test/Orleans.Serialization.UnitTests/RecordHierarchyTests.cs

This file was deleted.

Loading

0 comments on commit 62b0b27

Please sign in to comment.