Fix params arrays

[thomhurst]

Fixes #2942

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix params arrays

I've completed my review of this PR that fixes params arrays handling. Here's my comprehensive feedback:

Todo List:

• Read and analyze TupleArgumentHelper.cs changes
• Review ParamsArgumentsTests.cs improvements
• Check for code quality and best practices
• Look for potential bugs or edge cases
• Assess performance implications
• Evaluate test coverage improvements
• Provide comprehensive review feedback

---

✅ Code Quality & Best Practices

TupleArgumentHelper.cs:

• Good fix: The change from line 167-169 correctly handles single parameter params arrays by creating an array wrapper instead of passing the raw parameter
• Clear intent: Updated comment accurately reflects the new behavior ("create array with single element")
• Type safety: Proper use of elementType instead of paramsParam.Type ensures correct array element typing
• Consistent patterns: The fix aligns with the multiple-arguments case (lines 172-180) for consistency

ParamsArgumentsTests.cs:

• Comprehensive coverage: Excellent test expansion covering all params array scenarios:
  • Empty params arrays (EmptyParamsArray)
  • Single element params (SingleTypeInParamsArray, SingleStringInParamsArray)
  • Multiple elements params (MultipleTypesInParamsArray, MultipleStringsInParamsArray)
• Modern C# practices: Good use of file-scoped namespaces, proper async/await patterns
• Descriptive naming: Test method names clearly indicate their purpose

✅ Bug Analysis

Fixed Issue:
The core bug was in TupleArgumentHelper.cs:167 where single params arguments were incorrectly passed as the raw parameter type instead of being wrapped in an array. This would cause type mismatch errors when the method expects T[] but receives T.

Before (buggy):

var singleArgExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{paramsParam.Type.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";

After (fixed):

var singleElementExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{elementType.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";
argumentExpressions.Add($"new {elementType.GloballyQualified()}[] {{ {singleElementExpression} }}");

No new bugs identified - the fix correctly handles the edge case while maintaining existing functionality.

✅ Performance Considerations

• Minimal impact: The change only affects single-parameter params scenarios and adds one array allocation
• Proper array sizing: Single-element arrays are efficiently created with new T[] { element }
• No unnecessary boxing: Direct element type casting avoids performance overhead
• Generator efficiency: Source generator changes don't affect runtime performance since code is generated at compile-time

✅ Security Concerns

• No security issues: The changes are purely structural and don't introduce any security vulnerabilities
• Type safety maintained: Strong typing is preserved throughout the casting process
• Input validation: Existing CastHelper.Cast<T>() provides safe type conversion

✅ Test Coverage

Excellent improvement in test coverage:

Scenario	Before	After
Empty params	❌	✅ EmptyParamsArray
Single param (Type)	❌	✅ SingleTypeInParamsArray
Single param (string)	❌	✅ SingleStringInParamsArray
Multiple params (Type)	❌	✅ MultipleTypesInParamsArray
Multiple params (string)	❌	✅ MultipleStringsInParamsArray
Enum params	✅	✅ GetOperations (existing)

Test quality highlights:

• Assertions verify both array length and individual elements
• Different data types tested (Type, string, enum)
• Async test patterns follow TUnit conventions
• Comprehensive edge case coverage

🎯 Overall Assessment

This is a high-quality fix that:

• ✅ Correctly addresses the params array type handling bug
• ✅ Maintains backward compatibility
• ✅ Follows established code patterns
• ✅ Significantly improves test coverage
• ✅ Has minimal performance impact
• ✅ Introduces no security concerns

Recommendation: Approve - This PR effectively fixes the params array issue with proper testing and follows TUnit's development standards.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix params arrays

I'll provide a comprehensive review of this PR that fixes params arrays handling. Here's my todo list:

Todo List:

• Read and analyze TupleArgumentHelper.cs changes
• Review ParamsArgumentsTests.cs improvements
• Check snapshot test file changes
• Look for potential bugs or edge cases
• Assess performance implications
• Evaluate security concerns
• Check test coverage improvements
• Provide comprehensive review feedback

---

✅ Code Quality & Best Practices

TupleArgumentHelper.cs (TUnit.Core.SourceGenerator/CodeGenerators/Helpers/TupleArgumentHelper.cs:167-169):

• Excellent fix: The change correctly handles single parameter params arrays by creating an array wrapper instead of passing the raw parameter
• Clear intent: Updated comment accurately reflects the new behavior ("create array with single element")
• Type safety: Proper use of elementType instead of paramsParam.Type ensures correct array element typing
• Consistent patterns: The fix aligns with the multiple-arguments case (lines 172-180) for consistency

ParamsArgumentsTests.cs:

• Comprehensive coverage: Excellent test expansion covering all params array scenarios:
  • Empty params arrays (EmptyParamsArray)
  • Single element params (SingleTypeInParamsArray, SingleStringInParamsArray)
  • Multiple elements params (MultipleTypesInParamsArray, MultipleStringsInParamsArray)
• Modern C# practices: Good use of file-scoped namespaces, proper async/await patterns
• Descriptive naming: Test method names clearly indicate their purpose

✅ Bug Analysis

Fixed Issue:
The core bug was in TupleArgumentHelper.cs:167 where single params arguments were incorrectly passed as the raw parameter type instead of being wrapped in an array. This would cause type mismatch errors when the method expects T[] but receives T.

Before (buggy):

var singleArgExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{paramsParam.Type.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";

After (fixed):

var singleElementExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{elementType.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";
argumentExpressions.Add($"new {elementType.GloballyQualified()}[] {{ {singleElementExpression} }}");

No new bugs identified - the fix correctly handles the edge case while maintaining existing functionality.

✅ Performance Considerations

• Minimal impact: The change only affects single-parameter params scenarios and adds one array allocation
• Proper array sizing: Single-element arrays are efficiently created with new T[] { element }
• No unnecessary boxing: Direct element type casting avoids performance overhead
• Generator efficiency: Source generator changes don't affect runtime performance since code is generated at compile-time

✅ Security Concerns

• No security issues: The changes are purely structural and don't introduce any security vulnerabilities
• Type safety maintained: Strong typing is preserved throughout the casting process
• Input validation: Existing CastHelper.Cast<T>() provides safe type conversion

✅ Test Coverage

Excellent improvement in test coverage:

Scenario	Before	After
Empty params	❌	✅ EmptyParamsArray
Single param (Type)	❌	✅ SingleTypeInParamsArray
Single param (string)	❌	✅ SingleStringInParamsArray
Multiple params (Type)	❌	✅ MultipleTypesInParamsArray
Multiple params (string)	❌	✅ MultipleStringsInParamsArray
Enum params	✅	✅ GetOperations (existing)

Test quality highlights:

• Assertions verify both array length and individual elements
• Different data types tested (Type, string, enum)
• Async test patterns follow TUnit conventions
• Comprehensive edge case coverage

✅ Snapshot Test Analysis

The snapshot changes across 9 test files show consistent updates to generated source code, reflecting the params array fix. All changes appear to be updating the generated code to properly handle single-element params arrays with array wrapping.

🎯 Overall Assessment

This is a high-quality fix that:

• ✅ Correctly addresses the params array type handling bug
• ✅ Maintains backward compatibility
• ✅ Follows established code patterns
• ✅ Significantly improves test coverage
• ✅ Has minimal performance impact
• ✅ Introduces no security concerns
• ✅ Updates snapshot tests appropriately

Recommendation: Approve - This PR effectively fixes the params array issue with proper testing and follows TUnit's development standards.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix params arrays

I've completed a comprehensive review of this PR that fixes params arrays handling. Here's my detailed feedback:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze TupleArgumentHelper.cs changes
• Review ParamsArgumentsTests.cs improvements
• Check snapshot test file changes
• Look for potential bugs or edge cases
• Assess performance implications
• Evaluate security concerns
• Check test coverage improvements
• Provide comprehensive review feedback

---

✅ Code Quality & Best Practices

TupleArgumentHelper.cs (TUnit.Core.SourceGenerator/CodeGenerators/Helpers/TupleArgumentHelper.cs:167-169):

• Excellent fix: The change correctly handles single parameter params arrays by creating an array wrapper instead of passing the raw parameter
• Clear intent: Updated comment accurately reflects the new behavior ("create array with single element")
• Type safety: Proper use of elementType instead of paramsParam.Type ensures correct array element typing
• Consistent patterns: The fix aligns with the multiple-arguments case (lines 172-180) for consistency

ParamsArgumentsTests.cs:

• Comprehensive coverage: Excellent test expansion covering all params array scenarios:
  • Empty params arrays (EmptyParamsArray)
  • Single element params (SingleTypeInParamsArray, SingleStringInParamsArray)
  • Multiple elements params (MultipleTypesInParamsArray, MultipleStringsInParamsArray)
• Modern C# practices: Good use of file-scoped namespaces, proper async/await patterns
• Descriptive naming: Test method names clearly indicate their purpose
• Follows TUnit conventions: Proper use of ArgumentsAttribute and assertion patterns

✅ Bug Analysis

Fixed Issue:
The core bug was in TupleArgumentHelper.cs:167 where single params arguments were incorrectly passed as the raw parameter type instead of being wrapped in an array. This would cause type mismatch errors when the method expects T[] but receives T.

Before (buggy):

var singleArgExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{paramsParam.Type.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";

After (fixed):

var singleElementExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{elementType.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";
argumentExpressions.Add($"new {elementType.GloballyQualified()}[] {{ {singleElementExpression} }}");

No new bugs identified - the fix correctly handles the edge case while maintaining existing functionality.

✅ Performance Considerations

• Minimal impact: The change only affects single-parameter params scenarios and adds one array allocation
• Proper array sizing: Single-element arrays are efficiently created with new T[] { element }
• No unnecessary boxing: Direct element type casting avoids performance overhead
• Generator efficiency: Source generator changes don't affect runtime performance since code is generated at compile-time
• Consistent with TUnit's performance goals: Maintains the framework's emphasis on speed

✅ Security Concerns

• No security issues: The changes are purely structural and don't introduce any security vulnerabilities
• Type safety maintained: Strong typing is preserved throughout the casting process
• Input validation: Existing CastHelper.Cast<T>() provides safe type conversion
• AOT compatibility: Changes maintain AOT and trimming support as required by the codebase

✅ Test Coverage

Excellent improvement in test coverage:

Scenario	Before	After
Empty params	❌	✅ EmptyParamsArray
Single param (Type)	❌	✅ SingleTypeInParamsArray
Single param (string)	❌	✅ SingleStringInParamsArray
Multiple params (Type)	❌	✅ MultipleTypesInParamsArray
Multiple params (string)	❌	✅ MultipleStringsInParamsArray
Enum params	✅	✅ GetOperations (existing)

Test quality highlights:

• Assertions verify both array length and individual elements
• Different data types tested (Type, string, enum)
• Async test patterns follow TUnit conventions
• Comprehensive edge case coverage

✅ Snapshot Test Analysis

The snapshot changes across both test files show consistent updates to generated source code, reflecting the params array fix. Key observations:

• ArgsAsArrayTests.Test.verified.txt: Shows proper array wrapping for params scenarios (lines 82, 92, etc.)
• Tests2112.Test.verified.txt: Demonstrates the fix for the specific bug case with long[] params
• All changes appear to be updating the generated code to properly handle single-element params arrays with array wrapping

✅ Adherence to TUnit Guidelines

This PR follows all critical TUnit development rules:

• ✅ Behavioral parity: Fix applies to source-generated mode and maintains consistency
• ✅ Snapshot tests: Properly updated and committed
• ✅ Performance focus: Minimal performance impact, no regressions
• ✅ Modern C#: Uses contemporary patterns and conventions
• ✅ Comprehensive testing: Excellent test coverage for all scenarios

🎯 Overall Assessment

This is a high-quality fix that:

• ✅ Correctly addresses the params array type handling bug
• ✅ Maintains backward compatibility
• ✅ Follows established code patterns and TUnit conventions
• ✅ Significantly improves test coverage
• ✅ Has minimal performance impact
• ✅ Introduces no security concerns
• ✅ Updates snapshot tests appropriately
• ✅ Adheres to all TUnit development guidelines

Recommendation: Approve - This PR effectively fixes the params array issue with proper testing and follows TUnit's development standards excellently.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix params arrays

I've completed a comprehensive review of this PR that fixes params arrays handling. Here's my detailed feedback:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze TupleArgumentHelper.cs changes
• Review ParamsArgumentsTests.cs improvements
• Check snapshot test file changes
• Look for potential bugs or edge cases
• Assess performance implications
• Evaluate security concerns
• Check test coverage improvements
• Provide comprehensive review feedback

---

✅ Code Quality & Best Practices

TupleArgumentHelper.cs (TUnit.Core.SourceGenerator/CodeGenerators/Helpers/TupleArgumentHelper.cs:167-169):

• Excellent fix: The change correctly handles single parameter params arrays by creating an array wrapper instead of passing the raw parameter
• Clear intent: Updated comment accurately reflects the new behavior ("create array with single element")
• Type safety: Proper use of elementType instead of paramsParam.Type ensures correct array element typing
• Consistent patterns: The fix aligns with the multiple-arguments case (lines 172-180) for consistency

ParamsArgumentsTests.cs:

• Comprehensive coverage: Excellent test expansion covering all params array scenarios:
  • Empty params arrays (EmptyParamsArray)
  • Single element params (SingleTypeInParamsArray, SingleStringInParamsArray)
  • Multiple elements params (MultipleTypesInParamsArray, MultipleStringsInParamsArray)
• Modern C# practices: Good use of file-scoped namespaces, proper async/await patterns
• Descriptive naming: Test method names clearly indicate their purpose
• Follows TUnit conventions: Proper use of ArgumentsAttribute and assertion patterns

✅ Bug Analysis

Fixed Issue:
The core bug was in TupleArgumentHelper.cs:167 where single params arguments were incorrectly passed as the raw parameter type instead of being wrapped in an array. This would cause type mismatch errors when the method expects T[] but receives T.

Before (buggy):

var singleArgExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{paramsParam.Type.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";

After (fixed):

var singleElementExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{elementType.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";
argumentExpressions.Add($"new {elementType.GloballyQualified()}[] {{ {singleElementExpression} }}");

No new bugs identified - the fix correctly handles the edge case while maintaining existing functionality.

✅ Performance Considerations

• Minimal impact: The change only affects single-parameter params scenarios and adds one array allocation
• Proper array sizing: Single-element arrays are efficiently created with new T[] { element }
• No unnecessary boxing: Direct element type casting avoids performance overhead
• Generator efficiency: Source generator changes don't affect runtime performance since code is generated at compile-time
• Consistent with TUnit's performance goals: Maintains the framework's emphasis on speed

✅ Security Concerns

• No security issues: The changes are purely structural and don't introduce any security vulnerabilities
• Type safety maintained: Strong typing is preserved throughout the casting process
• Input validation: Existing CastHelper.Cast<T>() provides safe type conversion
• AOT compatibility: Changes maintain AOT and trimming support as required by the codebase

✅ Test Coverage

Excellent improvement in test coverage:

Scenario	Before	After
Empty params	❌	✅ EmptyParamsArray
Single param (Type)	❌	✅ SingleTypeInParamsArray
Single param (string)	❌	✅ SingleStringInParamsArray
Multiple params (Type)	❌	✅ MultipleTypesInParamsArray
Multiple params (string)	❌	✅ MultipleStringsInParamsArray
Enum params	✅	✅ GetOperations (existing)

Test quality highlights:

• Assertions verify both array length and individual elements
• Different data types tested (Type, string, enum)
• Async test patterns follow TUnit conventions
• Comprehensive edge case coverage

✅ Snapshot Test Analysis

The snapshot changes across both test files show consistent updates to generated source code, reflecting the params array fix. Key observations:

• ArgsAsArrayTests.Test.verified.txt: Shows proper array wrapping for params scenarios (lines 82, 92, etc.)
• Tests2112.Test.verified.txt: Demonstrates the fix for the specific bug case with long[] params
• All changes appear to be updating the generated code to properly handle single-element params arrays with array wrapping

✅ Adherence to TUnit Guidelines

This PR follows all critical TUnit development rules:

• ✅ Behavioral parity: Fix applies to source-generated mode and maintains consistency
• ✅ Snapshot tests: Properly updated and committed
• ✅ Performance focus: Minimal performance impact, no regressions
• ✅ Modern C#: Uses contemporary patterns and conventions
• ✅ Comprehensive testing: Excellent test coverage for all scenarios

🎯 Overall Assessment

This is a high-quality fix that:

• ✅ Correctly addresses the params array type handling bug
• ✅ Maintains backward compatibility
• ✅ Follows established code patterns and TUnit conventions
• ✅ Significantly improves test coverage
• ✅ Has minimal performance impact
• ✅ Introduces no security concerns
• ✅ Updates snapshot tests appropriately
• ✅ Adheres to all TUnit development guidelines

Recommendation: Approve - This PR effectively fixes the params array issue with proper testing and follows TUnit's development standards excellently.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix params arrays

I've completed a comprehensive review of this PR that fixes params arrays handling. Here's my detailed feedback:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze TupleArgumentHelper.cs changes
• Review ParamsArgumentsTests.cs improvements
• Check ReflectionTestDataCollector.cs changes
• Examine snapshot test file changes
• Look for potential bugs or edge cases
• Assess performance implications
• Evaluate security concerns
• Check test coverage improvements
• Provide comprehensive review feedback

---

✅ Code Quality & Best Practices

TupleArgumentHelper.cs (TUnit.Core.SourceGenerator/CodeGenerators/Helpers/TupleArgumentHelper.cs:167-169):

• Excellent fix: The change correctly handles single parameter params arrays by creating an array wrapper instead of passing the raw parameter
• Clear intent: Updated comment accurately reflects the new behavior ("create array with single element")
• Type safety: Proper use of elementType instead of paramsParam.Type ensures correct array element typing
• Consistent patterns: The fix aligns with the multiple-arguments case (lines 172-180) for consistency

ReflectionTestDataCollector.cs (lines 1446-1521):

• Comprehensive params handling: The reflection mode now properly handles params arrays with detailed argument processing
• Proper element type checking: Uses GetElementType() and ParamArrayAttribute detection
• Type conversion logic: Includes covariance checks and proper casting with CastHelper.Cast
• Excellent error handling: Graceful fallback when params element type can't be determined

ParamsArgumentsTests.cs:

• Comprehensive coverage: Excellent test expansion covering all params array scenarios:
  • Empty params arrays (EmptyParamsArray)
  • Single element params (SingleTypeInParamsArray, SingleStringInParamsArray)
  • Multiple elements params (MultipleTypesInParamsArray, MultipleStringsInParamsArray)
• Modern C# practices: Good use of file-scoped namespaces, proper async/await patterns
• Descriptive naming: Test method names clearly indicate their purpose
• Follows TUnit conventions: Proper use of ArgumentsAttribute and assertion patterns

✅ Bug Analysis

Fixed Issue:
The core bug was in TupleArgumentHelper.cs:167 where single params arguments were incorrectly passed as the raw parameter type instead of being wrapped in an array. This would cause type mismatch errors when the method expects T[] but receives T.

Before (buggy):

var singleArgExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{paramsParam.Type.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";

After (fixed):

var singleElementExpression = $"TUnit.Core.Helpers.CastHelper.Cast<{elementType.GloballyQualified()}>({argumentsArrayName}[{regularParamCount}])";
argumentExpressions.Add($"new {elementType.GloballyQualified()}[] {{ {singleElementExpression} }}");

Dual Mode Parity: The reflection mode changes ensure both source-generated and reflection modes handle params arrays identically, maintaining TUnit's critical behavioral parity requirement.

No new bugs identified - the fix correctly handles the edge case while maintaining existing functionality.

✅ Performance Considerations

• Minimal impact: The change only affects single-parameter params scenarios and adds one array allocation
• Proper array sizing: Single-element arrays are efficiently created with new T[] { element }
• No unnecessary boxing: Direct element type casting avoids performance overhead
• Generator efficiency: Source generator changes don't affect runtime performance since code is generated at compile-time
• Reflection optimizations: The reflection mode includes performance optimizations like pre-JIT with RuntimeHelpers.PrepareMethod
• Consistent with TUnit's performance goals: Maintains the framework's emphasis on speed

✅ Security Concerns

• No security issues: The changes are purely structural and don't introduce any security vulnerabilities
• Type safety maintained: Strong typing is preserved throughout the casting process
• Input validation: Existing CastHelper.Cast<T>() provides safe type conversion
• AOT compatibility: Changes maintain AOT and trimming support as required by the codebase
• Proper exception handling: Secure error handling with TargetInvocationException unwrapping

✅ Test Coverage

Excellent improvement in test coverage:

Scenario	Before	After
Empty params	❌	✅ EmptyParamsArray
Single param (Type)	❌	✅ SingleTypeInParamsArray
Single param (string)	❌	✅ SingleStringInParamsArray
Multiple params (Type)	❌	✅ MultipleTypesInParamsArray
Multiple params (string)	❌	✅ MultipleStringsInParamsArray
Enum params	✅	✅ GetOperations (existing)

Test quality highlights:

• Assertions verify both array length and individual elements
• Different data types tested (Type, string, enum)
• Async test patterns follow TUnit conventions
• Comprehensive edge case coverage

✅ Snapshot Test Analysis

The snapshot changes across both test files show consistent updates to generated source code, reflecting the params array fix. Key observations:

• ArgsAsArrayTests.Test.verified.txt: Shows proper array wrapping for params scenarios (lines 82, 92, etc.)
• Tests2112.Test.verified.txt: Demonstrates the fix for the specific bug case with long[] params (lines 92, 122, etc.)
• All changes appear to be updating the generated code to properly handle single-element params arrays with array wrapping

✅ Adherence to TUnit Guidelines

This PR follows all critical TUnit development rules:

• ✅ Behavioral parity: Fix applies to both source-generated and reflection modes with identical behavior
• ✅ Snapshot tests: Properly updated and committed
• ✅ Performance focus: Minimal performance impact, no regressions
• ✅ Modern C#: Uses contemporary patterns and conventions
• ✅ Comprehensive testing: Excellent test coverage for all scenarios
• ✅ AOT support: Maintains trimming and AOT compatibility

🎯 Overall Assessment

This is a high-quality fix that:

• ✅ Correctly addresses the params array type handling bug
• ✅ Maintains backward compatibility
• ✅ Follows established code patterns and TUnit conventions
• ✅ Significantly improves test coverage
• ✅ Has minimal performance impact
• ✅ Introduces no security concerns
• ✅ Updates snapshot tests appropriately
• ✅ Ensures behavioral parity between execution modes
• ✅ Adheres to all TUnit development guidelines

Recommendation: Approve - This PR effectively fixes the params array issue with proper testing, maintains dual-mode parity, and follows TUnit's development standards excellently.

---