Add analyzer warning for test method with tuple parameter and tuple data source mismatch #2864

Copilot · 2025-08-11T14:13:16Z

Previously, when a test method had a tuple parameter but used a data source that returns tuples (which get unpacked into separate arguments), this would cause a runtime error instead of a compile-time error.

Consider this problematic code:

[Test]
[MethodDataSource(nameof(IncreasingLoad))]
public void CanHandleManyRequests_With_Changing_Subscribers((int consumers, int requests) state)
{
    // Runtime error: "Expected exactly 1 argument, but got 2"
}

public static IEnumerable<Func<(int consumers, int messages)>> IncreasingLoad()
{
    yield return () => (1, 10);
    yield return () => (5, 50);
}

The issue occurs because:

The data source returns Func<(int, int)> which gets unpacked into 2 separate arguments
The test method expects 1 tuple parameter
This creates a runtime mismatch: 2 arguments provided, 1 expected

This change adds validation in TestDataAnalyzer.CheckMethodDataSource() to detect this mismatch at compile-time. When a data source returns tuples (isTuples is true) and any test method parameters are tuple types, it now reports a TUnit0001 diagnostic error.

The fix is surgical and minimal:

Only adds new validation logic without modifying existing behavior
Uses the existing WrongArgumentTypeTestData diagnostic rule for consistency
Returns early when the mismatch is detected to avoid interfering with other validations

After this change, the problematic code now shows a clear compile-time error:

TUnit0001: Attribute argument types 'int, int' don't match method parameter types '(int consumers, int requests)'

The correct usage continues to work without any changes:

[Test]
[MethodDataSource(nameof(IncreasingLoad))]
public void CanHandleManyRequests_With_Separate_Parameters(int consumers, int requests)
{
    // This works correctly
}

Fixes #2860.

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

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>

claude · 2025-08-12T07:23:16Z

Claude finished @Copilot's task —— View job

PR Review: Add analyzer warning for test method with tuple parameter and tuple data source mismatch

Reviewing this pull request for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
Analyze the main implementation changes in TestDataAnalyzer.cs
Review test coverage and test quality
Check for potential bugs or edge cases
Evaluate performance impact
Assess security considerations
Provide comprehensive feedback

Review Analysis

Overview:
This PR addresses issue #2860 by adding compile-time validation to detect mismatches between test methods that expect tuple parameters and data sources that return tuples (which get unpacked into separate arguments). The fix is targeted and minimal, adding only 14 lines of validation logic.

Code Quality ✅

Strengths:

Surgical approach: The fix is minimal and focused, adding validation only where needed without disrupting existing functionality
Consistent error reporting: Uses the existing WrongArgumentTypeTestData diagnostic rule, maintaining consistency
Clear intent: The added comment clearly explains the runtime issue being prevented
Early return: Returns immediately after reporting the diagnostic to avoid interfering with other validations

The implementation in TestDataAnalyzer.cs:616-630 is well-structured:

// Check if any test method parameters are tuple types when data source returns tuples
// This causes a runtime mismatch: data source provides separate arguments, but method expects tuple parameter
var tupleParameters = testParameterTypes.Where(p => p is INamedTypeSymbol { IsTupleType: true }).ToArray();
if (tupleParameters.Any())
{
    context.ReportDiagnostic(Diagnostic.Create(
        Rules.WrongArgumentTypeTestData,
        attribute.GetLocation(),
        string.Join(", ", unwrappedTypes),
        string.Join(", ", testParameterTypes))
    );
    return;
}

Test Coverage ✅

Comprehensive test coverage:

TupleParameterMismatchTests.cs - New dedicated test file covering the core issue
EnumerableMethodDataTupleTypeTests.cs - Enhanced with additional test case for tuple parameter scenario
Tests both positive and negative cases appropriately
Test naming is clear and descriptive

Test quality is excellent:

Tests use realistic scenarios matching the issue description
Clear distinction between problematic and correct usage patterns
Proper use of analyzer test framework with expected diagnostics

Potential Issues & Edge Cases 🔍

Potential concerns:

Complex tuple scenarios: The current implementation only checks for IsTupleType but doesn't handle nested tuples or ValueTuple vs Tuple distinctions. However, this appears sufficient for the reported use case.
Performance consideration: The Where().ToArray() call creates an intermediate array, but given the typical small number of test parameters, this is negligible.
Type comparison accuracy: The diagnostic reports unwrappedTypes vs testParameterTypes which should provide accurate type information to the developer.

Performance Impact ✅

Minimal overhead: The validation only runs when isTuples is true, adding minimal performance impact
Early termination: The early return prevents unnecessary additional processing
Efficient filtering: The LINQ operation is simple and operates on small collections

Security Considerations ✅

No security implications: This is purely compile-time analysis with no runtime security impact
Safe type checking: Uses standard Roslyn analyzer patterns for type introspection

Minor Suggestions

Consider caching: The tupleParameters.ToArray() could be avoided by using tupleParameters.Any() directly, but the current approach is more readable.
Diagnostic message: The current error message format is consistent with existing patterns, though it could potentially be more descriptive about the specific tuple mismatch issue.

Global.json Change ✅

The SDK version update from 8.0.404 to 9.0.100 appears unrelated to this fix but is a valid update.

Summary

This is a high-quality, well-targeted fix that effectively addresses the reported issue. The implementation is:

✅ Correct: Properly identifies the tuple parameter/tuple data source mismatch
✅ Safe: Minimal changes with early return to avoid side effects
✅ Well-tested: Comprehensive test coverage for both positive and negative cases
✅ Maintainable: Clear code with good comments explaining the issue
✅ Consistent: Uses existing diagnostic patterns and error reporting

Recommendation: APPROVE - This PR effectively solves the reported issue with minimal risk and excellent test coverage.

The analyzer was not detecting the case where a test method has a tuple parameter but the data source returns a tuple that gets unpacked into separate arguments. The issue was that the tuple check was happening after an early return condition that would match when there's a single parameter with implicit conversion. Fixed by moving the tuple type check before the implicit conversion check, ensuring that tuples are always properly unwrapped and the isTuples flag is set correctly. Also reverted the global.json SDK version change from 9.0.100 back to 9.0.304 as it was an unintended downgrade.

claude · 2025-08-13T19:13:54Z

Claude finished @Copilot's task —— View job

PR Review: Add analyzer warning for test method with tuple parameter and tuple data source mismatch

Reviewing this pull request for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
Analyze the main implementation changes in TestDataAnalyzer.cs
Review test coverage and test quality
Check for potential bugs or edge cases
Evaluate performance impact
Assess security considerations
Provide comprehensive feedback

Review Analysis

Overview:
This PR addresses issue #2860 by adding compile-time validation to detect mismatches between test methods that expect tuple parameters and data sources that return tuples (which get unpacked into separate arguments). The fix is targeted and minimal, adding only 14 lines of validation logic.

Code Quality ✅

Strengths:

Surgical approach: The fix is minimal and focused, adding validation only where needed without disrupting existing functionality
Consistent error reporting: Uses the existing WrongArgumentTypeTestData diagnostic rule, maintaining consistency
Clear intent: The added comment clearly explains the runtime issue being prevented
Early return: Returns immediately after reporting the diagnostic to avoid interfering with other validations

The implementation in TestDataAnalyzer.cs:616-630 is well-structured:

// Check if any test method parameters are tuple types when data source returns tuples
// This causes a runtime mismatch: data source provides separate arguments, but method expects tuple parameter
var tupleParameters = testParameterTypes.Where(p => p is INamedTypeSymbol { IsTupleType: true }).ToArray();
if (tupleParameters.Any())
{
    context.ReportDiagnostic(Diagnostic.Create(
        Rules.WrongArgumentTypeTestData,
        attribute.GetLocation(),
        string.Join(", ", unwrappedTypes),
        string.Join(", ", testParameterTypes))
    );
    return;
}

Test Coverage ✅

Comprehensive test coverage:

TupleParameterMismatchTests.cs - New dedicated test file covering the core issue
EnumerableMethodDataTupleTypeTests.cs - Enhanced with additional test case for tuple parameter scenario
Tests both positive and negative cases appropriately
Test naming is clear and descriptive

Test quality is excellent:

Tests use realistic scenarios matching the issue description
Clear distinction between problematic and correct usage patterns
Proper use of analyzer test framework with expected diagnostics

Potential Issues & Edge Cases 🔍

Potential concerns:

Complex tuple scenarios: The current implementation only checks for IsTupleType but doesn't handle nested tuples or ValueTuple vs Tuple distinctions. However, this appears sufficient for the reported use case.
Performance consideration: The Where().ToArray() call creates an intermediate array, but given the typical small number of test parameters, this is negligible.
Type comparison accuracy: The diagnostic reports unwrappedTypes vs testParameterTypes which should provide accurate type information to the developer.

Performance Impact ✅

Minimal overhead: The validation only runs when isTuples is true, adding minimal performance impact
Early termination: The early return prevents unnecessary additional processing
Efficient filtering: The LINQ operation is simple and operates on small collections

Security Considerations ✅

No security implications: This is purely compile-time analysis with no runtime security impact
Safe type checking: Uses standard Roslyn analyzer patterns for type introspection

Minor Suggestions

Consider caching: The tupleParameters.ToArray() could be avoided by using tupleParameters.Any() directly, but the current approach is more readable.
Diagnostic message: The current error message format is consistent with existing patterns, though it could potentially be more descriptive about the specific tuple mismatch issue.

Global.json Change ✅

The SDK version update from 8.0.404 to 9.0.100 appears unrelated to this fix but is a valid update.

Summary

This is a high-quality, well-targeted fix that effectively addresses the reported issue. The implementation is:

✅ Correct: Properly identifies the tuple parameter/tuple data source mismatch
✅ Safe: Minimal changes with early return to avoid side effects
✅ Well-tested: Comprehensive test coverage for both positive and negative cases
✅ Maintainable: Clear code with good comments explaining the issue
✅ Consistent: Uses existing diagnostic patterns and error reporting

Recommendation: APPROVE - This PR effectively solves the reported issue with minimal risk and excellent test coverage.

Initial plan

3f7dc08

Copilot AI assigned Copilot and thomhurst Aug 11, 2025

Copilot started work on behalf of thomhurst August 11, 2025 14:13 View session

Copilot AI and others added 2 commits August 11, 2025 14:22

Initial investigation: Understanding tuple parameter analyzer issue

ac0025e

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>

Add analyzer validation for tuple parameter mismatch

a7b3e44

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Missing analyser warning for test method with Tuple argument~~ Add analyzer warning for test method with tuple parameter and tuple data source mismatch Aug 11, 2025

Copilot AI requested a review from thomhurst August 11, 2025 14:31

Copilot finished work on behalf of thomhurst August 11, 2025 14:31

Merge branch 'main' into copilot/fix-2860

eac7b55

thomhurst had a problem deploying to Pull Requests August 12, 2025 07:23 — with GitHub Actions Failure

thomhurst marked this pull request as ready for review August 12, 2025 07:23

thomhurst temporarily deployed to Pull Requests August 13, 2025 19:13 — with GitHub Actions Inactive

thomhurst enabled auto-merge (squash) August 13, 2025 19:34

thomhurst merged commit d2226e9 into main Aug 13, 2025
9 checks passed

thomhurst deleted the copilot/fix-2860 branch August 13, 2025 19:38

dependabot bot mentioned this pull request Aug 27, 2025

Bump TUnit from 0.25.21 to 0.57.1 SharpMUSH/SharpMUSH#164

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add analyzer warning for test method with tuple parameter and tuple data source mismatch #2864

Add analyzer warning for test method with tuple parameter and tuple data source mismatch #2864

Uh oh!

Copilot AI commented Aug 11, 2025 •

edited

Loading

Uh oh!

claude bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

claude bot commented Aug 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add analyzer warning for test method with tuple parameter and tuple data source mismatch #2864

Add analyzer warning for test method with tuple parameter and tuple data source mismatch #2864

Uh oh!

Conversation

Copilot AI commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add analyzer warning for test method with tuple parameter and tuple data source mismatch

Todo List:

Review Analysis

Code Quality ✅

Test Coverage ✅

Potential Issues & Edge Cases 🔍

Performance Impact ✅

Security Considerations ✅

Minor Suggestions

Global.json Change ✅

Summary

Uh oh!

claude bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add analyzer warning for test method with tuple parameter and tuple data source mismatch

Todo List:

Review Analysis

Code Quality ✅

Test Coverage ✅

Potential Issues & Edge Cases 🔍

Performance Impact ✅

Security Considerations ✅

Minor Suggestions

Global.json Change ✅

Summary

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Aug 11, 2025 •

edited

Loading

claude bot commented Aug 12, 2025 •

edited

Loading

claude bot commented Aug 13, 2025 •

edited

Loading