cezarypiatek / MappingGenerator

:arrows_counterclockwise: "AutoMapper" like, Roslyn based, code fix provider that allows to generate mapping code in design time.
https://marketplace.visualstudio.com/items?itemName=54748ff9-45fc-43c2-8ec5-cf7912bc3b84.mappinggenerator
MIT License
1.03k stars 120 forks source link

Incorrect mapping for structs from dll #104

Closed alec-anikin closed 4 years ago

alec-anikin commented 4 years ago

Hi!

It works fine for structs declared in solution but not for external structs. VS2019, SDK 3.1.102

How to reproduce:

using System.Drawing;
using MappingGenerator.OnBuildGenerator;
namespace Test
{
    [MappingInterface]
    public interface IMapper
    {
        SomeClass Map(SomeClass other);
    }

    public sealed class SomeClass
    {
        public Point SomeStruct { get; }

        public SomeClass(Point someStruct)
        {
            SomeStruct = someStruct;
        }
    }
}

Generated mapping

public partial class Mapper : Test.IMapper
    {
        public virtual SomeClass Map(SomeClass other)
        {
            return new Protobuf.SomeClass(someStruct: new Point());
        }
    }

Thank you!

cezarypiatek commented 4 years ago

Just to be sure, the expected mapping code should be as follows?

public partial class Mapper : Test.IMapper
    {
        public virtual SomeClass Map(SomeClass other)
        {
            return new Protobuf.SomeClass(someStruct: other.SomeStruct);
        }
    }
alec-anikin commented 4 years ago

For project declared structs now mapping is

public partial class Mapper : Protobuf.IMapper
    {
        public virtual SomeClass Map(SomeClass other)
        {
            return new Protobuf.SomeClass(someStruct: new Protobuf.SomeStruct(value: other.SomeStruct.Value));
        }
    }

I think both versions are correct: someStruct: new Protobuf.SomeStruct(value: other.SomeStruct.Value) or someStruct: other.SomeStruct but the second one is simpler especially for multiple constructors cases.

cezarypiatek commented 4 years ago

@alec-anikin can you show the definition of the Point structure? The version from the external library. Does it contain fields or properties?

cezarypiatek commented 4 years ago

I've just tested it with MappingGenerator.OnBuildGenerator 1.13.329 and SmartCodeGenerator.Engine 1.2.9 and I've got the mapping code as follows

// ------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
// ------------------------------------------------------------------------------
using System;
using System.Diagnostics;
using System.Drawing;
using MappingGenerator.OnBuildGenerator;
using System.Linq;
using ConsoleApp11;

namespace ConsoleApp11
{
    [System.CodeDom.Compiler.GeneratedCode("MappingGenerator.OnBuildGenerator.OnBuildMappingGenerator", "1.13.329.0")]
    public partial class Mapper : ConsoleApp11.IMapper
    {
        public virtual SomeClass Map(SomeClass other)
        {
            return new ConsoleApp11.SomeClass(someStruct: new Point(x: other.SomeStruct.X, y: other.SomeStruct.Y));
        }
    }
}

It looks like it's working as expected. Can you send me a solution that reproduces that problem?

alec-anikin commented 4 years ago

OnBuildGenerator-Example-master.zip It sounds very strange but just add reference to another project from solution and generated code will be broken. May be two or more paths to the same type in the project is the cause.

cezarypiatek commented 4 years ago

Ok, I'm able to reproduce the issue but it completely doesn't make any sense to me. Removing reference to Lib project fixes the issue. I need to take a look closer to what's going there.

cezarypiatek commented 4 years ago

It looks like Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace (which is used for loading projects) doesn't play very well with netstandard projects. I've made a small test by replacing MSBuildWorkspace with https://github.com/daveaglick/Buildalyzer#roslyn-workspaces and it seems to solve the issue but I don't know what the ramifications could be. I need to run a couple more tests to make sure that there are no negative side effects.

cezarypiatek commented 4 years ago

Waiting for response https://github.com/daveaglick/Buildalyzer/issues/130

cezarypiatek commented 4 years ago

@alec-anikin A new version of SmartCodeGenerator.Engine has been released. Please update the SmartCodeGenerator.Engine package and verify if it works correctly. Solved with https://github.com/cezarypiatek/SmartCodeGenerator/pull/4

alec-anikin commented 4 years ago

It works fine! Thank you!