Merge pull request #250 from CommunityToolkit/dev/display-attribute-b…

…reaking-broadcast Fix [AlsoBroadcastChange] not working when followed by other attributes
CommunityToolkit · May 13, 2022 · 0fb704d · 0fb704d
2 parents 73e85b4 + 612a6ea
commit 0fb704d
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 7 deletions.
diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs
@@ -91,12 +91,19 @@ internal static class Execute
             {
                 // Gather dependent property and command names
                 if (TryGatherDependentPropertyChangedNames(fieldSymbol, attributeData, propertyChangedNames, builder) ||
-                    TryGatherDependentCommandNames(fieldSymbol, attributeData, notifiedCommandNames, builder) ||
-                    TryGetIsBroadcastingChanges(fieldSymbol, attributeData, builder, out alsoBroadcastChange))
+                    TryGatherDependentCommandNames(fieldSymbol, attributeData, notifiedCommandNames, builder))
                 {
                     continue;
                 }
 
+                // Check whether the property should also broadcast changes
+                if (TryGetIsBroadcastingChanges(fieldSymbol, attributeData, builder, out bool isBroadcastTargetValid))
+                {
+                    alsoBroadcastChange = isBroadcastTargetValid;
+
+                    continue;
+                }
+
                 // Track the current validation attribute, if applicable
                 if (attributeData.AttributeClass?.InheritsFromFullyQualifiedName("global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true)
                 {
@@ -332,21 +339,21 @@ bool IsCommandNameValidWithGeneratedMembers(string commandName)
         /// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
         /// <param name="attributeData">The <see cref="AttributeData"/> instance for <paramref name="fieldSymbol"/>.</param>
         /// <param name="diagnostics">The current collection of gathered diagnostics.</param>
-        /// <param name="alsoBroadcastChange">Whether or not the resulting property should also broadcast changes.</param>
+        /// <param name="isBroadcastTargetValid">Whether or not the the property is in a valid target that can broadcast changes.</param>
         /// <returns>Whether or not the generated property for <paramref name="fieldSymbol"/> used <c>[AlsoBroadcastChange]</c>.</returns>
         private static bool TryGetIsBroadcastingChanges(
             IFieldSymbol fieldSymbol,
             AttributeData attributeData,
             ImmutableArray<Diagnostic>.Builder diagnostics,
-            out bool alsoBroadcastChange)
+            out bool isBroadcastTargetValid)
         {
             if (attributeData.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoBroadcastChangeAttribute") == true)
             {
                 // If the containing type is valid, track it
                 if (fieldSymbol.ContainingType.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient") ||
                     fieldSymbol.ContainingType.HasOrInheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipientAttribute"))
                 {
-                    alsoBroadcastChange = true;
+                    isBroadcastTargetValid = true;
 
                     return true;
                 }
@@ -358,12 +365,12 @@ private static bool TryGetIsBroadcastingChanges(
                     fieldSymbol.ContainingType,
                     fieldSymbol.Name);
 
-                alsoBroadcastChange = false;
+                isBroadcastTargetValid = false;
 
                 return true;
             }
 
-            alsoBroadcastChange = false;
+            isBroadcastTargetValid = false;
 
             return false;
         }

diff --git a/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs b/tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs
@@ -608,6 +608,43 @@ public void Test_ObservableProperty_ModelWithCultureAwarePropertyName()
         CollectionAssert.AreEqual(new[] { nameof(model.InputFolder) }, propertyNames);
     }
 
+    // See https://github.com/CommunityToolkit/dotnet/issues/242
+    [TestMethod]
+    public void Test_ObservableProperty_ModelWithAlsoBroadcastChangeAndDisplayAttributeLast()
+    {
+        IMessenger messenger = new StrongReferenceMessenger();
+        ModelWithAlsoBroadcastChangeAndDisplayAttributeLast model = new(messenger);
+
+        List<string?> propertyNames = new();
+
+        model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
+
+        object newValue = new();
+        bool isMessageReceived = false;
+
+        messenger.Register<Test_ObservablePropertyAttribute, PropertyChangedMessage<object>>(this, (r, m) =>
+        {
+            if (m.Sender != model)
+            {
+                Assert.Fail();
+            }
+
+            if (m.NewValue != newValue)
+            {
+                Assert.Fail();
+            }
+
+            isMessageReceived = true;
+        });
+
+        model.SomeProperty = newValue;
+
+        Assert.AreEqual(model.SomeProperty, newValue);
+        Assert.IsTrue(isMessageReceived);
+
+        CollectionAssert.AreEqual(new[] { nameof(model.SomeProperty) }, propertyNames);
+    }
+
     public abstract partial class BaseViewModel : ObservableObject
     {
         public string? Content { get; set; }
@@ -991,4 +1028,13 @@ partial class ModelWithCultureAwarePropertyName
         [ObservableProperty]
         private int _inputFolder;
     }
+
+    [ObservableRecipient]
+    public sealed partial class ModelWithAlsoBroadcastChangeAndDisplayAttributeLast : ObservableValidator
+    {
+        [ObservableProperty]
+        [AlsoBroadcastChange]
+        [Display(Name = "Foo bar baz")]
+        private object? _someProperty;
+    }
 }