Fix NUnit migration losing test code in Assert.ThrowsAsync with constraint

[Copilot]

Fix NUnit Migration: Assert.ThrowsAsync with constraint loses test code

Problem

When NUnit Assert.ThrowsAsync(Is.TypeOf(typeof(ArgumentException)), async () => await Sut.Execute(10)) is migrated, it incorrectly becomes await Assert.That(Is.TypeOf(typeof(ArgumentException))).Throws(), losing the test lambda entirely.

Root Cause

The ConvertNUnitThrows method only handled the generic form (Assert.ThrowsAsync<T>(...)), not the non-generic constraint-based form (Assert.ThrowsAsync(constraint, action)).

Solution

• Analyze the issue and understand the code flow
• Update ConvertNUnitThrows to handle non-generic ThrowsAsync with constraint
• Extract exception type from Is.TypeOf constraint
• Convert to generic TUnit ThrowsAsync form
• Add comprehensive test cases for the fix
• Run tests and verify the fix works
• Ensure no regressions in existing functionality
• Fix fallback path for unrecognized constraint patterns
• Add test for unrecognized constraint fallback

Implementation Details

1. Enhanced ConvertNUnitThrows to detect and handle the 2-argument form of Assert.Throws/ThrowsAsync
2. Added TryExtractTypeFromConstraint method to parse Is.TypeOf(typeof(T)) constraint expressions
3. Added ExtractTypeFromTypeof helper to extract the type from typeof(T) expressions
4. Fixed fallback path to correctly handle unrecognized constraint patterns by using the second argument (action) instead of the first (constraint)
5. Added 6 test cases covering:
   • Generic ThrowsAsync (already working, now tested)
   • ThrowsAsync with Is.TypeOf constraint (new fix)
   • ThrowsAsync with complex test code (from bug report) (new fix)
   • Generic Throws (already working, now tested)
   • Throws with Is.TypeOf constraint (new fix)
   • ThrowsAsync with unrecognized constraint (fallback test)

Test Results

✅ All analyzer tests pass (441 tests)

• net9.0: 440 succeeded, 1 skipped (expected)
• Added 1 new test for unrecognized constraint fallback

✅ New tests specifically for this fix (6 tests)

• NUnit_ThrowsAsync_Generic_Converted ✓
• NUnit_ThrowsAsync_WithConstraint_TypeOf_Converted ✓
• NUnit_ThrowsAsync_WithConstraint_TypeOf_With_Code_To_Execute ✓ (exact bug scenario)
• NUnit_Throws_Generic_Converted ✓
• NUnit_Throws_WithConstraint_TypeOf_Converted ✓
• NUnit_ThrowsAsync_WithUnrecognizedConstraint_PreservesAction ✓ (fallback test)

Example Transformation

Before (bug):

// NUnit
Assert.ThrowsAsync(Is.TypeOf(typeof(ArgumentException)), async () => await Sut.Execute(10));

// Incorrect TUnit (loses test code)
await Assert.That(Is.TypeOf(typeof(ArgumentException))).Throws();

After (fixed):

// NUnit
Assert.ThrowsAsync(Is.TypeOf(typeof(ArgumentException)), async () => await Sut.Execute(10));

// Correct TUnit (preserves test code)
await Assert.ThrowsAsync<ArgumentException>(async () => await Sut.Execute(10));

Unrecognized constraint (fallback):

// NUnit with unrecognized constraint
Assert.ThrowsAsync(Is.InstanceOf<ArgumentException>(), async () => await SomeMethod());

// Correct TUnit (preserves action lambda in fallback)
await Assert.That(async () => await SomeMethod()).Throws();

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Bug]: Nunit migration removes testes code for specific case</issue_title>
<issue_description>### Description

When Nunit Assert looks like this:
Assert.ThrowsAsync(Is.TypeOf(typeof(ArgumentException)), async () => await Sut.Execute(10));
autmatic TUnit conversion replaces it with:
await Assert.That(Is.TypeOf(typeof(ArgumentException))).Throws();
which completely removes code that needs to be migrated

Expected Behavior

as above

Actual Behavior

as above

Steps to Reproduce

as above

TUnit Version

1.9.2

.NET Version

10

Operating System

Windows

IDE / Test Runner

dotnet CLI (dotnet test / dotnet run)

Error Output / Stack Trace

Additional Context

No response

IDE-Specific Issue?

• I've confirmed this issue occurs when running via dotnet test or dotnet run, not just in my IDE</issue_description>

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

• Fixes [Bug]: Nunit migration removes testes code for specific case #4234

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[thomhurst]

@copilot

Summary

Fixes NUnit migration that was discarding test code when migrating Assert.ThrowsAsync(Is.TypeOf(typeof(T)), action) to TUnit.

Critical Issues

Bug in fallback path (line 669) - The fallback for unsupported constraint patterns still uses the wrong argument. This will cause the same bug (losing test code) for any constraint pattern that TryExtractTypeFromConstraint doesn't recognize.

The fallback currently uses Arguments[0] (the constraint) instead of Arguments[1] (the action) when there are 2+ arguments. It should check the argument count and use the correct argument.

Suggestions

Add test for unrecognized constraint - Consider adding a test case for a constraint pattern that TryExtractTypeFromConstraint doesn't handle to ensure the fallback works correctly.

Verdict

REQUEST CHANGES - Critical bug in fallback path

[Copilot]

@copilot

Summary

...

Fixed in commit 1f3f59a. The fallback path now correctly checks the argument count and uses Arguments[1] (the action) for 2+ argument cases, preserving the test code for unrecognized constraint patterns. Added test case NUnit_ThrowsAsync_WithUnrecognizedConstraint_PreservesAction to verify the fallback behavior.

[thomhurst]

Summary

Fixes NUnit migration bug where Assert.ThrowsAsync(Is.TypeOf(typeof(T)), action) was incorrectly migrated, losing the test code.

Critical Issues

None found ✅

Previous Review Status

All issues from the previous review have been addressed:

• ✅ Fallback path bug fixed - Now correctly uses Arguments[1] (action) for 2+ argument cases instead of Arguments[0] (constraint)
• ✅ Test for unrecognized constraint added - NUnit_ThrowsAsync_WithUnrecognizedConstraint_PreservesAction verifies fallback behavior

Implementation Analysis

The fix correctly handles three scenarios:

1. Generic form (Assert.Throws<T>(() => ...)) - Already worked, now tested
2. Constraint-based form (Assert.Throws(Is.TypeOf(typeof(T)), () => ...)) - New fix extracts type from constraint
3. Fallback for unrecognized constraints - Correctly preserves action lambda by checking argument count

The TryExtractTypeFromConstraint method properly handles Is.TypeOf(typeof(T)) patterns and returns null for unrecognized patterns, triggering the fallback path which now correctly uses the action argument.

Test coverage is comprehensive with 6 test cases covering all scenarios including the exact bug report case.

Verdict

✅ APPROVE - No critical issues, previous review comments addressed

[Copilot]

Pull request overview

This PR fixes a critical bug in the NUnit to TUnit migration tool where Assert.ThrowsAsync(Is.TypeOf(typeof(T)), action) incorrectly lost the test code during migration, converting to await Assert.That(Is.TypeOf(typeof(T))).Throws() and dropping the action lambda entirely.

Key Changes:

• Enhanced the ConvertNUnitThrows method to detect and handle the 2-argument constraint-based form of Assert.Throws/ThrowsAsync
• Added helper methods to parse Is.TypeOf(typeof(T)) constraints and extract the exception type
• Fixed the fallback path to correctly preserve the action lambda for unrecognized constraint patterns
• Added comprehensive test coverage with 6 new test cases covering generic forms, constraint-based forms, and fallback scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs	Enhanced ConvertNUnitThrows to handle non-generic constraint-based forms; added TryExtractTypeFromConstraint and ExtractTypeFromTypeof helper methods to parse constraint expressions and extract exception types; improved fallback path to preserve action lambda
TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs	Added 6 comprehensive test cases covering generic ThrowsAsync/Throws, constraint-based forms with Is.TypeOf, the exact bug scenario, and fallback behavior for unrecognized constraints

Comments suppressed due to low confidence (3)

TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs:170

• This assignment to literal is useless, since its value is never read.

            if (firstArg.Expression is LiteralExpressionSyntax literal)

TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs:557

• The expression 'A == true' can be simplified to 'A'.

        if (arguments.Count >= 3 && IsLikelyComparerArgument(arguments[2]) == true)

TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs:578

• The expression 'A == true' can be simplified to 'A'.

        if (arguments.Count >= 3 && IsLikelyComparerArgument(arguments[2]) == true)

[Copilot]

Consider extending TryExtractTypeFromConstraint to also support Is.InstanceOf constraint patterns. While the current implementation correctly handles Is.TypeOf(typeof(T)), NUnit also supports Is.InstanceOf<T>() (generic form) and Is.InstanceOf(typeof(T)) (typeof form), which behave similarly to TypeOf but allow subclasses.

To extract the type from the generic form Is.InstanceOf<T>(), you would need to check if the invocation expression is a generic method and extract the type argument from the GenericNameSyntax. This would allow more NUnit assertions to be migrated to the optimized generic TUnit form instead of falling back to Assert.That(...).Throws().

Suggested change

	/// Attempts to extract the exception type from NUnit constraint expressions like Is.TypeOf(typeof(T)).
	/// Returns null if the type cannot be extracted.
	/// </summary>
	private TypeSyntax? TryExtractTypeFromConstraint(ExpressionSyntax constraint)
	{
	// Handle Is.TypeOf(typeof(T)) pattern
	if (constraint is InvocationExpressionSyntax invocation)
	{
	// Check if it's a method call like Is.TypeOf(...) or TypeOf(...)
	if (invocation.Expression is MemberAccessExpressionSyntax memberAccess &&
	memberAccess.Name.Identifier.Text == "TypeOf" &&
	invocation.ArgumentList.Arguments.Count > 0)
	{
	// Extract the argument to TypeOf - should be typeof(T)
	var typeofArg = invocation.ArgumentList.Arguments[0].Expression;
	return ExtractTypeFromTypeof(typeofArg);
	}
	// Handle standalone TypeOf(typeof(T)) calls
	if (invocation.Expression is IdentifierNameSyntax { Identifier.Text: "TypeOf" } &&
	invocation.ArgumentList.Arguments.Count > 0)
	{
	var typeofArg = invocation.ArgumentList.Arguments[0].Expression;
	return ExtractTypeFromTypeof(typeofArg);
	}
	}
	/// Attempts to extract the exception type from NUnit constraint expressions like
	/// Is.TypeOf(typeof(T)), Is.InstanceOf(typeof(T)) or Is.InstanceOf&lt;T&gt;().
	/// Returns null if the type cannot be extracted.
	/// </summary>
	private TypeSyntax? TryExtractTypeFromConstraint(ExpressionSyntax constraint)
	{
	if (constraint is InvocationExpressionSyntax invocation)
	{
	// Handle qualified method calls like Is.TypeOf(...), Is.InstanceOf(...), Is.InstanceOf<T>()
	if (invocation.Expression is MemberAccessExpressionSyntax memberAccess)
	{
	// Generic form: Is.InstanceOf<T>()
	if (memberAccess.Name is GenericNameSyntax genericName &&
	genericName.Identifier.Text == "InstanceOf" &&
	genericName.TypeArgumentList.Arguments.Count > 0)
	{
	return genericName.TypeArgumentList.Arguments[0];
	}
	// typeof form: Is.TypeOf(typeof(T)) or Is.InstanceOf(typeof(T))
	if ((memberAccess.Name.Identifier.Text == "TypeOf" ||
	memberAccess.Name.Identifier.Text == "InstanceOf") &&
	invocation.ArgumentList.Arguments.Count > 0)
	{
	var typeofArg = invocation.ArgumentList.Arguments[0].Expression;
	return ExtractTypeFromTypeof(typeofArg);
	}
	}
	// Handle standalone generic calls: InstanceOf<T>()
	if (invocation.Expression is GenericNameSyntax genericIdentifier &&
	genericIdentifier.Identifier.Text == "InstanceOf" &&
	genericIdentifier.TypeArgumentList.Arguments.Count > 0)
	{
	return genericIdentifier.TypeArgumentList.Arguments[0];
	}
	// Handle standalone typeof form calls: TypeOf(typeof(T)) or InstanceOf(typeof(T))
	if (invocation.Expression is IdentifierNameSyntax { Identifier.Text: var name } &&
	(name == "TypeOf" || name == "InstanceOf") &&
	invocation.ArgumentList.Arguments.Count > 0)
	{
	var typeofArg = invocation.ArgumentList.Arguments[0].Expression;
	return ExtractTypeFromTypeof(typeofArg);
	}
	}