Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 10, 2025

Summary

Fixed source generator to handle void-returning activity functions with improved robustness.

Changes Made

  • Fixed null safety issues in return type detection
  • Removed unsafe casts to INamedTypeSymbol
  • Fixed semantic issue where returnSymbol was set to object for void returns
  • Only include non-void return types in namespace resolution
  • Added TypeKind.Error check to handle compilation errors gracefully
  • Made returnType parameter nullable in constructor to avoid fragile placeholder pattern
  • All tests pass (43/43)

Technical Details

Addressed all review feedback:

  1. NPE protection: Added null check after GetTypeInfo().Type instead of using null-forgiving operator
  2. Error type handling: Added TypeKind.Error check to gracefully handle compilation errors
  3. Cast safety: Added proper type checking with pattern matching before casting to INamedTypeSymbol
  4. Semantic correctness: returnSymbol is now nullable and left as null for void returns, only used where it has meaning
  5. Constructor design: Made returnType parameter nullable, eliminating the need for fragile placeholder pattern

The generator correctly handles void, Task, and Task return types with robust error handling throughout.

Original prompt

This section details on the original issue you should resolve

<issue_title>Source generator doesn't handle activity functions that return void</issue_title>
<issue_description>I have the following activity definition:

[Function(nameof(FlakeyActivity))]
public static void FlakeyActivity([ActivityTrigger] object _)
{
    throw new ApplicationException("Kah-BOOOOM!!!");
}

Source-gen tried to produce the following:

public static Task<void> CallFlakeyActivityAsync(this TaskOrchestrationContext ctx, object _, TaskOptions? options = null)
{
    return ctx.CallActivityAsync<void>("FlakeyActivity", _, options);
}

Obviously, this didn't compile. It seems we need to handle this as a special-case.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits December 10, 2025 20:10
Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix source generator to handle activity functions returning void Fix source generator for void-returning activity functions Dec 10, 2025
Copilot AI requested a review from YunchuWang December 10, 2025 20:23
@YunchuWang
Copy link
Member

@copilot address my comments

@YunchuWang YunchuWang marked this pull request as ready for review December 22, 2025 20:17
Copilot AI review requested due to automatic review settings December 22, 2025 20:17
…semantics

Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a source generator bug where activity functions returning void or non-generic Task would produce invalid C# code (Task<void> doesn't compile).

Key Changes

  • Added void return type detection in DurableFunction.cs using SpecialType.System_Void and non-generic Task comparison
  • Modified generator to emit Task (not Task<T>) and call non-generic CallActivityAsync for void-returning activities
  • Made object parameters nullable to match the API signature of CallActivityAsync

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Generators/AzureFunctions/DurableFunction.cs Added ReturnsVoid property and detection logic for void and non-generic Task returns
src/Generators/DurableTaskSourceGenerator.cs Added conditional code generation based on ReturnsVoid flag to emit correct method signatures
src/Generators/AzureFunctions/TypedParameter.cs Made object parameters nullable when not already annotated as nullable
test/Generators.Tests/AzureFunctionsTests.cs Added test coverage for void return and Task (non-generic) return scenarios

Comment on lines +73 to +74
INamedTypeSymbol? returnSymbol = null;

Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "we'll use object as a placeholder since it won't be used", but this is misleading. The returnSymbol is actually used: it's passed to GetRenderedTypeExpression in the constructor (line 41) to set the ReturnType property, and it's added to the usedTypes list (line 117) to determine required namespaces. Consider revising the comment to more accurately reflect that object is used as a safe default type for namespace resolution, even though the actual return type won't be rendered in the generated code due to the ReturnsVoid flag.

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +84
{
INamedTypeSymbol? nonGenericTaskSymbol = model.Compilation.GetTypeByMetadataName("System.Threading.Tasks.Task");
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "we'll use object as a placeholder since it won't be used", but this is misleading. The returnSymbol is actually used: it's passed to GetRenderedTypeExpression in the constructor (line 41) to set the ReturnType property, and it's added to the usedTypes list (line 117) to determine required namespaces. Consider revising the comment to more accurately reflect that object is used as a safe default type for namespace resolution, even though the actual return type won't be rendered in the generated code due to the ReturnsVoid flag.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI commented Dec 22, 2025

@copilot address my comments

All comments addressed in c94763f:

  • Added null check for Type instead of using null-forgiving operator
  • Replaced unsafe casts with pattern matching and proper type checks
  • Made returnSymbol nullable and semantically correct (null for void returns)
  • Only include non-void types in namespace resolution

All 43 tests passing.

Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
@YunchuWang
Copy link
Member

@copilot address my comments

…e nullable

Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 22, 2025

@copilot address my comments

All comments addressed in 0e8964e:

  • Added TypeKind.Error check to handle compilation errors
  • Made returnType parameter nullable to avoid fragile placeholder pattern
  • returnSymbol is now passed as null for void returns

All 43 tests passing.

@YunchuWang
Copy link
Member

LGTM

@YunchuWang
Copy link
Member

verified locally working
image

image

@YunchuWang
Copy link
Member

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Summary

What changed?

  • Fixed source generator to properly handle activity functions returning void or non-generic Task
  • Added robust type checking with null safety, error type handling, and proper pattern matching
  • Made returnType parameter nullable in DurableFunction constructor to eliminate fragile placeholder pattern

Why is this change needed?

Activity functions returning void or Task (non-generic) caused the source generator to produce invalid C# code (Task<void> doesn't compile), breaking compilation for users with void-returning activities.

Issues / work items

  • Resolves #issue_number (from original issue description)

Project checklist

  • Release notes are not required for the next release
  • Backport is not required
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change? No

AI-assisted code disclosure (required)

Was an AI tool used?

  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot Workspace Agent
  • AI-assisted areas/files: All code changes in DurableFunction.cs, DurableTaskSourceGenerator.cs, TypedParameter.cs, and test files
  • What you changed after AI output: Addressed multiple rounds of code review feedback to improve null safety, remove unsafe casts, fix semantic issues, add error type handling, and make constructor parameter nullable

AI verification:

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed (43/43 tests passing)

Manual validation

  • Verified by reviewer locally with void-returning activity functions
  • Evidence: Screenshots showing generated code correctly produces Task CallLogHelloVoidAsync(...) for void returns

Notes for reviewers

  • Multiple iterations of code review feedback addressed
  • All edge cases for void, Task, and Task returns now handled robustly

@YunchuWang
Copy link
Member

@copilot address the comment

Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot address the comment

The constructor change is not a breaking change because DurableFunction is in a source generator assembly that's not user-facing. The types are only used by the Roslyn compiler at build time and cannot be directly referenced by user code. See my response to the specific comment thread for details.

@YunchuWang YunchuWang merged commit e3b230e into main Dec 23, 2025
9 checks passed
@YunchuWang YunchuWang deleted the copilot/fix-source-generator-void-handling branch December 23, 2025 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Source generator doesn't handle activity functions that return void

3 participants