RicoSuter / NJsonSchema

JSON Schema reader, generator and validator for .NET
http://NJsonSchema.org
MIT License
1.37k stars 529 forks source link

Private expression-bodied read-only properties should be ignored #1694

Closed mereth closed 2 months ago

mereth commented 4 months ago

Hi,

I'm currently encountering an issue with a generated OpenAPI schema that seems to be related on how properties are filtered by the SystemTextJsonReflectionService implementations.

public class TestClass
{
    private string Prop1 { get; set; }

    private string Prop2 { get; } = "Hello";

    private string Prop3 => "Hello";
}

Both Prop2 and Prop3 are not filtered because they don't have a setter.

The excluding test, extracted from SystemTextJsonReflectionService, doesn't seem handle setter or getter nullability properly :

if (accessorInfo.MemberInfo is PropertyInfo propertyInfo &&
    (propertyInfo.GetMethod?.IsPrivate == true || propertyInfo.GetMethod?.IsStatic == true) &&
    (propertyInfo.SetMethod?.IsPrivate == true || propertyInfo.SetMethod?.IsStatic == true) &&
    !propertyInfo.IsDefined(typeof(DataMemberAttribute)))
{
    continue;
}

Unlike the including test, from NewtonsoftJsonReflectionService

contextualType
.Properties
.Where(p => p.PropertyInfo.DeclaringType == contextualType.Type &&
            (p.PropertyInfo.GetMethod?.IsPrivate != true && p.PropertyInfo.GetMethod?.IsStatic == false ||
             p.PropertyInfo.SetMethod?.IsPrivate != true && p.PropertyInfo.SetMethod?.IsStatic == false ||
             p.PropertyInfo.IsDefined(typeof(DataMemberAttribute))))

I think the SystemTextJsonReflectionService test should changed for:

if (accessorInfo.MemberInfo is PropertyInfo propertyInfo &&
    (propertyInfo.GetMethod == null || propertyInfo.GetMethod.IsPrivate == true || propertyInfo.GetMethod.IsStatic == true) &&
    (propertyInfo.SetMethod == null || propertyInfo.SetMethod.IsPrivate == true || propertyInfo.SetMethod.IsStatic == true) &&
    !propertyInfo.IsDefined(typeof(DataMemberAttribute)))
{
    continue;
}

Regards

altso commented 4 months ago

I believe it's what causing https://github.com/RicoSuter/NSwag/issues/4681.