Merge pull request #1015 from CommunityToolkit/dev/better-modifiers-support

Support more partial property modifiers, ban pointer types

Author: Sergio0694

src/CommunityToolkit.Mvvm.SourceGenerators/AnalyzerReleases.Shipped.md

@@ -96,3 +96,4 @@ MVVMTK0051 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator
 MVVMTK0052 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0052
 MVVMTK0053 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0053
 MVVMTK0054 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0054
+MVVMTK0055 | CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator | Error | See https://aka.ms/mvvmtoolkit/errors/mvvmtk0055

src/CommunityToolkit.Mvvm.SourceGenerators/CommunityToolkit.Mvvm.SourceGenerators.projitems

@@ -41,6 +41,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)ComponentModel\TransitiveMembersGenerator.Execute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\AsyncVoidReturningRelayCommandMethodAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidGeneratedPropertyObservablePropertyAttributeAnalyzer.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidPointerTypeObservablePropertyAttributeAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\InvalidPartialPropertyLevelObservablePropertyAttributeAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\WinRTClassUsingNotifyPropertyChangedAttributesAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Diagnostics\Analyzers\WinRTGeneratedBindableCustomPropertyWithBasesMemberAnalyzer.cs" />

src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs

@@ -15,6 +15,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
 /// <param name="TypeNameWithNullabilityAnnotations">The type name for the generated property, including nullability annotations.</param>
 /// <param name="FieldName">The field name.</param>
 /// <param name="PropertyName">The generated property name.</param>
+/// <param name="PropertyModifers">The list of additional modifiers for the property (they are <see cref="SyntaxKind"/> values).</param>
 /// <param name="PropertyAccessibility">The accessibility of the property.</param>
 /// <param name="GetterAccessibility">The accessibility of the <see langword="get"/> accessor.</param>
 /// <param name="SetterAccessibility">The accessibility of the <see langword="set"/> accessor.</param>
@@ -23,7 +24,6 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
 /// <param name="NotifiedCommandNames">The sequence of commands to notify.</param>
 /// <param name="NotifyPropertyChangedRecipients">Whether or not the generated property also broadcasts changes.</param>
 /// <param name="NotifyDataErrorInfo">Whether or not the generated property also validates its value.</param>
-/// <param name="IsRequired">Whether or not the generated property should be marked as required.</param>
 /// <param name="IsOldPropertyValueDirectlyReferenced">Whether the old property value is being directly referenced.</param>
 /// <param name="IsReferenceTypeOrUnconstrainedTypeParameter">Indicates whether the property is of a reference type or an unconstrained type parameter.</param>
 /// <param name="IncludeMemberNotNullOnSetAccessor">Indicates whether to include nullability annotations on the setter.</param>
@@ -34,6 +34,7 @@ internal sealed record PropertyInfo(
     string TypeNameWithNullabilityAnnotations,
     string FieldName,
     string PropertyName,
+    EquatableArray<ushort> PropertyModifers,
     Accessibility PropertyAccessibility,
     Accessibility GetterAccessibility,
     Accessibility SetterAccessibility,
@@ -42,7 +43,6 @@ internal sealed record PropertyInfo(
     EquatableArray<string> NotifiedCommandNames,
     bool NotifyPropertyChangedRecipients,
     bool NotifyDataErrorInfo,
-    bool IsRequired,
     bool IsOldPropertyValueDirectlyReferenced,
     bool IsReferenceTypeOrUnconstrainedTypeParameter,
     bool IncludeMemberNotNullOnSetAccessor,

src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.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;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.ComponentModel;
@@ -98,7 +99,7 @@ public static bool IsCandidateValidForCompilation(MemberDeclarationSyntax node,
         public static bool IsCandidateSymbolValid(ISymbol memberSymbol)
         {
 #if ROSLYN_4_12_0_OR_GREATER
-            // We only need additional checks for properties (Roslyn already validates things for fields in our scenarios)
+            // We only need these additional checks for properties (Roslyn already validates things for fields in our scenarios)
             if (memberSymbol is IPropertySymbol propertySymbol)
             {
                 // Ensure that the property declaration is a partial definition with no implementation
@@ -115,6 +116,14 @@ public static bool IsCandidateSymbolValid(ISymbol memberSymbol)
             }
 #endif
 
+            // Pointer types are never allowed in either case
+            if (memberSymbol is
+                IPropertySymbol { Type.TypeKind: TypeKind.Pointer or TypeKind.FunctionPointer } or
+                IFieldSymbol { Type.TypeKind: TypeKind.Pointer or TypeKind.FunctionPointer })
+            {
+                return false;
+            }
+
             // We assume all other cases are supported (other failure cases will be detected later)
             return true;
         }
@@ -362,6 +371,9 @@ public static bool TryGetInfo(
 
             token.ThrowIfCancellationRequested();
 
+            // Get all additional modifiers for the member
+            ImmutableArray<SyntaxKind> propertyModifiers = GetPropertyModifiers(memberSyntax);
+
             // Retrieve the accessibility values for all components
             if (!TryGetAccessibilityModifiers(
                 memberSyntax,
@@ -378,16 +390,12 @@ public static bool TryGetInfo(
 
             token.ThrowIfCancellationRequested();
 
-            // Check whether the property should be required
-            bool isRequired = GetIsRequiredProperty(memberSymbol);
-
-            token.ThrowIfCancellationRequested();
-
             propertyInfo = new PropertyInfo(
                 memberSyntax.Kind(),
                 typeNameWithNullabilityAnnotations,
                 fieldName,
                 propertyName,
+                propertyModifiers.AsUnderlyingType(),
                 propertyAccessibility,
                 getterAccessibility,
                 setterAccessibility,
@@ -396,7 +404,6 @@ public static bool TryGetInfo(
                 notifiedCommandNames.ToImmutable(),
                 notifyRecipients,
                 notifyDataErrorInfo,
-                isRequired,
                 isOldPropertyValueDirectlyReferenced,
                 isReferenceTypeOrUnconstrainedTypeParameter,
                 includeMemberNotNullOnSetAccessor,
@@ -970,6 +977,45 @@ private static void GatherLegacyForwardedAttributes(
             }
         }
 
+        /// <summary>
+        /// Gathers all allowed property modifiers that should be forwarded to the generated property.
+        /// </summary>
+        /// <param name="memberSyntax">The <see cref="MemberDeclarationSyntax"/> instance to process.</param>
+        /// <returns>The returned set of property modifiers, if any.</returns>
+        private static ImmutableArray<SyntaxKind> GetPropertyModifiers(MemberDeclarationSyntax memberSyntax)
+        {
+            // Fields never need to carry additional modifiers along
+            if (memberSyntax.IsKind(SyntaxKind.FieldDeclaration))
+            {
+                return ImmutableArray<SyntaxKind>.Empty;
+            }
+
+            // We only allow a subset of all possible modifiers (aside from the accessibility modifiers)
+            ReadOnlySpan<SyntaxKind> candidateKinds =
+            [
+                SyntaxKind.NewKeyword,
+                SyntaxKind.VirtualKeyword,
+                SyntaxKind.SealedKeyword,
+                SyntaxKind.OverrideKeyword,
+#if ROSLYN_4_3_1_OR_GREATER
+                SyntaxKind.RequiredKeyword
+#endif
+            ];
+
+            using ImmutableArrayBuilder<SyntaxKind> builder = ImmutableArrayBuilder<SyntaxKind>.Rent();
+
+            // Track all modifiers from the allowed set on the input property declaration
+            foreach (SyntaxKind kind in candidateKinds)
+            {
+                if (memberSyntax.Modifiers.Any(kind))
+                {
+                    builder.Add(kind);
+                }
+            }
+
+            return builder.ToImmutable();
+        }
+
         /// <summary>
         /// Tries to get the accessibility of the property and accessors, if possible.
         /// If the target member is not a property, it will use the defaults.
@@ -1043,20 +1089,6 @@ private static bool TryGetAccessibilityModifiers(
             return true;
         }
 
-        /// <summary>
-        /// Checks whether an input member is a required property.
-        /// </summary>
-        /// <param name="memberSymbol">The input <see cref="ISymbol"/> instance to process.</param>
-        /// <returns>Whether <paramref name="memberSymbol"/> is a required property.</returns>
-        private static bool GetIsRequiredProperty(ISymbol memberSymbol)
-        {
-#if ROSLYN_4_3_1_OR_GREATER
-            return memberSymbol is IPropertySymbol { IsRequired: true };
-#else
-            return false;
-#endif
-        }
-
         /// <summary>
         /// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
         /// </summary>
@@ -1395,13 +1427,11 @@ private static SyntaxTokenList GetPropertyModifiers(PropertyInfo propertyInfo)
         {
             SyntaxTokenList propertyModifiers = propertyInfo.PropertyAccessibility.ToSyntaxTokenList();
 
-#if ROSLYN_4_3_1_OR_GREATER
-            // Add the 'required' modifier if the original member also had it
-            if (propertyInfo.IsRequired)
+            // Add all gathered modifiers
+            foreach (SyntaxKind modifier in propertyInfo.PropertyModifers.AsImmutableArray().FromUnderlyingType())
             {
-                propertyModifiers = propertyModifiers.Add(Token(SyntaxKind.RequiredKeyword));
+                propertyModifiers = propertyModifiers.Add(Token(modifier));
             }
-#endif
 
             // Add the 'partial' modifier if the original member is a partial property
             if (propertyInfo.AnnotatedMemberKind is SyntaxKind.PropertyDeclaration)

src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/InvalidPointerTypeObservablePropertyAttributeAnalyzer.cs

@@ -0,0 +1,64 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// 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.Immutable;
+using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;
+
+namespace CommunityToolkit.Mvvm.SourceGenerators;
+
+/// <summary>
+/// A diagnostic analyzer that generates an error whenever <c>[ObservableProperty]</c> is used with pointer types.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class InvalidPointerTypeObservablePropertyAttributeAnalyzer : DiagnosticAnalyzer
+{
+    /// <inheritdoc/>
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(InvalidObservablePropertyDeclarationReturnsPointerLikeType);
+
+    /// <inheritdoc/>
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+
+        context.RegisterCompilationStartAction(static context =>
+        {
+            // Get the [ObservableProperty] symbol
+            if (context.Compilation.GetTypeByMetadataName("CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute") is not INamedTypeSymbol observablePropertySymbol)
+            {
+                return;
+            }
+
+            context.RegisterSymbolAction(context =>
+            {
+                // Ensure that we have a valid target symbol to analyze
+                if (context.Symbol is not (IFieldSymbol or IPropertySymbol))
+                {
+                    return;
+                }
+
+                // If the property is not using [ObservableProperty], there's nothing to do
+                if (!context.Symbol.TryGetAttributeWithType(observablePropertySymbol, out AttributeData? observablePropertyAttribute))
+                {
+                    return;
+                }
+
+                // Emit a diagnostic if the type is a pointer type
+                if (context.Symbol is
+                    IPropertySymbol { Type.TypeKind: TypeKind.Pointer or TypeKind.FunctionPointer } or
+                    IFieldSymbol { Type.TypeKind: TypeKind.Pointer or TypeKind.FunctionPointer })
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(
+                        InvalidObservablePropertyDeclarationReturnsPointerLikeType,
+                        observablePropertyAttribute.GetLocation(),
+                        context.Symbol.ContainingType,
+                        context.Symbol.Name));
+                }
+            }, SymbolKind.Field, SymbolKind.Property);
+        });
+    }
+}

src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs

@@ -907,4 +907,20 @@ internal static class DiagnosticDescriptors
         isEnabledByDefault: true,
         description: "A property using [ObservableProperty] returns a byref-like value ([ObservableProperty] must be used on properties of a non byref-like type).",
         helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0054");
+
+    /// <summary>
+    /// Gets a <see cref="DiagnosticDescriptor"/> for when <c>[ObservableProperty]</c> is used on a property that returns a pointer type.
+    /// <para>
+    /// Format: <c>"The property {0}.{1} returns a pointer or function pointer value ([ObservableProperty] must be used on properties of a non pointer-like type)"</c>.
+    /// </para>
+    /// </summary>
+    public static readonly DiagnosticDescriptor InvalidObservablePropertyDeclarationReturnsPointerLikeType = new DiagnosticDescriptor(
+        id: "MVVMTK0055",
+        title: "Using [ObservableProperty] on a property that returns pointer-like",
+        messageFormat: """The property {0}.{1} returns a pointer or function pointer value ([ObservableProperty] must be used on properties of a non pointer-like type)""",
+        category: typeof(ObservablePropertyGenerator).FullName,
+        defaultSeverity: DiagnosticSeverity.Error,
+        isEnabledByDefault: true,
+        description: "A property using [ObservableProperty] returns a pointer-like value ([ObservableProperty] must be used on properties of a non pointer-like type).",
+        helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0055");
 }

src/CommunityToolkit.Mvvm.SourceGenerators/Extensions/SyntaxKindExtensions.cs

@@ -3,6 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Immutable;
+using System.Runtime.CompilerServices;
 using Microsoft.CodeAnalysis.CSharp;
 
 namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions;
@@ -12,6 +14,30 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions;
 /// </summary>
 internal static class SyntaxKindExtensions
 {
+    /// <summary>
+    /// Converts an <see cref="ImmutableArray{T}"/> of <see cref="SyntaxKind"/> values to one of their underlying type.
+    /// </summary>
+    /// <param name="array">The input <see cref="ImmutableArray{T}"/> value.</param>
+    /// <returns>The resulting <see cref="ImmutableArray{T}"/> of <see cref="ushort"/> values.</returns>
+    public static ImmutableArray<ushort> AsUnderlyingType(this ImmutableArray<SyntaxKind> array)
+    {
+        ushort[]? underlyingArray = (ushort[]?)(object?)Unsafe.As<ImmutableArray<SyntaxKind>, SyntaxKind[]?>(ref array);
+
+        return Unsafe.As<ushort[]?, ImmutableArray<ushort>>(ref underlyingArray);
+    }
+
+    /// <summary>
+    /// Converts an <see cref="ImmutableArray{T}"/> of <see cref="ushort"/> values to one of their real type.
+    /// </summary>
+    /// <param name="array">The input <see cref="ImmutableArray{T}"/> value.</param>
+    /// <returns>The resulting <see cref="ImmutableArray{T}"/> of <see cref="SyntaxKind"/> values.</returns>
+    public static ImmutableArray<SyntaxKind> FromUnderlyingType(this ImmutableArray<ushort> array)
+    {
+        SyntaxKind[]? typedArray = (SyntaxKind[]?)(object?)Unsafe.As<ImmutableArray<ushort>, ushort[]?>(ref array);
+
+        return Unsafe.As<SyntaxKind[]?, ImmutableArray<SyntaxKind>>(ref typedArray);
+    }
+
     /// <summary>
     /// Converts a <see cref="SyntaxKind"/> value to either "field" or "property" based on the kind.
     /// </summary>