Fix: source generation duplicate fix for partial classes

[thomhurst]

Fixes #3231

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Complete ✅

Todo List:

• Read and understand the modified generator files
• Analyze the test files to understand the issue being fixed
• Review the git diff to see specific changes
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Evaluate test coverage
• Provide comprehensive feedback

---

Code Review Feedback

✅ Issue Understanding

The PR correctly addresses issue #3231 regarding source generation duplicates for partial classes. When a partial class is split across multiple files, the previous implementation would generate duplicate source code for each part of the partial class, leading to compilation errors.

✅ Code Quality & Best Practices

DataSourceHelpersGenerator.cs:

• Good: The fix properly uses .Collect().SelectMany() with DistinctBy() to deduplicate types based on SymbolEqualityComparer.Default
• Good: Updated comment accurately reflects the new behavior ("Generate individual files for each unique type")
• Good: Maintains existing code structure and patterns

PropertyInjectionSourceGenerator.cs:

• Good: Consistent application of the same deduplication pattern
• Good: Added proper null check for empty properties collections (lines 94-98) which improves robustness
• Good: The logic correctly handles partial classes by comparing ClassSymbol instances

✅ Technical Correctness

The implementation correctly addresses the root cause:

1. Problem: Source generators were creating duplicate registrations for each partial class declaration
2. Solution: Collect all declarations, then deduplicate by type symbol before generating output
3. Result: Only one generated file per unique type, regardless of how many partial declarations exist

✅ Performance Considerations

• Positive: The .Collect().SelectMany() pattern is the recommended approach for deduplication in incremental generators
• Positive: SymbolEqualityComparer.Default is the correct and efficient way to compare type symbols
• Positive: Early null checks prevent unnecessary processing
• Minimal Impact: The additional collection step has negligible performance impact and prevents duplicate work

✅ Security Assessment

• No concerns: Changes are purely compilation-time source generation improvements
• Safe: No runtime security implications
• Appropriate: Uses framework-provided symbol comparison utilities

✅ Test Coverage

Excellent test case design:

• Realistic scenario: Tests the exact issue - partial class with data source properties split across files
• Comprehensive: Covers both the property declaration (TestsPart2.cs) and usage (TestsPart1.cs)
• Appropriate: Uses [EngineTest(ExpectedResult.Pass)] to ensure the fix resolves compilation issues
• Well-structured: Clean separation of concerns between test parts and data class

🟡 Minor Suggestions

1. PropertyInjectionSourceGenerator.cs:107-109: Consider extracting the null check logic to a helper method if this pattern is used elsewhere:

   private static bool CanSetProperty(IPropertySymbol property)
   {
       return property.SetMethod != null || property.SetMethod?.IsInitOnly == true;
   }

   This could be simplified to:

   private static bool CanSetProperty(IPropertySymbol property)
   {
       return property.SetMethod is { } setMethod && (setMethod.IsInitOnly || !setMethod.IsInitOnly);
   }

   But the current implementation is clear and correct.

✅ Overall Assessment

This is a high-quality fix that:

• ✅ Correctly identifies and solves the partial class duplication issue
• ✅ Uses appropriate source generator patterns
• ✅ Maintains code consistency across both generators
• ✅ Includes excellent test coverage
• ✅ Has minimal performance impact
• ✅ Follows TUnit's architectural patterns

Recommendation: APPROVE - This fix is well-implemented, thoroughly tested, and addresses the issue without introducing any concerns.

---