Improve AlsoNotify handling in ReactiveGenerator

ChrisPulman · ChrisPulman · commit 3c3b9c3dc7f0 · 2025-12-24T16:39:13.000Z
Refactors the logic for extracting AlsoNotify property names from [Reactive] attributes to be more robust, handling various constructor argument forms and fallback scenarios. Adds validation in test helpers to ensure generated code includes property change notifications for all AlsoNotify properties specified via nameof(). Updates test baselines to reflect improved code generation.
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveFromPartialWithAlsoNotify#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveFromPartialWithAlsoNotify#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_test4"/>
@@ -20,6 +19,7 @@ public int Test4
             set
             {
                 this.RaiseAndSetIfChanged(ref _test4, value);
+                this.RaisePropertyChanged(nameof(OtherNotifyProperty));
             }
         }
     }
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveProperiesWithAttributes#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveProperiesWithAttributes#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_test3"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveProperties#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactiveProperties#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_test1"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesCalledValue#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesCalledValue#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="this.value"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithAccess#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithAccess#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_test2"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithAttributesAccessAndInheritance#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithAttributesAccessAndInheritance#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_name"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithIdenticalClass#TestNs1.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithIdenticalClass#TestNs1.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs1
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_name"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithIdenticalClass#TestNs2.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithIdenticalClass#TestNs2.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs2
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_name"/>
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithInit#TestNs.TestVM.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithInit#TestNs.TestVM.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs
 {
-    
-    public partial class TestVM
+        public partial class TestVM
     {
         
         /// <inheritdoc cref="_mustBeSet"/>
@@ -25,4 +24,4 @@ public string MustBeSet
     }
 }
 #nullable restore
-#pragma warning restore
+#pragma warning restore
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass1.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass1.Properties.g.verified.cs
@@ -9,7 +9,6 @@ namespace TestNs1
 {
     public partial class TestViewModel3
 {
-
     public partial class TestInnerClass1
     {
         
@@ -41,4 +40,4 @@ public int TestInner11
 
 }
 #nullable restore
-#pragma warning restore
+#pragma warning restore
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass2+TestInnerClass3.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass2+TestInnerClass3.Properties.g.verified.cs
@@ -11,7 +11,6 @@ public partial class TestViewModel3
 {
 public partial class TestInnerClass2
 {
-
     public partial class TestInnerClass3
     {
         
@@ -44,4 +43,4 @@ public int TestInner33
 
 }
 #nullable restore
-#pragma warning restore
+#pragma warning restore
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass2.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3+TestInnerClass2.Properties.g.verified.cs
@@ -9,7 +9,6 @@ namespace TestNs1
 {
     public partial class TestViewModel3
 {
-
     public partial class TestInnerClass2
     {
         
@@ -41,4 +40,4 @@ public int TestInner22
 
 }
 #nullable restore
-#pragma warning restore
+#pragma warning restore
diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3.Properties.g.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVE/ReactiveGeneratorTests.FromReactivePropertiesWithNestedClass#TestNs1.TestViewModel3.Properties.g.verified.cs
@@ -7,8 +7,7 @@
 
 namespace TestNs1
 {
-    
-    public partial class TestViewModel3
+        public partial class TestViewModel3
     {
         
         /// <inheritdoc cref="_testVM3Property"/>
@@ -37,4 +36,4 @@ public float TestVM3Property2
     }
 }
 #nullable restore
-#pragma warning restore
+#pragma warning restore
diff --git a/src/ReactiveUI.SourceGenerator.Tests/TestHelper.cs b/src/ReactiveUI.SourceGenerator.Tests/TestHelper.cs
@@ -3,6 +3,7 @@
 // The ReactiveUI and contributors licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for full license information.
 
+using System.Text.RegularExpressions;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 
@@ -26,8 +27,8 @@ namespace ReactiveUI.SourceGenerator.Tests;
 /// It provides utilities to initialize dependencies, run generators, and verify the output.
 /// </summary>
 /// <typeparam name="T">Type of Incremental Generator.</typeparam>
-/// <seealso cref="System.IDisposable" />
-public sealed class TestHelper<T> : IDisposable
+/// <seealso cref="IDisposable" />
+public sealed partial class TestHelper<T> : IDisposable
         where T : IIncrementalGenerator, new()
 {
     /// <summary>
@@ -209,7 +210,7 @@ public SettingsTask RunGeneratorAndCheck(
         if (rerunCompilation)
         {
             // Run the generator and capture diagnostics.
-            var rerunDriver = driver.RunGeneratorsAndUpdateCompilation(compilation, out _, out var diagnostics);
+            var rerunDriver = driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics);
 
             // If any warnings or errors are found, log them to the test output before throwing an exception.
             var offendingDiagnostics = diagnostics
@@ -226,12 +227,89 @@ public SettingsTask RunGeneratorAndCheck(
                 throw new InvalidOperationException("Compilation failed due to the above diagnostics.");
             }
 
+            // Validate generated code contains expected features
+            ValidateGeneratedCode(code, rerunDriver);
+
             return VerifyGenerator(rerunDriver);
         }
 
         // If rerun is not needed, simply run the generator.
         return VerifyGenerator(driver.RunGenerators(compilation));
     }
 
+    [GeneratedRegex(@"\[Reactive\((?:.*?nameof\((\w+)\))+", RegexOptions.Singleline)]
+    private static partial Regex ReactiveRegex();
+
+    [GeneratedRegex(@"nameof\((\w+)\)")]
+    private static partial Regex NameOfRegex();
+
+    /// <summary>
+    /// Validates that generated code contains expected features based on the source code attributes.
+    /// </summary>
+    /// <param name="sourceCode">The original source code.</param>
+    /// <param name="driver">The generator driver with generated output.</param>
+    private static void ValidateGeneratedCode(string sourceCode, GeneratorDriver driver)
+    {
+        var runResult = driver.GetRunResult();
+        var generatedTrees = runResult.Results.SelectMany(r => r.GeneratedSources).ToList();
+        var allGeneratedCode = string.Join("\n", generatedTrees.Select(t => t.SourceText.ToString()));
+
+        // Check for AlsoNotify feature in Reactive attributes
+        // Pattern matches: [Reactive(nameof(PropertyName))] or [Reactive(nameof(Prop1), nameof(Prop2))]
+        var alsoNotifyPattern = ReactiveRegex();
+        var nameofPattern = NameOfRegex();
+        var matches = alsoNotifyPattern.Matches(sourceCode);
+
+        TestContext.Out.WriteLine("=== VALIDATION DEBUG ===");
+        TestContext.Out.WriteLine("Found {0} Reactive attributes with nameof", matches.Count);
+
+        if (matches.Count > 0)
+        {
+            foreach (Match match in matches)
+            {
+                TestContext.Out.WriteLine("Checking attribute: {0}", match.Value);
+
+                // Extract all nameof() references within this attribute
+                var nameofMatches = nameofPattern.Matches(match.Value);
+                TestContext.Out.WriteLine("Found {0} nameof references in this attribute", nameofMatches.Count);
+
+                foreach (Match nameofMatch in nameofMatches)
+                {
+                    var propertyToNotify = nameofMatch.Groups[1].Value;
+                    TestContext.Out.WriteLine("Checking for notification of property: {0}", propertyToNotify);
+
+                    // Verify that the generated code contains calls to raise property changed for the additional property
+                    // Check for various forms of property change notification
+                    var hasNotification =
+                        allGeneratedCode.Contains($"this.RaisePropertyChanged(nameof({propertyToNotify}))") ||
+                        allGeneratedCode.Contains($"this.RaisePropertyChanged(\"{propertyToNotify}\")") ||
+                        allGeneratedCode.Contains($"RaisePropertyChanged(nameof({propertyToNotify}))") ||
+                        allGeneratedCode.Contains($"RaisePropertyChanged(\"{propertyToNotify}\")");
+
+                    TestContext.Out.WriteLine("Has notification: {0}", hasNotification);
+
+                    if (!hasNotification)
+                    {
+                        var errorMessage = $"Generated code does not include AlsoNotify for property '{propertyToNotify}'. " +
+                                         $"Expected to find property change notification for '{propertyToNotify}' in the generated code.\n" +
+                                         $"Source attribute: {match.Value}";
+
+                        TestContext.Out.WriteLine("=== VALIDATION FAILURE ===");
+                        TestContext.Out.WriteLine(errorMessage);
+                        TestContext.Out.WriteLine("=== SOURCE CODE SNIPPET ===");
+                        TestContext.Out.WriteLine(match.Value);
+                        TestContext.Out.WriteLine("=== GENERATED CODE ===");
+                        TestContext.Out.WriteLine(allGeneratedCode);
+                        TestContext.Out.WriteLine("=== END ===");
+
+                        throw new InvalidOperationException(errorMessage);
+                    }
+                }
+            }
+        }
+
+        TestContext.Out.WriteLine("=== END VALIDATION DEBUG ===");
+    }
+
     private SettingsTask VerifyGenerator(GeneratorDriver driver) => Verify(driver).UseDirectory(VerifiedFilePath()).ScrubLinesContaining("[global::System.CodeDom.Compiler.GeneratedCode(\"");
 }
diff --git a/src/ReactiveUI.SourceGenerators.Roslyn/Reactive/ReactiveGenerator.Execute.cs b/src/ReactiveUI.SourceGenerators.Roslyn/Reactive/ReactiveGenerator.Execute.cs

Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,7 @@`
`7`	`7`
`8`	`8`	`namespace TestNs`
`9`	`9`	`{`
`10`		`-`
`11`		`- public partial class TestVM`
	`10`	`+ public partial class TestVM`
`12`	`11`	`{`
`13`	`12`
`14`	`13`	`/// <inheritdoc cref="_test4"/>`
`@@ -20,6 +19,7 @@ public int Test4`
`20`	`19`	`set`
`21`	`20`	`{`
`22`	`21`	`this.RaiseAndSetIfChanged(ref _test4, value);`
	`22`	`+ this.RaisePropertyChanged(nameof(OtherNotifyProperty));`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,7 @@`
`7`	`7`
`8`	`8`	`namespace TestNs1`
`9`	`9`	`{`
`10`		`-`
`11`		`- public partial class TestVM`
	`10`	`+ public partial class TestVM`
`12`	`11`	`{`
`13`	`12`
`14`	`13`	`/// <inheritdoc cref="_name"/>`
Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,7 @@`
`7`	`7`
`8`	`8`	`namespace TestNs2`
`9`	`9`	`{`
`10`		`-`
`11`		`- public partial class TestVM`
	`10`	`+ public partial class TestVM`
`12`	`11`	`{`
`13`	`12`
`14`	`13`	`/// <inheritdoc cref="_name"/>`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,6 @@ namespace TestNs1`
`9`	`9`	`{`
`10`	`10`	`public partial class TestViewModel3`
`11`	`11`	`{`
`12`		`-`
`13`	`12`	`public partial class TestInnerClass1`
`14`	`13`	`{`
`15`	`14`
`@@ -41,4 +40,4 @@ public int TestInner11`
`41`	`40`
`42`	`41`	`}`
`43`	`42`	`#nullable restore`
`44`		`-#pragma warning restore`
	`43`	`+#pragma warning restore`