Feat: add recursive depth support to projection mappings

[TimothyMakkison]

Added max recursive depth support for projection mappings.

Description

Added attributes and properties for MaxDepth. InlineExpressionMappingBuilderContext uses an ImmutableDictionary<(ISymbol, ISymbol), int> to track types mapped by its parents, returning default when the limit is reached. The loop is prevented from occuring by disabling FindMapping for previously seen types.

Undecided/ not done

• What should the names be.
• How should negative/0 numbers be handled ie errors or revert back to current behaviour.
• What should the default depth be set to.
• This could be used in MappingBuilderContext for improved loop detection, perhaps this could be explored in a separate PR.
• Need to add tests and documentation 😞

WIP
Fixes #785, #820, #99

Checklist

• The existing code style is followed
• The commit message follows our guidelines
• Performed a self-review of my code
• Hard-to-understand areas of my code are commented
• The documentation is updated (as applicable)
• Unit tests are added/updated
• Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

[latonz]

Does this really resolve #849?

[TimothyMakkison]

Mb 😅, knew this had a load of related issues and saw "circular reference" in the title. I'll fix it

[latonz]

Thank you for this PR, nice work!
Is there sth. which hinders us to implement this for all kind of mappings instead of just IQueryable projections? I think it should almost never apply to non-IQueryable mappings anyway (as the mappings are re-used). WDYT?

[TimothyMakkison]

Is there sth. which hinders us to implement this for all kind of mappings instead of just IQueryable projections?

Don't think so, just didn't see a massive need to.

I think it should almost never apply to non-IQueryable mappings anyway (as the mappings are re-used). WDYT?

It could be used to enhance loop detection. The current implementation relies on the parent contexts type bing the same as the current mapped type, this fails to detect mapping like: A->B->A->B.
Perhaps there is a use case for a nested mapping that eventually terminates without using IReferenceHandler 🤷.

[latonz]

@TimothyMakkison I think I was confused, IReferenceHandler does only work at runtime, but what we are discussing here is the compile time 🙈 That all sounds good 😊

[latonz]

Thank you for the updates.

I think extracting ImmutableDictionary<TypeMappingKey, int> and the calls to it into a new struct/class (eg. MappingRecoursingDepthTracker) with a simple public API would probably make sense to simplify the logic inside the InlineExpressionMappingBuilderContext. WDYT?

Is MaxRecursiveDepth the correct term? Wouldn't a noun fit better eg. MaxRecursionDepth?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.14%. Comparing base (f08b7b4) to head (6028284).
Report is 137 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #878      +/-   ##
==========================================
- Coverage   91.28%   91.14%   -0.14%     
==========================================
  Files         234      237       +3     
  Lines        8086     8252     +166     
  Branches     1011     1031      +20     
==========================================
+ Hits         7381     7521     +140     
- Misses        456      482      +26     
  Partials      249      249              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[latonz]

@TimothyMakkison what is the status on this one? Should I review again?

[TimothyMakkison]

@TimothyMakkison what is the status on this one? Should I review again?

Super sorry for abandoned this.

Iirc this PR was nearish to completion

• Haven't added documentation
• Doesn't handle negative recursive values. Iirc I wasnt sure when to raise the diagnostic?
• Didn't add improved loop detection. Feel like it should be a different PR.

I'll try to finish it but I'm happy for someone else to complete it.

edit: just realised I can make MapperMaxRecursionDepthAttribute acceot a uint to prevent negative numbers