Add support for nested tuples in data source handling and tests

[thomhurst]

Fixes #2950

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Add support for nested tuples in data source handling and tests

Review Checklist:

• Analyze all changed files
• Check code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage
• Verify adherence to TUnit coding standards

Overall Assessment

This is a well-implemented feature that successfully addresses issue #2950 by adding comprehensive support for nested tuples in data source handling. The implementation correctly maintains behavioral parity between source-generated and reflection modes as required by TUnit's architecture.

✅ Strengths

1. Comprehensive Implementation

• Multi-layered approach: Changes span analyzer, source generator, core attributes, helpers, and engine - ensuring full coverage
• Dual mode support: Both source-generated and reflection execution paths are properly handled
• AOT compatibility: Uses ITuple interface for modern .NET and fallback patterns for older versions

2. Intelligent Tuple Handling

The new ToObjectArrayWithTypes() method in DataSourceHelpers.cs:379-428 is particularly well-designed:

// Special case: If there's a single parameter that expects a tuple type,
// and the item is a tuple, don't unwrap
if (expectedTypes.Length == 1 && IsTupleType(expectedTypes[0]))
{
    return [item];
}

3. Robust Analyzer Enhancements

The analyzer changes in TestDataAnalyzer.cs:767-812 properly handle nested tuple scenarios:

• Correctly handles single parameter expecting complete tuple vs multiple parameters expecting unwrapped elements
• Maintains type safety checking for complex nested structures

4. Excellent Test Coverage

The test suite covers diverse scenarios:

• Simple nested tuples: (int, (int, int))
• Deeply nested: ((int, int), (int, int))
• Mixed types: (string, (int, bool))
• Arrays with tuples: (int[], (string, double))
• Async enumerables and various return patterns

⚠️ Areas for Improvement

1. Code Duplication

The IsTupleType() method is duplicated across multiple files:

• DataSourceHelpers.cs:479-503
• DataUnwrapper.cs:48-72
• Similar logic in AotTupleProcessorGenerator.cs:246-255

Recommendation: Consider consolidating into a shared utility class.

2. Performance Considerations

In DataSourceHelpers.cs:263-271, the tuple unwrapping recreates fresh data for each parameter:

return unwrapped.Select((_, index) => new Func<Task<object?>>(() =>
{
    var freshData = InvokeIfFunc(data);  // Re-invokes for each parameter
    var freshUnwrapped = UnwrapTupleAot(freshData);
    return Task.FromResult<object?>(index < freshUnwrapped.Length ? freshUnwrapped[index] : null);
})).ToArray();

This could be inefficient for expensive data source methods. Consider caching the unwrapped result.

3. Magic Number Usage

The tuple handling uses magic numbers (7, 8) related to ValueTuple's nested structure:

• AotTupleProcessorGenerator.cs:525-552

Recommendation: Extract these as named constants with explanatory comments about ValueTuple's 8-element limitation.

4. Error Handling

Limited error handling for malformed nested tuples or type mismatches. Consider adding more descriptive error messages for debugging.

🔍 Specific Code Quality Issues

Minor Issues:

1. MethodDataSourceAttribute.cs:146: The tuple check could be more explicit:

   // Current
   if (methodResult is System.Collections.IEnumerable enumerable and not string && !DataSourceHelpers.IsTuple(methodResult))
   
   // Suggested: More explicit about why tuples are excluded
   && !DataSourceHelpers.IsTuple(methodResult) // Tuples should be treated as single values, not collections

2. TestDataAnalyzer.cs:768-779: The special case logic is correct but could benefit from a comment explaining the tuple unwrapping decision tree.

🛡️ Security & Performance

Security: ✅ No security concerns identified. The implementation properly uses [UnconditionalSuppressMessage] attributes for AOT scenarios where reflection is necessary.

Performance: ⚠️ Generally good, but the re-invocation pattern mentioned above could impact performance for expensive data sources.

📋 Adherence to TUnit Standards

✅ Excellent adherence to TUnit coding standards:

• Uses modern C# patterns (collection initializers, pattern matching)
• Maintains AOT compatibility with proper fallbacks
• Follows TUnit's dual execution mode requirements
• Uses ValueTask appropriately
• Proper use of [UnconditionalSuppressMessage] for AOT scenarios

🎯 Recommendations

1. Consolidate duplicate IsTupleType() methods into a shared utility
2. Add performance optimization for repeated data source invocations in tuple unwrapping
3. Extract magic numbers related to ValueTuple structure as named constants
4. Consider adding integration tests that verify both source-generated and reflection modes produce identical results for nested tuples

🏁 Final Verdict

This is a high-quality implementation that successfully solves the nested tuple problem while maintaining TUnit's architectural principles. The code is well-structured, properly tested, and handles edge cases appropriately. The minor improvements suggested above would enhance maintainability and performance but don't detract from the overall quality of the solution.

Recommendation: ✅ Approve with minor suggestions

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Add support for nested tuples in data source handling and tests

Review Checklist:

• Analyze all changed files systematically
• Check code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage
• Verify adherence to TUnit coding standards
• Check dual execution mode parity (source-gen vs reflection)

---

Executive Summary

This PR successfully addresses issue #2950 by implementing intelligent nested tuple support across TUnit's data source handling infrastructure. The implementation correctly handles the dual execution modes requirement and provides comprehensive test coverage.

Code Quality & Best Practices ✅

Strengths:

• AOT Compatibility: Proper use of ITuple interface with fallbacks for older .NET versions (DataSourceHelpers.cs:87-98)
• Modern C# Patterns: Clean use of collection initializers, pattern matching, and expression-bodied members
• Consistent Naming: Follows TUnit conventions with descriptive method names
• Proper Suppression: Appropriate use of [UnconditionalSuppressMessage] for AOT scenarios

Recommendations:

• Consider adding XML documentation for the new UnwrapTupleWithTypes method for better API understanding
• The complex switch statements in UnwrapTupleAot could benefit from comments explaining the fallback strategy

Potential Issues & Bugs 🔍

No Critical Issues Found - The implementation appears solid with proper error handling:

1. Null Safety: Proper null checks throughout (DataSourceHelpers.cs:81-84, DataUnwrapper.cs:22-26)
2. Type Safety: Intelligent type checking before unwrapping tuples
3. Edge Cases: Handles empty enumerables gracefully (MethodDataSourceAttribute.cs:134-138)

Minor Considerations:

• The reflection fallback in DataSourceHelpers.cs:137-140 maintains backward compatibility but adds a small performance overhead in non-AOT scenarios
• Complex type matching logic could potentially miss edge cases with custom tuple implementations, but this is acceptable given the comprehensive testing

Performance Considerations ⚡

Excellent Performance Design:

1. AOT Optimization: Uses ITuple interface for fast access on modern .NET (DataSourceHelpers.cs:88-97)
2. Minimal Allocations: Efficient array creation and reuse patterns
3. Lazy Evaluation: Factory pattern ensures data sources are invoked only when needed (DataSourceHelpers.cs:265-270)
4. Cached Type Checking: IsTupleType() method avoids repeated reflection calls

Performance Impact: ✅ Low to None - The changes add negligible overhead and actually optimize tuple handling

Security Concerns 🔒

No Security Issues Identified:

• Proper use of [DynamicallyAccessedMembers] attributes for AOT trimming
• No exposed reflection patterns that could be exploited
• Input validation present for null values and type mismatches

Test Coverage Assessment 📋

Comprehensive Test Suite - Excellent coverage across scenarios:

1. NestedTupleDataSourceTests.cs: 8 test methods covering:

   • Simple nested tuples: (int, (int, int))
   • Deep nesting: ((int, int), (int, int))
   • Triple nesting: (int, (int, (int, int)))
   • Mixed types: (string, (int, bool))
   • Arrays in tuples: (int[], (string, double))
   • IEnumerable and IAsyncEnumerable scenarios

2. Edge Case Coverage: Tests both single-parameter and multi-parameter scenarios

3. Real-world Scenarios: NestedTupleTest.cs demonstrates the original issue fix

Missing Test Cases (Minor):

• Tests with Tuple<T> class types (only ValueTuple tested)
• Error scenarios with malformed tuple data sources

TUnit Coding Standards Adherence ✅

Fully Compliant:

1. Dual Execution Mode Parity: ✅

   • Analyzer updates: Enhanced tuple type checking
   • Source generator: Handles nested tuples in generated code
   • Runtime: DataUnwrapper and DataSourceHelpers support both modes

2. Performance Priority: ✅

   • AOT-compatible implementations
   • Efficient tuple unwrapping strategies
   • Minimal reflection usage

3. Modern C# Usage: ✅

   • Collection initializers, pattern matching
   • Expression-bodied members where appropriate
   • Proper async/await patterns

Technical Deep Dive 🔧

Intelligent Tuple Handling Logic:

The implementation's key innovation is context-aware tuple unwrapping:

// From TestDataAnalyzer.cs:772-779
if (testParameterTypes.Length == 1 && 
    testParameterTypes[0] is INamedTypeSymbol paramTupleType && 
    paramTupleType.IsTupleType &&
    SymbolEqualityComparer.Default.Equals(tupleType, testParameterTypes[0]))
{
    // Return the tuple as-is for single parameter expecting the same tuple
    return ImmutableArray.Create(type);
}

This logic correctly handles the scenario where:

• Data source returns: (int, (int, int))
• Test method expects: Single parameter of type (int, (int, int)) → Don't unwrap
• Test method expects: Two parameters int, (int, int) → Unwrap to separate arguments

Recommendations & Action Items 📝

Code Quality Improvements:

1. Add XML documentation for UnwrapTupleWithTypes method
2. Consider adding debug logging for complex tuple unwrapping decisions

Future Enhancements:

1. Add performance benchmarks for nested tuple scenarios
2. Consider supporting Tuple class types in addition to ValueTuple

Overall Assessment ⭐

Score: 9/10 - Excellent Implementation

This PR demonstrates exceptional engineering practices:

• ✅ Solves the stated problem completely
• ✅ Maintains backward compatibility
• ✅ Follows TUnit architectural principles
• ✅ Provides comprehensive test coverage
• ✅ Optimizes for performance and AOT compatibility
• ✅ Handles edge cases gracefully

The implementation is production-ready and significantly enhances TUnit's tuple handling capabilities while maintaining the framework's performance and reliability standards.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Add support for nested tuples in data source handling and tests

Review Checklist:

• Read TUnit development guidelines from CLAUDE.md
• Analyze all 14 changed files systematically
• Check code quality and adherence to TUnit standards
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage
• Verify dual execution mode parity (source-gen vs reflection)
• Provide comprehensive feedback

---

✅ Overall Assessment

This PR successfully addresses issue #2950 by implementing intelligent nested tuple handling across TUnit's data source infrastructure. The implementation demonstrates excellent adherence to TUnit's architectural principles and coding standards.

🎯 Key Strengths

1. Comprehensive Multi-Layer Implementation

• Complete coverage: Changes span analyzer, source generator, core helpers, engine, and attributes
• Dual execution mode parity: Both source-generated and reflection paths properly handle nested tuples
• Backward compatibility: Existing tuple functionality remains intact

2. Intelligent Tuple Unwrapping Logic

The core innovation is context-aware tuple handling in DataSourceHelpers.cs:379-428:

// Special case: If there's a single parameter that expects a tuple type,
// and the item is a tuple, don't unwrap
if (expectedTypes.Length == 1 && IsTupleType(expectedTypes[0]))
{
    return [item];
}

This correctly handles:

• Data source returns: (int, (int, int))
• Test method expects: Single parameter (int, (int, int)) → Don't unwrap
• Test method expects: Two parameters int, (int, int) → Unwrap to separate arguments

3. Excellent AOT Compatibility

• Uses ITuple interface for modern .NET with proper fallbacks DataSourceHelpers.cs:87-98
• Proper use of [UnconditionalSuppressMessage] for trimming scenarios
• Explicit handling of ValueTuple and Tuple types for maximum compatibility

4. Robust Test Coverage

NestedTupleDataSourceTests.cs provides comprehensive scenarios:

• Simple nested: (int, (int, int))
• Deeply nested: ((int, int), (int, int))
• Triple nested: (int, (int, (int, int)))
• Mixed types: (string, (int, bool))
• Arrays in tuples: (int[], (string, double))
• Async enumerables and various return patterns

⚠️ Areas for Improvement

1. Code Duplication

The IsTupleType() method is duplicated across:

• DataSourceHelpers.cs:479-503
• DataUnwrapper.cs:48-72

Recommendation: Extract to a shared utility class like TupleTypeHelper.

2. Performance Consideration

In DataSourceHelpers.cs:265-270, tuple unwrapping re-invokes data source for each parameter:

return unwrapped.Select((_, index) => new Func<Task<object?>>(() =>
{
    var freshData = InvokeIfFunc(data);  // Re-invokes for each parameter
    var freshUnwrapped = UnwrapTupleAot(freshData);
    return Task.FromResult<object?>(freshUnwrapped[index]);
})).ToArray();

This could be expensive for costly data source methods. Consider caching unwrapped results.

3. Magic Numbers

ValueTuple handling uses magic numbers (7, 8) related to its nested structure without explanation.

Recommendation: Extract as named constants with explanatory comments about ValueTuple's design limitations.

🔍 Code Quality Analysis

Excellent TUnit Standards Adherence ✅

• Modern C#: Collection initializers, pattern matching, expression-bodied members
• Performance focus: Minimal allocations, efficient AOT-compatible implementations
• Dual mode support: Proper analyzer and source generator integration
• Proper async patterns: ValueTask usage where appropriate

Security & AOT Compatibility ✅

• Proper reflection suppression attributes for trimming
• No security concerns identified
• AOT-friendly patterns throughout

Analyzer Integration ✅

The analyzer changes in TestDataAnalyzer.cs:784-790 correctly handle nested tuple scenarios:

if (testParameterTypes.Length == 1 && 
    testParameterTypes[0] is INamedTypeSymbol paramTupleType && 
    paramTupleType.IsTupleType &&
    SymbolEqualityComparer.Default.Equals(tupleType, testParameterTypes[0]))
{
    return ImmutableArray.Create(type);
}

Public API Changes ✅

The new ToObjectArrayWithTypes() method is properly exposed in public API snapshots across all target frameworks.

🚀 Performance Impact

Minimal to No Performance Regression:

• Uses efficient ITuple interface on modern .NET
• Fallback patterns are well-optimized
• AOT-compatible implementations avoid runtime reflection overhead

🧪 Test Coverage Assessment

Comprehensive and Well-Designed:

• Covers real-world scenarios from the original issue
• Tests both single-parameter and multi-parameter scenarios
• Includes edge cases like deeply nested tuples
• Tests various data source patterns (Func, IEnumerable, IAsyncEnumerable)

Minor Gap: Missing tests for Tuple<T> class types (only ValueTuple<T> tested).

🏗️ Architecture Compliance

Perfectly Aligned with TUnit Goals:

• ✅ Fast: AOT-optimized implementations
• ✅ Modern: Uses latest C# patterns and .NET features
• ✅ Reliable: Comprehensive test coverage and backward compatibility
• ✅ Enjoyable: Intuitive tuple handling that "just works"

📋 Recommendations

1. Consolidate duplicate IsTupleType() methods into shared utility
2. Add performance optimization for repeated data source invocations
3. Extract magic numbers as named constants
4. Consider adding tests for Tuple<T> class types
5. Add XML documentation for the new ToObjectArrayWithTypes method

🏁 Final Verdict

Score: 9.5/10 - Excellent Implementation

This PR represents exceptional engineering quality that:

• ✅ Completely solves the stated problem
• ✅ Maintains TUnit's architectural principles
• ✅ Provides comprehensive test coverage
• ✅ Optimizes for performance and AOT compatibility
• ✅ Handles complex edge cases gracefully
• ✅ Maintains backward compatibility

The implementation is production-ready and significantly enhances TUnit's tuple handling capabilities while preserving the framework's core performance and reliability characteristics.

Recommendation: ✅ Approve - Ready for merge with minor suggestions for future optimization.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Add support for nested tuples in data source handling and tests

Review Checklist:

• Read TUnit development guidelines from CLAUDE.md
• Analyze all 14 changed files systematically
• Check code quality and adherence to TUnit standards
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage
• Verify dual execution mode parity (source-gen vs reflection)
• Provide comprehensive feedback

---

✅ Overall Assessment

This PR successfully addresses issue #2950 by implementing intelligent nested tuple handling across TUnit's data source infrastructure. The implementation demonstrates excellent adherence to TUnit's architectural principles and coding standards.

🎯 Key Strengths

1. Comprehensive Multi-Layer Implementation

• Complete coverage: Changes span analyzer, source generator, core helpers, engine, and attributes
• Dual execution mode parity: Both source-generated and reflection paths properly handle nested tuples
• Backward compatibility: Existing tuple functionality remains intact

2. Intelligent Tuple Unwrapping Logic

The core innovation is context-aware tuple handling in DataSourceHelpers.cs:379-428:

// Special case: If there's a single parameter that expects a tuple type,
// and the item is a tuple, don't unwrap
if (expectedTypes.Length == 1 && IsTupleType(expectedTypes[0]))
{
    return [item];
}

This correctly handles:

• Data source returns: (int, (int, int))
• Test method expects: Single parameter (int, (int, int)) → Don't unwrap
• Test method expects: Two parameters int, (int, int) → Unwrap to separate arguments

3. Excellent AOT Compatibility

• Uses ITuple interface for modern .NET with proper fallbacks DataSourceHelpers.cs:87-98
• Proper use of [UnconditionalSuppressMessage] for trimming scenarios
• Explicit handling of ValueTuple and Tuple types for maximum compatibility

4. Robust Test Coverage

NestedTupleDataSourceTests.cs provides comprehensive scenarios:

• Simple nested: (int, (int, int))
• Deeply nested: ((int, int), (int, int))
• Triple nested: (int, (int, (int, int)))
• Mixed types: (string, (int, bool))
• Arrays in tuples: (int[], (string, double))
• Async enumerables and various return patterns

⚠️ Areas for Improvement

1. Code Duplication

The IsTupleType() method is duplicated across:

• DataSourceHelpers.cs:479-503
• DataUnwrapper.cs:48-72

Recommendation: Extract to a shared utility class like TupleTypeHelper.

2. Performance Consideration

In DataSourceHelpers.cs:265-270, tuple unwrapping re-invokes data source for each parameter:

return unwrapped.Select((_, index) => new Func<Task<object?>>(() =>
{
    var freshData = InvokeIfFunc(data);  // Re-invokes for each parameter
    var freshUnwrapped = UnwrapTupleAot(freshData);
    return Task.FromResult<object?>(freshUnwrapped[index]);
})).ToArray();

This could be expensive for costly data source methods. Consider caching unwrapped results.

3. Magic Numbers

ValueTuple handling uses magic numbers (7, 8) related to its nested structure without explanation.

Recommendation: Extract as named constants with explanatory comments about ValueTuple's design limitations.

🔍 Code Quality Analysis

Excellent TUnit Standards Adherence ✅

• Modern C#: Collection initializers, pattern matching, expression-bodied members
• Performance focus: Minimal allocations, efficient AOT-compatible implementations
• Dual mode support: Proper analyzer and source generator integration
• Proper async patterns: ValueTask usage where appropriate

Security & AOT Compatibility ✅

• Proper reflection suppression attributes for trimming
• No security concerns identified
• AOT-friendly patterns throughout

Analyzer Integration ✅

The analyzer changes in TestDataAnalyzer.cs:784-790 correctly handle nested tuple scenarios:

if (testParameterTypes.Length == 1 && 
    testParameterTypes[0] is INamedTypeSymbol paramTupleType && 
    paramTupleType.IsTupleType &&
    SymbolEqualityComparer.Default.Equals(tupleType, testParameterTypes[0]))
{
    return ImmutableArray.Create(type);
}

Public API Changes ✅

The new ToObjectArrayWithTypes() method is properly exposed in public API snapshots across all target frameworks.

🚀 Performance Impact

Minimal to No Performance Regression:

• Uses efficient ITuple interface on modern .NET
• Fallback patterns are well-optimized
• AOT-compatible implementations avoid runtime reflection overhead

🧪 Test Coverage Assessment

Comprehensive and Well-Designed:

• Covers real-world scenarios from the original issue
• Tests both single-parameter and multi-parameter scenarios
• Includes edge cases like deeply nested tuples
• Tests various data source patterns (Func, IEnumerable, IAsyncEnumerable)

Minor Gap: Missing tests for Tuple<T> class types (only ValueTuple<T> tested).

🏗️ Architecture Compliance

Perfectly Aligned with TUnit Goals:

• ✅ Fast: AOT-optimized implementations
• ✅ Modern: Uses latest C# patterns and .NET features
• ✅ Reliable: Comprehensive test coverage and backward compatibility
• ✅ Enjoyable: Intuitive tuple handling that "just works"

📋 Recommendations

1. Consolidate duplicate IsTupleType() methods into shared utility
2. Add performance optimization for repeated data source invocations
3. Extract magic numbers as named constants
4. Consider adding tests for Tuple<T> class types
5. Add XML documentation for the new ToObjectArrayWithTypes method

🏁 Final Verdict

Score: 9.5/10 - Excellent Implementation

This PR represents exceptional engineering quality that:

• ✅ Completely solves the stated problem
• ✅ Maintains TUnit's architectural principles
• ✅ Provides comprehensive test coverage
• ✅ Optimizes for performance and AOT compatibility
• ✅ Handles complex edge cases gracefully
• ✅ Maintains backward compatibility

The implementation is production-ready and significantly enhances TUnit's tuple handling capabilities while preserving the framework's core performance and reliability characteristics.

Recommendation: ✅ Approve - Ready for merge with minor suggestions for future optimization.

---