Skip to content

Commit 01efc75

Browse files
Sergio0694Sergio0694
authored
Merge pull request #582 from CommunityToolkit/dev/new-property-hooks
Add new OnPropertyNameChanging and Changed overloads
2 parents e6e0940 + 845503d commit 01efc75

File tree

6 files changed

+390
-11
lines changed

6 files changed

+390
-11
lines changed

‎src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
1717
/// <param name="NotifiedCommandNames">The sequence of commands to notify.</param>
1818
/// <param name="NotifyPropertyChangedRecipients">Whether or not the generated property also broadcasts changes.</param>
1919
/// <param name="NotifyDataErrorInfo">Whether or not the generated property also validates its value.</param>
20+
/// <param name="IsOldPropertyValueDirectlyReferenced">Whether the old property value is being directly referenced.</param>
21+
/// <param name="IsReferenceType">Indicates whether the property is of a reference type.</param>
2022
/// <param name="ForwardedAttributes">The sequence of forwarded attributes for the generated property.</param>
2123
internal sealed record PropertyInfo(
2224
string TypeNameWithNullabilityAnnotations,
@@ -27,4 +29,6 @@ internal sealed record PropertyInfo(
2729
EquatableArray<string> NotifiedCommandNames,
2830
bool NotifyPropertyChangedRecipients,
2931
bool NotifyDataErrorInfo,
32+
bool IsOldPropertyValueDirectlyReferenced,
33+
bool IsReferenceType,
3034
EquatableArray<AttributeInfo> ForwardedAttributes);

‎src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs

Lines changed: 146 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ public static bool TryGetInfo(
111111
bool hasOrInheritsClassLevelNotifyPropertyChangedRecipients = false;
112112
bool hasOrInheritsClassLevelNotifyDataErrorInfo = false;
113113
bool hasAnyValidationAttributes = false;
114+
bool isOldPropertyValueDirectlyReferenced = IsOldPropertyValueDirectlyReferenced(fieldSymbol, propertyName);
115+
bool isReferenceType = fieldSymbol.Type.IsReferenceType;
114116

115117
// Track the property changing event for the property, if the type supports it
116118
if (shouldInvokeOnPropertyChanging)
@@ -263,6 +265,8 @@ public static bool TryGetInfo(
263265
notifiedCommandNames.ToImmutable(),
264266
notifyRecipients,
265267
notifyDataErrorInfo,
268+
isOldPropertyValueDirectlyReferenced,
269+
isReferenceType,
266270
forwardedAttributes.ToImmutable());
267271

268272
diagnostics = builder.ToImmutable();
@@ -637,6 +641,38 @@ private static bool TryGetNotifyDataErrorInfo(
637641
return false;
638642
}
639643

644+
/// <summary>
645+
/// Checks whether the generated code has to directly reference the old property value.
646+
/// </summary>
647+
/// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
648+
/// <param name="propertyName">The name of the property being generated.</param>
649+
/// <returns>Whether the generated code needs direct access to the old property value.</returns>
650+
private static bool IsOldPropertyValueDirectlyReferenced(IFieldSymbol fieldSymbol, string propertyName)
651+
{
652+
// Check On<PROPERTY_NAME>Changing(<PROPERTY_TYPE> oldValue, <PROPERTY_TYPE> newValue) first
653+
foreach (ISymbol symbol in fieldSymbol.ContainingType.GetMembers($"On{propertyName}Changing"))
654+
{
655+
// No need to be too specific as we're not expecting false positives (which also wouldn't really
656+
// cause any problems anyway, just produce slightly worse codegen). Just checking the number of
657+
// parameters is good enough, and keeps the code very simple and cheap to run.
658+
if (symbol is IMethodSymbol { Parameters.Length: 2 })
659+
{
660+
return true;
661+
}
662+
}
663+
664+
// Do the same for On<PROPERTY_NAME>Changed(<PROPERTY_TYPE> oldValue, <PROPERTY_TYPE> newValue)
665+
foreach (ISymbol symbol in fieldSymbol.ContainingType.GetMembers($"On{propertyName}Changed"))
666+
{
667+
if (symbol is IMethodSymbol { Parameters.Length: 2 })
668+
{
669+
return true;
670+
}
671+
}
672+
673+
return false;
674+
}
675+
640676
/// <summary>
641677
/// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
642678
/// </summary>
@@ -683,10 +719,9 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
683719
string name => IdentifierName(name)
684720
};
685721

686-
if (propertyInfo.NotifyPropertyChangedRecipients)
722+
if (propertyInfo.NotifyPropertyChangedRecipients || propertyInfo.IsOldPropertyValueDirectlyReferenced)
687723
{
688-
// If broadcasting changes are required, also store the old value.
689-
// This code generates a statement as follows:
724+
// Store the old value for later. This code generates a statement as follows:
690725
//
691726
// <PROPERTY_TYPE> __oldValue = <FIELD_EXPRESSIONS>;
692727
setterStatements.Add(
@@ -705,6 +740,23 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
705740
InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing"))
706741
.AddArgumentListArguments(Argument(IdentifierName("value")))));
707742

743+
// Optimization: if the previous property value is not being referenced (which we can check by looking for an existing
744+
// symbol matching the name of either of these generated methods), we can pass a default expression and avoid generating
745+
// a field read, which won't otherwise be elided by Roslyn. Otherwise, we just store the value in a local as usual.
746+
ArgumentSyntax oldPropertyValueArgument = propertyInfo.IsOldPropertyValueDirectlyReferenced switch
747+
{
748+
true => Argument(IdentifierName("__oldValue")),
749+
false => Argument(LiteralExpression(SyntaxKind.DefaultLiteralExpression, Token(SyntaxKind.DefaultKeyword)))
750+
};
751+
752+
// Also call the overload after that:
753+
//
754+
// On<PROPERTY_NAME>Changing(<OLD_PROPERTY_VALUE_EXPRESSION>, value);
755+
setterStatements.Add(
756+
ExpressionStatement(
757+
InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changing"))
758+
.AddArgumentListArguments(oldPropertyValueArgument, Argument(IdentifierName("value")))));
759+
708760
// Gather the statements to notify dependent properties
709761
foreach (string propertyName in propertyInfo.PropertyChangingNames)
710762
{
@@ -751,6 +803,14 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
751803
InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changed"))
752804
.AddArgumentListArguments(Argument(IdentifierName("value")))));
753805

806+
// Do the same for the overload, as above:
807+
//
808+
// On<PROPERTY_NAME>Changed(<OLD_PROPERTY_VALUE_EXPRESSION>, value);
809+
setterStatements.Add(
810+
ExpressionStatement(
811+
InvocationExpression(IdentifierName($"On{propertyInfo.PropertyName}Changed"))
812+
.AddArgumentListArguments(oldPropertyValueArgument, Argument(IdentifierName("value")))));
813+
754814
// Gather the statements to notify dependent properties
755815
foreach (string propertyName in propertyInfo.PropertyChangedNames)
756816
{
@@ -872,6 +932,8 @@ public static ImmutableArray<MemberDeclarationSyntax> GetOnPropertyChangeMethods
872932
// Construct the generated method as follows:
873933
//
874934
// /// <summary>Executes the logic for when <see cref="<PROPERTY_NAME>"/> is changing.</summary>
935+
// /// <param name="value">The new property value being set.</param>
936+
// /// <remarks>This method is invoked right before the value of <see cref="<PROPERTY_NAME>"/> is changed.</remarks>
875937
// [global::System.CodeDom.Compiler.GeneratedCode("...", "...")]
876938
// partial void On<PROPERTY_NAME>Changing(<PROPERTY_TYPE> value);
877939
MemberDeclarationSyntax onPropertyChangingDeclaration =
@@ -884,12 +946,56 @@ public static ImmutableArray<MemberDeclarationSyntax> GetOnPropertyChangeMethods
884946
.AddArgumentListArguments(
885947
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))),
886948
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString()))))))
887-
.WithOpenBracketToken(Token(TriviaList(Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> is changing.</summary>")), SyntaxKind.OpenBracketToken, TriviaList())))
949+
.WithOpenBracketToken(Token(TriviaList(
950+
Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> is changing.</summary>"),
951+
Comment("/// <param name=\"value\">The new property value being set.</param>"),
952+
Comment($"/// <remarks>This method is invoked right before the value of <see cref=\"{propertyInfo.PropertyName}\"/> is changed.</remarks>")), SyntaxKind.OpenBracketToken, TriviaList())))
953+
.WithSemicolonToken(Token(SyntaxKind.SemicolonToken));
954+
955+
// Prepare the nullable type for the previous property value. This is needed because if the type is a reference
956+
// type, the previous value might be null even if the property type is not nullable, as the first invocation would
957+
// happen when the property is first set to some value that is not null (but the backing field would still be so).
958+
// As a cheap way to check whether we need to add nullable, we can simply check whether the type name with nullability
959+
// annotations ends with a '?'. If it doesn't and the type is a reference type, we add it. Otherwise, we keep it.
960+
TypeSyntax oldValueTypeSyntax = propertyInfo.IsReferenceType switch
961+
{
962+
true when !propertyInfo.TypeNameWithNullabilityAnnotations.EndsWith("?")
963+
=> IdentifierName($"{propertyInfo.TypeNameWithNullabilityAnnotations}?"),
964+
_ => parameterType
965+
};
966+
967+
// Construct the generated method as follows:
968+
//
969+
// /// <summary>Executes the logic for when <see cref="<PROPERTY_NAME>"/> is changing.</summary>
970+
// /// <param name="oldValue">The previous property value that is being replaced.</param>
971+
// /// <param name="newValue">The new property value being set.</param>
972+
// /// <remarks>This method is invoked right before the value of <see cref="<PROPERTY_NAME>"/> is changed.</remarks>
973+
// [global::System.CodeDom.Compiler.GeneratedCode("...", "...")]
974+
// partial void On<PROPERTY_NAME>Changing(<OLD_VALUE_TYPE> oldValue, <PROPERTY_TYPE> newValue);
975+
MemberDeclarationSyntax onPropertyChanging2Declaration =
976+
MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changing"))
977+
.AddModifiers(Token(SyntaxKind.PartialKeyword))
978+
.AddParameterListParameters(
979+
Parameter(Identifier("oldValue")).WithType(oldValueTypeSyntax),
980+
Parameter(Identifier("newValue")).WithType(parameterType))
981+
.AddAttributeLists(
982+
AttributeList(SingletonSeparatedList(
983+
Attribute(IdentifierName("global::System.CodeDom.Compiler.GeneratedCode"))
984+
.AddArgumentListArguments(
985+
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))),
986+
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString()))))))
987+
.WithOpenBracketToken(Token(TriviaList(
988+
Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> is changing.</summary>"),
989+
Comment("/// <param name=\"oldValue\">The previous property value that is being replaced.</param>"),
990+
Comment("/// <param name=\"newValue\">The new property value being set.</param>"),
991+
Comment($"/// <remarks>This method is invoked right before the value of <see cref=\"{propertyInfo.PropertyName}\"/> is changed.</remarks>")), SyntaxKind.OpenBracketToken, TriviaList())))
888992
.WithSemicolonToken(Token(SyntaxKind.SemicolonToken));
889993

890994
// Construct the generated method as follows:
891995
//
892996
// /// <summary>Executes the logic for when <see cref="<PROPERTY_NAME>"/> ust changed.</summary>
997+
// /// <param name="value">The new property value that was set.</param>
998+
// /// <remarks>This method is invoked right after the value of <see cref="<PROPERTY_NAME>"/> is changed.</remarks>
893999
// [global::System.CodeDom.Compiler.GeneratedCode("...", "...")]
8941000
// partial void On<PROPERTY_NAME>Changed(<PROPERTY_TYPE> value);
8951001
MemberDeclarationSyntax onPropertyChangedDeclaration =
@@ -902,10 +1008,44 @@ public static ImmutableArray<MemberDeclarationSyntax> GetOnPropertyChangeMethods
9021008
.AddArgumentListArguments(
9031009
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))),
9041010
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString()))))))
905-
.WithOpenBracketToken(Token(TriviaList(Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> just changed.</summary>")), SyntaxKind.OpenBracketToken, TriviaList())))
1011+
.WithOpenBracketToken(Token(TriviaList(
1012+
Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> just changed.</summary>"),
1013+
Comment("/// <param name=\"value\">The new property value that was set.</param>"),
1014+
Comment($"/// <remarks>This method is invoked right after the value of <see cref=\"{propertyInfo.PropertyName}\"/> is changed.</remarks>")), SyntaxKind.OpenBracketToken, TriviaList())))
1015+
.WithSemicolonToken(Token(SyntaxKind.SemicolonToken));
1016+
1017+
// Construct the generated method as follows:
1018+
//
1019+
// /// <summary>Executes the logic for when <see cref="<PROPERTY_NAME>"/> ust changed.</summary>
1020+
// /// <param name="oldValue">The previous property value that was replaced.</param>
1021+
// /// <param name="newValue">The new property value that was set.</param>
1022+
// /// <remarks>This method is invoked right after the value of <see cref="<PROPERTY_NAME>"/> is changed.</remarks>
1023+
// [global::System.CodeDom.Compiler.GeneratedCode("...", "...")]
1024+
// partial void On<PROPERTY_NAME>Changed(<OLD_VALUE_TYPE> oldValue, <PROPERTY_TYPE> newValue);
1025+
MemberDeclarationSyntax onPropertyChanged2Declaration =
1026+
MethodDeclaration(PredefinedType(Token(SyntaxKind.VoidKeyword)), Identifier($"On{propertyInfo.PropertyName}Changed"))
1027+
.AddModifiers(Token(SyntaxKind.PartialKeyword))
1028+
.AddParameterListParameters(
1029+
Parameter(Identifier("oldValue")).WithType(oldValueTypeSyntax),
1030+
Parameter(Identifier("newValue")).WithType(parameterType))
1031+
.AddAttributeLists(
1032+
AttributeList(SingletonSeparatedList(
1033+
Attribute(IdentifierName("global::System.CodeDom.Compiler.GeneratedCode"))
1034+
.AddArgumentListArguments(
1035+
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).FullName))),
1036+
AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(typeof(ObservablePropertyGenerator).Assembly.GetName().Version.ToString()))))))
1037+
.WithOpenBracketToken(Token(TriviaList(
1038+
Comment($"/// <summary>Executes the logic for when <see cref=\"{propertyInfo.PropertyName}\"/> just changed.</summary>"),
1039+
Comment("/// <param name=\"oldValue\">The previous property value that was replaced.</param>"),
1040+
Comment("/// <param name=\"newValue\">The new property value that was set.</param>"),
1041+
Comment($"/// <remarks>This method is invoked right after the value of <see cref=\"{propertyInfo.PropertyName}\"/> is changed.</remarks>")), SyntaxKind.OpenBracketToken, TriviaList())))
9061042
.WithSemicolonToken(Token(SyntaxKind.SemicolonToken));
9071043

908-
return ImmutableArray.Create(onPropertyChangingDeclaration, onPropertyChangedDeclaration);
1044+
return ImmutableArray.Create(
1045+
onPropertyChangingDeclaration,
1046+
onPropertyChanging2Declaration,
1047+
onPropertyChangedDeclaration,
1048+
onPropertyChanged2Declaration);
9091049
}
9101050

9111051
/// <summary>

‎tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
</PropertyGroup>
1111
<ItemGroup>
1212
<Compile Include="$(MSBuildThisFileDirectory)Helpers\CSharpAnalyzerWithLanguageVersionTest{TAnalyzer}.cs" />
13+
<Compile Include="$(MSBuildThisFileDirectory)Test_SourceGeneratorsCodegen.cs" />
1314
<Compile Include="$(MSBuildThisFileDirectory)Test_SourceGeneratorsDiagnostics.cs" />
1415
</ItemGroup>
1516
</Project>

0 commit comments

Comments
 (0)