Merge pull request #248 from CommunityToolkit/dev/block-conflicting-generated-properties

Block [ObservableProperty] on properties causing conflicts

Author: Sergio0694

CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md

@@ -30,3 +30,4 @@ MVVMTK0020 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
 MVVMTK0021 | CommunityToolkit.Mvvm.SourceGenerators.ObservableRecipientGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0022 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0023 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0024 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error

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

@@ -71,6 +71,20 @@ internal static class Execute
                 return null;
             }
 
+            // Check for special cases that are explicitly not allowed
+            if (IsGeneratedPropertyInvalid(propertyName, fieldSymbol.Type))
+            {
+                builder.Add(
+                    InvalidObservablePropertyError,
+                    fieldSymbol,
+                    fieldSymbol.ContainingType,
+                    fieldSymbol.Name);
+
+                diagnostics = builder.ToImmutable();
+
+                return null;
+            }
+
             ImmutableArray<string>.Builder propertyChangedNames = ImmutableArray.CreateBuilder<string>();
             ImmutableArray<string>.Builder propertyChangingNames = ImmutableArray.CreateBuilder<string>();
             ImmutableArray<string>.Builder notifiedCommandNames = ImmutableArray.CreateBuilder<string>();
@@ -178,6 +192,29 @@ private static bool IsTargetTypeValid(
             return isObservableObject || hasObservableObjectAttribute || hasINotifyPropertyChangedAttribute;
         }
 
+        /// <summary>
+        /// Checks whether the generated property would be a special case that is marked as invalid.
+        /// </summary>
+        /// <param name="propertyName">The property name.</param>
+        /// <param name="propertyType">The property type.</param>
+        /// <returns>Whether the generated property is invalid.</returns>
+        private static bool IsGeneratedPropertyInvalid(string propertyName, ITypeSymbol propertyType)
+        {
+            // If the generated property name is called "Property" and the type is either object or it is PropertyChangedEventArgs or
+            // PropertyChangingEventArgs (or a type derived from either of those two types), consider it invalid. This is needed because
+            // if such a property was generated, the partial On<PROPERTY_NAME>Changing and OnPropertyChanging(PropertyChangingEventArgs)
+            // methods, as well as the partial On<PROPERTY_NAME>Changed and OnPropertyChanged(PropertyChangedEventArgs) methods.
+            if (propertyName == "Property")
+            {
+                return
+                    propertyType.SpecialType == SpecialType.System_Object ||
+                    propertyType.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.PropertyChangedEventArgs") ||
+                    propertyType.HasOrInheritsFromFullyQualifiedName("global::System.ComponentModel.PropertyChangingEventArgs");
+            }
+
+            return false;
+        }
+
         /// <summary>
         /// Tries to gather dependent properties from the given attribute.
         /// </summary>

CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs

@@ -379,4 +379,20 @@ internal static class DiagnosticDescriptors
         isEnabledByDefault: true,
         description: "Methods with multiple overloads cannot be annotated with [ICommand], as command methods must be unique within their containing type.",
         helpLinkUri: "https://aka.ms/mvvmtoolkit");
+
+    /// <summary>
+    /// Gets a <see cref="DiagnosticDescriptor"/> indicating when a generated property created with <c>[ObservableProperty]</c> would cause conflicts with other generated members.
+    /// <para>
+    /// Format: <c>"The field {0}.{1} cannot be used to generate an observable property, as its name or type would cause conflicts with other generated members"</c>.
+    /// </para>
+    /// </summary>
+    public static readonly DiagnosticDescriptor InvalidObservablePropertyError = new DiagnosticDescriptor(
+        id: "MVVMTK0024",
+        title: "Invalid generated property declaration",
+        messageFormat: "The field {0}.{1} cannot be used to generate an observable property, as its name or type would cause conflicts with other generated members",
+        category: typeof(ObservablePropertyGenerator).FullName,
+        defaultSeverity: DiagnosticSeverity.Error,
+        isEnabledByDefault: true,
+        description: "The fields annotated with [ObservableProperty] cannot result in a property name or have a type that would cause conflicts with other generated members.",
+        helpLinkUri: "https://aka.ms/mvvmtoolkit");
 }

CommunityToolkit.Mvvm.SourceGenerators/Extensions/ITypeSymbolExtensions.cs

@@ -13,6 +13,25 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions;
 /// </summary>
 internal static class ITypeSymbolExtensions
 {
+    /// <summary>
+    /// Checks whether or not a given <see cref="ITypeSymbol"/> has or inherits from a specified type.
+    /// </summary>
+    /// <param name="typeSymbol">The target <see cref="ITypeSymbol"/> instance to check.</param>
+    /// <param name="name">The full name of the type to check for inheritance.</param>
+    /// <returns>Whether or not <paramref name="typeSymbol"/> is or inherits from <paramref name="name"/>.</returns>
+    public static bool HasOrInheritsFromFullyQualifiedName(this ITypeSymbol typeSymbol, string name)
+    {
+        for (ITypeSymbol? currentType = typeSymbol; currentType is not null; currentType = currentType.BaseType)
+        {
+            if (currentType.HasFullyQualifiedName(name))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /// <summary>
     /// Checks whether or not a given <see cref="ITypeSymbol"/> inherits from a specified type.
     /// </summary>

tests/CommunityToolkit.Mvvm.SourceGenerators.UnitTests/Test_SourceGeneratorsDiagnostics.cs

@@ -1105,6 +1105,89 @@ private void GreetUser(object value)
         VerifyGeneratedDiagnostics<ICommandGenerator>(source, "MVVMTK0023");
     }
 
+    [TestMethod]
+    public void InvalidObservablePropertyError_Object()
+    {
+        string source = @"
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                public partial class MyViewModel : ObservableObject
+                {
+                    [ObservableProperty]
+                    public object property;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
+    }
+
+    [TestMethod]
+    public void InvalidObservablePropertyError_PropertyChangingEventArgs()
+    {
+        string source = @"
+            using System.ComponentModel;
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                public partial class MyViewModel : ObservableObject
+                {
+                    [ObservableProperty]
+                    public PropertyChangingEventArgs property;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
+    }
+
+    [TestMethod]
+    public void InvalidObservablePropertyError_PropertyChangedEventArgs()
+    {
+        string source = @"
+            using System.ComponentModel;
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                public partial class MyViewModel : ObservableObject
+                {
+                    [ObservableProperty]
+                    public PropertyChangedEventArgs property;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
+    }
+
+    [TestMethod]
+    public void InvalidObservablePropertyError_CustomTypeDerivedFromPropertyChangedEventArgs()
+    {
+        string source = @"
+            using System.ComponentModel;
+            using CommunityToolkit.Mvvm.ComponentModel;
+
+            namespace MyApp
+            {
+                public class MyPropertyChangedEventArgs : PropertyChangedEventArgs
+                {
+                    public MyPropertyChangedEventArgs(string propertyName)
+                        : base(propertyName)
+                    {
+                    }
+                }
+
+                public partial class MyViewModel : ObservableObject
+                {
+                    [ObservableProperty]
+                    public MyPropertyChangedEventArgs property;
+                }
+            }";
+
+        VerifyGeneratedDiagnostics<ObservablePropertyGenerator>(source, "MVVMTK0024");
+    }
+
     /// <summary>
     /// Verifies the output of a source generator.
     /// </summary>