fix: member access for anonymous captures#278
Conversation
…est coverage for member access scenarios
… IQueryable for SelectExprInfo
📝 WalkthroughWalkthroughRefactors analyzer capture extraction to identify root identifiers in member-access chains; adds helpers and alias-generation in SelectExprInfo for anonymous-capture member accesses; updates SelectExpr codegen to use AsEnumerable()/AsQueryable() for capture paths; adds tests for member-access on captured objects/fields. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as "Developer code\n(SelectExpr usage)"
participant Analyzer as "LocalVariableCaptureAnalyzer\n(src/Linqraft.Analyzer)"
participant Generator as "SelectExprInfo codegen\n(src/Linqraft.Core)"
participant Runtime as "Runtime / Query execution"
Dev->>Analyzer: Analyze lambda projection\n(detect identifiers, member-access roots, anonymous captures)
Analyzer-->>Dev: Return capture metadata (root ids, capture map)
Dev->>Generator: Generate SelectExpr method\n(include captures, anonymous aliases)
Generator-->>Runtime: Emit projection with aliases\n(use AsEnumerable().Select(...) where captures exist)
Runtime->>Runtime: Execute query using generated projection\n(access captured members via aliases)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0969cbd0d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/Linqraft.Core/SelectExprInfo.cs
Outdated
| var memberName = memberAccess.Name.Identifier.Text; | ||
| var captureName = capturePropertyName ?? memberName; | ||
|
|
||
| if (!aliasMap.TryGetValue(rootName, out var members)) | ||
| { |
There was a problem hiding this comment.
Handle nested member access when building capture aliases
The alias generator only records the terminal member name (memberAccess.Name) for each capture and ignores any intermediate segments in the member-access chain. That means a capture like new { request.Inner.FromDate } will produce var request = new { FromDate = captureObj.FromDate }, but the select expression still refers to request.Inner.FromDate, so the generated code won’t compile because request has no Inner property. This regression is introduced by the new member-access aliasing logic and would affect any capture that uses nested member access.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Linqraft.Core/SelectExprInfoNamed.cs (1)
184-258:⚠️ Potential issue | 🟡 MinorGate
AsEnumerable()to anonymous captures only. The current code unconditionally appliesAsEnumerable()to all captures, but the justification in the comment—"avoid expression tree compilation with dynamic"—applies only to anonymous type captures wheredynamicis used. Non-anonymous captures are cast to their actual type and do not require client-side evaluation. ApplyingAsEnumerable()universally forces unnecessary materialization of the entire sequence into memory. Consider moving theAsEnumerable()call inside theif (isAnonymousType)block:Suggested refactor
if (isAnonymousType && CaptureArgumentType != null) { // ... dynamic extraction ... sb.Append(GenerateAnonymousCaptureMemberAccessAliases()); sb.AppendLine( $" var converted = matchedQuery.AsEnumerable().Select({LambdaParameterName} => new {dtoName}" ); } else { // For non-anonymous types, use direct Select without AsEnumerable var captureTypeName = CaptureArgumentType?.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) ?? "object"; sb.AppendLine($" var capture = ({captureTypeName})captureParam;"); sb.AppendLine( $" var converted = matchedQuery.Select({LambdaParameterName} => new {dtoName}" ); }src/Linqraft.Core/SelectExprInfoAnonymous.cs (1)
109-169:⚠️ Potential issue | 🟠 MajorAlign SelectExprInfoAnonymous with Named/Explicit paths to avoid CS1963 in expression trees when using captures.
When the source is IQueryable and captures with dynamic aliases are involved, property assignments that reference the aliases will create dynamic member access operations inside the expression tree, triggering CS1963. SelectExprInfoNamed and SelectExprInfoExplicitDto already handle this by calling
.AsEnumerable().Select()instead of.Select()directly. Apply the same fix to SelectExprInfoAnonymous for consistency and to avoid expression tree compilation with dynamic members.🛠️ Suggested fix (align anonymous path with enumerable fallback)
- sb.AppendLine($" var converted = matchedQuery.Select({LambdaParameterName} => new"); + sb.AppendLine($" var converted = matchedQuery.AsEnumerable().Select({LambdaParameterName} => new");For methods with captures, add conversion back to the appropriate return type:
- sb.AppendLine($" return converted as object as {returnTypePrefix}<TResult>;"); + if (hasCapture) + { + sb.AppendLine($" return converted.AsQueryable() as object as {returnTypePrefix}<TResult>;"); + } + else + { + sb.AppendLine($" return converted as object as {returnTypePrefix}<TResult>;"); + }
🧹 Nitpick comments (2)
src/Linqraft.Core/SelectExprInfoExplicitDto.cs (1)
525-585: GateAsEnumerable()to the anonymous-type capture path where dynamic code is generated.The code applies
AsEnumerable()unconditionally to all captures (line 552), but the justification in the comment—"avoid expression tree compilation with dynamic"—only applies whenisAnonymousTypeis true. Non-anonymous captures use straightforward casting without dynamic operations and don't require this fallback. Consider wrapping theAsEnumerable()call to apply only when handling anonymous types or whenGenerateAnonymousCaptureMemberAccessAliases()emits code, to preserve provider translation for simpler capture scenarios.src/Linqraft.Core/SelectExprInfo.cs (1)
187-208: Consider usingCodeFormatter.IndentSpacesfor consistent indentation.The generated code uses hardcoded 4-space indentation (e.g.,
" var"," {"," "), whereas the rest of this file usesCodeFormatter.IndentSpaces(indents)for consistent formatting. If the generated alias code needs to align with surrounding generated code, consider parameterizing the indentation level.♻️ Example refactor to use CodeFormatter
- protected string GenerateAnonymousCaptureMemberAccessAliases() + protected string GenerateAnonymousCaptureMemberAccessAliases(int baseIndent = 1) { // ... existing code ... + var spaces = CodeFormatter.IndentSpaces(baseIndent); + var innerSpaces = CodeFormatter.IndentSpaces(baseIndent + CodeFormatter.IndentSize); + var sb = new StringBuilder(); foreach (var kvp in aliasMap) { var rootName = kvp.Key; var members = kvp.Value; - sb.AppendLine($" var {rootName} = new"); - sb.AppendLine(" {"); + sb.AppendLine($"{spaces}var {rootName} = new"); + sb.AppendLine($"{spaces}{{"); // ... rest of formatting ... }
|
@copilot fix all review comments |
|
@arika0093 I've opened a new pull request, #279, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * fix: handle nested member access in anonymous capture aliases Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * refactor: improve code clarity per review feedback Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Linqraft.Core/SelectExprInfo.cs`:
- Around line 203-219: The loop that walks pathParts and casts child to
Dictionary<string, object> can throw InvalidCastException when a shorter path
was previously added as a leaf (a string) and a longer path reuses that prefix;
update the logic around current, child, pathParts and captureName so you check
the runtime type before casting: if child is already a Dictionary<string,
object> proceed normally, but if child is a leaf (e.g., string) replace it with
a new Dictionary<string, object> that preserves the previous leaf value under a
reserved key (for example "__leaf") and then continue; similarly, when setting
the lastPart, if current[lastPart] already exists and is a Dictionary, write the
captureName into that dictionary under "__leaf" instead of overwriting the
dictionary. This change should be applied to the code using variables current,
child, pathParts and captureName to avoid InvalidCastException.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* refactor: simplify variable capture logic and enhance test coverage for SelectExpr * feat: implement anonymous capture member access aliases and enhance test coverage for member access scenarios * refactor: update anonymous capture handling and improve conversion to IQueryable for SelectExprInfo * fix: handle nested member access in anonymous capture aliases (#279) * Initial plan * fix: handle nested member access in anonymous capture aliases Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * refactor: improve code clarity per review feedback Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Update src/Linqraft.Core/SelectExprInfo.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit d06bd3a)
* chore: update changelog for release with version from package build (cherry picked from commit 1a6ac55) * docs: update CLAUDE.md for clarity and structure improvements (cherry picked from commit 5844f5c) * Bump Swashbuckle.AspNetCore.SwaggerUI from 10.0.1 to 10.1.0 (#260) --- updated-dependencies: - dependency-name: Swashbuckle.AspNetCore.SwaggerUI dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 7965f8b) * Replace manual and Roslyn-based syntax highlighting with Shiki (#262) * Initial plan * Add Shiki highlighting service and update CodeBlock component Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Add Shiki CSS styling and update to latest version Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Replace PreviewPane with ShikiPreviewPane in Playground Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Remove unused CSharpSyntaxHighlighter registration Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Address code review feedback: add using directives and clarify documentation Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * chore: remove unused file --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> Co-authored-by: Arika Ishinami <delete0093@gmail.com> (cherry picked from commit d813ba0) * [skip-ci] chore: format code with csharpier (cherry picked from commit fe3caff) * Clean up configuration and add IncludeBuildOutput property (#263) * chore: clean up cliff.toml configuration by removing unused sections and comments * chore: add IncludeBuildOutput property to project file (cherry picked from commit 4c39282) * Update GitHub Sponsors username in FUNDING.yml (cherry picked from commit 2032ec7) * Bump csharpier from 1.2.3 to 1.2.4 (#264) --- updated-dependencies: - dependency-name: csharpier dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 21cc7d2) * Bump csharpier from 1.2.4 to 1.2.5 (#265) --- updated-dependencies: - dependency-name: csharpier dependency-version: 1.2.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 5c8b321) * chore(deps): bump actions/checkout from 4 to 6 (#266) Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit e4a8909) * chore(deps): bump dependabot/fetch-metadata from 2.4.0 to 2.5.0 (#267) Bumps [dependabot/fetch-metadata](https://github.com/dependabot/fetch-metadata) from 2.4.0 to 2.5.0. - [Release notes](https://github.com/dependabot/fetch-metadata/releases) - [Commits](dependabot/fetch-metadata@08eff52...21025c7) --- updated-dependencies: - dependency-name: dependabot/fetch-metadata dependency-version: 2.5.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit aa2fdd9) * chore(deps): bump actions/upload-artifact from 4 to 6 (#268) Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 6. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v6) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 7405906) * chore(deps): bump actions/setup-dotnet from 4 to 5 (#269) Bumps [actions/setup-dotnet](https://github.com/actions/setup-dotnet) from 4 to 5. - [Release notes](https://github.com/actions/setup-dotnet/releases) - [Commits](actions/setup-dotnet@v4...v5) --- updated-dependencies: - dependency-name: actions/setup-dotnet dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit f8d9764) * chore(deps): bump actions/download-artifact from 5 to 7 (#270) Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5 to 7. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v5...v7) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit e5a0eec) * chore(deps): bump stefanzweifel/git-auto-commit-action from 4 to 7 (#271) Bumps [stefanzweifel/git-auto-commit-action](https://github.com/stefanzweifel/git-auto-commit-action) from 4 to 7. - [Release notes](https://github.com/stefanzweifel/git-auto-commit-action/releases) - [Changelog](https://github.com/stefanzweifel/git-auto-commit-action/blob/master/CHANGELOG.md) - [Commits](stefanzweifel/git-auto-commit-action@v4...v7) --- updated-dependencies: - dependency-name: stefanzweifel/git-auto-commit-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 5fa1e2a) * Bump Microsoft.AspNetCore.Components.WebAssembly from 10.0.1 to 10.0.2 (#272) --- updated-dependencies: - dependency-name: Microsoft.AspNetCore.Components.WebAssembly dependency-version: 10.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit ea0b3a8) * Bump Microsoft.AspNetCore.Components.WebAssembly.DevServer from 10.0.1 to 10.0.2 (#273) --- updated-dependencies: - dependency-name: Microsoft.AspNetCore.Components.WebAssembly.DevServer dependency-version: 10.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 5bd0087) * Bump Microsoft.AspNetCore.OpenApi from 10.0.1 to 10.0.2 (#274) --- updated-dependencies: - dependency-name: Microsoft.AspNetCore.OpenApi dependency-version: 10.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 79c9a81) * Bump Swashbuckle.AspNetCore.SwaggerUI from 10.1.0 to 10.1.1 (#277) --- updated-dependencies: - dependency-name: Swashbuckle.AspNetCore.SwaggerUI dependency-version: 10.1.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit d4ee6a3) * fix: member access for anonymous captures (#278) * refactor: simplify variable capture logic and enhance test coverage for SelectExpr * feat: implement anonymous capture member access aliases and enhance test coverage for member access scenarios * refactor: update anonymous capture handling and improve conversion to IQueryable for SelectExprInfo * fix: handle nested member access in anonymous capture aliases (#279) * Initial plan * fix: handle nested member access in anonymous capture aliases Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * refactor: improve code clarity per review feedback Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> * Update src/Linqraft.Core/SelectExprInfo.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit d06bd3a) * [skip-ci] chore: format code with csharpier (cherry picked from commit be05ead)
close #276
Summary by CodeRabbit
Bug Fixes
Refactor
Tests