Generate static methods in nonstatic classes

[trejjam]

Generate static methods in nonstatic classes

Description

This PR adds the Mapper configuration option GenerateStaticMethods which is a switch that generates static methods inside a non-static class.

Fixes #673

Checklist

• The existing code style is followed
• The commit message follows our guidelines
• Performed a self-review of my code
• Hard-to-understand areas of my code are commented
• The documentation is updated (as applicable)
• Unit tests are added/updated
• Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c5f0c54) 91.40% compared to head (ff6fac9) 91.48%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
+ Coverage   91.40%   91.48%   +0.08%     
==========================================
  Files         214      214              
  Lines        7189     7212      +23     
  Branches      869      872       +3     
==========================================
+ Hits         6571     6598      +27     
+ Misses        411      408       -3     
+ Partials      207      206       -1     

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

[trejjam]

Hi, @latonz, @TimothyMakkison,

can I ask you for your opinion about this functionality? And if so, can I ask for feedback on implementation?

Thank you,
Jan

[trejjam]

The goal is to support this code:

public interface IMyInterface
{
    static abstract CarDto ToDto(Car car);
}

[Mapper(GenerateStaticMethods = true)]
public partial class CarMapper : IMyInterface
{
    public static partial CarDto ToDto(Car car);
}

[latonz]

Sorry for my late feedback, I had the pleasure to enjoy some vacation days 🏝️
Thank you for opening the corresponding issue and submitting this PR, I understand your use case.
On a first glance the current implementation has several not yet addressed problems (which probably aren't that easy to fix):

• Generated nested mapping methods are not static
  (to reproduce simply add public static partial Riok.Mapperly.IntegrationTests.Dto.TestObjectDto Map(Riok.Mapperly.IntegrationTests.Models.TestObject value); to the TestMapperWithStaticMethods in your integration tests)
• Non-Static user defined mappings are called from generated static mapping methods
• Non-Static user implemented mappings are called from generated static mapping methods
• Non-Static object factories are called from generated static mapping methods
• Non-Static external mappers are called from generated static mapping methods (probably due to rebasing, only landed recently in main)

I will comment on the issue #673 with the concerns that I have for this feature.

[trejjam]

Thanks, I will take a look at the missing parts

[latonz]

@trejjam thank you for your efforts! But before you are going to continue working on this PR we should probably agree on the design. I added my bits here.

[trejjam]

Yes, for sure :)

[trejjam]

Hi @latonz,
I finally found some time to finish this PR. I will be happy if you find some time for a review.

[latonz]

Thank you for the updates, I added my feedback 😊

[trejjam]

Hi, thank you for the review! I processed all recommendations.

I left changes as separate commits, so it's easier to navigate through changes. Please let me know for the final rebase/squash

[latonz]

Thank you for the updates, only a few comments left and this can get merged 👍

[latonz]

I just checked my earlier comment #681 (comment) and noticed that there is still a problem with object factories and external mappers. In pseudo-static mappers, instance object factories and external mappers are still beeing used without a diagnostic. These should be ignored and a diagnostic should be emitted. ExtractObjectFactories can probably be moved after ExtractUserMappings. Then _mapperDescriptor.Static can be used in ExtractUserMappings and ExtractObjectFactories.

[trejjam]

I will take a look at the factories and external mappers

[trejjam]

I dropped one test which I would say did not test what it was saying so: 2b6db69

[trejjam]

I added tests for external mappers and object factories

[github-actions]

🎉 This PR is included in version 3.3.0-next.3 🎉

The release is available on:

• GitHub release
• v3.3.0-next.3

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

• GitHub release
• v3.3.0

Your semantic-release bot 📦🚀