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

…-membernotnull

Add [MemberNotNull] to [ObservableProperty] set accessors

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

@@ -18,7 +18,8 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
 /// <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="IsOldPropertyValueDirectlyReferenced">Whether the old property value is being directly referenced.</param>
-/// <param name="IsReferenceType">Indicates whether the property is of a reference type.</param>
+/// <param name="IsReferenceTypeOrUnconstraindTypeParameter">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>
 /// <param name="ForwardedAttributes">The sequence of forwarded attributes for the generated property.</param>
 internal sealed record PropertyInfo(
  string TypeNameWithNullabilityAnnotations,
@@ -30,5 +31,6 @@ internal sealed record PropertyInfo(
  bool NotifyPropertyChangedRecipients,
  bool NotifyDataErrorInfo,
  bool IsOldPropertyValueDirectlyReferenced,
- bool IsReferenceType,
+ bool IsReferenceTypeOrUnconstraindTypeParameter,
+ bool IncludeMemberNotNullOnSetAccessor,
  EquatableArray<AttributeInfo> ForwardedAttributes);

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

@@ -112,7 +112,13 @@ public static bool TryGetInfo(
  bool hasOrInheritsClassLevelNotifyDataErrorInfo = false;
  bool hasAnyValidationAttributes = false;
  bool isOldPropertyValueDirectlyReferenced = IsOldPropertyValueDirectlyReferenced(fieldSymbol, propertyName);
- bool isReferenceType = fieldSymbol.Type.IsReferenceType;
+
+ // Get the nullability info for the property
+ GetNullabilityInfo(
+ fieldSymbol,
+ semanticModel,
+ out bool isReferenceTypeOrUnconstraindTypeParameter,
+ out bool includeMemberNotNullOnSetAccessor);
 
  // Track the property changing event for the property, if the type supports it
  if (shouldInvokeOnPropertyChanging)
@@ -261,7 +267,8 @@ public static bool TryGetInfo(
  notifyRecipients,
  notifyDataErrorInfo,
  isOldPropertyValueDirectlyReferenced,
- isReferenceType,
+ isReferenceTypeOrUnconstraindTypeParameter,
+ includeMemberNotNullOnSetAccessor,
  forwardedAttributes.ToImmutable());
 
  diagnostics = builder.ToImmutable();
@@ -668,6 +675,49 @@ private static bool IsOldPropertyValueDirectlyReferenced(IFieldSymbol fieldSymbo
  return false;
  }
 
+ /// <summary>
+ /// Gets the nullability info on the generated property
+ /// </summary>
+ /// <param name="fieldSymbol">The input <see cref="IFieldSymbol"/> instance to process.</param>
+ /// <param name="semanticModel">The <see cref="SemanticModel"/> instance for the current run.</param>
+ /// <param name="isReferenceTypeOrUnconstraindTypeParameter">Whether the property type supports nullability.</param>
+ /// <param name="includeMemberNotNullOnSetAccessor">Whether <see cref="MemberNotNullAttribute"/> should be used on the setter.</param>
+ /// <returns></returns>
+ private static void GetNullabilityInfo(
+ IFieldSymbol fieldSymbol,
+ SemanticModel semanticModel,
+ out bool isReferenceTypeOrUnconstraindTypeParameter,
+ out bool includeMemberNotNullOnSetAccessor)
+ {
+ // We're using IsValueType here and not IsReferenceType to also cover unconstrained type parameter cases.
+ // This will cover both reference types as well T when the constraints are not struct or unmanaged.
+ // If this is true, it means the field storage can potentially be in a null state (even if not annotated).
+ isReferenceTypeOrUnconstraindTypeParameter = !fieldSymbol.Type.IsValueType;
+
+ // This is used to avoid nullability warnings when setting the property from a constructor, in case the field
+ // was marked as not nullable. Nullability annotations are assumed to always be enabled to make the logic simpler.
+ // Consider this example:
+ //
+ // partial class MyViewModel : ObservableObject
+ // {
+ // public MyViewModel()
+ // {
+ // Name = "Bob";
+ // }
+ //
+ // [ObservableProperty]
+ // private string name;
+ // }
+ //
+ // The [MemberNotNull] attribute is needed on the setter for the generated Name property so that when Name
+ // is set, the compiler can determine that the name backing field is also being set (to a non null value).
+ // Of course, this can only be the case if the field type is also of a type that could be in a null state.
+ includeMemberNotNullOnSetAccessor =
+ isReferenceTypeOrUnconstraindTypeParameter &&
+ fieldSymbol.Type.NullableAnnotation != NullableAnnotation.Annotated &&
+ semanticModel.Compilation.HasAccessibleTypeWithMetadataName("System.Diagnostics.CodeAnalysis.MemberNotNullAttribute");
+ }
+
  /// <summary>
  /// Gets a <see cref="CompilationUnitSyntax"/> instance with the cached args for property changing notifications.
  /// </summary>
@@ -880,6 +930,27 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
  .Select(static a => AttributeList(SingletonSeparatedList(a.GetSyntax())))
  .ToImmutableArray();
 
+ // Prepare the setter for the generated property:
+ //
+ // set
+ // {
+ // <BODY>
+ // }
+ AccessorDeclarationSyntax setAccessor = AccessorDeclaration(SyntaxKind.SetAccessorDeclaration).WithBody(Block(setterIfStatement));
+
+ // Add the [MemberNotNull] attribute if needed:
+ //
+ // [MemberNotNull("<FIELD_NAME>")]
+ // <SET_ACCESSOR>
+ if (propertyInfo.IncludeMemberNotNullOnSetAccessor)
+ {
+ setAccessor = setAccessor.AddAttributeLists(
+ AttributeList(SingletonSeparatedList(
+ Attribute(IdentifierName("global::System.Diagnostics.CodeAnalysis.MemberNotNull"))
+ .AddArgumentListArguments(
+ AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(propertyInfo.FieldName)))))));
+ }
+
  // Construct the generated property as follows:
  //
  // /// <inheritdoc cref="<FIELD_NAME>"/>
@@ -889,10 +960,7 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
  // public <FIELD_TYPE><NULLABLE_ANNOTATION?> <PROPERTY_NAME>
  // {
  // get => <FIELD_NAME>;
- // set
- // {
- // <BODY>
- // }
+ // <SET_ACCESSOR>
  // }
  return
  PropertyDeclaration(propertyType, Identifier(propertyInfo.PropertyName))
@@ -910,8 +978,7 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
  AccessorDeclaration(SyntaxKind.GetAccessorDeclaration)
  .WithExpressionBody(ArrowExpressionClause(IdentifierName(propertyInfo.FieldName)))
  .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)),
- AccessorDeclaration(SyntaxKind.SetAccessorDeclaration)
- .WithBody(Block(setterIfStatement)));
+ setAccessor);
  }
 
  /// <summary>
@@ -952,7 +1019,7 @@ public static ImmutableArray<MemberDeclarationSyntax> GetOnPropertyChangeMethods
  // happen when the property is first set to some value that is not null (but the backing field would still be so).
  // As a cheap way to check whether we need to add nullable, we can simply check whether the type name with nullability
  // annotations ends with a '?'. If it doesn't and the type is a reference type, we add it. Otherwise, we keep it.
- TypeSyntax oldValueTypeSyntax = propertyInfo.IsReferenceType switch
+ TypeSyntax oldValueTypeSyntax = propertyInfo.IsReferenceTypeOrUnconstraindTypeParameter switch
  {
  true when !propertyInfo.TypeNameWithNullabilityAnnotations.EndsWith("?")
  => IdentifierName($"{propertyInfo.TypeNameWithNullabilityAnnotations}?"),