Merge pull request #202 from CommunityToolkit/dev/fix-target-member-diagnostics

Fix incorrect MVVMTK0015 with inherited members

Author: Sergio0694

CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs

@@ -187,21 +187,19 @@ private static bool TryGatherDependentPropertyChangedNames(
             // Validates a property name using existing properties
             bool IsPropertyNameValid(string propertyName)
             {
-                return fieldSymbol.ContainingType.GetMembers(propertyName).OfType<IPropertySymbol>().Any();
+                return fieldSymbol.ContainingType.GetAllMembers(propertyName).OfType<IPropertySymbol>().Any();
             }
 
             // Validate a property name including generated properties too
             bool IsPropertyNameValidWithGeneratedMembers(string propertyName)
             {
-                foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
+                foreach (ISymbol member in fieldSymbol.ContainingType.GetAllMembers())
                 {
                     if (member is IFieldSymbol otherFieldSymbol &&
                         !SymbolEqualityComparer.Default.Equals(fieldSymbol, otherFieldSymbol) &&
                         otherFieldSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") &&
                         propertyName == GetGeneratedPropertyName(otherFieldSymbol))
                     {
-                        propertyChangedNames.Add(propertyName);
-
                         return true;
                     }
                 }
@@ -256,7 +254,7 @@ bool IsCommandNameValid(string commandName, out bool shouldLookForGeneratedMembe
             {
                 // Each target must be a string matching the name of a property from the containing type of the annotated field, and the
                 // property must be of type IRelayCommand, or any type that implements that interface (to avoid generating invalid code).
-                if (fieldSymbol.ContainingType.GetMembers(commandName).OfType<IPropertySymbol>().FirstOrDefault() is IPropertySymbol propertySymbol)
+                if (fieldSymbol.ContainingType.GetAllMembers(commandName).OfType<IPropertySymbol>().FirstOrDefault() is IPropertySymbol propertySymbol)
                 {
                     // If there is a property member with the specified name, check that it's valid. If it isn't, the
                     // target is definitely not valid, and the additional checks below can just be skipped. The property
@@ -285,7 +283,7 @@ bool IsCommandNameValid(string commandName, out bool shouldLookForGeneratedMembe
             // Validate a command name including generated command too
             bool IsCommandNameValidWithGeneratedMembers(string commandName)
             {
-                foreach (ISymbol member in fieldSymbol.ContainingType.GetMembers())
+                foreach (ISymbol member in fieldSymbol.ContainingType.GetAllMembers())
                 {
                     if (member is IMethodSymbol methodSymbol &&
                         methodSymbol.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.Input.ICommandAttribute") &&

CommunityToolkit.Mvvm.SourceGenerators/Extensions/INamedTypeSymbolExtensions.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using System.Text;
 using Microsoft.CodeAnalysis;
 
@@ -39,4 +40,37 @@ static StringBuilder BuildFrom(ISymbol? symbol, StringBuilder builder)
         // those characters at the moment, see: https://github.com/dotnet/roslyn/issues/58476.
         return BuildFrom(symbol, new StringBuilder(256)).ToString().Replace('`', '-').Replace('+', '.');
     }
+
+    /// <summary>
+    /// Gets all member symbols from a given <see cref="INamedTypeSymbol"/> instance, including inherited ones.
+    /// </summary>
+    /// <param name="symbol">The input <see cref="INamedTypeSymbol"/> instance.</param>
+    /// <returns>A sequence of all member symbols for <paramref name="symbol"/>.</returns>
+    public static IEnumerable<ISymbol> GetAllMembers(this INamedTypeSymbol symbol)
+    {
+        for (INamedTypeSymbol? currentSymbol = symbol; currentSymbol is { SpecialType: not SpecialType.System_Object }; currentSymbol = currentSymbol.BaseType)
+        {
+            foreach (ISymbol memberSymbol in currentSymbol.GetMembers())
+            {
+                yield return memberSymbol;
+            }
+        }
+    }
+
+    /// <summary>
+    /// Gets all member symbols from a given <see cref="INamedTypeSymbol"/> instance, including inherited ones.
+    /// </summary>
+    /// <param name="symbol">The input <see cref="INamedTypeSymbol"/> instance.</param>
+    /// <param name="name">The name of the members to look for.</param>
+    /// <returns>A sequence of all member symbols for <paramref name="symbol"/>.</returns>
+    public static IEnumerable<ISymbol> GetAllMembers(this INamedTypeSymbol symbol, string name)
+    {
+        for (INamedTypeSymbol? currentSymbol = symbol; currentSymbol is not null; currentSymbol = currentSymbol.BaseType)
+        {
+            foreach (ISymbol memberSymbol in currentSymbol.GetMembers(name))
+            {
+                yield return memberSymbol;
+            }
+        }
+    }
 }

tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs

@@ -515,6 +515,75 @@ public void Test_ObservableProperty_NullabilityAnnotations_Complex()
     }
 #endif
 
+    // See https://github.com/CommunityToolkit/dotnet/issues/201
+    [TestMethod]
+    public void Test_ObservableProperty_InheritedMembersAsAttributeTargets()
+    {
+        ConcreteViewModel model = new();
+
+        List<string?> propertyNames = new();
+        List<object?> canExecuteChangedArgs = new();
+
+        model.PropertyChanged += (s, e) => propertyNames.Add(e.PropertyName);
+        model.DoSomethingCommand.CanExecuteChanged += (s, _) => canExecuteChangedArgs.Add(s);
+        model.ManualCommand.CanExecuteChanged += (s, _) => canExecuteChangedArgs.Add(s);
+
+        model.A = nameof(model.A);
+        model.B = nameof(model.B);
+        model.C = nameof(model.C);
+        model.D = nameof(model.D);
+
+        CollectionAssert.AreEqual(new[]
+        {
+            nameof(model.A),
+            nameof(model.Content),
+            nameof(model.B),
+            nameof(model.SomeGeneratedProperty),
+            nameof(model.C),
+            nameof(model.D)
+        }, propertyNames);
+
+        CollectionAssert.AreEqual(new[] { model.DoSomethingCommand, model.ManualCommand }, canExecuteChangedArgs);
+    }
+
+    public abstract partial class BaseViewModel : ObservableObject
+    {
+        public string? Content { get; set; }
+
+        [ObservableProperty]
+        private string? someGeneratedProperty;
+
+        [ICommand]
+        private void DoSomething()
+        {
+        }
+
+        public IRelayCommand ManualCommand { get; } = new RelayCommand(() => { });
+    }
+
+    public partial class ConcreteViewModel : BaseViewModel
+    {
+        // Inherited property
+        [ObservableProperty]
+        [AlsoNotifyChangeFor(nameof(Content))]
+        private string? _a;
+
+        // Inherited generated property
+        [ObservableProperty]
+        [AlsoNotifyChangeFor(nameof(SomeGeneratedProperty))]
+        private string? _b;
+
+        // Inherited generated command
+        [ObservableProperty]
+        [AlsoNotifyCanExecuteFor(nameof(DoSomethingCommand))]
+        private string? _c;
+
+        // Inherited manual command
+        [ObservableProperty]
+        [AlsoNotifyCanExecuteFor(nameof(ManualCommand))]
+        private string? _d;
+    }
+
     public partial class SampleModel : ObservableObject
     {
         /// <summary>