Unify member-path resolution behind a shared MemberPathResolver

[rolandbanks]

Summary

Consolidates seven independent, hand-rolled implementations that resolve a LINQ MemberExpression chain into an OData path segment - used by $filter, top-level $select, $orderby/NavigateTo, top-level $expand, ExpandWithSelect, the NestedExpandBuilder nested-configure callback, and Any/All predicates - into one shared internal MemberPathResolver.

Zero intended behavior change. ~11 characterization tests were added first, in their own commit, before any production code changed, pinning today's exact output for nested/dotted property paths across all seven call sites - including four pre-existing truncation bugs/quirks that are deliberately preserved, not fixed, in this PR:

• .OrderBy(p => p.Nav.Prop) / .NavigateTo(p => p.Nav.Prop) silently truncate to Prop, dropping Nav/.
• .Select(p => p.Nav.Prop) silently truncates to Nav, dropping /Prop (opposite direction).
• .ExpandWithSelect(nav, x => x.Deeper.Prop) and NestedExpandBuilder's nested .Select(x => x.Deeper.Prop) truncate the same way as OrderBy.

As a side effect, this also replaces the Any/All lambda parser's List.Insert(0,...) (O(n²)) walk with the same Stack-based (O(n)) approach already used elsewhere, with byte-identical output.

Why: while reviewing an external PR that patches [JsonPropertyName]-awareness into exactly one of these seven call sites (creating an inconsistency - .Filter() would honor the attribute, .OrderBy() wouldn't), we found the underlying duplication was the real problem. This PR does the prerequisite consolidation only. Adopting [JsonPropertyName] support, or fixing the four truncation bugs above, is intentionally out of scope here - each becomes a single-place change to MemberPathResolver once this lands.

Verified

• Full test suite green on every commit: 786/786 passing (777 pre-existing + ~11 new characterization tests).
• Clean Release build, zero warnings (TreatWarningsAsErrors).
• Characterization test assertions are byte-identical from the first (tests-only) commit through the final migration commit - confirmed via git diff on those files.
• MemberPathResolver and the relocated ExpandSegment record remain internal - no public API surface added.
• 14 independent adversarial code reviews (2 per migrated function, one control-flow angle and one edge-case angle) each hand-traced old vs. new logic and found zero behavioral divergence - covering Stack vs. List enumeration order, Convert-vs-loose-Unary gating differences (deliberately preserved as two distinct methods, not unified), reference-equality on the lambda parameter, closure evaluation, and FieldInfo vs. PropertyInfo members.

Test plan

• dotnet build -c Release - 0 warnings, 0 errors
• dotnet test - 786/786 passed
• Diffed characterization test files between first and last commit - empty diff
• Multi-agent adversarial review of each of the 7 migrations

🤖 Generated with Claude Code

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 medium

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
Complexity	1 medium

View in Codacy

🟢 Metrics 23 complexity · -4 duplication

Metric	Results
Complexity	23
Duplication	-4

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

This PR consolidates LINQ member-expression resolution into a shared MemberPathResolver, which is a positive architectural step. However, the current implementation is not up to standards due to high cyclomatic complexity and inconsistent application across query builders.

The MemberPathResolver.cs file introduces a significant complexity spike (delta of 30), primarily driven by the IsNavigationProperty method, which exceeds project thresholds and presents a maintenance risk. Furthermore, while the intent was to unify path resolution, the implementation of top-level $select parsing in ODataQueryBuilder.cs remains inconsistent, using the new resolver for anonymous types (NewExpression) but reverting to legacy direct member access for single properties. This inconsistency could lead to unpredictable OData URL generation and should be addressed before merging to ensure the characterization of legacy quirks is stable.

About this PR

• Consolidation of path resolution is incomplete. ODataQueryBuilder.ExpressionParsing.cs still contains significant parsing logic (e.g., GetSelectFieldNames) that operates independently of the MemberPathResolver. This bypasses the goal of a single source of truth for member-path translation.
• There is systemic inconsistency in how the new resolver is utilized. Several files still access member.Member.Name directly instead of using the resolver's leaf-name methods, which undermines the goal of simplifying future support for attributes like [JsonPropertyName].

1 comment outside of the diff

PanoramicData.OData.Client/NestedExpandBuilder.cs

_{line 41 ⚪ LOW RISK}
Suggestion: ```suggestion
_selectFields.Add(MemberPathResolver.GetLeafMemberNameOrEmpty(member));

Test suggestions

• Verify $filter correctly resolves full nested paths (p.A.B -> A/B)
• Verify top-level $select preserves legacy first-segment truncation quirk (p.A.B -> A)
• Verify $orderby preserves legacy leaf-segment truncation quirk (p.A.B -> B)
• Verify NavigateTo(p.A.B) preserves legacy leaf-segment truncation (A/B -> B)
• Verify ExpandWithSelect and NestedExpandBuilder.Select preserve leaf-segment truncation
• Verify Any/All predicates correctly resolve full nested paths inside the lambda (f => f.A.B -> A/B)
• Verify nested closure member access evaluation (captured.Nested.Prop -> Value) in Any/All predicates

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🔴 HIGH RISK}

The IsNavigationProperty method exceeds complexity limits (15 vs limit of 8). This is largely due to the serial type checks for scalar types. You can simplify this and reduce the branching factor by using a lookup set for known scalar types and grouping the boolean checks.

Try running the following prompt in your IDE agent:

Refactor the IsNavigationProperty method in MemberPathResolver.cs to reduce its cyclomatic complexity below 8. Consolidate the individual type checks (string, DateTime, DateTimeOffset, DateOnly, TimeOnly, TimeSpan, Guid, decimal, byte[]) into a static HashSet<Type> and simplify the conditional logic for primitives, enums, and collections.

See Issue in Codacy

[codacy-production]

_{🟡 MEDIUM RISK}

The property resolution logic is inconsistent between 'NewExpression' and 'MemberExpression' branches. While the 'NewExpression' branch resolves to the leaf name ('FirstName'), the single property branch in 'ExpressionParsing.cs' resolves to the first path segment ('BestFriend'). These should be unified behind a consistent strategy in 'MemberPathResolver' to ensure the generated OData $select is predictable.

[codacy-production]

Pull Request Overview

This PR successfully consolidates seven disparate LINQ MemberExpression resolution implementations into a shared internal MemberPathResolver. It achieves the goal of eliminating duplication and improves the performance of Any/All predicates from O(n²) to O(n) using a stack-based approach. All acceptance criteria, including the preservation of legacy path-truncation quirks, have been verified via characterization tests.

However, the PR is currently not up to standards according to the quality analysis. The main concern is the significant increase in cyclomatic complexity within MemberPathResolver.cs (+30). Specifically, the IsNavigationProperty method exceeds complexity thresholds and requires refactoring to maintainable patterns like switch expressions or static HashSets before this PR should be merged.

Test suggestions

• Filter resolves full nested member paths (e.g., p.A.B -> A/B)
• OrderBy/NavigateTo truncates nested paths to the leaf segment (e.g., p.A.B -> B)
• Top-level Select truncates nested paths to the root segment (e.g., p.A.B -> A)
• ExpandWithSelect and NestedExpandBuilder.Select truncate nested paths to the leaf segment
• Any/All predicates resolve full nested member paths correctly in the lambda body
• Nested closure member access (captured objects) in Any predicates evaluates correctly

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🟡 MEDIUM RISK}

The IsNavigationProperty method has high cyclomatic complexity (15) due to the sequential if-statements used for type classification. This should be refactored into a more concise switch expression or a static HashSet for scalar types to improve maintainability and satisfy quality standards.

[codacy-production]

_{⚪ LOW RISK}

Nitpick: The Name element in the Segments stack is redundant because it is already available via the MemberInfo.Name property. Consider changing the return type to Stack<MemberInfo> to simplify the walk and its consumers.