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

Disable explicit cast for widening conversions #1638

Open
arthrp opened this issue Dec 14, 2024 · 4 comments
Open

Disable explicit cast for widening conversions #1638

arthrp opened this issue Dec 14, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@arthrp
Copy link

arthrp commented Dec 14, 2024

Is your feature request related to a problem? Please describe.
Currently there's always an explicit cast when casting one numeric type to another. However, it's not needed for widening conversions! Consider the following example:

public class DtoA
{
    public byte Count { get; set; }
}

public class DtoB
{
    public int Count { get; set; }
}

[Mapper]
public static partial class Mapper
{
    public static partial DtoB Map(DtoA a);
}

It produces this mapper:

    public static partial class Mapper
    {
        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.1.1.0")]
        public static partial global::MapperlyExample.DtoB Map(global::MapperlyExample.DtoA a)
        {
            var target = new global::MapperlyExample.DtoB();
            target.Count = (int)a.Count;
            return target;
        }
    }

The cast is redundant. Some might also view the explicit cast as a code smell, since if you change the DtoA Count to be long:

public class DtoA
{
    public long Count { get; set; }
}

the mapping code stays the same - but now you have potential data loss as it's a narrowing conversion!

Describe the solution you'd like
Either:

  • Change default behavior not to produce explicit casts in mappers on numeric types widening conversions
    OR
  • Introduce parameter to control that behavior i.e. [Mapper(NoExplicitWideningCast = true)]
@arthrp arthrp added the enhancement New feature or request label Dec 14, 2024
@TonEnfer
Copy link
Contributor

Hi @arthrp
It seems that what you are asking for can be achieved to some extent with the following setup: [Mapper(EnabledConversions = MappingConversionType.All & ~MappingConversionType.ExplicitCast)]. Docs
Maybe this solution will suit you?

@arthrp
Copy link
Author

arthrp commented Dec 15, 2024

Thanks, but applying [Mapper(EnabledConversions = MappingConversionType.All & ~MappingConversionType.ExplicitCast)] actually produces identical mapping code to the one shown above (with explicit cast). Also same when allowing only implicit casts EnabledConversions = MappingConversionType.ImplicitCast - looks like a bug? :)

@TonEnfer
Copy link
Contributor

TonEnfer commented Dec 15, 2024

Indeed, the above code with byte to int mapping still produces code with cast, but the following code does not generate cast, instead it produces error RMG007:

public class DtoA
{
    public long Count { get; set; }
}

public class DtoB
{
    public int Count { get; set; }
}

[Mapper(EnabledConversions = MappingConversionType.All & ~MappingConversionType.ExplicitCast)]
public static partial class AnimalCloningExtensions
{
    public static partial DtoB Map(DtoA source); //<- RMG007 Could not map member ConsoleApp1.DtoA.Count of type long to ConsoleApp1.DtoB.Count of type int
}

Are you getting a different result? If so, I think you should provide information about your environment and code sample (see the bug issue template) and wait for the developers to respond

@latonz
Copy link
Contributor

latonz commented Dec 16, 2024

As mentioned by @TonEnfer, your use case should be covered by the EnabledConversions configuration. Note that Mapperly also adds the cast syntax for implicit conversions. This approach simplifies code generation, as the type may not always be implicitly clear (e.g., in lambdas). However, this should not cause any issues, since it is not possible to define both an explicit and an implicit operator for the same conversion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants