Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

Assign default or invalid values for unmapped fields. #7

Open
cezarypiatek opened this issue Mar 14, 2018 · 17 comments
Open

Assign default or invalid values for unmapped fields. #7

cezarypiatek opened this issue Mar 14, 2018 · 17 comments
Labels
enhancement New feature or request

Comments

@cezarypiatek
Copy link
Owner

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

@cezarypiatek cezarypiatek added the enhancement New feature or request label Mar 14, 2018
@OmiCron07
Copy link

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.

@cezarypiatek
Copy link
Owner Author

cezarypiatek commented Mar 15, 2018

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.

@p3t3rix
Copy link

p3t3rix commented Oct 7, 2019

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
Copy link

p3t3rix commented Oct 14, 2019

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

@cezarypiatek
Copy link
Owner Author

@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 #79

@p3t3rix
Copy link

p3t3rix commented Oct 17, 2019

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.

@cezarypiatek
Copy link
Owner Author

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

@ishimko
Copy link

ishimko commented Feb 3, 2020

@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?

@cezarypiatek
Copy link
Owner Author

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.

@ishimko
Copy link

ishimko commented Feb 4, 2020

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; }
}

@cezarypiatek
Copy link
Owner Author

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

@LaBetonneuse
Copy link

@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. 😃

@cezarypiatek
Copy link
Owner Author

cezarypiatek commented Aug 20, 2020

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
Copy link
Owner Author

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

@LaBetonneuse
Copy link

LaBetonneuse commented Aug 23, 2020 via email

@cezarypiatek
Copy link
Owner Author

@LaBetonneuse Empty constructor brackets have been removed in 1.17.435

@cezarypiatek
Copy link
Owner Author

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

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

No branches or pull requests

5 participants