Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IReadOnlyCollection as interface param uses Length instead of Count when source is an array #1106

Closed
kockicica opened this issue Feb 7, 2024 · 11 comments · Fixed by #1108 or #1125
Closed
Labels

Comments

@kockicica
Copy link

Describe the bug

Enumerable mapping methods with interface parameters uses 'Length', property that does not exist on System.Collections.Generic.IReadOnlyCollection when source is an array

Declaration code

source

public class Model.PagerMessage {
    ...
    public PagerContact[]     Recipients { get; set; }

}

target

public class Data.PagerMessage : DomainObject<Guid> {

    ...
    public virtual IList<PagerContact> Recipients { get; set; }

}

mapping

    [MapperIgnoreTarget(nameof(Data.PagerMessage.Id))]
    [MapperIgnoreTarget(nameof(Data.PagerMessage.Created))]
    private partial Data.PagerMessage _MapDtoToPagerMessage(Model.PagerMessage source);

Actual generated code

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.4.0.0")]
    private global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact> MapToList(global::System.Collections.Generic.IReadOnlyCollection<global::Tekelija.Model.Dardo.PagerContact> source)
    {
        var target = new global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact>(source.Length);
        foreach (var item in source)
        {
            target.Add(_MapPagerContactDtoToPagerContact(item));
        }
        return target;
    }

Expected generated code

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.4.0.0")]
    private global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact> MapToList(global::System.Collections.Generic.IReadOnlyCollection<global::Tekelija.Model.Dardo.PagerContact> source)
    {
        var target = new global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact>(source.Count);
        foreach (var item in source)
        {
            target.Add(_MapPagerContactDtoToPagerContact(item));
        }
        return target;
    }

Environment (please complete the following information):

  • Mapperly Version: 3.4.0-next.3
  • .NET Version: .NET 8.0.100
  • Target Framework: net8.0
  • Compiler Version: 4.8.0-3.23524.11 (f43cd10b)
  • C# Language Version: 12.0
  • IDE: Rider 2023.3.3
  • OS: windows 11
@kockicica kockicica added the bug Something isn't working label Feb 7, 2024
@latonz
Copy link
Contributor

latonz commented Feb 7, 2024

Thank you for opening this issue, looks like a regression. I‘ll look into it.

@TimothyMakkison
Copy link
Collaborator

Probably line 208 in EnumerableMappingBuilder, uses the correct base types but uses the original source types count property.

Copy link

🎉 This issue has been resolved in version 3.4.0-next.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kockicica
Copy link
Author

I dont know should I open new issue or update this, so let's try former: something similar still exists in 3.4.0/next.4.

    private partial IList<Data.PagerContact>? _MapDtoPagerContactsToPagerContacts(Model.PagerContact[]? source);

Actual generated code

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.4.0.0")]
    private partial global::System.Collections.Generic.IList<global::Tekelija.TAPaging.Data.PagerContact>? _MapDtoPagerContactsToPagerContacts(global::Tekelija.Model.Dardo.PagerContact[]? source)
    {
        if (source == null)
            return default;
        var target = new global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact>(source.Count);
        foreach (var item in source)
        {
            target.Add(_MapPagerContactDtoToPagerContact(item));
        }
        return target;
    }

Expected generated code

    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "3.4.0.0")]
    private partial global::System.Collections.Generic.IList<global::Tekelija.TAPaging.Data.PagerContact>? _MapDtoPagerContactsToPagerContacts(global::Tekelija.Model.Dardo.PagerContact[]? source)
    {
        if (source == null)
            return default;
        var target = new global::System.Collections.Generic.List<global::Tekelija.TAPaging.Data.PagerContact>(source.Length);
        foreach (var item in source)
        {
            target.Add(_MapPagerContactDtoToPagerContact(item));
        }
        return target;
    }

@latonz
Copy link
Contributor

latonz commented Feb 18, 2024

@kockicica I cannot reproduce it with the latest next release 😞 Sometimes when upgrading a source generator package the IDE does not correctly regenerate the code with the new source generator. Have you restarted your IDE after upgrading the Mapperly package?

@kockicica
Copy link
Author

Oh' I've restarted everything, several times - without effect unfortunately. 😄 Here's is a simple example: https://github.com/kockicica/mapperly-test

@latonz
Copy link
Contributor

latonz commented Feb 18, 2024

Thank you for the repro, now I got it. The issue still exists when the parameter of the user defined mapping method is an array. I onl tested enumerables/arrays as object properties which is fixed. I‘ll look into it in the next days.

Copy link

🎉 This issue has been resolved in version 3.4.0-next.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@latonz
Copy link
Contributor

latonz commented Feb 20, 2024

This is hopefully really fixed in 3.4.0-next.5. Could you (@kockicica) verify this?

@kockicica
Copy link
Author

Just tried and yes, i think that we can finally mark this as resolved for good. 😃 Thanks a lot.

Copy link

🎉 This issue has been resolved in version 3.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants