From 84b2a29ee63fcfcc6791feb7eb41db198326c61e Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 23 Apr 2021 18:40:32 +0200 Subject: [PATCH] Improved code generation for ValidateAllProperties generator Improved type safety, more AOT-friendly, removed Unsafe.As hack --- ...ValidatorValidateAllPropertiesGenerator.cs | 39 +++++++++++++++---- .../ComponentModel/ObservableValidator.cs | 6 +-- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs b/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs index 516f7178..8d160cba 100644 --- a/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs +++ b/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.cs @@ -64,7 +64,11 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators foreach (INamedTypeSymbol classSymbol in syntaxReceiver.GatheredInfo) { - // Create a static method to validate all properties in a given class. + // Create a static factory method creating a delegate that can be used to validate all properties in a given class. + // This pattern is used so that the library doesn't have to use MakeGenericType(...) at runtime, nor use unsafe casts + // over the created delegate to be able to cache it as an Action instance. This pattern enables the same + // functionality and with almost identical performance (not noticeable in this context anyway), but while preserving + // full runtime type safety (as a safe cast is used to validate the input argument), and with less reflection needed. // This code takes a class symbol and produces a compilation unit as follows: // // // Licensed to the .NET Foundation under one or more agreements. @@ -84,9 +88,14 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators // { // [global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)] // [global::System.Obsolete("This method is not intended to be called directly by user code")] - // public static void ValidateAllProperties( instance) + // public static global::System.Action CreateAllPropertiesValidator( _) // { - // + // static void ValidateAllProperties( instance) + // { + // + // } + // + // return static o => ValidateAllProperties(()o); // } // } // } @@ -102,8 +111,8 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators Token(SyntaxKind.StaticKeyword), Token(SyntaxKind.PartialKeyword)).AddAttributeLists(classAttributes).AddMembers( MethodDeclaration( - PredefinedType(Token(SyntaxKind.VoidKeyword)), - Identifier("ValidateAllProperties")).AddAttributeLists( + GenericName("global::System.Action").AddTypeArgumentListArguments(PredefinedType(Token(SyntaxKind.ObjectKeyword))), + Identifier("CreateAllPropertiesValidator")).AddAttributeLists( AttributeList(SingletonSeparatedList( Attribute(IdentifierName("global::System.ComponentModel.EditorBrowsable")).AddArgumentListArguments( AttributeArgument(ParseExpression("global::System.ComponentModel.EditorBrowsableState.Never"))))), @@ -114,8 +123,24 @@ namespace Microsoft.Toolkit.Mvvm.SourceGenerators Literal("This method is not intended to be called directly by user code"))))))).AddModifiers( Token(SyntaxKind.PublicKeyword), Token(SyntaxKind.StaticKeyword)).AddParameterListParameters( - Parameter(Identifier("instance")).WithType(IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)))) - .WithBody(Block(EnumerateValidationStatements(classSymbol, validationSymbol).ToArray()))))) + Parameter(Identifier("_")).WithType(IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)))) + .WithBody(Block( + LocalFunctionStatement( + PredefinedType(Token(SyntaxKind.VoidKeyword)), + Identifier("ValidateAllProperties")) + .AddModifiers(Token(SyntaxKind.StaticKeyword)) + .AddParameterListParameters( + Parameter(Identifier("instance")).WithType(IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)))) + .WithBody(Block(EnumerateValidationStatements(classSymbol, validationSymbol).ToArray())), + ReturnStatement( + SimpleLambdaExpression(Parameter(Identifier("o"))) + .AddModifiers(Token(SyntaxKind.StaticKeyword)) + .WithExpressionBody( + InvocationExpression(IdentifierName("ValidateAllProperties")) + .AddArgumentListArguments(Argument( + CastExpression( + IdentifierName(classSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)), + IdentifierName("o"))))))))))) .NormalizeWhitespace() .ToFullString(); diff --git a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs index dead1935..0390ba60 100644 --- a/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs +++ b/Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs @@ -476,11 +476,9 @@ namespace Microsoft.Toolkit.Mvvm.ComponentModel static Action GetValidationAction(Type type) { if (type.Assembly.GetType("Microsoft.Toolkit.Mvvm.ComponentModel.__Internals.__ObservableValidatorExtensions") is Type extensionsType && - extensionsType.GetMethod("ValidateAllProperties", new[] { type }) is MethodInfo methodInfo) + extensionsType.GetMethod("CreateAllPropertiesValidator", new[] { type }) is MethodInfo methodInfo) { - Type delegateType = typeof(Action<>).MakeGenericType(type); - - return Unsafe.As>(methodInfo.CreateDelegate(delegateType)); + return (Action)methodInfo.Invoke(null, new object?[] { null })!; } return GetValidationActionFallback(type);