Skip to content

Commit

Permalink
Fix SG nullability annotations for required and init properties. (#10…
Browse files Browse the repository at this point in the history
  • Loading branch information
eiriktsarpalis authored Sep 19, 2024
1 parent 128fdfd commit 3bb186e
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ public static bool IsNullable(this IParameterSymbol parameter)
{
// Because System.Text.Json cannot distinguish between nullable and non-nullable type parameters,
// (e.g. the same metadata is being used for both KeyValuePair<string, string?> and KeyValuePair<string, string>),
// we derive nullability annotations from the original definition of the field and not instation.
// we derive nullability annotations from the original definition of the field and not its instantiation.
// This preserves compatibility with the capabilities of the reflection-based NullabilityInfo reader.
parameter = parameter.OriginalDefinition;
return !IsInputTypeNonNullable(parameter, parameter.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ private static void GenerateCtorParamMetadataInitFunc(SourceWriter writer, strin
Name = {{FormatStringLiteral(spec.Name)}},
ParameterType = typeof({{spec.ParameterType.FullyQualifiedName}}),
Position = {{spec.ParameterIndex}},
IsNullable = {{FormatBoolLiteral(spec.IsNullable)}},
IsMemberInitializer = true,
},
""");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,7 @@ private void ProcessMember(
ParameterType = property.PropertyType,
MatchesConstructorParameter = matchingConstructorParameter is not null,
ParameterIndex = matchingConstructorParameter?.ParameterIndex ?? paramCount++,
IsNullable = property.PropertyType.CanBeNull && !property.IsSetterNonNullableAnnotation,
};

(propertyInitializers ??= new()).Add(propertyInitializer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@ public sealed record PropertyInitializerGenerationSpec
public required int ParameterIndex { get; init; }

public required bool MatchesConstructorParameter { get; init; }

public required bool IsNullable { get; init; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ public static IEnumerable<object[]> GetTypesWithNonNullablePropertyGetter()
yield return Wrap(typeof(NotNullableSpecialTypePropertiesClass), nameof(NotNullableSpecialTypePropertiesClass.JsonDocument));
yield return Wrap(typeof(NullableObliviousConstructorParameter), nameof(NullableObliviousConstructorParameter.Property));
yield return Wrap(typeof(NotNullGenericPropertyClass<string>), nameof(NotNullGenericPropertyClass<string>.Property));
yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableInitProperty.Property));
yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableRequiredProperty.Property));

static object[] Wrap(Type type, string propertyName) => [type, propertyName];
}
Expand Down Expand Up @@ -125,6 +127,8 @@ public static IEnumerable<object[]> GetTypesWithNullablePropertyGetter()
yield return Wrap(typeof(NullableObliviousPropertyClass), nameof(NullableObliviousPropertyClass.Property));
yield return Wrap(typeof(GenericPropertyClass<string>), nameof(GenericPropertyClass<string>.Property));
yield return Wrap(typeof(NullableGenericPropertyClass<string>), nameof(NullableGenericPropertyClass<string>.Property));
yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableInitProperty.Property));
yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableRequiredProperty.Property));

static object[] Wrap(Type type, string propertyName) => [type, propertyName];
}
Expand Down Expand Up @@ -191,6 +195,8 @@ public static IEnumerable<object[]> GetTypesWithNonNullablePropertySetter()
yield return Wrap(typeof(DisallowNullConstructorParameter), nameof(DisallowNullConstructorParameter.Property));
yield return Wrap(typeof(DisallowNullConstructorParameter<string>), nameof(DisallowNullConstructorParameter<string>.Property));
yield return Wrap(typeof(NotNullGenericConstructorParameter<string>), nameof(NotNullGenericConstructorParameter<string>.Property));
yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableInitProperty.Property));
yield return Wrap(typeof(ClassWithNonNullableInitProperty), nameof(ClassWithNonNullableRequiredProperty.Property));

static object[] Wrap(Type type, string propertyName) => [type, propertyName];
}
Expand Down Expand Up @@ -249,6 +255,8 @@ public static IEnumerable<object[]> GetTypesWithNullablePropertySetter()
yield return Wrap(typeof(AllowNullConstructorParameter<string>), nameof(AllowNullConstructorParameter<string>.Property));
yield return Wrap(typeof(GenericConstructorParameter<string>), nameof(GenericConstructorParameter<string>.Property));
yield return Wrap(typeof(NullableGenericConstructorParameter<string>), nameof(NullableGenericConstructorParameter<string>.Property));
yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableInitProperty.Property));
yield return Wrap(typeof(ClassWithNullableInitProperty), nameof(ClassWithNullableRequiredProperty.Property));

static object[] Wrap(Type type, string propertyName) => [type, propertyName];
}
Expand Down Expand Up @@ -765,5 +773,25 @@ public class NullableFieldClass
[JsonInclude]
public string? Field;
}

public class ClassWithNullableInitProperty
{
public string? Property { get; init; }
}

public class ClassWithNonNullableInitProperty
{
public string Property { get; init; }
}

public class ClassWithNullableRequiredProperty
{
public required string? Property { get; set; }
}

public class ClassWithNonNullableRequiredProperty
{
public required string Property { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ protected NullableAnnotationsTests_Metadata(JsonSerializerWrapper serializer)
[JsonSerializable(typeof(NullableGenericConstructorParameter<string>))]
[JsonSerializable(typeof(NotNullGenericConstructorParameter<string>))]
[JsonSerializable(typeof(NotNullablePropertyWithIgnoreConditions))]
[JsonSerializable(typeof(ClassWithNullableInitProperty))]
[JsonSerializable(typeof(ClassWithNonNullableInitProperty))]
[JsonSerializable(typeof(ClassWithNullableRequiredProperty))]
[JsonSerializable(typeof(ClassWithNonNullableRequiredProperty))]
internal sealed partial class NullableAnnotationsTestsContext_Metadata
: JsonSerializerContext { }
}
Expand Down Expand Up @@ -128,6 +132,10 @@ protected NullableAnnotationsTests_Default(JsonSerializerWrapper serializer)
[JsonSerializable(typeof(NullableGenericConstructorParameter<string>))]
[JsonSerializable(typeof(NotNullGenericConstructorParameter<string>))]
[JsonSerializable(typeof(NotNullablePropertyWithIgnoreConditions))]
[JsonSerializable(typeof(ClassWithNullableInitProperty))]
[JsonSerializable(typeof(ClassWithNonNullableInitProperty))]
[JsonSerializable(typeof(ClassWithNullableRequiredProperty))]
[JsonSerializable(typeof(ClassWithNonNullableRequiredProperty))]
internal sealed partial class NullableAnnotationsTestsContext_Default
: JsonSerializerContext
{ }
Expand Down

0 comments on commit 3bb186e

Please sign in to comment.