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

Downcasting objects ignores explicit MapProperty instructions #1684

Open
3 tasks done
brutaldev opened this issue Jan 15, 2025 · 2 comments
Open
3 tasks done

Downcasting objects ignores explicit MapProperty instructions #1684

brutaldev opened this issue Jan 15, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@brutaldev
Copy link

  • I have read the documentation, including the FAQ
  • I can reproduce the bug using the latest prerelease version
  • I have searched existing discussion and issue to avoid duplicates

Describe the bug
When defining MapProperty rules on a method that maps from a parent type to sub-type the generated code is a downcast and the MapProperty instruction is ignored. A real-world example where this was picked up is a class that encrypts items in its base class. When you need to convert it back to the base class the encrypted values need to be copied back into the base class properties unencrypted. MapProperty is used to instruct it to do this but the generated code does not include this instruction.

Declaration code

public class ComplexType
{
  public List<string> Strings { get; set; } = [];
}

public class AnotherComplexType : ComplexType
{
  public List<string> SameStrings { get; set; } = [];
}

[Mapper]
public static partial class MyMapper
{
  [MapProperty(nameof(AnotherComplexType.SameStrings), nameof(ComplexType.Strings))]
  public static partial ComplexType BrokenMapper(this AnotherComplexType source);
}

Actual relevant generated code

public static partial class MyMapper
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.1.1.0")]
    public static partial global::MapperlyInheritanceMapPropertyBug.ComplexType BrokenMapper(this global::MapperlyInheritanceMapPropertyBug.AnotherComplexType source)
    {
        return (global::MapperlyInheritanceMapPropertyBug.ComplexType)source;
    }
}

Expected relevant generated code

public static partial global::MapperlyInheritanceMapPropertyBug.ComplexType WorkingMapper(this global::MapperlyInheritanceMapPropertyBug.AnotherComplexType source)
{
  var target = (global::MapperlyInheritanceMapPropertyBug.ComplexType)source;

  // Use MapProperty instructions.
  target.Strings = source.SameStrings;

  return target;
}

OR

public static partial global::MapperlyInheritanceMapPropertyBug.ComplexType WorkingMapper(this global::MapperlyInheritanceMapPropertyBug.AnotherComplexType source)
{
  var target = new global::MapperlyInheritanceMapPropertyBug.ComplexType();
  target.Strings = global::System.Linq.Enumerable.ToList(source.SameStrings);
  return target;
}

Reported relevant diagnostics
None

Environment (please complete the following information):

  • Mapperly Version: 4.1.1
  • Nullable reference types: Enabled
  • .NET Version: 9.0.0
  • Target Framework: net9.0
  • Compiler Version: 4.12.0-3.24572.7 (dfa7fc6b)
  • C# Language Version: 13.0
  • IDE: Visual Studio v17.12.3
  • OS: Windows 11

Additional context
Using UseDeepCloning works correctly producing the following generated code:

public static partial class MyMapper
{
    [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.1.1.0")]
    public static partial global::MapperlyInheritanceMapPropertyBug.ComplexType BrokenMapper(this global::MapperlyInheritanceMapPropertyBug.AnotherComplexType source)
    {
        var target = new global::MapperlyInheritanceMapPropertyBug.ComplexType();
        target.Strings = global::System.Linq.Enumerable.ToList(source.SameStrings);
        return target;
    }
}

However, this applies to the entire Mapper class not just specific mapper methods and in complex mappers with dozens of types you would sacrifice reference copying performance across the board.

@brutaldev brutaldev added the bug Something isn't working label Jan 15, 2025
@latonz latonz added enhancement New feature or request and removed bug Something isn't working labels Jan 20, 2025
@latonz
Copy link
Contributor

latonz commented Jan 20, 2025

Thanks for reporting. Currently this is the expected behaviour, as Mapperly considers a cast as a "complete" mapping and doesn't apply any further mapping rules. I can see that there are use-cases for which additional configuration rules make sense, I changed it from a bug to a feature request 😊

@brutaldev
Copy link
Author

The more I think about it and go over this with colleagues of mine, we seem to agree this violates POLA. If you've supplied mapping rules the expectation is that they are honoured. I think it's certainly a surprise when attributes are ignored.

The code in question was a conversion from AutoMapper that was quickly picked up as not working despite the correct code conversion being put in place. And not to say AutoMapper made all the correct assumptions about things, but if you supplied a FromMember rule it would certainly not ignore it.

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

2 participants