From 135ba8581d0271e316906461a2ecba966eacdef1 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 22:32:54 +0100 Subject: [PATCH 1/7] Add missing XML docs for property hooks --- .../ObservablePropertyGenerator.Execute.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index 031a9d473..62317662f 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -872,6 +872,8 @@ public static ImmutableArray GetOnPropertyChangeMethods // Construct the generated method as follows: // // /// Executes the logic for when is changing. + // /// The new property value being set. + // /// This method is invoked right before the value of is changed. // [global::System.CodeDom.Compiler.GeneratedCode("...", "...")] // partial void OnChanging( value); MemberDeclarationSyntax onPropertyChangingDeclaration = @@ -884,12 +886,17 @@ public static ImmutableArray GetOnPropertyChangeMethods .AddArgumentListArguments( AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))), AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString())))))) - .WithOpenBracketToken(Token(TriviaList(Comment($"/// Executes the logic for when is changing.")), SyntaxKind.OpenBracketToken, TriviaList()))) + .WithOpenBracketToken(Token(TriviaList( + Comment($"/// Executes the logic for when is changing."), + Comment("/// The new property value being set."), + Comment($"/// This method is invoked right before the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); // Construct the generated method as follows: // // /// Executes the logic for when ust changed. + // /// The new property value that was set. + // /// This method is invoked right after the value of is changed. // [global::System.CodeDom.Compiler.GeneratedCode("...", "...")] // partial void OnChanged( value); MemberDeclarationSyntax onPropertyChangedDeclaration = @@ -902,7 +909,10 @@ public static ImmutableArray GetOnPropertyChangeMethods .AddArgumentListArguments( AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))), AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString())))))) - .WithOpenBracketToken(Token(TriviaList(Comment($"/// Executes the logic for when just changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) + .WithOpenBracketToken(Token(TriviaList( + Comment($"/// Executes the logic for when just changed."), + Comment("/// The new property value that was set."), + Comment($"/// This method is invoked right after the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); return ImmutableArray.Create(onPropertyChangingDeclaration, onPropertyChangedDeclaration); From 4350cef5b02ed92ce251a2e00c7aa75dbcfe9796 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 22:37:48 +0100 Subject: [PATCH 2/7] Generate new property hook overloads --- .../ObservablePropertyGenerator.Execute.cs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index 62317662f..c46806964 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -892,6 +892,33 @@ public static ImmutableArray GetOnPropertyChangeMethods Comment($"/// This method is invoked right before the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); + // Construct the generated method as follows: + // + // /// Executes the logic for when is changing. + // /// The previous property value that is being replaced. + // /// The new property value being set. + // /// This method is invoked right before the value of is changed. + // [global::System.CodeDom.Compiler.GeneratedCode("...", "...")] + // partial void OnChanging( oldValue, newValue); + MemberDeclarationSyntax onPropertyChanging2Declaration = + MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changing")) + .AddModifiers(Token(SyntaxKind.PartialKeyword)) + .AddParameterListParameters( + Parameter(Identifier("oldValue")).WithType(parameterType), + Parameter(Identifier("newValue")).WithType(parameterType)) + .AddAttributeLists( + AttributeList(SingletonSeparatedList( + Attribute(IdentifierName("global::System.CodeDom.Compiler.GeneratedCode")) + .AddArgumentListArguments( + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))), + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString())))))) + .WithOpenBracketToken(Token(TriviaList( + Comment($"/// Executes the logic for when is changing."), + Comment("/// The previous property value that is being replaced."), + Comment("/// The new property value being set."), + Comment($"/// This method is invoked right before the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) + .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); + // Construct the generated method as follows: // // /// Executes the logic for when ust changed. @@ -915,7 +942,38 @@ public static ImmutableArray GetOnPropertyChangeMethods Comment($"/// This method is invoked right after the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); - return ImmutableArray.Create(onPropertyChangingDeclaration, onPropertyChangedDeclaration); + // Construct the generated method as follows: + // + // /// Executes the logic for when ust changed. + // /// The previous property value that was replaced. + // /// The new property value that was set. + // /// This method is invoked right after the value of is changed. + // [global::System.CodeDom.Compiler.GeneratedCode("...", "...")] + // partial void OnChanged( oldValue, newValue); + MemberDeclarationSyntax onPropertyChanged2Declaration = + MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changed")) + .AddModifiers(Token(SyntaxKind.PartialKeyword)) + .AddParameterListParameters( + Parameter(Identifier("oldValue")).WithType(parameterType), + Parameter(Identifier("newValue")).WithType(parameterType)) + .AddAttributeLists( + AttributeList(SingletonSeparatedList( + Attribute(IdentifierName("global::System.CodeDom.Compiler.GeneratedCode")) + .AddArgumentListArguments( + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))), + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString())))))) + .WithOpenBracketToken(Token(TriviaList( + Comment($"/// Executes the logic for when just changed."), + Comment("/// The previous property value that was replaced."), + Comment("/// The new property value that was set."), + Comment($"/// This method is invoked right after the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) + .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); + + return ImmutableArray.Create( + onPropertyChangingDeclaration, + onPropertyChanging2Declaration, + onPropertyChangedDeclaration, + onPropertyChanged2Declaration); } /// From 2542767121adb7377227d6f45837c83ef01f72b2 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 22:44:25 +0100 Subject: [PATCH 3/7] Generate calls to new property hook overloads --- .../ObservablePropertyGenerator.Execute.cs | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index c46806964..a1edf240c 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -683,19 +683,15 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf string name => IdentifierName(name) }; - if (propertyInfo.NotifyPropertyChangedRecipients) - { - // If broadcasting changes are required, also store the old value. - // This code generates a statement as follows: - // - // __oldValue = ; - setterStatements.Add( - LocalDeclarationStatement( - VariableDeclaration(propertyType) - .AddVariables( - VariableDeclarator(Identifier("__oldValue")) - .WithInitializer(EqualsValueClause(fieldExpression))))); - } + // Store the old value for later. This code generates a statement as follows: + // + // __oldValue = ; + setterStatements.Add( + LocalDeclarationStatement( + VariableDeclaration(propertyType) + .AddVariables( + VariableDeclarator(Identifier("__oldValue")) + .WithInitializer(EqualsValueClause(fieldExpression))))); // Add the OnPropertyChanging() call first: // @@ -705,6 +701,14 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing")) .AddArgumentListArguments(Argument(IdentifierName("value"))))); + // Also call the overload after that: + // + // OnChanging(__oldValue, value); + setterStatements.Add( + ExpressionStatement( + InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing")) + .AddArgumentListArguments(Argument(IdentifierName("__oldValue")), Argument(IdentifierName("value"))))); + // Gather the statements to notify dependent properties foreach (string propertyName in propertyInfo.PropertyChangingNames) { @@ -751,6 +755,14 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changed")) .AddArgumentListArguments(Argument(IdentifierName("value"))))); + // Do the same for the overload, as above: + // + // OnChanged(__oldValue, value); + setterStatements.Add( + ExpressionStatement( + InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changed")) + .AddArgumentListArguments(Argument(IdentifierName("__oldValue")), Argument(IdentifierName("value"))))); + // Gather the statements to notify dependent properties foreach (string propertyName in propertyInfo.PropertyChangedNames) { From f07bb2015de9042026e90f05ed8c216f3d9fb352 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 23:07:01 +0100 Subject: [PATCH 4/7] Optimize codegen when hooks are not used --- .../ComponentModel/Models/PropertyInfo.cs | 2 + .../ObservablePropertyGenerator.Execute.cs | 72 +++++++++++++++---- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs index c02e123ca..f8c1378e5 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs @@ -17,6 +17,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models; /// The sequence of commands to notify. /// Whether or not the generated property also broadcasts changes. /// Whether or not the generated property also validates its value. +/// Whether the old property value is being directly referenced. /// The sequence of forwarded attributes for the generated property. internal sealed record PropertyInfo( string TypeNameWithNullabilityAnnotations, @@ -27,4 +28,5 @@ internal sealed record PropertyInfo( EquatableArray NotifiedCommandNames, bool NotifyPropertyChangedRecipients, bool NotifyDataErrorInfo, + bool IsOldPropertyValueDirectlyReferenced, EquatableArray ForwardedAttributes); diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index a1edf240c..43a67ce4d 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -111,6 +111,7 @@ public static bool TryGetInfo( bool hasOrInheritsClassLevelNotifyPropertyChangedRecipients = false; bool hasOrInheritsClassLevelNotifyDataErrorInfo = false; bool hasAnyValidationAttributes = false; + bool isOldPropertyValueDirectlyReferenced = IsOldPropertyValueDirectlyReferenced(fieldSymbol, propertyName); // Track the property changing event for the property, if the type supports it if (shouldInvokeOnPropertyChanging) @@ -263,6 +264,7 @@ public static bool TryGetInfo( notifiedCommandNames.ToImmutable(), notifyRecipients, notifyDataErrorInfo, + isOldPropertyValueDirectlyReferenced, forwardedAttributes.ToImmutable()); diagnostics = builder.ToImmutable(); @@ -637,6 +639,38 @@ private static bool TryGetNotifyDataErrorInfo( return false; } + /// + /// Checks whether the generated code has to directly reference the old property value. + /// + /// The input instance to process. + /// The name of the property being generated. + /// Whether the generated code needs direct access to the old property value. + private static bool IsOldPropertyValueDirectlyReferenced(IFieldSymbol fieldSymbol, string propertyName) + { + // Check OnChanging( oldValue, newValue) first + foreach (ISymbol symbol in fieldSymbol.ContainingType.GetMembers($"On{propertyName}Changing")) + { + // No need to be too specific as we're not expecting false positives (which also wouldn't really + // cause any problems anyway, just produce slightly worse codegen). Just checking the number of + // parameters is good enough, and keeps the code very simple and cheap to run. + if (symbol is IMethodSymbol { Parameters.Length: 2 }) + { + return true; + } + } + + // Do the same for OnChanged( oldValue, newValue) + foreach (ISymbol symbol in fieldSymbol.ContainingType.GetMembers($"On{propertyName}Changed")) + { + if (symbol is IMethodSymbol { Parameters.Length: 2 }) + { + return true; + } + } + + return false; + } + /// /// Gets a instance with the cached args for property changing notifications. /// @@ -683,15 +717,18 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf string name => IdentifierName(name) }; - // Store the old value for later. This code generates a statement as follows: - // - // __oldValue = ; - setterStatements.Add( - LocalDeclarationStatement( - VariableDeclaration(propertyType) - .AddVariables( - VariableDeclarator(Identifier("__oldValue")) - .WithInitializer(EqualsValueClause(fieldExpression))))); + if (propertyInfo.NotifyPropertyChangedRecipients || propertyInfo.IsOldPropertyValueDirectlyReferenced) + { + // Store the old value for later. This code generates a statement as follows: + // + // __oldValue = ; + setterStatements.Add( + LocalDeclarationStatement( + VariableDeclaration(propertyType) + .AddVariables( + VariableDeclarator(Identifier("__oldValue")) + .WithInitializer(EqualsValueClause(fieldExpression))))); + } // Add the OnPropertyChanging() call first: // @@ -701,13 +738,22 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing")) .AddArgumentListArguments(Argument(IdentifierName("value"))))); + // Optimization: if the previous property value is not being referenced (which we can check by looking for an existing + // symbol matching the name of either of these generated methods), we can pass a default expression and avoid generating + // a field read, which won't otherwise be elided by Roslyn. Otherwise, we just store the value in a local as usual. + ArgumentSyntax oldPropertyValueArgument = propertyInfo.IsOldPropertyValueDirectlyReferenced switch + { + true => Argument(IdentifierName("__oldValue")), + false => Argument(LiteralExpression(SyntaxKind.DefaultLiteralExpression, Token(SyntaxKind.DefaultKeyword))) + }; + // Also call the overload after that: // - // OnChanging(__oldValue, value); + // OnChanging(, value); setterStatements.Add( ExpressionStatement( InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing")) - .AddArgumentListArguments(Argument(IdentifierName("__oldValue")), Argument(IdentifierName("value"))))); + .AddArgumentListArguments(oldPropertyValueArgument, Argument(IdentifierName("value"))))); // Gather the statements to notify dependent properties foreach (string propertyName in propertyInfo.PropertyChangingNames) @@ -757,11 +803,11 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf // Do the same for the overload, as above: // - // OnChanged(__oldValue, value); + // OnChanged(, value); setterStatements.Add( ExpressionStatement( InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changed")) - .AddArgumentListArguments(Argument(IdentifierName("__oldValue")), Argument(IdentifierName("value"))))); + .AddArgumentListArguments(oldPropertyValueArgument, Argument(IdentifierName("value"))))); // Gather the statements to notify dependent properties foreach (string propertyName in propertyInfo.PropertyChangedNames) From c684646108bc0166fcb49868a66bf9dda35f6920 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 23:19:46 +0100 Subject: [PATCH 5/7] Handle nullability in new hooks with previous value --- .../ComponentModel/Models/PropertyInfo.cs | 2 ++ .../ObservablePropertyGenerator.Execute.cs | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs index f8c1378e5..59e5b33fe 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs @@ -18,6 +18,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models; /// Whether or not the generated property also broadcasts changes. /// Whether or not the generated property also validates its value. /// Whether the old property value is being directly referenced. +/// Indicates whether the property is of a reference type. /// The sequence of forwarded attributes for the generated property. internal sealed record PropertyInfo( string TypeNameWithNullabilityAnnotations, @@ -29,4 +30,5 @@ internal sealed record PropertyInfo( bool NotifyPropertyChangedRecipients, bool NotifyDataErrorInfo, bool IsOldPropertyValueDirectlyReferenced, + bool IsReferenceType, EquatableArray ForwardedAttributes); diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index 43a67ce4d..d84c3f96e 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -112,6 +112,7 @@ public static bool TryGetInfo( bool hasOrInheritsClassLevelNotifyDataErrorInfo = false; bool hasAnyValidationAttributes = false; bool isOldPropertyValueDirectlyReferenced = IsOldPropertyValueDirectlyReferenced(fieldSymbol, propertyName); + bool isReferenceType = fieldSymbol.Type.IsReferenceType; // Track the property changing event for the property, if the type supports it if (shouldInvokeOnPropertyChanging) @@ -265,6 +266,7 @@ public static bool TryGetInfo( notifyRecipients, notifyDataErrorInfo, isOldPropertyValueDirectlyReferenced, + isReferenceType, forwardedAttributes.ToImmutable()); diagnostics = builder.ToImmutable(); @@ -950,6 +952,18 @@ public static ImmutableArray GetOnPropertyChangeMethods Comment($"/// This method is invoked right before the value of is changed.")), SyntaxKind.OpenBracketToken, TriviaList()))) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)); + // Prepare the nullable type for the previous property value. This is needed because if the type is a reference + // type, the previous value might be null even if the property type is not nullable, as the first invocation would + // happen when the property is first set to some value that is not null (but the backing field would still be so). + // As a cheap way to check whether we need to add nullable, we can simply check whether the type name with nullability + // annotations ends with a '?'. If it doesn't and the type is a reference type, we add it. Otherwise, we keep it. + TypeSyntax oldValueTypeSyntax = propertyInfo.IsReferenceType switch + { + true when !propertyInfo.TypeNameWithNullabilityAnnotations.EndsWith("?") + => IdentifierName($"{propertyInfo.TypeNameWithNullabilityAnnotations}?"), + _ => parameterType + }; + // Construct the generated method as follows: // // /// Executes the logic for when is changing. @@ -962,7 +976,7 @@ public static ImmutableArray GetOnPropertyChangeMethods MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changing")) .AddModifiers(Token(SyntaxKind.PartialKeyword)) .AddParameterListParameters( - Parameter(Identifier("oldValue")).WithType(parameterType), + Parameter(Identifier("oldValue")).WithType(oldValueTypeSyntax), Parameter(Identifier("newValue")).WithType(parameterType)) .AddAttributeLists( AttributeList(SingletonSeparatedList( @@ -1012,7 +1026,7 @@ public static ImmutableArray GetOnPropertyChangeMethods MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changed")) .AddModifiers(Token(SyntaxKind.PartialKeyword)) .AddParameterListParameters( - Parameter(Identifier("oldValue")).WithType(parameterType), + Parameter(Identifier("oldValue")).WithType(oldValueTypeSyntax), Parameter(Identifier("newValue")).WithType(parameterType)) .AddAttributeLists( AttributeList(SingletonSeparatedList( From 7f4c8800aa5b05e9fbbe7d09e0188c9a8a254f97 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sun, 22 Jan 2023 23:40:08 +0100 Subject: [PATCH 6/7] Add functional tests for new generated methods --- .../Test_ObservablePropertyAttribute.cs | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs b/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs index 44adc66aa..e9d5606d2 100644 --- a/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs +++ b/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs @@ -438,6 +438,60 @@ public void Test_OnPropertyChangingAndChangedPartialMethods() Assert.AreEqual(99, model.NumberChangedValue); } + [TestMethod] + public void Test_OnPropertyChangingAndChangedPartialMethods_WithPreviousValues() + { + ViewModelWithImplementedUpdateMethods2 model = new(); + + Assert.AreEqual(null, model.Name); + Assert.AreEqual(0, model.Number); + + CollectionAssert.AreEqual(Array.Empty<(string, string)>(), model.OnNameChangingValues); + CollectionAssert.AreEqual(Array.Empty<(string, string)>(), model.OnNameChangedValues); + CollectionAssert.AreEqual(Array.Empty<(int, int)>(), model.OnNumberChangingValues); + CollectionAssert.AreEqual(Array.Empty<(int, int)>(), model.OnNumberChangedValues); + + model.Name = "Bob"; + + CollectionAssert.AreEqual(new[] { ((string?)null, "Bob") }, model.OnNameChangingValues); + CollectionAssert.AreEqual(new[] { ((string?)null, "Bob") }, model.OnNameChangedValues); + + Assert.AreEqual("Bob", model.Name); + + CollectionAssert.AreEqual(new[] { ((string?)null, "Bob") }, model.OnNameChangingValues); + CollectionAssert.AreEqual(new[] { ((string?)null, "Bob") }, model.OnNameChangedValues); + + model.Name = "Alice"; + + CollectionAssert.AreEqual(new[] { (null, "Bob"), ("Bob", "Alice") }, model.OnNameChangingValues); + CollectionAssert.AreEqual(new[] { (null, "Bob"), ("Bob", "Alice") }, model.OnNameChangedValues); + + Assert.AreEqual("Alice", model.Name); + + CollectionAssert.AreEqual(new[] { (null, "Bob"), ("Bob", "Alice") }, model.OnNameChangingValues); + CollectionAssert.AreEqual(new[] { (null, "Bob"), ("Bob", "Alice") }, model.OnNameChangedValues); + + model.Number = 42; + + CollectionAssert.AreEqual(new[] { (0, 42) }, model.OnNumberChangingValues); + CollectionAssert.AreEqual(new[] { (0, 42) }, model.OnNumberChangedValues); + + Assert.AreEqual(42, model.Number); + + CollectionAssert.AreEqual(new[] { (0, 42) }, model.OnNumberChangingValues); + CollectionAssert.AreEqual(new[] { (0, 42) }, model.OnNumberChangedValues); + + model.Number = 77; + + CollectionAssert.AreEqual(new[] { (0, 42), (42, 77) }, model.OnNumberChangingValues); + CollectionAssert.AreEqual(new[] { (0, 42), (42, 77) }, model.OnNumberChangedValues); + + Assert.AreEqual(77, model.Number); + + CollectionAssert.AreEqual(new[] { (0, 42), (42, 77) }, model.OnNumberChangingValues); + CollectionAssert.AreEqual(new[] { (0, 42), (42, 77) }, model.OnNumberChangedValues); + } + [TestMethod] public void Test_OnPropertyChangingAndChangedPartialMethodWithAdditionalValidation() { @@ -1253,6 +1307,43 @@ partial void OnNumberChanged(int value) } } + public partial class ViewModelWithImplementedUpdateMethods2 : ObservableObject + { + [ObservableProperty] + public string? name; + + [ObservableProperty] + public int number; + + public List<(string? Old, string? New)> OnNameChangingValues { get; } = new(); + + public List<(string? Old, string? New)> OnNameChangedValues { get; } = new(); + + public List<(int Old, int New)> OnNumberChangingValues { get; } = new(); + + public List<(int Old, int New)> OnNumberChangedValues { get; } = new(); + + partial void OnNameChanging(string? oldValue, string? newValue) + { + OnNameChangingValues.Add((oldValue, newValue)); + } + + partial void OnNameChanged(string? oldValue, string? newValue) + { + OnNameChangedValues.Add((oldValue, newValue)); + } + + partial void OnNumberChanging(int oldValue, int newValue) + { + OnNumberChangingValues.Add((oldValue, newValue)); + } + + partial void OnNumberChanged(int oldValue, int newValue) + { + OnNumberChangedValues.Add((oldValue, newValue)); + } + } + public partial class ViewModelWithImplementedUpdateMethodAndAdditionalValidation : ObservableObject { private int step; From 845503d7f78ab58fc1ce119481d711e0642638f8 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 23 Jan 2023 00:00:17 +0100 Subject: [PATCH 7/7] Add codegen test for new partial property hooks --- ....Mvvm.SourceGenerators.UnitTests.projitems | 1 + .../Test_SourceGeneratorsCodegen.cs | 143 ++++++++++++++++++ .../Test_SourceGeneratorsDiagnostics.cs | 10 +- 3 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests.projitems b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests.projitems index ab60a1c18..881fdb625 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests.projitems +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests.projitems @@ -10,6 +10,7 @@ + \ No newline at end of file diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs new file mode 100644 index 000000000..8cf4b9daa --- /dev/null +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsCodegen.cs @@ -0,0 +1,143 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.ComponentModel.DataAnnotations; +using System.IO; +using System.Linq; +using CommunityToolkit.Mvvm.ComponentModel; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace CommunityToolkit.Mvvm.SourceGenerators.UnitTests; + +[TestClass] +public class Test_SourceGeneratorsCodegen +{ + [TestMethod] + public void ObservablePropertyWithPartialMethodWithPreviousValuesNotUsed_DoesNotGenerateFieldReadAndMarksOldValueAsNullable() + { + string source = """ + using System.ComponentModel; + using CommunityToolkit.Mvvm.ComponentModel; + + #nullable enable + + namespace MyApp; + + partial class MyViewModel : ObservableObject + { + [ObservableProperty] + private string name = null!; + } + """; + + string result = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + partial class MyViewModel + { + /// + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + public string Name + { + get => name; + set + { + if (!global::System.Collections.Generic.EqualityComparer.Default.Equals(name, value)) + { + OnNameChanging(value); + OnNameChanging(default, value); + OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name); + name = value; + OnNameChanged(value); + OnNameChanged(default, value); + OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name); + } + } + } + + /// Executes the logic for when is changing. + /// The new property value being set. + /// This method is invoked right before the value of is changed. + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")] + partial void OnNameChanging(string value); + /// Executes the logic for when is changing. + /// The previous property value that is being replaced. + /// The new property value being set. + /// This method is invoked right before the value of is changed. + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")] + partial void OnNameChanging(string? oldValue, string newValue); + /// Executes the logic for when just changed. + /// The new property value that was set. + /// This method is invoked right after the value of is changed. + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")] + partial void OnNameChanged(string value); + /// Executes the logic for when just changed. + /// The previous property value that was replaced. + /// The new property value that was set. + /// This method is invoked right after the value of is changed. + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.1.0.0")] + partial void OnNameChanged(string? oldValue, string newValue); + } + } + """; + + VerifyGenerateSources(source, new[] { new ObservablePropertyGenerator() }, ("MyApp.MyViewModel.g.cs", result)); + } + + /// + /// Generates the requested sources + /// + /// The input source to process. + /// The generators to apply to the input syntax tree. + /// The source files to compare. + private static void VerifyGenerateSources(string source, IIncrementalGenerator[] generators, params (string Filename, string Text)[] results) + { + // Ensure CommunityToolkit.Mvvm and System.ComponentModel.DataAnnotations are loaded + Type observableObjectType = typeof(ObservableObject); + Type validationAttributeType = typeof(ValidationAttribute); + + // Get all assembly references for the loaded assemblies (easy way to pull in all necessary dependencies) + IEnumerable references = + from assembly in AppDomain.CurrentDomain.GetAssemblies() + where !assembly.IsDynamic + let reference = MetadataReference.CreateFromFile(assembly.Location) + select reference; + + SyntaxTree syntaxTree = CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp11)); + + // Create a syntax tree with the input source + CSharpCompilation compilation = CSharpCompilation.Create( + "original", + new SyntaxTree[] { syntaxTree }, + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + GeneratorDriver driver = CSharpGeneratorDriver.Create(generators).WithUpdatedParseOptions((CSharpParseOptions)syntaxTree.Options); + + // Run all source generators on the input source code + _ = driver.RunGeneratorsAndUpdateCompilation(compilation, out Compilation outputCompilation, out ImmutableArray diagnostics); + + // Ensure that no diagnostics were generated + CollectionAssert.AreEquivalent(Array.Empty(), diagnostics); + + foreach ((string filename, string text) in results) + { + SyntaxTree generatedTree = outputCompilation.SyntaxTrees.Single(tree => Path.GetFileName(tree.FilePath) == filename); + + Assert.AreEqual(text, generatedTree.ToString()); + } + + GC.KeepAlive(observableObjectType); + GC.KeepAlive(validationAttributeType); + } +} diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs index 6e15ce95e..fcb059fb5 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs @@ -7,14 +7,14 @@ using System.Collections.Immutable; using System.ComponentModel.DataAnnotations; using System.Linq; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using CommunityToolkit.Mvvm.ComponentModel; -using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Text.RegularExpressions; using System.Threading.Tasks; +using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.SourceGenerators.UnitTests.Helpers; -using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.VisualStudio.TestTools.UnitTesting; namespace CommunityToolkit.Mvvm.SourceGenerators.UnitTests;