MappingGenerator icon indicating copy to clipboard operation
MappingGenerator copied to clipboard

Assign default or invalid values for unmapped fields.

Open cezarypiatek opened this issue 7 years ago • 17 comments

Something like that https://github.com/angularsen/roslyn-analyzers#sample

cezarypiatek avatar Mar 14 '18 21:03 cezarypiatek

Just to be clear, I want the analyzer to give me errors when there are new properties that are not in the mapping, because of newly added properties for example. And if some properties are voluntary left unset, we just have to comment these properties out to suppress the errors.

OmiCron07 avatar Mar 15 '18 13:03 OmiCron07

Sounds exactly like the mentioned https://github.com/angularsen/roslyn-analyzers#sample project. I'm not gonna duplicate this functionality (besides I think it's out of the scope of this project). Use mentioned project to keep tracking changes in properties set. You can generate mapping with MappingGenerator and add this magical comment to enable tracking changes. If I misunderstood you, please correct me.

cezarypiatek avatar Mar 15 '18 14:03 cezarypiatek

I just wanted to add that while the mentioned project does mark these properties it is not possible to abstract this magic comment so that your team members will remember it. I haven't found a sufficient solution for this problem yet.

p3t3rix avatar Oct 07 '19 16:10 p3t3rix

I found another project that does something similar but is not as easily dismissable as a comment: https://github.com/oliverhanappi/ManualMappingGuard

p3t3rix avatar Oct 14 '19 11:10 p3t3rix

@p3t3rix please forgive me, but is's hard for me to understand what's your problem and your expectation against the solution. Can you elaborate a little bit more and provide examples? I'm currently working on the mechanism to generate mapping implementation during the build - maybe this is what you actually need. You can track the progress here https://github.com/cezarypiatek/MappingGenerator/issues/79

cezarypiatek avatar Oct 14 '19 16:10 cezarypiatek

I have nothing against the solution, i just wanted to list alternatives for people like me that think that an annotation is a cleaner way to express that a method is a mapping method (instead of a comment). At the moment we use the MappingGenerator and the mentioned manual mapping guard together and it works for us.

p3t3rix avatar Oct 17 '19 14:10 p3t3rix

Instead of setting up default value MappingGenerator should generate a comment with information about unmapped properties.

cezarypiatek avatar Nov 11 '19 11:11 cezarypiatek

@cezarypiatek when doing "add parameter" refactoring using Rider/ReSharper, TODO "variable" is used for a new parameter. It leads to compilation error and brings attention to places which require fix.

Does it make sense to follow this approach in MappingGenerator?

For example:

public class TestMapper
{
    public static AccountDTO Map(AccountEntity entity)
    {
        return new AccountDTO()
        {
            BankName = entity.BankName,
            Number = entity.Number,
            Unmapped = TODO
        };
    }
}
    
public class AccountDTO
{
    public string BankName { get; set; }
    public string Number { get; set; }
    public string Unmapped { get; set; }
}
    
public class AccountEntity
{
    public string BankName { get; set; }
    public string Number { get; set; }
}

UPD:

I've tried to hack in this behavior quickly and can see a number of tests failing because of missing properties now produce this TODO code. Is this intentional or just a result copy-paste in test data?

ishymko avatar Feb 03 '20 18:02 ishymko

Hi @vanashimko, I'm using Data-Driven approach for testing MappingGenerator. I think this is quite expected that a lot of tests can fail when you change one of the fundamental behavior. I don't know why there are unmapped fields in the expected outputs - some of them may be intentional and maybe some of them are caused by the potential errors in MappingGenerator. I think this requires an individual investigation case by case.

Adding TODO in the same form as Resharp does it will produce an invalid code. This form of code generation should be available as a separate position in Roslyn menu.

cezarypiatek avatar Feb 03 '20 20:02 cezarypiatek

Thanks for quick response, @cezarypiatek! Sorry, I'm a bit confused on what is this issue about.

I thought that TODO fits ok in "assign invalid values". And now I see your comment "MappingGenerator should generate a comment with information about unmapped properties".

So are you about a comment which does not break code and make it non-compilable (as TODO variable does) and works as a default behavior in existing code fix? Something like:

public class TestMapper
{
    public static AccountDTO Map(AccountEntity entity)
    {
        return new AccountDTO()
        {
            BankName = entity.BankName,
            Number = entity.Number,
            // TODO: Unmapped = ?
        };
    }
}
    
public class AccountDTO
{
    public string BankName { get; set; }
    public string Number { get; set; }
    public string Unmapped { get; set; }
}
    
public class AccountEntity
{
    public string BankName { get; set; }
    public string Number { get; set; }
}

ishymko avatar Feb 04 '20 05:02 ishymko

@vanashimko yes, the approach with comments is the preferred one in the default behavior.

cezarypiatek avatar Feb 04 '20 17:02 cezarypiatek

@cezarypiatek, since it's the preferred approach, is it going to be implemented? Will/does it work both ways, like so (notice the Unmapped2 property) ?:

public class TestMapper
{
    public static AccountDTO Map(AccountEntity entity)
    {
        return new AccountDTO
        {
            BankName = entity.BankName,
            Number = entity.Number,
            // TODO: Unmapped = ?,
            // TODO: ? = Unmapped2
        };
    }
}
    
public class AccountDTO
{
    public string BankName { get; set; }
    public string Number { get; set; }
    public string Unmapped { get; set; }
}
    
public class AccountEntity
{
    public string BankName { get; set; }
    public string Number { get; set; }
    public string Unmapped2 { get; set; }
}

Also, the brackets on the AccountDto constructor could be removed (and should be, according to my analyzer).

Thanks for the great work, even though I haven't had the opportunity to try yet because I need all properties to be processed, beit in a TODO comment. 😃

LaBetonneuse avatar Aug 20 '20 13:08 LaBetonneuse

In spite of appearances, it's a difficult feature to implement. I've made a few attempts but I failed because there are some edge cases that I don't know how to cover. // TODO: ? = Unmapped2 <<-- this one is impossible to implement with the current MappingGenerator design.
Currently, I'm working on #133 which is very complex and time-consuming. When I finish it I will try once again to deal with this issue.

cezarypiatek avatar Aug 20 '20 21:08 cezarypiatek

For validation in design time, if all members of the target object are assigned, you can use my another extension which is described here Immutable types in C# with Roslyn - this can be solved with [InitRequire] attribute or with /FullInitRequired/ marker.

image

image

cezarypiatek avatar Aug 22 '20 07:08 cezarypiatek

OK, I'll have a look. Thanks. 😊


De : Cezary Piątek [email protected] Envoyé : samedi 22 août 2020 09:00 À : cezarypiatek/MappingGenerator Cc : guillaume; Comment Objet : Re: [cezarypiatek/MappingGenerator] Assign default or invalid values for unmapped fields. (#7)

For validation in design time, if all members of the target object are assigned, you can use my another extension which is described here Immutable types in C# with Roslynhttps://cezarypiatek.github.io/post/immutable-types-with-roslyn/ - this can be solved with [InitRequire] attribute or with /FullInitRequired/ marker.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/cezarypiatek/MappingGenerator/issues/7#issuecomment-678605701, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACJGQSHAGGQAWUPW5XTLHR3SB5UI3ANCNFSM4EVM3EXA.

LaBetonneuse avatar Aug 23 '20 17:08 LaBetonneuse

@LaBetonneuse Empty constructor brackets have been removed in 1.17.435

cezarypiatek avatar Aug 31 '20 20:08 cezarypiatek

The new version of MappingGenerator have a nice UI configure which allows to control how unmapped target fields are handled.

image

Please update your MappingGenerator extension.

If you have further suggestion please report them via the new issue tracker https://github.com/cezarypiatek/MappingGeneratorIssueTracker

cezarypiatek avatar Jun 01 '21 15:06 cezarypiatek