Mapperly v4.0.0 will no longer map to protobuf RepeatedField (complains about Capacity field)

[EraYaN]

On v3.6.0 Mapperly successfully maps IEnumerble/ICollection to RepeatedField (this one

On v4.0.0 this generated a bunch of RMG012 (which we have a error severity in our configuration)

Was there some special handling in v3.6 that made this just work? (maybe one of the interfaces?)

Example:

RMG012	The member Capacity on the mapping target type Google.Protobuf.Collections.RepeatedField<TypeA?> was not found on the mapping source type System.Collections.Generic.ICollection<InternalTypeA>

With a mapping for TypeA? and InternalTypeA existing and working.

[latonz]

This comes with the new feature which maps properties of collections #1364. Therefore the property Capacity of the RepeatedField is unmapped and results in the diagnostic. I see that this may not be the expected behaviour here, but I'm unsure how we could fix this 🤔 Maybe we should just ignore Capacity, Count, ... members for collection mappings by default unless they are explicitly mapped. WDYT?

[TimothyMakkison]

Perhaps for the time being, you could use a custom map, mapping IEnumerable<T> to RepeatedFields<T> to circumvent the issue.

Regarding #1364, when I first tried to do the PR, I added exception for properties such as Count, Capacity, IsReadOnly etc. Would this be an acceptable solution?

[latonz]

Unfortunately generic mappings are not yet implemented, see #1285.

Currently all members of collection types in the system namespace are ignored (e.g. when mapping from ICollection to a custom type all members of ICollection are ignored). If we want to ignore additional members we would have to decide which, for example, IsReadOnly may be critical for some use-cases and it could be dangerous to silently ignore it...

[latonz]

@EraYaN I just released 4.1.0-next.0 with support for Capacity members. I think this should resolve the issue you have. It doesn't really resolve the underlaying issue though. Let me know if it works for you.

[EraYaN]

That does not seem to work, I'll see if I can make a minimal example. And yes protobuf MapField also seems to now have some trouble in that case with the IsReadOnly field. I'll see if I can make something that shows both of those.

Maybe there is a way to ignore these globally on a per type basis. Cause we use these extensively of course.

Although actually using the Capacity in the mapper would be even better (to pre-allocate the right number of items etc) in the target list etc. It currently seems to do Count * 2 for List targets.

[latonz]

A minimal example would be nice to investigate, thank you. Mapperly should also try to set the capacity if known, if that doesn‘t work I would be glad if you could create a minimal example for this too.

[EraYaN]

Alright while making a demonstrator seems like it doesn't like a missing setter on ICollection properties.

internal ICollection<int> Numbers { get; } = [];

vs

internal ICollection<int> Numbers { get; set; } = [];

Although on the -next.0 version it reports those as RMG020i.Name.

Code lives here:
https://github.com/EraYaN/mapperly-test

I have made a second commit that adds the setters (check the configurations DebugWithSetters and Debug for example).

We'd like to have models without setters. If you change the ICollection to IList it seems to generate a line like:

target.Numbers.Capacity = input.Numbers.Count + target.Numbers.Count;

So I didn't see clearly and it does in fact do it correctly with the capacity.

[latonz]

Okay, I got it. In general Mapperly 4.1.0-next.0 always tries to set the capacity on the target collection, either via a capacity property or via EnsureCapacity. If the target does not have such a member (e.g. if the target is ICollection<T>), the capacity cannot be initialized. If the capacity is not mapped but is present on the source, RMG20 is reported. This is the case in your sample when the setters are disabled. If the setters are disabled, instead of a foreach-add mapping a simpler source.ToList() mapping is used, therefore there is no need to map any properties and RMG020 is not reported. I'll look into how we can improve the situation here, if the target is a system collection, we should probably not report RMG020 even if there are unmapped members.