Merge pull request #141 from CommunityToolkit/dev/observable-property…

…-name-clash-diagnostics Extend source generator diagnostics in several scenarios
CommunityToolkit · Mar 15, 2022 · 65e1767 · 65e1767
2 parents 2372ea9 + 216d4ea
commit 65e1767
Show file tree

Hide file tree

Showing 19 changed files with 1,127 additions and 106 deletions.
diff --git a/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Unshipped.md b/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Unshipped.md
@@ -18,3 +18,11 @@ MVVMTK0010 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error |
 MVVMTK0011 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0012 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
 MVVMTK0013 | CommunityToolkit.Mvvm.SourceGenerators.ICommandGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0014 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0015 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0016 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0017 | CommunityToolkit.Mvvm.SourceGenerators.INotifyPropertyChangedGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0018 | CommunityToolkit.Mvvm.SourceGenerators.ObservableObjectGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0019 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0020 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/error
+MVVMTK0021 | CommunityToolkit.Mvvm.SourceGenerators.ObservableRecipientGenerator | Error | See https://aka.ms/mvvmtoolkit/error
diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/INotifyPropertyChangedGenerator.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/INotifyPropertyChangedGenerator.cs
@@ -34,7 +34,7 @@ public INotifyPropertyChangedGenerator()
     {
         static INotifyPropertyChangedInfo GetInfo(INamedTypeSymbol typeSymbol, AttributeData attributeData)
         {
-            bool includeAdditionalHelperMethods = attributeData.GetNamedArgument<bool>("IncludeAdditionalHelperMethods", true);
+            bool includeAdditionalHelperMethods = attributeData.GetNamedArgument("IncludeAdditionalHelperMethods", true);
 
             return new(includeAdditionalHelperMethods);
         }
@@ -57,6 +57,17 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, INotifyP
             return false;
         }
 
+        // Check if the type uses [INotifyPropertyChanged] or [ObservableObject] already (in the type hierarchy too)
+        if (typeSymbol.HasOrInheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") ||
+            typeSymbol.InheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.INotifyPropertyChangedAttribute"))
+        {
+            builder.Add(InvalidAttributeCombinationForINotifyPropertyChangedAttributeError, typeSymbol, typeSymbol);
+
+            diagnostics = builder.ToImmutable();
+
+            return false;
+        }
+
         diagnostics = builder.ToImmutable();
 
         return true;

diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableObjectGenerator.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableObjectGenerator.cs
@@ -60,6 +60,17 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, object?
             return false;
         }
 
+        // Check if the type uses [INotifyPropertyChanged] or [ObservableObject] already (in the type hierarchy too)
+        if (typeSymbol.InheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") ||
+            typeSymbol.HasOrInheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.INotifyPropertyChangedAttribute"))
+        {
+            builder.Add(InvalidAttributeCombinationForObservableObjectAttributeError, typeSymbol, typeSymbol);
+
+            diagnostics = builder.ToImmutable();
+
+            return false;
+        }
+
         diagnostics = builder.ToImmutable();
 
         return true;

diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs
diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.cs
@@ -7,7 +7,6 @@
 using System.Linq;
 using System.Text;
 using CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
-using CommunityToolkit.Mvvm.SourceGenerators.Diagnostics;
 using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
 using CommunityToolkit.Mvvm.SourceGenerators.Models;
 using Microsoft.CodeAnalysis;
@@ -38,20 +37,32 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
         // Filter the fields using [ObservableProperty]
         IncrementalValuesProvider<IFieldSymbol> fieldSymbolsWithAttribute =
             fieldSymbols
-            .Where(static item => item.GetAttributes().Any(a => a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") == true));
+            .Where(static item => item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"));
+
+        // Get diagnostics for fields using [AlsoNotifyChangeFor] and [AlsoNotifyCanExecuteFor], but not [ObservableProperty]
+        IncrementalValuesProvider<Diagnostic> fieldSymbolsWithOrphanedDependentAttributeWithErrors =
+            fieldSymbols
+            .Where(static item =>
+                (item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyChangeForAttribute") ||
+                 item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.AlsoNotifyCanExecuteForAttribute")) &&
+                 !item.HasAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"))
+            .Select(static (item, _) => Execute.GetDiagnosticForFieldWithOrphanedDependentAttributes(item));
+
+        // Output the diagnostics
+        context.ReportDiagnostics(fieldSymbolsWithOrphanedDependentAttributeWithErrors);
 
         // Filter by language version
         context.FilterWithLanguageVersion(ref fieldSymbolsWithAttribute, LanguageVersion.CSharp8, UnsupportedCSharpLanguageVersionError);
 
         // Gather info for all annotated fields
-        IncrementalValuesProvider<(HierarchyInfo Hierarchy, Result<PropertyInfo> Info)> propertyInfoWithErrors =
+        IncrementalValuesProvider<(HierarchyInfo Hierarchy, Result<PropertyInfo?> Info)> propertyInfoWithErrors =
             fieldSymbolsWithAttribute
             .Select(static (item, _) =>
             {
                 HierarchyInfo hierarchy = HierarchyInfo.From(item.ContainingType);
-                PropertyInfo propertyInfo = Execute.GetInfo(item, out ImmutableArray<Diagnostic> diagnostics);
+                PropertyInfo? propertyInfo = Execute.TryGetInfo(item, out ImmutableArray<Diagnostic> diagnostics);
 
-                return (hierarchy, new Result<PropertyInfo>(propertyInfo, diagnostics));
+                return (hierarchy, new Result<PropertyInfo?>(propertyInfo, diagnostics));
             });
 
         // Output the diagnostics
@@ -60,7 +71,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
         // Get the filtered sequence to enable caching
         IncrementalValuesProvider<(HierarchyInfo Hierarchy, PropertyInfo Info)> propertyInfo =
             propertyInfoWithErrors
-            .Select(static (item, _) => (item.Hierarchy, item.Info.Value))
+            .Select(static (item, _) => (item.Hierarchy, Info: item.Info.Value))
+            .Where(static item => item.Info is not null)!
             .WithComparers(HierarchyInfo.Comparer.Default, PropertyInfo.Comparer.Default);
 
         // Split and group by containing type

diff --git a/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableRecipientGenerator.cs b/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableRecipientGenerator.cs
@@ -39,7 +39,7 @@ static ObservableRecipientInfo GetInfo(INamedTypeSymbol typeSymbol, AttributeDat
             string typeName = typeSymbol.Name;
             bool hasExplicitConstructors = !(typeSymbol.InstanceConstructors.Length == 1 && typeSymbol.InstanceConstructors[0] is { Parameters.IsEmpty: true, IsImplicitlyDeclared: true });
             bool isAbstract = typeSymbol.IsAbstract;
-            bool isObservableValidator = typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
+            bool isObservableValidator = typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
             bool hasOnActivatedMethod = typeSymbol.GetMembers().Any(m => m is IMethodSymbol { Parameters.IsEmpty: true, Name: "OnActivated" });
             bool hasOnDeactivatedMethod = typeSymbol.GetMembers().Any(m => m is IMethodSymbol { Parameters.IsEmpty: true, Name: "OnDeactivated" });
 
@@ -70,7 +70,7 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, Observab
         ImmutableArray<Diagnostic>.Builder builder = ImmutableArray.CreateBuilder<Diagnostic>();
 
         // Check if the type already inherits from ObservableRecipient
-        if (typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient"))
+        if (typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipient"))
         {
             builder.Add(DuplicateObservableRecipientError, typeSymbol, typeSymbol);
 
@@ -79,10 +79,20 @@ protected override bool ValidateTargetType(INamedTypeSymbol typeSymbol, Observab
             return false;
         }
 
+        // Check if the type already inherits [ObservableRecipient]
+        if (typeSymbol.InheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableRecipientAttribute"))
+        {
+            builder.Add(InvalidAttributeCombinationForObservableRecipientAttributeError, typeSymbol, typeSymbol);
+
+            diagnostics = builder.ToImmutable();
+
+            return false;
+        }
+
         // In order to use [ObservableRecipient], the target type needs to inherit from ObservableObject,
         // or be annotated with [ObservableObject] or [INotifyPropertyChanged] (with additional helpers).
-        if (!typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject") &&
-            !typeSymbol.HasOrInheritsAttribute(static a => a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") == true) &&
+        if (!typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObject") &&
+            !typeSymbol.HasOrInheritsAttributeWithFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableObjectAttribute") &&
             !typeSymbol.HasOrInheritsAttribute(static a =>
                 a.AttributeClass?.HasFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.INotifyPropertyChangedAttribute") == true &&
                 !a.HasNamedArgument("IncludeAdditionalHelperMethods", false)))

diff --git a/...rceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.Execute.cs b/...rceGenerators/ComponentModel/ObservableValidatorValidateAllPropertiesGenerator.Execute.cs
@@ -29,7 +29,7 @@ private static class Execute
         /// <returns>Whether <paramref name="typeSymbol"/> inherits from <c>ObservableValidator</c>.</returns>
         public static bool IsObservableValidator(INamedTypeSymbol typeSymbol)
         {
-            return typeSymbol.InheritsFrom("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
+            return typeSymbol.InheritsFromFullyQualifiedName("global::CommunityToolkit.Mvvm.ComponentModel.ObservableValidator");
         }
 
         /// <summary>
@@ -61,7 +61,7 @@ public static ValidationInfo GetInfo(INamedTypeSymbol typeSymbol)
                 }
 
                 // Skip the current member if there are no validation attributes applied to it
-                if (!attributes.Any(a => a.AttributeClass?.InheritsFrom(
+                if (!attributes.Any(a => a.AttributeClass?.InheritsFromFullyQualifiedName(
                     "global::System.ComponentModel.DataAnnotations.ValidationAttribute") == true))
                 {
                     continue;