Merge pull request #970 from CommunityToolkit/dev/partial-property-fixes

Bug fixes to generated partial properties

Author: Sergio0694

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

@@ -23,6 +23,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
 /// <param name="NotifiedCommandNames">The sequence of commands to notify.</param>
 /// <param name="NotifyPropertyChangedRecipients">Whether or not the generated property also broadcasts changes.</param>
 /// <param name="NotifyDataErrorInfo">Whether or not the generated property also validates its value.</param>
+/// <param name="IsRequired">Whether or not the generated property should be marked as required.</param>
 /// <param name="IsOldPropertyValueDirectlyReferenced">Whether the old property value is being directly referenced.</param>
 /// <param name="IsReferenceTypeOrUnconstrainedTypeParameter">Indicates whether the property is of a reference type or an unconstrained type parameter.</param>
 /// <param name="IncludeMemberNotNullOnSetAccessor">Indicates whether to include nullability annotations on the setter.</param>
@@ -41,6 +42,7 @@ internal sealed record PropertyInfo(
     EquatableArray<string> NotifiedCommandNames,
     bool NotifyPropertyChangedRecipients,
     bool NotifyDataErrorInfo,
+    bool IsRequired,
     bool IsOldPropertyValueDirectlyReferenced,
     bool IsReferenceTypeOrUnconstrainedTypeParameter,
     bool IncludeMemberNotNullOnSetAccessor,

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;
         }
 
+        /// <summary>
+        /// Checks whether an input member is a required property.
+        /// </summary>
+        /// <param name="memberSymbol">The input <see cref="ISymbol"/> instance to process.</param>
+        /// <returns>Whether <paramref name="memberSymbol"/> is a required property.</returns>
+        private static bool GetIsRequiredProperty(ISymbol memberSymbol)
+        {
+#if ROSLYN_4_3_1_OR_GREATER
+            return memberSymbol is IPropertySymbol { IsRequired: true };
+#else
+            return false;
+#endif
+        }
+
         /// <summary>
         /// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
         /// </summary>
@@ -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:
             //
             // <XML_SUMMARY>
@@ -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);
         }
 
+        /// <summary>
+        /// Gets all modifiers that need to be added to a generated property.
+        /// </summary>
+        /// <param name="propertyInfo">The input <see cref="PropertyInfo"/> instance to process.</param>
+        /// <returns>The list of necessary modifiers for <paramref name="propertyInfo"/>.</returns>
+        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;
+        }
+
         /// <summary>
         /// Gets the <see cref="MemberDeclarationSyntax"/> instances for the <c>OnPropertyChanging</c> and <c>OnPropertyChanged</c> methods for the input field.
         /// </summary>

src/CommunityToolkit.Mvvm/ComponentModel/Attributes/NotifyPropertyChangedRecipientsAttribute.cs

@@ -7,7 +7,7 @@
 namespace CommunityToolkit.Mvvm.ComponentModel;
 
 /// <summary>
-/// An attribute that can be used to support <see cref="ObservablePropertyAttribute"/> in generated properties, when applied to fields
+/// An attribute that can be used to support <see cref="ObservablePropertyAttribute"/> in generated properties, when applied to fields and properties
 /// contained in a type that is either inheriting from <see cref="ObservableRecipient"/>, or annotated with <see cref="ObservableRecipientAttribute"/>.
 /// When this attribute is used, the generated property setter will also call <see cref="ObservableRecipient.Broadcast{T}(T, T, string?)"/>.
 /// 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;
 /// }
 /// </code>
 /// </para>
@@ -27,10 +27,10 @@ namespace CommunityToolkit.Mvvm.ComponentModel;
 /// <code>
 /// 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);
 ///     }
 /// }
 /// </code>
@@ -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.
 /// </para>
 /// </summary>
-[AttributeUsage(AttributeTargets.Field | AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
+/// <remarks>
+/// Just like <see cref="ObservablePropertyAttribute"/>, this attribute can also be used on fields as well.
+/// </remarks>
+[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
 public sealed class NotifyPropertyChangedRecipientsAttribute : Attribute
 {
 }

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; }

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 = """
+            // <auto-generated/>
+            #pragma warning disable
+            #nullable enable
+            namespace MyApp
+            {
+                /// <inheritdoc/>
+                partial class MyViewModel
+                {
+                    /// <inheritdoc/>
+                    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", <ASSEMBLY_VERSION>)]
+                    [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
+                    public required partial int Number
+                    {
+                        get => field;
+                        private set
+                        {
+                            if (!global::System.Collections.Generic.EqualityComparer<int>.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);
+                            }
+                        }
+                    }
+
+                    /// <summary>Executes the logic for when <see cref="Number"/> is changing.</summary>
+                    /// <param name="value">The new property value being set.</param>
+                    /// <remarks>This method is invoked right before the value of <see cref="Number"/> is changed.</remarks>
+                    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", <ASSEMBLY_VERSION>)]
+                    partial void OnNumberChanging(int value);
+                    /// <summary>Executes the logic for when <see cref="Number"/> is changing.</summary>
+                    /// <param name="oldValue">The previous property value that is being replaced.</param>
+                    /// <param name="newValue">The new property value being set.</param>
+                    /// <remarks>This method is invoked right before the value of <see cref="Number"/> is changed.</remarks>
+                    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", <ASSEMBLY_VERSION>)]
+                    partial void OnNumberChanging(int oldValue, int newValue);
+                    /// <summary>Executes the logic for when <see cref="Number"/> just changed.</summary>
+                    /// <param name="value">The new property value that was set.</param>
+                    /// <remarks>This method is invoked right after the value of <see cref="Number"/> is changed.</remarks>
+                    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", <ASSEMBLY_VERSION>)]
+                    partial void OnNumberChanged(int value);
+                    /// <summary>Executes the logic for when <see cref="Number"/> just changed.</summary>
+                    /// <param name="oldValue">The previous property value that was replaced.</param>
+                    /// <param name="newValue">The new property value that was set.</param>
+                    /// <remarks>This method is invoked right after the value of <see cref="Number"/> is changed.</remarks>
+                    [global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", <ASSEMBLY_VERSION>)]
+                    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()
     {