-
Notifications
You must be signed in to change notification settings - Fork 120
Feature/mapping get only lists #117
base: master
Are you sure you want to change the base?
Feature/mapping get only lists #117
Conversation
I noticed after upgrading the test project to netcoreapp3.1 this one test fails without any other changes to the code, and it seems a bit flaky sometimes the enum is wrong but other times not. |
I have made a separate pull request to update netcoreapp3.1 first, this will fix the failing unit test |
@cezarypiatek I will update this pr soon with the latest changes of the upstream branch and use dotnet 3.0. Could you let me know if this is the right direction for this solution? |
@Wazowsk1 please rebase your branch to master and push it again. Then I will be able to review your changes. |
f36acb3
to
2f6fae0
Compare
@Wazowsk1 I've started do review your code. Hell lot of the work 👍 Please give me a couple of days for this. |
...pingGenerator/Mappings/MappingImplementors/SingleParameterForeachMappingMethodImplementor.cs
Outdated
Show resolved
Hide resolved
return true; | ||
} | ||
|
||
if (ContainsCollectionWithoutSetter(MappingHelper.GetElementType(member.Type))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to handle recursive types to avoid stack overflow exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the mappingPath check in the implementor
{ | ||
foreach (var member in type.GetMembers().Where(ObjectHelper.IsPublicPropertySymbol).OfType<IPropertySymbol>()) | ||
{ | ||
if (ObjectHelper.IsSimpleType(member.Type) || !MappingHelper.IsCollection(member.Type)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this condition will prevent inspections of complex objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, see test 037
0e0f951
to
621642d
Compare
#112