Merge pull request #4219 from Sergio0694/bugfix/value-named-generated…

…-property Fixed [ObservableProperty] for fields named "value"
CommunityToolkit · Sep 2, 2021 · e4b8a68 · e4b8a68
2 parents 166e101 + 8d0d473
commit e4b8a68
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 6 deletions.
diff --git a/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs b/Microsoft.Toolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs
@@ -240,6 +240,14 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
                 }
             }
 
+            // In case the backing field is exactly named "value", we need to add the "this." prefix to ensure that comparisons and assignments
+            // with it in the generated setter body are executed correctly and without conflicts with the implicit value parameter.
+            ExpressionSyntax fieldExpression = fieldSymbol.Name switch
+            {
+                "value" => MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ThisExpression(), IdentifierName("value")),
+                string name => IdentifierName(name)
+            };
+
             BlockSyntax setterBlock;
 
             if (validationAttributes.Count > 0)
@@ -263,10 +271,10 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
 
                     // Generate the inner setter block as follows:
                     //
-                    // if (!global::System.Collections.Generic.EqualityComparer<<FIELD_TYPE>>.Default.Equals(<FIELD_NAME>, value))
+                    // if (!global::System.Collections.Generic.EqualityComparer<<FIELD_TYPE>>.Default.Equals(this.<FIELD_NAME>, value))
                     // {
                     //     OnPropertyChanging(global::Microsoft.Toolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedOrChangingArgs.PropertyNamePropertyChangingEventArgs); // Optional
-                    //     <FIELD_NAME> = value;
+                    //     this.<FIELD_NAME> = value;
                     //     OnPropertyChanged(global::Microsoft.Toolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedOrChangingArgs.PropertyNamePropertyChangedEventArgs);
                     //     ValidateProperty(value, <PROPERTY_NAME>);
                     //     OnPropertyChanged(global::Microsoft.Toolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedOrChangingArgs.Property1PropertyChangedEventArgs); // Optional
@@ -291,7 +299,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
                                             IdentifierName("Default")),
                                         IdentifierName("Equals")))
                                 .AddArgumentListArguments(
-                                    Argument(IdentifierName(fieldSymbol.Name)),
+                                    Argument(fieldExpression),
                                     Argument(IdentifierName("value")))),
                             Block(
                                 ExpressionStatement(
@@ -303,7 +311,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
                                 ExpressionStatement(
                                     AssignmentExpression(
                                         SyntaxKind.SimpleAssignmentExpression,
-                                        IdentifierName(fieldSymbol.Name),
+                                        fieldExpression,
                                         IdentifierName("value"))),
                                 ExpressionStatement(
                                     InvocationExpression(IdentifierName("OnPropertyChanged"))
@@ -346,7 +354,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
                     ExpressionStatement(
                         AssignmentExpression(
                             SyntaxKind.SimpleAssignmentExpression,
-                            IdentifierName(fieldSymbol.Name),
+                            fieldExpression,
                             IdentifierName("value"))),
                     ExpressionStatement(
                         InvocationExpression(IdentifierName("OnPropertyChanged"))
@@ -384,7 +392,7 @@ private static PropertyDeclarationSyntax CreatePropertyDeclaration(
                                         IdentifierName("Default")),
                                     IdentifierName("Equals")))
                             .AddArgumentListArguments(
-                                    Argument(IdentifierName(fieldSymbol.Name)),
+                                    Argument(fieldExpression),
                                     Argument(IdentifierName("value")))),
                         updateAndNotificationBlock));
             }

diff --git a/UnitTests/UnitTests.NetCore/Mvvm/Test_ObservablePropertyAttribute.cs b/UnitTests/UnitTests.NetCore/Mvvm/Test_ObservablePropertyAttribute.cs
@@ -119,6 +119,42 @@ public void Test_ValidationAttributes()
             Assert.AreEqual(testAttribute.Animal, Animal.Llama);
         }
 
+        // See https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/4216
+        [TestCategory("Mvvm")]
+        [TestMethod]
+        public void Test_ObservablePropertyWithValueNamedField()
+        {
+            var model = new ModelWithValueProperty();
+
+            List<string?> propertyNames = new();
+
+            model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
+
+            model.Value = "Hello world";
+
+            Assert.AreEqual(model.Value, "Hello world");
+
+            CollectionAssert.AreEqual(new[] { nameof(model.Value) }, propertyNames);
+        }
+
+        // See https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/4216
+        [TestCategory("Mvvm")]
+        [TestMethod]
+        public void Test_ObservablePropertyWithValueNamedField_WithValidationAttributes()
+        {
+            var model = new ModelWithValuePropertyWithValidation();
+
+            List<string?> propertyNames = new();
+
+            model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
+
+            model.Value = "Hello world";
+
+            Assert.AreEqual(model.Value, "Hello world");
+
+            CollectionAssert.AreEqual(new[] { nameof(model.Value) }, propertyNames);
+        }
+
         public partial class SampleModel : ObservableObject
         {
             /// <summary>
@@ -195,5 +231,19 @@ public enum Animal
             Dog,
             Llama
         }
+
+        public partial class ModelWithValueProperty : ObservableObject
+        {
+            [ObservableProperty]
+            private string value;
+        }
+
+        public partial class ModelWithValuePropertyWithValidation : ObservableValidator
+        {
+            [ObservableProperty]
+            [Required]
+            [MinLength(5)]
+            private string value;
+        }
     }
 }