From a87344925ce72be01b1c2dbd05594500f1beebd7 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 28 Oct 2024 09:09:00 -0700 Subject: [PATCH 1/3] Fix targets for '[NotifyPropertyChangedRecipients]' --- .../NotifyPropertyChangedRecipientsAttribute.cs | 15 +++++++++------ .../Attributes/ObservablePropertyAttribute.cs | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/NotifyPropertyChangedRecipientsAttribute.cs b/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/NotifyPropertyChangedRecipientsAttribute.cs index f387acd8e..d758bbe17 100644 --- a/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/NotifyPropertyChangedRecipientsAttribute.cs +++ b/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/NotifyPropertyChangedRecipientsAttribute.cs @@ -7,7 +7,7 @@ namespace CommunityToolkit.Mvvm.ComponentModel; /// -/// An attribute that can be used to support in generated properties, when applied to fields +/// An attribute that can be used to support in generated properties, when applied to fields and properties /// contained in a type that is either inheriting from , or annotated with . /// When this attribute is used, the generated property setter will also call . /// This allows generated properties to opt-in into broadcasting behavior without having to fallback into a full explicit observable property. @@ -18,7 +18,7 @@ namespace CommunityToolkit.Mvvm.ComponentModel; /// { /// [ObservableProperty] /// [NotifyPropertyChangedRecipients] -/// private string username; +/// public partial string Username; /// } /// /// @@ -27,10 +27,10 @@ namespace CommunityToolkit.Mvvm.ComponentModel; /// /// partial class MyViewModel /// { -/// public string Username +/// public partial string Username /// { -/// get => username; -/// set => SetProperty(ref username, value, broadcast: true); +/// get => field; +/// set => SetProperty(ref field, value, broadcast: true); /// } /// } /// @@ -39,7 +39,10 @@ namespace CommunityToolkit.Mvvm.ComponentModel; /// This attribute can also be added to a class, and if so it will affect all generated properties in that type and inherited types. /// /// -[AttributeUsage(AttributeTargets.Field | AttributeTargets.Class, AllowMultiple = false, Inherited = false)] +/// +/// Just like , this attribute can also be used on fields as well. +/// +[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Class, AllowMultiple = false, Inherited = false)] public sealed class NotifyPropertyChangedRecipientsAttribute : Attribute { } diff --git a/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/ObservablePropertyAttribute.cs b/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/ObservablePropertyAttribute.cs index c6ae78763..83311270c 100644 --- a/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/ObservablePropertyAttribute.cs +++ b/src/CommunityToolkit.Mvvm/ComponentModel/Attributes/ObservablePropertyAttribute.cs @@ -20,7 +20,7 @@ namespace CommunityToolkit.Mvvm.ComponentModel; /// partial class MyViewModel : ObservableObject /// { /// [ObservableProperty] -/// public partial string name { get; set; } +/// public partial string Name { get; set; } /// /// [ObservableProperty] /// public partial bool IsEnabled { get; set; } From e82074f29002965b4fccac76ecec0b0860c348e7 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 28 Oct 2024 09:19:23 -0700 Subject: [PATCH 2/3] Support required properties in codegen --- .../ComponentModel/Models/PropertyInfo.cs | 2 + .../ObservablePropertyGenerator.Execute.cs | 53 ++++++++++++++++--- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs index 367d9a751..c2c8feed6 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs @@ -23,6 +23,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 or not the generated property should be marked as required. /// Whether the old property value is being directly referenced. /// Indicates whether the property is of a reference type or an unconstrained type parameter. /// Indicates whether to include nullability annotations on the setter. @@ -41,6 +42,7 @@ internal sealed record PropertyInfo( EquatableArray NotifiedCommandNames, bool NotifyPropertyChangedRecipients, bool NotifyDataErrorInfo, + bool IsRequired, bool IsOldPropertyValueDirectlyReferenced, bool IsReferenceTypeOrUnconstrainedTypeParameter, bool IncludeMemberNotNullOnSetAccessor, diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs index ce3c5d98c..751c4bbc6 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs @@ -348,6 +348,11 @@ public static bool TryGetInfo( token.ThrowIfCancellationRequested(); + // Check whether the property should be required + bool isRequired = GetIsRequiredProperty(memberSymbol); + + token.ThrowIfCancellationRequested(); + propertyInfo = new PropertyInfo( memberSyntax.Kind(), typeNameWithNullabilityAnnotations, @@ -361,6 +366,7 @@ public static bool TryGetInfo( notifiedCommandNames.ToImmutable(), notifyRecipients, notifyDataErrorInfo, + isRequired, isOldPropertyValueDirectlyReferenced, isReferenceTypeOrUnconstrainedTypeParameter, includeMemberNotNullOnSetAccessor, @@ -1028,6 +1034,20 @@ private static bool TryGetAccessibilityModifiers( return true; } + /// + /// Checks whether an input member is a required property. + /// + /// The input instance to process. + /// Whether is a required property. + private static bool GetIsRequiredProperty(ISymbol memberSymbol) + { +#if ROSLYN_4_3_1_OR_GREATER + return memberSymbol is IPropertySymbol { IsRequired: true }; +#else + return false; +#endif + } + /// /// Gets a instance with the cached args for property changing notifications. /// @@ -1324,11 +1344,6 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf // Also add any forwarded attributes setAccessor = setAccessor.AddAttributeLists(forwardedSetAccessorAttributes); - // Prepare the modifiers for the property - SyntaxTokenList propertyModifiers = propertyInfo.AnnotatedMemberKind is SyntaxKind.PropertyDeclaration - ? propertyInfo.PropertyAccessibility.ToSyntaxTokenList().Add(Token(SyntaxKind.PartialKeyword)) - : propertyInfo.PropertyAccessibility.ToSyntaxTokenList(); - // Construct the generated property as follows: // // @@ -1352,7 +1367,7 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf .WithOpenBracketToken(Token(TriviaList(Comment(xmlSummary)), SyntaxKind.OpenBracketToken, TriviaList())), AttributeList(SingletonSeparatedList(Attribute(IdentifierName("global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage"))))) .AddAttributeLists(forwardedPropertyAttributes) - .WithModifiers(propertyModifiers) + .WithModifiers(GetPropertyModifiers(propertyInfo)) .AddAccessorListAccessors( AccessorDeclaration(SyntaxKind.GetAccessorDeclaration) .WithModifiers(propertyInfo.GetterAccessibility.ToSyntaxTokenList()) @@ -1362,6 +1377,32 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf setAccessor); } + /// + /// Gets all modifiers that need to be added to a generated property. + /// + /// The input instance to process. + /// The list of necessary modifiers for . + private static SyntaxTokenList GetPropertyModifiers(PropertyInfo propertyInfo) + { + SyntaxTokenList propertyModifiers = propertyInfo.PropertyAccessibility.ToSyntaxTokenList(); + +#if ROSLYN_4_3_1_OR_GREATER + // Add the 'required' modifier if the original member also had it + if (propertyInfo.IsRequired) + { + propertyModifiers = propertyModifiers.Add(Token(SyntaxKind.RequiredKeyword)); + } +#endif + + // Add the 'partial' modifier if the original member is a partial property + if (propertyInfo.AnnotatedMemberKind is SyntaxKind.PropertyDeclaration) + { + propertyModifiers = propertyModifiers.Add(Token(SyntaxKind.PartialKeyword)); + } + + return propertyModifiers; + } + /// /// Gets the instances for the OnPropertyChanging and OnPropertyChanged methods for the input field. /// From 73e7a88a4bd18ee88762742cb63fdbd8f1d16442 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 28 Oct 2024 09:19:31 -0700 Subject: [PATCH 3/3] Add unit test for required properties --- .../Test_SourceGeneratorsCodegen.cs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn4110.UnitTests/Test_SourceGeneratorsCodegen.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn4110.UnitTests/Test_SourceGeneratorsCodegen.cs index 658d20aeb..9d7092888 100644 --- a/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn4110.UnitTests/Test_SourceGeneratorsCodegen.cs +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn4110.UnitTests/Test_SourceGeneratorsCodegen.cs @@ -92,6 +92,82 @@ partial int Number VerifyGenerateSources(source, new[] { new ObservablePropertyGenerator() }, LanguageVersion.Preview, ("MyApp.MyViewModel.g.cs", result)); } + // See https://github.com/CommunityToolkit/dotnet/issues/969 + [TestMethod] + public void ObservablePropertyWithValueType_OnPartialProperty_RequiredProperty_WorksCorrectly() + { + string source = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + namespace MyApp; + + partial class MyViewModel : ObservableObject + { + [ObservableProperty] + public required partial int Number { get; private set; } + } + """; + + string result = """ + // + #pragma warning disable + #nullable enable + namespace MyApp + { + /// + partial class MyViewModel + { + /// + [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", )] + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + public required partial int Number + { + get => field; + private set + { + if (!global::System.Collections.Generic.EqualityComparer.Default.Equals(field, value)) + { + OnNumberChanging(value); + OnNumberChanging(default, value); + OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Number); + field = value; + OnNumberChanged(value); + OnNumberChanged(default, value); + OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Number); + } + } + } + + /// 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", )] + partial void OnNumberChanging(int 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", )] + partial void OnNumberChanging(int oldValue, int 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", )] + partial void OnNumberChanged(int 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", )] + partial void OnNumberChanged(int oldValue, int newValue); + } + } + """; + + VerifyGenerateSources(source, new[] { new ObservablePropertyGenerator() }, LanguageVersion.Preview, ("MyApp.MyViewModel.g.cs", result)); + } + [TestMethod] public void ObservablePropertyWithValueType_OnPartialProperty_WithExplicitModifiers_WorksCorrectly1() {