Skip to content

Commit 4b1adeb

Browse files
authored
Merge pull request #160 from CommunityToolkit/dev/nullable-generated-properties
Fix nullability annotations in generated properties
2 parents bf4b5f6 + 393a223 commit 4b1adeb

File tree

4 files changed

+136
-20
lines changed

4 files changed

+136
-20
lines changed

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
1414
/// <summary>
1515
/// A model representing an generated property
1616
/// </summary>
17-
/// <param name="TypeName">The type name for the generated property.</param>
18-
/// <param name="IsNullableReferenceType">Whether or not the property is of a nullable reference type.</param>
17+
/// <param name="TypeNameWithNullabilityAnnotations">The type name for the generated property, including nullability annotations.</param>
1918
/// <param name="FieldName">The field name.</param>
2019
/// <param name="PropertyName">The generated property name.</param>
2120
/// <param name="PropertyChangingNames">The sequence of property changing properties to notify.</param>
@@ -24,8 +23,7 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.ComponentModel.Models;
2423
/// <param name="AlsoBroadcastChange">Whether or not the generated property also broadcasts changes.</param>
2524
/// <param name="ValidationAttributes">The sequence of validation attributes for the generated property.</param>
2625
internal sealed record PropertyInfo(
27-
string TypeName,
28-
bool IsNullableReferenceType,
26+
string TypeNameWithNullabilityAnnotations,
2927
string FieldName,
3028
string PropertyName,
3129
ImmutableArray<string> PropertyChangingNames,
@@ -42,8 +40,7 @@ public sealed class Comparer : Comparer<PropertyInfo, Comparer>
4240
/// <inheritdoc/>
4341
protected override void AddToHashCode(ref HashCode hashCode, PropertyInfo obj)
4442
{
45-
hashCode.Add(obj.TypeName);
46-
hashCode.Add(obj.IsNullableReferenceType);
43+
hashCode.Add(obj.TypeNameWithNullabilityAnnotations);
4744
hashCode.Add(obj.FieldName);
4845
hashCode.Add(obj.PropertyName);
4946
hashCode.AddRange(obj.PropertyChangingNames);
@@ -57,8 +54,7 @@ protected override void AddToHashCode(ref HashCode hashCode, PropertyInfo obj)
5754
protected override bool AreEqual(PropertyInfo x, PropertyInfo y)
5855
{
5956
return
60-
x.TypeName == y.TypeName &&
61-
x.IsNullableReferenceType == y.IsNullableReferenceType &&
57+
x.TypeNameWithNullabilityAnnotations == y.TypeNameWithNullabilityAnnotations &&
6258
x.FieldName == y.FieldName &&
6359
x.PropertyName == y.PropertyName &&
6460
x.PropertyChangingNames.SequenceEqual(y.PropertyChangingNames) &&

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ internal static class Execute
4949
}
5050

5151
// Get the property type and name
52-
string typeName = fieldSymbol.Type.GetFullyQualifiedName();
53-
bool isNullableReferenceType = fieldSymbol.Type is { IsReferenceType: true, NullableAnnotation: NullableAnnotation.Annotated };
52+
string typeNameWithNullabilityAnnotations = fieldSymbol.Type.GetFullyQualifiedNameWithNullabilityAnnotations();
5453
string fieldName = fieldSymbol.Name;
5554
string propertyName = GetGeneratedPropertyName(fieldSymbol);
5655

@@ -124,8 +123,7 @@ internal static class Execute
124123
diagnostics = builder.ToImmutable();
125124

126125
return new(
127-
typeName,
128-
isNullableReferenceType,
126+
typeNameWithNullabilityAnnotations,
129127
fieldName,
130128
propertyName,
131129
propertyChangingNames.ToImmutable(),
@@ -406,10 +404,8 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
406404
{
407405
ImmutableArray<StatementSyntax>.Builder setterStatements = ImmutableArray.CreateBuilder<StatementSyntax>();
408406

409-
// Get the property type syntax (adding the nullability annotation, if needed)
410-
TypeSyntax propertyType = propertyInfo.IsNullableReferenceType
411-
? NullableType(IdentifierName(propertyInfo.TypeName))
412-
: IdentifierName(propertyInfo.TypeName);
407+
// Get the property type syntax
408+
TypeSyntax propertyType = IdentifierName(propertyInfo.TypeNameWithNullabilityAnnotations);
413409

414410
// In case the backing field is exactly named "value", we need to add the "this." prefix to ensure that comparisons and assignments
415411
// with it in the generated setter body are executed correctly and without conflicts with the implicit value parameter.
@@ -602,10 +598,8 @@ public static MemberDeclarationSyntax GetPropertySyntax(PropertyInfo propertyInf
602598
/// <returns>The generated <see cref="MemberDeclarationSyntax"/> instances for the <c>OnPropertyChanging</c> and <c>OnPropertyChanged</c> methods.</returns>
603599
public static ImmutableArray<MemberDeclarationSyntax> GetOnPropertyChangeMethodsSyntax(PropertyInfo propertyInfo)
604600
{
605-
// Get the parameter type syntax (adding the nullability annotation, if needed)
606-
TypeSyntax parameterType = propertyInfo.IsNullableReferenceType
607-
? NullableType(IdentifierName(propertyInfo.TypeName))
608-
: IdentifierName(propertyInfo.TypeName);
601+
// Get the property type syntax
602+
TypeSyntax parameterType = IdentifierName(propertyInfo.TypeNameWithNullabilityAnnotations);
609603

610604
// Construct the generated method as follows:
611605
//

CommunityToolkit.Mvvm.SourceGenerators/Extensions/ISymbolExtensions.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,16 @@ public static string GetFullyQualifiedName(this ISymbol symbol)
2222
return symbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
2323
}
2424

25+
/// <summary>
26+
/// Gets the fully qualified name for a given symbol, including nullability annotations
27+
/// </summary>
28+
/// <param name="symbol">The input <see cref="ISymbol"/> instance.</param>
29+
/// <returns>The fully qualified name for <paramref name="symbol"/>.</returns>
30+
public static string GetFullyQualifiedNameWithNullabilityAnnotations(this ISymbol symbol)
31+
{
32+
return symbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.AddMiscellaneousOptions(SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier));
33+
}
34+
2535
/// <summary>
2636
/// Checks whether or not a given type symbol has a specified full name.
2737
/// </summary>

tests/CommunityToolkit.Mvvm.UnitTests/Test_ObservablePropertyAttribute.cs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.ComponentModel.DataAnnotations;
99
using System.Linq;
1010
using System.Reflection;
11+
using System.Runtime.CompilerServices;
1112
using System.Threading.Tasks;
1213
using CommunityToolkit.Mvvm.ComponentModel;
1314
using CommunityToolkit.Mvvm.Input;
@@ -417,6 +418,103 @@ private void Test_AlsoBroadcastChange_Test<T>(Func<IMessenger, T> factory, Actio
417418
Assert.AreEqual(propertyName, messages[0].Message.PropertyName);
418419
}
419420

421+
#if NET6_0_OR_GREATER
422+
// See https://github.com/CommunityToolkit/dotnet/issues/155
423+
[TestMethod]
424+
public void Test_ObservableProperty_NullabilityAnnotations_Simple()
425+
{
426+
// List<string?>?
427+
NullabilityInfoContext context = new();
428+
NullabilityInfo info = context.Create(typeof(NullableRepro).GetProperty(nameof(NullableRepro.NullableList))!);
429+
430+
Assert.AreEqual(typeof(List<string>), info.Type);
431+
Assert.AreEqual(NullabilityState.Nullable, info.ReadState);
432+
Assert.AreEqual(NullabilityState.Nullable, info.WriteState);
433+
Assert.AreEqual(1, info.GenericTypeArguments.Length);
434+
435+
NullabilityInfo elementInfo = info.GenericTypeArguments[0];
436+
437+
Assert.AreEqual(typeof(string), elementInfo.Type);
438+
Assert.AreEqual(NullabilityState.Nullable, elementInfo.ReadState);
439+
Assert.AreEqual(NullabilityState.Nullable, elementInfo.WriteState);
440+
}
441+
442+
// See https://github.com/CommunityToolkit/dotnet/issues/155
443+
[TestMethod]
444+
public void Test_ObservableProperty_NullabilityAnnotations_Complex()
445+
{
446+
// Foo<Foo<string?, int>.Bar<object?>?, StrongBox<Foo<int, string?>.Bar<object>?>?>?
447+
NullabilityInfoContext context = new();
448+
NullabilityInfo info = context.Create(typeof(NullableRepro).GetProperty(nameof(NullableRepro.NullableMess))!);
449+
450+
Assert.AreEqual(typeof(Foo<Foo<string?, int>.Bar<object?>?, StrongBox<Foo<int, string?>.Bar<object>?>?>), info.Type);
451+
Assert.AreEqual(NullabilityState.Nullable, info.ReadState);
452+
Assert.AreEqual(NullabilityState.Nullable, info.WriteState);
453+
Assert.AreEqual(2, info.GenericTypeArguments.Length);
454+
455+
NullabilityInfo leftInfo = info.GenericTypeArguments[0];
456+
457+
Assert.AreEqual(typeof(Foo<string?, int>.Bar<object?>), leftInfo.Type);
458+
Assert.AreEqual(NullabilityState.Nullable, leftInfo.ReadState);
459+
Assert.AreEqual(NullabilityState.Nullable, leftInfo.WriteState);
460+
Assert.AreEqual(3, leftInfo.GenericTypeArguments.Length);
461+
462+
NullabilityInfo leftInfo0 = leftInfo.GenericTypeArguments[0];
463+
464+
Assert.AreEqual(typeof(string), leftInfo0.Type);
465+
Assert.AreEqual(NullabilityState.Nullable, leftInfo0.ReadState);
466+
Assert.AreEqual(NullabilityState.Nullable, leftInfo0.WriteState);
467+
468+
NullabilityInfo leftInfo1 = leftInfo.GenericTypeArguments[1];
469+
470+
Assert.AreEqual(typeof(int), leftInfo1.Type);
471+
Assert.AreEqual(NullabilityState.NotNull, leftInfo1.ReadState);
472+
Assert.AreEqual(NullabilityState.NotNull, leftInfo1.WriteState);
473+
474+
NullabilityInfo leftInfo2 = leftInfo.GenericTypeArguments[2];
475+
476+
Assert.AreEqual(typeof(object), leftInfo2.Type);
477+
Assert.AreEqual(NullabilityState.Nullable, leftInfo2.ReadState);
478+
Assert.AreEqual(NullabilityState.Nullable, leftInfo2.WriteState);
479+
480+
NullabilityInfo rightInfo = info.GenericTypeArguments[1];
481+
482+
Assert.AreEqual(typeof(StrongBox<Foo<int, string?>.Bar<object>?>), rightInfo.Type);
483+
Assert.AreEqual(NullabilityState.Nullable, rightInfo.ReadState);
484+
Assert.AreEqual(NullabilityState.Nullable, rightInfo.WriteState);
485+
Assert.AreEqual(1, rightInfo.GenericTypeArguments.Length);
486+
487+
NullabilityInfo rightInnerInfo = rightInfo.GenericTypeArguments[0];
488+
489+
Assert.AreEqual(typeof(Foo<int, string?>.Bar<object>), rightInnerInfo.Type);
490+
Assert.AreEqual(NullabilityState.Nullable, rightInnerInfo.ReadState);
491+
Assert.AreEqual(NullabilityState.Nullable, rightInnerInfo.WriteState);
492+
Assert.AreEqual(3, rightInnerInfo.GenericTypeArguments.Length);
493+
494+
NullabilityInfo rightInfo0 = rightInnerInfo.GenericTypeArguments[0];
495+
496+
Assert.AreEqual(typeof(int), rightInfo0.Type);
497+
Assert.AreEqual(NullabilityState.NotNull, rightInfo0.ReadState);
498+
Assert.AreEqual(NullabilityState.NotNull, rightInfo0.WriteState);
499+
500+
NullabilityInfo rightInfo1 = rightInnerInfo.GenericTypeArguments[1];
501+
502+
Assert.AreEqual(typeof(string), rightInfo1.Type);
503+
Assert.AreEqual(NullabilityState.Nullable, rightInfo1.ReadState);
504+
Assert.AreEqual(NullabilityState.Nullable, rightInfo1.WriteState);
505+
506+
NullabilityInfo rightInfo2 = rightInnerInfo.GenericTypeArguments[2];
507+
508+
Assert.AreEqual(typeof(object), rightInfo2.Type);
509+
//Assert.AreEqual(NullabilityState.NotNull, rightInfo2.ReadState);
510+
//Assert.AreEqual(NullabilityState.NotNull, rightInfo2.WriteState);
511+
512+
// The commented out lines are to work around a bug in the NullabilityInfo API in .NET 6.
513+
// This has been fixed for .NET 7: https://github.com/dotnet/runtime/pull/63556. The test
514+
// cases above can be uncommented when the .NET 7 target (or a more recent version) is added.
515+
}
516+
#endif
517+
420518
public partial class SampleModel : ObservableObject
421519
{
422520
/// <summary>
@@ -704,4 +802,22 @@ public BroadcastingViewModelWithInheritedAttribute(IMessenger messenger)
704802
[AlsoBroadcastChange]
705803
private string? name2;
706804
}
805+
806+
#if NET6_0_OR_GREATER
807+
private partial class NullableRepro : ObservableObject
808+
{
809+
[ObservableProperty]
810+
private List<string?>? nullableList;
811+
812+
[ObservableProperty]
813+
private Foo<Foo<string?, int>.Bar<object?>?, StrongBox<Foo<int, string?>.Bar<object>?>?>? nullableMess;
814+
}
815+
816+
private class Foo<T1, T2>
817+
{
818+
public class Bar<T>
819+
{
820+
}
821+
}
822+
#endif
707823
}

0 commit comments

Comments
 (0)