Code Generator Tries To Write To ReadOnly Properties

[TheBrambleShark]

Given the following example model...

[GenerateSerde]
public partial record class Person
(
    [property: FixedFieldInfo(0, 10)] string FirstName,
    [property: FixedFieldInfo(10, 10)] string LastName,
    [property: FixedFieldInfo(20, 5)] int EmployeeId,
    [property: FixedFieldInfo(25, 8, format: "MMddyyyy")] DateTime Birthday
)
{
    [FixedFieldInfo(25, 3, "D3")]
    public int Age => DateOnly.FromDateTime(DateTime.Today).Year - Birthday.Year;
}

Person.ISerde.g.cs will generate with the following code (comments own):

Serde.FixedWidth.Tests.Person Serde.IDeserialize<Serde.FixedWidth.Tests.Person>.Deserialize(IDeserializer deserializer)
{
    string _l_firstname = default!;
    string _l_lastname = default!;
    int _l_employeeid = default!;
    System.DateTime _l_birthday = default!;
    int _l_age = default!;

    byte _r_assignedValid = 0;

    var _l_serdeInfo = global::Serde.SerdeInfoProvider.GetInfo(this);
    var typeDeserialize = deserializer.ReadType(_l_serdeInfo);
    while (true)
    {
        var (_l_index_, _) = typeDeserialize.TryReadIndexWithName(_l_serdeInfo);
        if (_l_index_ == Serde.ITypeDeserializer.EndOfType)
        {
            break;
        }

        switch (_l_index_)
        {
            // Snip
            // Case 4 should generate with an empty body, or only with the appropriate checks.
            case 4:
                Serde.DeserializeException.ThrowIfDuplicate(_r_assignedValid, 4, _l_serdeInfo);
                _l_age = typeDeserialize.ReadI32(_l_serdeInfo, _l_index_);
                _r_assignedValid |= ((byte)1) << 4;
                break;
            case Serde.ITypeDeserializer.IndexNotFound:
                typeDeserialize.SkipValue(_l_serdeInfo, _l_index_);
                break;
            default:
                throw new InvalidOperationException("Unexpected index: " + _l_index_);
        }
    }
    if ((_r_assignedValid & 0b11111) != 0b11111)
    {
        throw Serde.DeserializeException.UnassignedMember();
    }

    // Or, Age should not be assigned here.
    var newType = new Serde.FixedWidth.Tests.Person(_l_firstname, _l_lastname, _l_employeeid, _l_birthday) {
        Age = _l_age,
    };

    return newType;
}

[TheBrambleShark]

Actually, it seems there's a built-in solution for this.

This resolves the issue, but we should not have to manually specify SkipSerialize = true if the property is readonly.

[FixedFieldInfo(25, 3, "D3")]
[SerdeMemberOptions(SkipDeserialize = true)]
public int Age => DateOnly.FromDateTime(DateTime.Today).Year - Birthday.Year;

[agocke]

I went back and forth on behavior here. The reason why it's explicit right now is because I wanted serialize/deserialize to be symmetrical by default -- if a member is serialized it should be deserialized.

I think there are the following options:

1. Skip deserialization and serialization for read-only members (questionable)
2. Skip deserialization for read-only members, but keep serialization (not symmetrical or obvious)
3. Provide a better experience (error on the member with a message about what to do in this case?)
4. Nothing

I'm not sure what the best option is. At least (3) seems good.

[agocke]

Actually, let me add one more option: skip serialization and deserialization for non-auto props.

[TheBrambleShark]

My thoughts:

Default should be option 3. Display an error on the type and advise the below courses of action:

Set handling on the type by specifying an option with the SerdeTypeOptions attribute, allowing users to select between the default, option 2, or option 5.

For individual properties like in my case, an alternative would be to annotate the property as I'm doing now. Or perhaps a dedicated SuppressDeserializeAttribute and SuppressSerializeAttribute could be added for this, if you don't mind the overlap in function.

If users want to skip serialization and deserialization, a mechanism for this also already exists via SerdeMemberOptions.

[TheBrambleShark]

Now that I'm back on my computer, here are some API proposals:

public enum ReadOnlyMemberHandling
{
    Error = 0,    // Option 3 - give us a good error message
    Ignore,       // Do not serialize or deserialize non-auto props.
    SerializeOnly // Option 2
}

If ReadOnlyMemberHandling.Error is selected (default if the property is not specified or the type is not annotated with a SerdeTypeOptions attribute), suppress the error if the property is annotated with SerdeMemberOptions and SkipDeserialize = true. This is where the SuppressDeserialize attribute could be used, but I'm not 100% convinced we should necessarily add a dedicated attribute just for this condition when SerdeMemberOptions works just as well.

If ReadOnlyMemberHandling.Ignore is selected, the type should only serialize properties that are opted in. To opt in, annotate with new attribute SerializeSerdeMember. This will be parameterless - if someone wishes to specify a proxy they can combine this with SerdeMemberOptions.

---

Or if we wish to keep things simple, I do like the idea of just giving a better error and suppressing if we tell it to skip deserializing. I also like the idea of removing ReadOnlyMemberHandling.Ignore and just letting people choose SerializeOnly, which would be a shorthand of annotating every affected member and handling it on the type level.

[agocke]

The few times I've run into this I had ~6 properties and only one of them was read-only. Was it different for you? I'm just weighing the value of an additional type-level option vs. just using SerializeMemberOptions.

I'm sold on the error though.

[TheBrambleShark]

I've never actually encountered this in the wild. It was just something that came up when writing tests for my fixed-width parser.

Given that, I think it may be sufficient for now to work on better error reporting and suggest that the property be made writable (init counts) or annotated with SerializeMemberOptions with SkipDeserialize = true. Autofix appreciated, though I'm not sure if one or the other should be recommended. I think that'd be highly situational. I've never tried providing two fix providers for an issue so I'm not sure if we can recommend both.

If there is later feedback that this is a more common issue than we think, we can revisit this to add the type-level option.