Remove UseProvidedBody string matching from DelegateBodySyntaxExtractor#96
Conversation
Refactor the delegate body extraction to use structural syntax analysis instead of searching for the "UseProvidedBody" method name string. Changes: - DelegateBodySyntaxExtractor now finds the lambda by looking at the outermost invocation's lambda argument in the method's return expression, instead of scanning for "UseProvidedBody" string - FluentBodyResult gets HasDelegateBody flag indicating RuntimeDelegateBody was set in BodyGenerationData - BodyGenerationDataExtractor sets HasDelegateBody when RuntimeDelegateBody is non-null - GeneratesMethodGenerationPipeline executes the method first, then uses HasDelegateBody to decide if syntax extraction is needed - Add missing WithCompileTimeConstants implementations to fix pre-existing build errors in DataMethodBodyBuilders Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com> Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/ea25cd90-2a07-42c1-8091-f255cb2c6687
…umption Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com> Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/ea25cd90-2a07-42c1-8091-f255cb2c6687
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR removes fragile string-matching for detecting UseProvidedBody and instead uses BodyGenerationData.RuntimeDelegateBody to decide when to extract a delegate body, while updating syntax extraction to look at the generator method’s return expression structure.
Changes:
- Added
HasDelegateBodytoFluentBodyResultand populated it fromBodyGenerationData.RuntimeDelegateBody. - Reordered fluent-body generation flow to execute the generator method first, then conditionally run syntax extraction only when a delegate body was provided.
- Updated
DelegateBodySyntaxExtractorto extract the lambda body from the outermost invocation in the generator method’s return expression (instead of scanning descendants for"UseProvidedBody"), and added missing fluent builder stages for compile-time constants.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodGenerationPipeline.cs | Switches fluent-body flow to runtime-execute first and then conditionally syntax-extract delegate bodies. |
| EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodExecutionRuntime.cs | Extends FluentBodyResult to include HasDelegateBody. |
| EasySourceGenerators.Generators/IncrementalGenerators/DelegateBodySyntaxExtractor.cs | Changes extraction strategy to use the generator method’s return expression shape and adds GetReturnExpression. |
| EasySourceGenerators.Generators/IncrementalGenerators/BodyGenerationDataExtractor.cs | Sets HasDelegateBody based on presence of RuntimeDelegateBody. |
| EasySourceGenerators.Generators/DataBuilding/DataMethodBodyBuilders.cs | Adds missing WithCompileTimeConstants and Stage5 builder implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (result!.HasDelegateBody) | ||
| { | ||
| string? delegateBody = DelegateBodySyntaxExtractor.TryExtractDelegateBody(methodInfo.Syntax); | ||
| if (delegateBody != null) | ||
| { |
There was a problem hiding this comment.
When HasDelegateBody is true but TryExtractDelegateBody returns null (e.g., non-lambda argument or unexpected return-expression shape), the code silently falls back to GenerateSimplePartialMethod. For parameterized delegates, ReturnValue will be null and the generated partial method will return default, which is incorrect and hard to diagnose. Consider reporting a diagnostic (and failing generation for that target) when HasDelegateBody is true but extraction fails, instead of falling back to the runtime-evaluated ReturnValue.
| if (method.Body != null) | ||
| { | ||
| ReturnStatementSyntax? returnStatement = method.Body.Statements | ||
| .OfType<ReturnStatementSyntax>() | ||
| .FirstOrDefault(); | ||
| return returnStatement?.Expression; | ||
| } |
There was a problem hiding this comment.
GetReturnExpression’s comment says the method “assumes … at most one return statement”, but the implementation uses FirstOrDefault(), which will silently pick the first return if there are multiple. Consider enforcing the assumption (e.g., SingleOrDefault + diagnostic) or updating the comment/logic so unexpected shapes don’t lead to extracting the wrong expression without any signal.
DelegateBodySyntaxExtractorsearched for lambdas by matching the string"UseProvidedBody"against method names in the syntax tree. This is fragile and unnecessary sinceBodyGenerationData.RuntimeDelegateBodyalready tells us whether a delegate body was provided.Approach
Instead of string-matching a method name, use the data from
BodyGenerationDatato decide when to extract, and structural position to decide what to extract:FluentBodyResult— AddedHasDelegateBodyflag, set whenRuntimeDelegateBodyis non-nullGeneratesMethodGenerationPipeline— Reversed the flow: execute the generator method first, then useHasDelegateBodyto conditionally trigger syntax extractionDelegateBodySyntaxExtractor— Replaced descendant-node scan for"UseProvidedBody"with structural lookup: find the outermostInvocationExpressionSyntaxin the method's return expression and extract its lambda argumentPre-existing fix
Added missing
WithCompileTimeConstantsand Stage5 implementations inDataMethodBodyBuilders.csto fix build errors on this branch.Original prompt
⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.