Conversation
…tional access Fix three bugs that cause build errors in NuGet-packaged scenarios: 1. LQRE001 false positive for null-conditional chains: Add MemberBindingExpressionSyntax handling in IsLocalAccess so entity?.Nav.Property patterns are correctly recognized as local access instead of outer variable references. 2. Chained null-conditional type inference: Add handling in ResolveConditionalAccessType for MemberAccessExpressionSyntax rooted in MemberBindingExpressionSyntax (e.g., entity?.Nav.Property), walking the chain to resolve the final type instead of falling back to object. 3. Inner lambda parameter type resolution: When the semantic model cannot resolve types for inner lambda parameters (common in NuGet-packaged generator scenarios), walk up the syntax tree to find the enclosing lambda and resolve the element type from the receiver collection. Also handles LINQ method chain unwrapping (OrderByDescending, Where, etc.) and Cast<T>() type overrides. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughExtended the analyzer to recognize member bindings in conditional-access expressions for local-access detection. Enhanced type inference in the source generator to handle Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4af9b015a3
ℹ️ 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".
| node is ParenthesizedLambdaExpressionSyntax parenthesizedLambda | ||
| && parenthesizedLambda.ParameterList.Parameters.Any(p => | ||
| string.Equals( | ||
| p.Identifier.ValueText, | ||
| parameterName, |
There was a problem hiding this comment.
TryResolveInnerLambdaParameterType currently treats any matching parameter in a parenthesized lambda as the sequence element, then returns the collection element/member type. In the packaged-generator scenario this fallback is used when Roslyn gives ErrorType for lambda parameters, so expressions like .Select((item, index) => index) will resolve index as item’s type instead of int, producing incorrect projected DTO/property types. This branch should only apply to the element parameter (or explicitly handle index/result-selector parameters).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple Linqraft analyzer/source-generator edge cases that previously caused build errors (or incorrect generated DTO types) when Linqraft 0.10.0 is consumed as a NuGet package—specifically around chained null-conditional access and inner-lambda parameter type inference.
Changes:
- Analyzer: treat
MemberBindingExpressionSyntaxas a local access when it’s rooted in a local conditional-access receiver (prevents false-positiveLQRE001on?.chains). - Source generator: improve type resolution for (1) chained conditional-access member paths and (2) inner lambda parameters when Roslyn returns error types in packaged scenarios; also adds
Cast<T>()element-type override handling. - Tests: add analyzer and runtime regressions for chained
?.access and inner-lambda type inference.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Linqraft.Analyzer/AnalyzerHelpers.cs | Extends local-access detection to handle MemberBindingExpressionSyntax under conditional access. |
| src/Linqraft.Core/SourceGenerator/ProjectionTemplateBuilder.BuildContext.cs | Adds inner-lambda type fallback, chained conditional-access type walking, LINQ-chain unwrapping, and Cast<T>() override support. |
| tests/Linqraft.Tests.Analyzer/AnalyzerSmokeTests.cs | Adds regression test ensuring chained ?. member access doesn’t trigger LQRE001. |
| tests/Linqraft.Tests/Nullability/Issue_ChainedNullConditionalAndInnerLambdaTypeTest.cs | Adds runtime regressions verifying generated DTO property types for chained ?. and inner-lambda projections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fallback: try to resolve via ResolveNamedType | ||
| return castTypeSyntax.ToString(); |
There was a problem hiding this comment.
In TryExtractCastTypeArgument, the fallback path says it will try ResolveNamedType, but it currently returns castTypeSyntax.ToString(). In the same NuGet/error-type scenarios this helper targets, returning a non–fully-qualified type name can produce uncompilable generated code (missing usings / wrong namespace). Consider either (a) threading defaultNamespace/useGlobalNamespaceFallback into this helper and using ResolveNamedType/ToFullyQualifiedTypeName consistently, or (b) returning null on unresolved Cast so it doesn’t override the inferred element type.
| // Fallback: try to resolve via ResolveNamedType | |
| return castTypeSyntax.ToString(); | |
| // Fallback: do not override the inferred element type when the cast type cannot be resolved | |
| return null; |
| // Verify the generated DTO property type is correct | ||
| var prop = typeof(OrderManageDto).GetProperty(nameof(OrderManageDto.TagCategories))!; | ||
| prop.PropertyType.ShouldNotBe(typeof(IEnumerable<object>)); | ||
| } |
There was a problem hiding this comment.
This regression test only asserts TagCategories is not IEnumerable. That will catch the original regression, but it can still pass if the generator infers an incorrect non-object type. Consider asserting the exact closed generic type (e.g., IEnumerable) so the test fails on any wrong inference, not just object.
|
|
||
| // Verify the generated DTO property type is List<string>, not List<object> | ||
| var prop = typeof(OrderWithTagsDto).GetProperty(nameof(OrderWithTagsDto.TagLabels))!; | ||
| prop.PropertyType.ShouldNotBe(typeof(List<object>)); |
There was a problem hiding this comment.
This regression test only asserts TagLabels is not List. Similar to the enum case, this can still pass if the generator infers a different incorrect non-object type. Consider asserting the exact type (e.g., List) to make the regression guard stricter.
| prop.PropertyType.ShouldNotBe(typeof(List<object>)); | |
| prop.PropertyType.ShouldBe(typeof(List<string>)); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Linqraft.Tests.Analyzer/AnalyzerSmokeTests.cs`:
- Around line 914-950: Add the missing Linqraft.SelectExprExtensions stub so the
test compiles: define a static class named SelectExprExtensions with the
SelectExpr<TSrc,TDest> extension method signature used in the test (matching the
call SelectExpr<OrderLine, OrderLineViewDto> in QueryHelper) and place it into
the test source string alongside the other helper stubs; this ensures SelectExpr
is resolved during compilation and the analyzer runs against valid code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c6aaee0-facf-46da-a6c4-b15074d79eca
📒 Files selected for processing (4)
src/Linqraft.Analyzer/AnalyzerHelpers.cssrc/Linqraft.Core/SourceGenerator/ProjectionTemplateBuilder.BuildContext.cstests/Linqraft.Tests.Analyzer/AnalyzerSmokeTests.cstests/Linqraft.Tests/Nullability/Issue_ChainedNullConditionalAndInnerLambdaTypeTest.cs
| const string source = """ | ||
| using System; | ||
| using System.Linq; | ||
| using Linqraft; | ||
|
|
||
| public class Product | ||
| { | ||
| public int ProductId { get; set; } | ||
| public ProductDetail? Detail { get; set; } | ||
| } | ||
|
|
||
| public class ProductDetail | ||
| { | ||
| public string Description { get; set; } = ""; | ||
| } | ||
|
|
||
| public class OrderLine | ||
| { | ||
| public int Id { get; set; } | ||
| public Product? Product { get; set; } | ||
| public DateTimeOffset CreatedAt { get; set; } | ||
| } | ||
|
|
||
| public class OrderLineViewDto { } | ||
|
|
||
| public static class QueryHelper | ||
| { | ||
| public static void QueryOrderLines(IQueryable<OrderLine> query) | ||
| { | ||
| query.SelectExpr<OrderLine, OrderLineViewDto>(l => new | ||
| { | ||
| ProductDescription = l.Product?.Detail.Description, | ||
| l.CreatedAt, | ||
| }); | ||
| } | ||
| } | ||
| """; |
There was a problem hiding this comment.
Missing SelectExprExtensions stub in test source.
The test source uses SelectExpr<OrderLine, OrderLineViewDto> at line 943, but unlike other smoke tests in this file (e.g., Missing_capture_reports_LQRE001 at lines 34-38), it doesn't include the Linqraft.SelectExprExtensions class definition. This will cause SelectExpr to be unresolved during compilation, potentially causing the test to pass for the wrong reason (no diagnostic because the code doesn't compile cleanly).
🔧 Proposed fix to add the missing stub
const string source = """
using System;
using System.Linq;
using Linqraft;
+ namespace Linqraft
+ {
+ public static class SelectExprExtensions
+ {
+ public static IQueryable<TResult> SelectExpr<TIn, TResult>(this IQueryable<TIn> query, Func<TIn, object> selector)
+ where TIn : class => throw null!;
+ }
+ }
+
public class Product📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const string source = """ | |
| using System; | |
| using System.Linq; | |
| using Linqraft; | |
| public class Product | |
| { | |
| public int ProductId { get; set; } | |
| public ProductDetail? Detail { get; set; } | |
| } | |
| public class ProductDetail | |
| { | |
| public string Description { get; set; } = ""; | |
| } | |
| public class OrderLine | |
| { | |
| public int Id { get; set; } | |
| public Product? Product { get; set; } | |
| public DateTimeOffset CreatedAt { get; set; } | |
| } | |
| public class OrderLineViewDto { } | |
| public static class QueryHelper | |
| { | |
| public static void QueryOrderLines(IQueryable<OrderLine> query) | |
| { | |
| query.SelectExpr<OrderLine, OrderLineViewDto>(l => new | |
| { | |
| ProductDescription = l.Product?.Detail.Description, | |
| l.CreatedAt, | |
| }); | |
| } | |
| } | |
| """; | |
| const string source = """ | |
| using System; | |
| using System.Linq; | |
| using Linqraft; | |
| namespace Linqraft | |
| { | |
| public static class SelectExprExtensions | |
| { | |
| public static IQueryable<TResult> SelectExpr<TIn, TResult>(this IQueryable<TIn> query, Func<TIn, object> selector) | |
| where TIn : class => throw null!; | |
| } | |
| } | |
| public class Product | |
| { | |
| public int ProductId { get; set; } | |
| public ProductDetail? Detail { get; set; } | |
| } | |
| public class ProductDetail | |
| { | |
| public string Description { get; set; } = ""; | |
| } | |
| public class OrderLine | |
| { | |
| public int Id { get; set; } | |
| public Product? Product { get; set; } | |
| public DateTimeOffset CreatedAt { get; set; } | |
| } | |
| public class OrderLineViewDto { } | |
| public static class QueryHelper | |
| { | |
| public static void QueryOrderLines(IQueryable<OrderLine> query) | |
| { | |
| query.SelectExpr<OrderLine, OrderLineViewDto>(l => new | |
| { | |
| ProductDescription = l.Product?.Detail.Description, | |
| l.CreatedAt, | |
| }); | |
| } | |
| } | |
| """; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Linqraft.Tests.Analyzer/AnalyzerSmokeTests.cs` around lines 914 - 950,
Add the missing Linqraft.SelectExprExtensions stub so the test compiles: define
a static class named SelectExprExtensions with the SelectExpr<TSrc,TDest>
extension method signature used in the test (matching the call
SelectExpr<OrderLine, OrderLineViewDto> in QueryHelper) and place it into the
test source string alongside the other helper stubs; this ensures SelectExpr is
resolved during compilation and the analyzer runs against valid code.
…TryExtractCastTypeArgument Agent-Logs-Url: https://github.com/arika0093/Linqraft/sessions/14a406ee-725d-421c-94b3-fc214ceac301 Co-authored-by: arika0093 <4524647+arika0093@users.noreply.github.com>
|
All three review items addressed in db4f906:
All 282 tests pass. |
Summary
Fix three bugs that cause build errors when Linqraft 0.10.0 is consumed as a NuGet package.
Bugs Fixed
Bug 1: LQRE001 False Positive for Null-Conditional Chains
entity?.Nav.Propertypatterns incorrectly triggeredLQRE001: SelectExpr references outer value without capture.Root cause:
IsLocalAccessinAnalyzerHelpers.csdidn't handleMemberBindingExpressionSyntax.When a member access chain follows
?.(e.g.,l.CreatedUser?.UserSettings.DisplayName), the.UserSettingspart is aMemberBindingExpressionSyntax. The analyzer couldn't trace it back to the locallparameter and falsely flaggedDisplayNameas an outer variable reference.Fix: Added a
MemberBindingExpressionSyntaxcase toIsLocalAccessthat walks up to the enclosingConditionalAccessExpressionSyntaxand checks whether its receiver is local.Bug 2: Chained Null-Conditional Type Inference →
objectentity?.Nav.Propertygenerated DTO properties typed asobjectinstead of the actual property type.Root cause:
ResolveConditionalAccessTypeonly handledMemberBindingExpressionSyntaxdirectly after?.(e.g.,?.Property), but not chainedMemberAccessExpressionSyntaxrooted in a binding (e.g.,?.Nav.Property).Fix: Added
ResolveChainedConditionalAccessTypeandContainsMemberBindinghelpers. WhenWhenNotNullis aMemberAccessExpressionSyntaxrooted in aMemberBindingExpressionSyntax, the chain is walked member-by-member from the receiver type to resolve the final type.Bug 3: Inner Lambda Member Access →
objectin NuGet-Packaged Scenarioscollection.Select(r => r.Property)generatedIEnumerable<object>instead of the correct element type (e.g.,IEnumerable<TagCategory>). Also affected scalar properties inside nested anonymous objects (e.g.,.Select(lh => new { lh.CreatedAt, ... })).Root cause: In NuGet-packaged source generator scenarios, the Roslyn semantic model returns
ErrorTypefor inner lambda parameters. The existing fallback resolution only handled the outer selector parameter (xinx => new { ... }), not parameters of nested lambdas (e.g.,rin.Select(r => r.RoleId)).Fix: Added the following helpers:
TryResolveInnerLambdaParameterType: walks the syntax tree from the expression up to the enclosing lambda, finds the invocation it's passed to, and resolves the element type from the receiver collection.UnwrapLinqChain/IsElementTypePreservingLinqMethod: strips element-type-preserving LINQ calls (OrderByDescending,Where,ThenBy, etc.) from the receiver chain to reach the root collection property.TryExtractCastTypeArgument: when aCast<T>()call wraps theSelect, usesTas the effective element type (handles patterns like.Select(x => x.Prop).Cast<DateTimeOffset?>().FirstOrDefault()).Files Changed
src/Linqraft.Analyzer/AnalyzerHelpers.csMemberBindingExpressionSyntaxcase toIsLocalAccesssrc/Linqraft.Core/SourceGenerator/ProjectionTemplateBuilder.BuildContext.cstests/Linqraft.Tests.Analyzer/AnalyzerSmokeTests.csentity?.Nav.Propertypatternstests/Linqraft.Tests/Nullability/Issue_ChainedNullConditionalAndInnerLambdaTypeTest.csVerification
Branch
fix/type-inference-and-analyzer-bugsSummary by CodeRabbit