From 6357654fba46108c6869e0d8057bf771bd0fa222 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 6 Aug 2024 15:50:15 -0300 Subject: [PATCH 1/5] Fix support for externally declared records FIxes improper codec codegen for records declared in referenced projects/assemblies. Roslyn does not guarantee the symbols contain the backing fields for generated properties (see https://github.com/dotnet/roslyn/discussions/72374#discussioncomment-8655916) and it also doesn't even report `record struct` symbols as records at all (see https://github.com/dotnet/roslyn/issues/69326). This makes for a very inconsistent experience when dealing with types defined in external assemblies that don't use the Orleans SDK themselves. We implement a heuristics here to determine primary constructors that can be relied upon to detect them consistently: 1. A ctor with non-zero parameters 2. All parameters match by name exactly with corresponding properties 3. All matching properties have getter AND setter annotated with [CompilerGenerated]. In addition, since the backing field isn't available at all in these records, and the corresponding property isn't settable (it's generated as `init set`), we leverage unsafe accessors (see https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.unsafeaccessorattribute?view=net-8.0) instead. The code checks whether the `FieldAccessorDescription` has an initializer syntax or not to determine whether to generate the original code or the new accessor version. The signature of the accessor matches the delegate that is generated for the regular backing field case, so there is no need to modify other call sites. Fixes #9092 --- Orleans.sln | 7 +++ src/Orleans.CodeGenerator/CodeGenerator.cs | 35 +++++++++-- src/Orleans.CodeGenerator/CopierGenerator.cs | 30 +++++++++- .../FieldIdAssignmentHelper.cs | 12 ++++ src/Orleans.CodeGenerator/PropertyUtility.cs | 16 +++++ .../SerializerGenerator.cs | 59 +++++++++++++++---- .../TestSerializerExternalModels/Models.cs | 25 ++++++++ .../TestSerializerExternalModels.csproj | 10 ++++ .../GeneratedSerializerTests.cs | 47 ++++++++++++++- .../Orleans.Serialization.UnitTests.csproj | 1 + 10 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 test/Misc/TestSerializerExternalModels/Models.cs create mode 100644 test/Misc/TestSerializerExternalModels/TestSerializerExternalModels.csproj diff --git a/Orleans.sln b/Orleans.sln index ee159384b5..0df56b5714 100644 --- a/Orleans.sln +++ b/Orleans.sln @@ -242,6 +242,8 @@ Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "Orleans.Serialization.FShar EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Orleans.Serialization.MessagePack", "src\Orleans.Serialization.MessagePack\Orleans.Serialization.MessagePack.csproj", "{F50F81B6-E9B5-4143-B66B-A1AD913F6E9C}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestSerializerExternalModels", "test\Misc\TestSerializerExternalModels\TestSerializerExternalModels.csproj", "{5D587DDE-036D-4694-A314-8DDF270AC031}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -632,6 +634,10 @@ Global {F50F81B6-E9B5-4143-B66B-A1AD913F6E9C}.Debug|Any CPU.Build.0 = Debug|Any CPU {F50F81B6-E9B5-4143-B66B-A1AD913F6E9C}.Release|Any CPU.ActiveCfg = Release|Any CPU {F50F81B6-E9B5-4143-B66B-A1AD913F6E9C}.Release|Any CPU.Build.0 = Release|Any CPU + {5D587DDE-036D-4694-A314-8DDF270AC031}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {5D587DDE-036D-4694-A314-8DDF270AC031}.Debug|Any CPU.Build.0 = Debug|Any CPU + {5D587DDE-036D-4694-A314-8DDF270AC031}.Release|Any CPU.ActiveCfg = Release|Any CPU + {5D587DDE-036D-4694-A314-8DDF270AC031}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -748,6 +754,7 @@ Global {84B44F1D-B7FE-40E3-82F0-730A55AC8613} = {316CDCC7-323F-4264-9FC9-667662BB1F80} {B2D53D3C-E44A-4C9B-AAEE-28FB8C1BDF62} = {A6573187-FD0D-4DF7-91D1-03E07E470C0A} {F50F81B6-E9B5-4143-B66B-A1AD913F6E9C} = {4CD3AA9E-D937-48CA-BB6C-158E12257D23} + {5D587DDE-036D-4694-A314-8DDF270AC031} = {70BCC54E-1618-4742-A079-07588065E361} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {7BFB3429-B5BB-4DB1-95B4-67D77A864952} diff --git a/src/Orleans.CodeGenerator/CodeGenerator.cs b/src/Orleans.CodeGenerator/CodeGenerator.cs index a137f5e0f7..8ba5fdcff2 100644 --- a/src/Orleans.CodeGenerator/CodeGenerator.cs +++ b/src/Orleans.CodeGenerator/CodeGenerator.cs @@ -145,10 +145,10 @@ public CompilationUnitSyntax GenerateCode(CancellationToken cancellationToken) if (symbol.IsRecord) { // If there is a primary constructor then that will be declared before the copy constructor - // A record always generates a copy constructor and marks it as implicitly declared + // A record always generates a copy constructor and marks it as compiler generated // todo: find an alternative to this magic var potentialPrimaryConstructor = symbol.Constructors[0]; - if (!potentialPrimaryConstructor.IsImplicitlyDeclared) + if (!potentialPrimaryConstructor.IsImplicitlyDeclared && !potentialPrimaryConstructor.IsCompilerGenerated()) { constructorParameters = potentialPrimaryConstructor.Parameters; } @@ -160,6 +160,23 @@ public CompilationUnitSyntax GenerateCode(CancellationToken cancellationToken) { constructorParameters = annotatedConstructors[0].Parameters; } + else + { + // record structs from referenced assemblies do not return IsRecord=true + // above. See https://github.com/dotnet/roslyn/issues/69326 + // So we implement the same heuristics from ShouldIncludePrimaryConstructorParameters + // to detect a primary constructor. + var properties = symbol.GetMembers().OfType().ToImmutableArray(); + var primaryConstructor = symbol.GetMembers() + .OfType() + .Where(m => m.MethodKind == MethodKind.Constructor && m.Parameters.Length > 0) + // Check for a ctor where all parameters have a corresponding compiler-generated prop. + .FirstOrDefault(ctor => ctor.Parameters.All(prm => + properties.Any(prop => prop.Name.Equals(prm.Name, StringComparison.Ordinal) && prop.IsCompilerGenerated()))); + + if (primaryConstructor != null) + constructorParameters = primaryConstructor.Parameters; + } } } @@ -273,8 +290,18 @@ bool ShouldIncludePrimaryConstructorParameters(INamedTypeSymbol t) } } - // Default to true for records, false otherwise. - return t.IsRecord; + // Default to true for records. + if (t.IsRecord) + return true; + + var properties = t.GetMembers().OfType().ToImmutableArray(); + + return t.GetMembers() + .OfType() + .Where(m => m.MethodKind == MethodKind.Constructor && m.Parameters.Length > 0) + // Check for a ctor where all parameters have a corresponding compiler-generated prop. + .Any(ctor => ctor.Parameters.All(prm => + properties.Any(prop => prop.Name.Equals(prm.Name, StringComparison.Ordinal) && prop.IsCompilerGenerated()))); } } } diff --git a/src/Orleans.CodeGenerator/CopierGenerator.cs b/src/Orleans.CodeGenerator/CopierGenerator.cs index 89867672b3..0ab6436591 100644 --- a/src/Orleans.CodeGenerator/CopierGenerator.cs +++ b/src/Orleans.CodeGenerator/CopierGenerator.cs @@ -117,11 +117,39 @@ static MemberDeclarationSyntax GetFieldDeclaration(GeneratedFieldDescription des { switch (description) { - case FieldAccessorDescription accessor: + case FieldAccessorDescription accessor when accessor.InitializationSyntax != null: return FieldDeclaration(VariableDeclaration(accessor.FieldType, SingletonSeparatedList(VariableDeclarator(accessor.FieldName).WithInitializer(EqualsValueClause(accessor.InitializationSyntax))))) .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.StaticKeyword), Token(SyntaxKind.ReadOnlyKeyword)); + case FieldAccessorDescription accessor when accessor.InitializationSyntax == null: + //[UnsafeAccessor(UnsafeAccessorKind.Method, Name = "set_Amount")] + //extern static void SetAmount(External instance, int value); + return + MethodDeclaration( + PredefinedType(Token(SyntaxKind.VoidKeyword)), + accessor.AccessorName) + .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.ExternKeyword), Token(SyntaxKind.StaticKeyword)) + .AddAttributeLists(AttributeList(SingletonSeparatedList( + Attribute(IdentifierName("System.Runtime.CompilerServices.UnsafeAccessor")) + .AddArgumentListArguments( + AttributeArgument( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName("System.Runtime.CompilerServices.UnsafeAccessorKind"), + IdentifierName("Method"))), + AttributeArgument( + LiteralExpression( + SyntaxKind.StringLiteralExpression, + Literal($"set_{accessor.FieldName}"))) + .WithNameEquals(NameEquals("Name")))))) + .WithParameterList( + ParameterList(SeparatedList(new[] + { + Parameter(Identifier("instance")).WithType(accessor.ContainingType), + Parameter(Identifier("value")).WithType(description.FieldType) + }))) + .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); default: return FieldDeclaration(VariableDeclaration(description.FieldType, SingletonSeparatedList(VariableDeclarator(description.FieldName)))) .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.ReadOnlyKeyword)); diff --git a/src/Orleans.CodeGenerator/FieldIdAssignmentHelper.cs b/src/Orleans.CodeGenerator/FieldIdAssignmentHelper.cs index 71c30724b3..9bd95ba4e0 100644 --- a/src/Orleans.CodeGenerator/FieldIdAssignmentHelper.cs +++ b/src/Orleans.CodeGenerator/FieldIdAssignmentHelper.cs @@ -74,6 +74,18 @@ private bool ExtractFieldIdAnnotations() { _symbols[member] = (id.Value, false); } + else if (PropertyUtility.GetMatchingPrimaryConstructorParameter(prop, _constructorParameters) is { } prm) + { + id = CodeGenerator.GetId(_libraryTypes, prop); + if (id.HasValue) + { + _symbols[member] = (id.Value, true); + } + else + { + _symbols[member] = ((uint)_constructorParameters.IndexOf(prm), true); + } + } } if (member is IFieldSymbol field) diff --git a/src/Orleans.CodeGenerator/PropertyUtility.cs b/src/Orleans.CodeGenerator/PropertyUtility.cs index 809a1ff15f..250a3f6568 100644 --- a/src/Orleans.CodeGenerator/PropertyUtility.cs +++ b/src/Orleans.CodeGenerator/PropertyUtility.cs @@ -18,6 +18,22 @@ public static class PropertyUtility return GetMatchingProperty(field, field.ContainingType.GetMembers()); } + public static bool IsCompilerGenerated(this ISymbol? symbol) + => symbol?.GetAttributes().Any(a => a.AttributeClass?.Name == "CompilerGeneratedAttribute") == true; + + public static bool IsCompilerGenerated(this IPropertySymbol? property) + => property?.GetMethod.IsCompilerGenerated() == true && property.SetMethod.IsCompilerGenerated(); + + public static IParameterSymbol? GetMatchingPrimaryConstructorParameter(IPropertySymbol property, IEnumerable constructorParameters) + { + if (!property.IsCompilerGenerated()) + return null; + + return constructorParameters.FirstOrDefault(p => + string.Equals(p.Name, property.Name, StringComparison.Ordinal) && + SymbolEqualityComparer.Default.Equals(p.Type, property.Type)); + } + public static IPropertySymbol? GetMatchingProperty(IFieldSymbol field, IEnumerable memberSymbols) { var propertyName = PropertyMatchRegex.Match(field.Name); diff --git a/src/Orleans.CodeGenerator/SerializerGenerator.cs b/src/Orleans.CodeGenerator/SerializerGenerator.cs index 3f131687b4..16b21b6d4e 100644 --- a/src/Orleans.CodeGenerator/SerializerGenerator.cs +++ b/src/Orleans.CodeGenerator/SerializerGenerator.cs @@ -137,11 +137,39 @@ static MemberDeclarationSyntax GetFieldDeclaration(GeneratedFieldDescription des SingletonSeparatedList(VariableDeclarator(type.FieldName) .WithInitializer(EqualsValueClause(TypeOfExpression(type.CodecFieldType)))))) .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.ReadOnlyKeyword)); - case FieldAccessorDescription accessor: + case FieldAccessorDescription accessor when accessor.InitializationSyntax != null: return FieldDeclaration(VariableDeclaration(accessor.FieldType, SingletonSeparatedList(VariableDeclarator(accessor.FieldName).WithInitializer(EqualsValueClause(accessor.InitializationSyntax))))) .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.StaticKeyword), Token(SyntaxKind.ReadOnlyKeyword)); + case FieldAccessorDescription accessor when accessor.InitializationSyntax == null: + //[UnsafeAccessor(UnsafeAccessorKind.Method, Name = "set_Amount")] + //extern static void SetAmount(External instance, int value); + return + MethodDeclaration( + PredefinedType(Token(SyntaxKind.VoidKeyword)), + accessor.AccessorName) + .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.ExternKeyword), Token(SyntaxKind.StaticKeyword)) + .AddAttributeLists(AttributeList(SingletonSeparatedList( + Attribute(IdentifierName("System.Runtime.CompilerServices.UnsafeAccessor")) + .AddArgumentListArguments( + AttributeArgument( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName("System.Runtime.CompilerServices.UnsafeAccessorKind"), + IdentifierName("Method"))), + AttributeArgument( + LiteralExpression( + SyntaxKind.StringLiteralExpression, + Literal($"set_{accessor.FieldName}"))) + .WithNameEquals(NameEquals("Name")))))) + .WithParameterList( + ParameterList(SeparatedList(new[] + { + Parameter(Identifier("instance")).WithType(accessor.ContainingType), + Parameter(Identifier("value")).WithType(description.FieldType) + }))) + .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); default: return FieldDeclaration(VariableDeclaration(description.FieldType, SingletonSeparatedList(VariableDeclarator(description.FieldName)))) .AddModifiers(Token(SyntaxKind.PrivateKeyword), Token(SyntaxKind.ReadOnlyKeyword)); @@ -1069,11 +1097,16 @@ public CodecFieldTypeFieldDescription(TypeSyntax fieldType, string fieldName, Ty internal sealed class FieldAccessorDescription : GeneratedFieldDescription { - public FieldAccessorDescription(TypeSyntax fieldType, string fieldName, ExpressionSyntax initializationSyntax) : base(fieldType, fieldName) - => InitializationSyntax = initializationSyntax; + public FieldAccessorDescription(TypeSyntax containingType, TypeSyntax fieldType, string fieldName, string accessorName, ExpressionSyntax initializationSyntax = null) : base(fieldType, fieldName) + { + ContainingType = containingType; + AccessorName = accessorName; + InitializationSyntax = initializationSyntax; + } public override bool IsInjected => false; - + public readonly string AccessorName; + public readonly TypeSyntax ContainingType; public readonly ExpressionSyntax InitializationSyntax; } @@ -1325,7 +1358,6 @@ public ExpressionSyntax GetSetter(ExpressionSyntax instance, ExpressionSyntax va return AssignmentExpression( SyntaxKind.SimpleAssignmentExpression, instance.Member(Field.Name), - value); } @@ -1357,20 +1389,26 @@ public ExpressionSyntax GetSetter(ExpressionSyntax instance, ExpressionSyntax va public FieldAccessorDescription GetGetterFieldDescription() { if (IsGettableField || IsGettableProperty) return null; - return GetFieldAccessor(ContainingType, TypeSyntax, MemberName, GetterFieldName, LibraryTypes, false); + return GetFieldAccessor(ContainingType, TypeSyntax, MemberName, GetterFieldName, LibraryTypes, false, + IsPrimaryConstructorParameter && IsProperty); } public FieldAccessorDescription GetSetterFieldDescription() { if (IsSettableField || IsSettableProperty) return null; - return GetFieldAccessor(ContainingType, TypeSyntax, MemberName, SetterFieldName, LibraryTypes, true); + return GetFieldAccessor(ContainingType, TypeSyntax, MemberName, SetterFieldName, LibraryTypes, true, + IsPrimaryConstructorParameter && IsProperty); } - public static FieldAccessorDescription GetFieldAccessor(INamedTypeSymbol containingType, TypeSyntax fieldType, string fieldName, string accessorName, LibraryTypes library, bool setter) + public static FieldAccessorDescription GetFieldAccessor(INamedTypeSymbol containingType, TypeSyntax fieldType, string fieldName, string accessorName, LibraryTypes library, bool setter, bool useUnsafeAccessor = false) { - var valueType = containingType.IsValueType; var containingTypeSyntax = containingType.ToTypeSyntax(); + if (useUnsafeAccessor) + return new(containingTypeSyntax, fieldType, fieldName, accessorName); + + var valueType = containingType.IsValueType; + var delegateType = (setter ? (valueType ? library.ValueTypeSetter_2 : library.Action_2) : (valueType ? library.ValueTypeGetter_2 : library.Func_2)) .ToTypeSyntax(containingTypeSyntax, fieldType); @@ -1381,7 +1419,8 @@ public static FieldAccessorDescription GetFieldAccessor(INamedTypeSymbol contain InvocationExpression(fieldAccessorUtility.Member(accessorMethod)) .AddArgumentListArguments(Argument(TypeOfExpression(containingTypeSyntax)), Argument(fieldName.GetLiteralExpression()))); - return new(delegateType, accessorName, accessorInvoke); + // Existing case, accessor is the field in both cases + return new(containingTypeSyntax, delegateType, accessorName, accessorName, accessorInvoke); } } } diff --git a/test/Misc/TestSerializerExternalModels/Models.cs b/test/Misc/TestSerializerExternalModels/Models.cs new file mode 100644 index 0000000000..74e0e5d595 --- /dev/null +++ b/test/Misc/TestSerializerExternalModels/Models.cs @@ -0,0 +1,25 @@ +using Orleans; + +namespace UnitTests.SerializerExternalModels; + +[GenerateSerializer] +public record struct Person2ExternalStruct(int Age, string Name) +{ + [Id(0)] + public string FavouriteColor { get; set; } + + [Id(1)] + public string StarSign { get; set; } +} + +#if NET6_0_OR_GREATER +[GenerateSerializer] +public record Person2External(int Age, string Name) +{ + [Id(0)] + public string FavouriteColor { get; set; } + + [Id(1)] + public string StarSign { get; set; } +} +#endif diff --git a/test/Misc/TestSerializerExternalModels/TestSerializerExternalModels.csproj b/test/Misc/TestSerializerExternalModels/TestSerializerExternalModels.csproj new file mode 100644 index 0000000000..d83003b5bd --- /dev/null +++ b/test/Misc/TestSerializerExternalModels/TestSerializerExternalModels.csproj @@ -0,0 +1,10 @@ + + + UnitTests.SerializerExternalModels + SerializerExternalModels + $(TestTargetFrameworks);netcoreapp3.1 + + + + + \ No newline at end of file diff --git a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs index 479b1e0a0a..8969d37f46 100644 --- a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs +++ b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs @@ -11,6 +11,13 @@ using System.IO.Pipelines; using Xunit; using System.Linq; +using UnitTests.SerializerExternalModels; +using Orleans; + +[assembly: GenerateCodeForDeclaringAssembly(typeof(Person2ExternalStruct))] +#if NET6_0_OR_GREATER +[assembly: GenerateCodeForDeclaringAssembly(typeof(Person2External))] +#endif namespace Orleans.Serialization.UnitTests; @@ -126,6 +133,44 @@ public void GeneratedRecordWithPCtorSerializersRoundTripThroughCodec() Assert.Equal(original.StarSign, result.StarSign); } + [Fact] + public void GeneratedLibExternalRecordStructWithPCtorSerializersRoundTripThroughCodec() + { + var original = new Person2ExternalStruct(2, "harry") + { + FavouriteColor = "redborine", + StarSign = "Aquaricorn" + }; + + var result = RoundTripThroughCodec(original); + + Assert.Equal(original.Age, result.Age); + Assert.Equal(original.Name, result.Name); + Assert.Equal(original.FavouriteColor, result.FavouriteColor); + Assert.Equal(original.StarSign, result.StarSign); + } + +#if NET6_0_OR_GREATER + + [Fact] + public void GeneratedLibExternalRecordWithPCtorSerializersRoundTripThroughCodec() + { + var original = new Person2External(2, "harry") + { + FavouriteColor = "redborine", + StarSign = "Aquaricorn" + }; + + var result = RoundTripThroughCodec(original); + + Assert.Equal(original.Age, result.Age); + Assert.Equal(original.Name, result.Name); + Assert.Equal(original.FavouriteColor, result.FavouriteColor); + Assert.Equal(original.StarSign, result.StarSign); + } + +#endif + #if NET6_0_OR_GREATER [Fact] public void RequiredMembersAreSupported() @@ -648,4 +693,4 @@ private object RoundTripThroughUntypedSerializer(object original, out string for return result; } -} \ No newline at end of file +} diff --git a/test/Orleans.Serialization.UnitTests/Orleans.Serialization.UnitTests.csproj b/test/Orleans.Serialization.UnitTests/Orleans.Serialization.UnitTests.csproj index ba53e60c26..d92b600acf 100644 --- a/test/Orleans.Serialization.UnitTests/Orleans.Serialization.UnitTests.csproj +++ b/test/Orleans.Serialization.UnitTests/Orleans.Serialization.UnitTests.csproj @@ -29,6 +29,7 @@ + From 9e64309e4b647be9d5171608acf269845130f99a Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Mon, 5 Aug 2024 17:52:18 -0300 Subject: [PATCH 2/5] Skip compiler-generated ctors in records In externally defined symbols, we need to also check for `[CompilerGenerated]`. --- src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs b/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs index 07b60bc5f7..d076d399b4 100644 --- a/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs +++ b/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs @@ -173,7 +173,7 @@ public bool IsEmptyConstructable t = t.BaseType; } - foreach (var ctor in Type.Constructors) + foreach (var ctor in Type.Constructors.Where(x => x.IsImplicitlyDeclared)) { if (ctor.Parameters.Length != 0) { From 7b48cc29931df0c2f70cfabc81393afd9069fc2f Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 6 Aug 2024 15:27:45 -0300 Subject: [PATCH 3/5] Ignore design-time build value when debugging generator This allows running F5 on the generator project and troubleshoot issues. By checking for an already attached debugger, we can debug also scenarios of code generation where the roslyn component starts it with DTB=true. --- src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj | 1 + src/Orleans.CodeGenerator/OrleansSourceGenerator.cs | 3 ++- src/Orleans.CodeGenerator/Properties/launchSettings.json | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 src/Orleans.CodeGenerator/Properties/launchSettings.json diff --git a/src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj b/src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj index af653ca935..c05d097cd9 100644 --- a/src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj +++ b/src/Orleans.CodeGenerator/Orleans.CodeGenerator.csproj @@ -10,6 +10,7 @@ true false true + true diff --git a/src/Orleans.CodeGenerator/OrleansSourceGenerator.cs b/src/Orleans.CodeGenerator/OrleansSourceGenerator.cs index b23ba45ba4..992e75573d 100644 --- a/src/Orleans.CodeGenerator/OrleansSourceGenerator.cs +++ b/src/Orleans.CodeGenerator/OrleansSourceGenerator.cs @@ -23,7 +23,8 @@ public void Execute(GeneratorExecutionContext context) return; } - if (context.AnalyzerConfigOptions.GlobalOptions.TryGetValue("build_property.orleans_designtimebuild", out var isDesignTimeBuild) + if (!Debugger.IsAttached && + context.AnalyzerConfigOptions.GlobalOptions.TryGetValue("build_property.orleans_designtimebuild", out var isDesignTimeBuild) && string.Equals("true", isDesignTimeBuild, StringComparison.OrdinalIgnoreCase)) { return; diff --git a/src/Orleans.CodeGenerator/Properties/launchSettings.json b/src/Orleans.CodeGenerator/Properties/launchSettings.json new file mode 100644 index 0000000000..3b0cf0fa23 --- /dev/null +++ b/src/Orleans.CodeGenerator/Properties/launchSettings.json @@ -0,0 +1,8 @@ +{ + "profiles": { + "Roslyn": { + "commandName": "DebugRoslynComponent", + "targetProject": "..\\..\\test\\Orleans.Serialization.UnitTests\\Orleans.Serialization.UnitTests.csproj" + } + } +} \ No newline at end of file From 050c1c9a921d4975308c35a12a156385b55c4bde Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Tue, 27 Aug 2024 14:52:31 -0700 Subject: [PATCH 4/5] Add additional test cases for generic & readonly scenario --- .../IsExternalInit.cs | 4 + .../TestSerializerExternalModels/Models.cs | 30 ++++++++ .../GeneratedSerializerTests.cs | 74 ++++++++++++++++++- 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/Misc/TestSerializerExternalModels/IsExternalInit.cs diff --git a/test/Misc/TestSerializerExternalModels/IsExternalInit.cs b/test/Misc/TestSerializerExternalModels/IsExternalInit.cs new file mode 100644 index 0000000000..b36792be01 --- /dev/null +++ b/test/Misc/TestSerializerExternalModels/IsExternalInit.cs @@ -0,0 +1,4 @@ +namespace System.Runtime.CompilerServices; + +// required for record serialization support for downlevel +internal static class IsExternalInit {} diff --git a/test/Misc/TestSerializerExternalModels/Models.cs b/test/Misc/TestSerializerExternalModels/Models.cs index 74e0e5d595..05e4645f82 100644 --- a/test/Misc/TestSerializerExternalModels/Models.cs +++ b/test/Misc/TestSerializerExternalModels/Models.cs @@ -12,6 +12,26 @@ public record struct Person2ExternalStruct(int Age, string Name) public string StarSign { get; set; } } +[GenerateSerializer] +public record struct GenericPersonExternalStruct(T CtorParam, string Name) +{ + [Id(0)] + public T BodyParam { get; set; } + + [Id(1)] + public string StarSign { get; set; } +} + +[GenerateSerializer] +public readonly record struct ReadonlyGenericPersonExternalStruct(T CtorParam, string Name) +{ + [Id(0)] + public T BodyParam { get; init; } + + [Id(1)] + public string StarSign { get; init; } +} + #if NET6_0_OR_GREATER [GenerateSerializer] public record Person2External(int Age, string Name) @@ -22,4 +42,14 @@ public record Person2External(int Age, string Name) [Id(1)] public string StarSign { get; set; } } + +[GenerateSerializer] +public record GenericPersonExternal(T CtorParam, string Name) +{ + [Id(0)] + public T BodyParam { get; set; } + + [Id(1)] + public string StarSign { get; set; } +} #endif diff --git a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs index 8969d37f46..b04544f0a6 100644 --- a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs +++ b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs @@ -151,7 +151,6 @@ public void GeneratedLibExternalRecordStructWithPCtorSerializersRoundTripThrough } #if NET6_0_OR_GREATER - [Fact] public void GeneratedLibExternalRecordWithPCtorSerializersRoundTripThroughCodec() { @@ -169,6 +168,79 @@ public void GeneratedLibExternalRecordWithPCtorSerializersRoundTripThroughCodec( Assert.Equal(original.StarSign, result.StarSign); } + [Fact] + public void GeneratedLibExternalGenericRecordStructWithPCtorSerializersRoundTripThroughCodec() + { + var originalInner = new Person2External(2, "harry") + { + FavouriteColor = "redborine", + StarSign = "Aquaricorn" + }; + + var original = new GenericPersonExternalStruct(originalInner, "harry") + { + BodyParam = originalInner, + StarSign = "Aquaricorn" + }; + + var result = RoundTripThroughCodec(original); + + Assert.Equal(original.CtorParam, result.CtorParam); + Assert.Equal(original.Name, result.Name); + Assert.Equal(original.BodyParam, result.BodyParam); + Assert.Equal(original.StarSign, result.StarSign); + Assert.Same(result.CtorParam, result.BodyParam); + } + + [Fact] + public void GeneratedReadonlyLibExternalGenericRecordStructWithPCtorSerializersRoundTripThroughCodec() + { + var originalInner = new Person2External(2, "harry") + { + FavouriteColor = "redborine", + StarSign = "Aquaricorn" + }; + + var original = new ReadonlyGenericPersonExternalStruct(originalInner, "harry") + { + BodyParam = originalInner, + StarSign = "Aquaricorn" + }; + + var result = RoundTripThroughCodec(original); + + Assert.Equal(original.CtorParam, result.CtorParam); + Assert.Equal(original.Name, result.Name); + Assert.Equal(original.BodyParam, result.BodyParam); + Assert.Equal(original.StarSign, result.StarSign); + Assert.Same(result.CtorParam, result.BodyParam); + } +#endif + +#if NET9_0_OR_GREATER + [Fact] + public void GeneratedLibExternalGenericRecordWithPCtorSerializersRoundTripThroughCodec() + { + var originalInner = new Person2External(2, "harry") + { + FavouriteColor = "redborine", + StarSign = "Aquaricorn" + }; + + var original = new GenericPersonExternal(originalInner, "harry") + { + BodyParam = originalInner, + StarSign = "Aquaricorn" + }; + + var result = RoundTripThroughCodec(original); + + Assert.Equal(original.CtorParam, result.CtorParam); + Assert.Equal(original.Name, result.Name); + Assert.Equal(original.BodyParam, result.BodyParam); + Assert.Equal(original.StarSign, result.StarSign); + Assert.Same(result.CtorParam, result.BodyParam); + } #endif #if NET6_0_OR_GREATER From e919b7391b7d217be3328f23192c6965d20e2bda Mon Sep 17 00:00:00 2001 From: Reuben Bond Date: Mon, 30 Sep 2024 10:49:36 -0700 Subject: [PATCH 5/5] Remove IsImplicitlyDeclared check --- src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs b/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs index d076d399b4..07b60bc5f7 100644 --- a/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs +++ b/src/Orleans.CodeGenerator/Model/SerializableTypeDescription.cs @@ -173,7 +173,7 @@ public bool IsEmptyConstructable t = t.BaseType; } - foreach (var ctor in Type.Constructors.Where(x => x.IsImplicitlyDeclared)) + foreach (var ctor in Type.Constructors) { if (ctor.Parameters.Length != 0) {