fix: prevent collection mapping using invalid length property

[TimothyMakkison]

Fix bug where array to list mappings would attempt to use an invalid Length property on the lowered IReadOnlyCollection source.

Fixes #1106

• EnumerableToList will always use Count instead of the non lowered sources CountPropertyName
  • I don't think this will cause issues, I did consider using CollectionInfoBuilder to get the new CountPropertyName
• Added ArrayToListShouldImproveNullability test
• Not sure if a similar issues exists with BuildEnumerableToArrayMapping, I think all calls to this method will always use Count for the original and lowered types. Perhaps CountPropertyName should be used here

Checklist

• The existing code style is followed
• The commit message follows our guidelines
• Performed a self-review of my code
• Unit tests are added/updated

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (b9998a8) 91.38% compared to head (6dc09cb) 91.39%.

Files	Patch %	Lines
...numerables/EnsureCapacity/EnsureCapacityBuilder.cs	71.42%	0 Missing and 2 partials ⚠️
...iptors/MappingBuilders/DictionaryMappingBuilder.cs	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1108      +/-   ##
==========================================
+ Coverage   91.38%   91.39%   +0.01%     
==========================================
  Files         221      221              
  Lines        7300     7301       +1     
  Branches      931      931              
==========================================
+ Hits         6671     6673       +2     
+ Misses        411      410       -1     
  Partials      218      218              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[latonz]

Thank you for looking into this and contributing a fix!

Doesn't a similar problem exist in EnumerableMappingBuilder.BuildCustomTypeMapping with the EnsureCapacityBuilder.TryBuildEnsureCapacity(ctx) call using the old / wrong source count property? IMO the best solution would be to create a new CollectionInfos instance when overwriting the source type and passing that directly to TryBuildEnsureCapacity.
I think also DictionaryMappingBuilder.BuildCustomTypeMapping has a potential similar problem... I don't think it is a real problem there now (as it is always the Count property for dictionary), but I don't think the code now is really correct.

[TimothyMakkison]

Another approach would probably be in
EnumerableMappingBuilder.BuildCollectionTypeForICollection instead of only returning the INamedTypeSymbol, also return the name of the count property:

CollectionInfo for IReadOnlyCollection / ICollection (I'd probably prefer this 🤔).

I opted to return CollectionInfo from BuildCollectionTypeForICollection.

• Made CollectionInfoBuilder.BuildCollectionInfo public, passing in SymbolAccessor and WellKnownTypes is a bit of pain.
• Updated TyrBuildEnsureCapacity to TryBuildEnsureCapacity(MappingBuilderContext ctx, CollectionInfo source, CollectionInfo target)
• Updated all mappers to use the new EnsureCapacity
• DictionaryMappingBuilder uses the correct IsCountKnown and CountProperty values.

[latonz]

Thank you for the updates, one small feedback point.

[github-actions]

🎉 This PR is included in version 3.4.0-next.4 🎉

The release is available on:

• GitHub release
• v3.4.0-next.4

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 3.4.0 🎉

The release is available on:

• GitHub release
• v3.4.0

Your semantic-release bot 📦🚀