Skip to content

Add [MemberNotNull] to [ObservableProperty] set accessors #646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sergio0694 merged 3 commits into from
Mar 14, 2023
Merged

Add [MemberNotNull] to [ObservableProperty] set accessors #646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c11bb0e
Add [MemberNotNull] to [ObservableProperty] set accessor
Sergio0694 Mar 14, 2023
d2fe8a0
Add unit tests for [MemberNotNull] generation
Sergio0694 Mar 14, 2023
673801a
Update generator for unconstrained generic nullability
Sergio0694 Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/PropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -30,5 +31,6 @@ internal sealed record PropertyInfo(
bool NotifyPropertyChangedRecipients,
bool NotifyDataErrorInfo,
bool IsOldPropertyValueDirectlyReferenced,
bool IsReferenceType,
bool IsReferenceTypeOrUnconstraindTypeParameter,
bool IncludeMemberNotNullOnSetAccessor,
EquatableArray<AttributeInfo> ForwardedAttributes);
85 changes: 76 additions & 9 deletions ...munityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservablePropertyGenerator.Execute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -261,7 +267,8 @@ public static bool TryGetInfo(
notifyRecipients,
notifyDataErrorInfo,
isOldPropertyValueDirectlyReferenced,
isReferenceType,
isReferenceTypeOrUnconstraindTypeParameter,
includeMemberNotNullOnSetAccessor,
forwardedAttributes.ToImmutable());

diagnostics = builder.ToImmutable();
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>"/>
Expand All @@ -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))
Expand All @@ -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>
Expand Down Expand Up @@ -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}?"),
Expand Down
Loading