feat: comprehensive migration code fixer improvements

[thomhurst]

Summary

• Add extensive assertion conversion support for NUnit, MSTest, and xUnit migration code fixers
• Add format string message preservation (Assert.AreEqual(5, x, "Expected {0}", x) → string.Format())
• Add custom comparer detection using semantic model (generates TODO comments instead of misinterpreting as message)
• Add AsyncMethodSignatureRewriter to convert void test methods to async Task when assertions are converted
• Add TestAttributeEnsurer to add [Test] attribute when data attributes exist (NUnit → TUnit migration)

NUnit Additions

• Assert.Throws<T>() / Assert.ThrowsAsync<T>() → Throws.InstanceOf<T>()
• Assert.Pass(), Assert.Fail(), Assert.Inconclusive(), Assert.Ignore()
• Is.SubsetOf(), Is.SupersetOf(), Is.EquivalentTo()
• Is.Unique → HasDistinctItems()
• Is.Ordered → IsInAscendingOrder()
• Is.InRange() → IsBetween()

MSTest Additions

• Assert.Inconclusive() → Assert.Skip()
• CollectionAssert.AreEquivalent() / AreNotEquivalent()
• CollectionAssert.AllItemsAreUnique() → HasDistinctItems()
• CollectionAssert.IsSubsetOf() / IsNotSubsetOf()
• CollectionAssert.AllItemsAreInstancesOfType()

xUnit Additions

• Assert.ThrowsAny<T>() / ThrowsAnyAsync<T>()
• Assert.Single() → HasSingleItem()
• Assert.Distinct() → HasDistinctItems()
• Assert.Subset() / Assert.Superset()
• Assert.Equivalent() → IsEquivalentTo()
• Assert.All() → AllSatisfy()

Test plan

• NUnit migration tests: 124/124 passed
• MSTest migration tests: 84/84 passed
• xUnit migration tests: 131/132 passed (1 infrastructure failure on net10.0)

🤖 Generated with Claude Code

[thomhurst]

Summary

This PR adds comprehensive migration code fixer improvements for NUnit, MSTest, and xUnit migrations to TUnit, including format string message preservation, custom comparer detection, async method signature conversion, and test attribute ensuring.

Critical Issues

None found ✅

Suggestions

1. String Format Safety (Low Priority)

In AssertionRewriter.cs:146-168, the CreateMessageExpression method generates string.Format() calls. While this is safe in the context of a code fixer (it generates code, doesn't execute it), consider adding a comment explaining that the generated code preserves the original format string from the migrated assertion:

/// <summary>
/// Creates a message expression, wrapping in string.Format if format args are present.
/// NOTE: This generates code; it does not execute string.Format. The format string
/// comes from the original test framework assertion being migrated.
/// </summary>

This clarifies the security model for future maintainers.

2. AllInterfaces Performance (Low Priority)

In AssertionRewriter.cs:188-191, the IsLikelyComparerArgument method uses namedType.AllInterfaces.Any(...). While this is fine for a code fixer (not a hot path), consider caching the semantic model lookups if this proves to be a bottleneck during migration of large codebases.

3. Test Coverage Clarification

The PR description mentions "xUnit migration tests: 131/132 passed (1 infrastructure failure on net10.0)". Consider opening a follow-up issue to track this infrastructure failure, even if it's unrelated to this PR.

TUnit Rules Compliance

✅ Rule 1 (Dual-Mode): Not applicable - changes are to analyzers/code fixers only, not core engine metadata collection.

✅ Rule 2 (Snapshot Testing): No .received.txt files committed. No changes to source generator output or public APIs that would require snapshot updates.

✅ Rule 3 (No VSTest): No references to Microsoft.VisualStudio.TestPlatform found.

✅ Rule 4 (Performance): Not applicable - code fixers run during migration (one-time), not in test execution hot paths.

✅ Rule 5 (AOT): No new reflection usage introduced. The SemanticModel.GetTypeInfo usage is part of Roslyn's semantic analysis (compile-time, not runtime reflection).

Code Quality Observations

Strengths:

• Well-structured with proper separation of concerns (new rewriters as separate classes)
• Good use of semantic model for comparer detection instead of string matching
• Comprehensive TODO comment generation for unsupported features
• Proper trivia preservation throughout transformations
• Thorough test coverage (124/124 NUnit, 84/84 MSTest, 131/132 xUnit)

Design Decisions:

• AsyncMethodSignatureRewriter correctly handles the order of modifiers (async after access modifiers)
• TestAttributeEnsurer handles NUnit's [TestCase] → TUnit's [Test] + [Arguments] requirement
• Message extraction properly handles both simple messages and format strings with arguments

Verdict

✅ APPROVE - No critical issues. The suggestions above are optional improvements that don't block merging. This is a solid enhancement to TUnit's migration capabilities.

[Copilot]

Pull request overview

This PR adds comprehensive migration code fixer improvements for NUnit, MSTest, and xUnit, significantly expanding assertion conversion support and adding infrastructure for automatic async/await transformation and test attribute management.

Key Changes:

• Added AsyncMethodSignatureRewriter to automatically convert void test methods to async Task when assertions are converted to await
• Added TestAttributeEnsurer to add [Test] attributes when data attributes exist (handles NUnit's [TestCase] → TUnit's [Test] + [Arguments] requirement)
• Extended AssertionRewriter base class with message preservation support, format string handling via string.Format(), and custom comparer detection using semantic model
• Added extensive assertion conversions across all three frameworks (30+ new assertion types including collection assertions, subset/superset operations, and exception handling improvements)

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
TUnit.Analyzers.CodeFixers/Base/AsyncMethodSignatureRewriter.cs	New class to convert void methods to async Task when they contain await expressions
TUnit.Analyzers.CodeFixers/Base/TestAttributeEnsurer.cs	New class to ensure [Test] attribute is present when data attributes exist
TUnit.Analyzers.CodeFixers/Base/AssertionRewriter.cs	Extended with message support, format args handling, and comparer detection
TUnit.Analyzers.CodeFixers/Base/BaseMigrationCodeFixProvider.cs	Integrated AsyncMethodSignatureRewriter and TestAttributeEnsurer into migration pipeline
TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs	Added comprehensive NUnit assertion conversions including Throws, Pass/Fail/Skip, and negation patterns
TUnit.Analyzers.CodeFixers/MSTestMigrationCodeFixProvider.cs	Added MSTest message preservation and collection assertions (AreEquivalent, AllItemsAreUnique, IsSubsetOf, etc.)
TUnit.Analyzers.CodeFixers/XUnitMigrationCodeFixProvider.cs	Added xUnit assertion conversions including ThrowsAny, IsNotType, Single, Distinct, Subset/Superset, and Equivalent
TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs	Added tests for new conversions and async signature transformation; removed redundant attribute mapping tests
TUnit.Analyzers.Tests/MSTestMigrationAnalyzerTests.cs	Added tests for message preservation across assertion types

[Copilot]

The ExtractMessageWithFormatArgs method doesn't validate that format arguments are actually format placeholders before attempting to wrap them in string.Format. If a testing framework allows passing extra parameters that aren't format args (like tolerance values or comparers), this could incorrectly wrap them in string.Format. Consider checking if the message expression is actually a string literal with format placeholders before treating additional arguments as format args.

Suggested change

	// Check if there are additional format arguments after the message
	if (arguments.Count > messageIndex + 1)
	{
	var formatArgs = arguments.Skip(messageIndex + 1).ToArray();
	return (message, formatArgs);
	}
	// If there are no additional arguments after the message, there's nothing to format.
	if (arguments.Count <= messageIndex + 1)
	{
	return (message, null);
	}
	// Only treat trailing arguments as format arguments when the message is a string
	// literal that actually contains format placeholders like "{0}", "{1}", etc.
	if (message is LiteralExpressionSyntax literal &&
	literal.IsKind(SyntaxKind.StringLiteralExpression))
	{
	var text = literal.Token.ValueText;
	var hasPlaceholder = false;
	for (var i = 0; i < text.Length - 1; i++)
	{
	if (text[i] == '{' && char.IsDigit(text[i + 1]))
	{
	hasPlaceholder = true;
	break;
	}
	}
	if (hasPlaceholder)
	{
	var formatArgs = arguments.Skip(messageIndex + 1).ToArray();
	return (message, formatArgs);
	}
	}
	// Message is not a format string; ignore trailing arguments for formatting purposes.

[Copilot]

The comment on line 392 mentions creating "x => x.GetType() == expectedType or x is Type", but the implementation on lines 393-399 creates an incomplete IsPatternExpression that is never used. The variable isExpression is declared but never referenced. This dead code should be removed.

[Copilot]

The hardcoded whitespace string with 16 spaces creates fragile indentation that will break when placed at different nesting levels. The leading trivia from the original node should be preserved instead to maintain consistent indentation in the migrated code.

Suggested change

	return result.WithLeadingTrivia(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using Assert.That(...).IsEquivalentTo() or a custom condition."),
	SyntaxFactory.EndOfLine("\n"),
	SyntaxFactory.Whitespace(" "));
	}
	private ExpressionSyntax CreateNotEqualWithComparerComment(SeparatedSyntaxList<ArgumentSyntax> arguments)
	{
	var result = CreateTUnitAssertion("IsNotEqualTo", arguments[1].Expression, arguments[0]);
	return result.WithLeadingTrivia(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using a custom condition."),
	SyntaxFactory.EndOfLine("\n"),
	SyntaxFactory.Whitespace(" "));
	var originalLeadingTrivia = result.GetLeadingTrivia();
	return result.WithLeadingTrivia(
	SyntaxFactory.TriviaList(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using Assert.That(...).IsEquivalentTo() or a custom condition."),
	SyntaxFactory.EndOfLine("\n"))
	.AddRange(originalLeadingTrivia));
	}
	private ExpressionSyntax CreateNotEqualWithComparerComment(SeparatedSyntaxList<ArgumentSyntax> arguments)
	{
	var result = CreateTUnitAssertion("IsNotEqualTo", arguments[1].Expression, arguments[0]);
	var originalLeadingTrivia = result.GetLeadingTrivia();
	return result.WithLeadingTrivia(
	SyntaxFactory.TriviaList(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using a custom condition."),
	SyntaxFactory.EndOfLine("\n"))
	.AddRange(originalLeadingTrivia));

[Copilot]

The hardcoded whitespace string with 16 spaces creates fragile indentation that will break when placed at different nesting levels. The leading trivia from the original node should be preserved instead to maintain consistent indentation in the migrated code.

Suggested change

	return result.WithLeadingTrivia(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using Assert.That(...).IsEquivalentTo() or a custom condition."),
	SyntaxFactory.EndOfLine("\n"),
	SyntaxFactory.Whitespace(" "));
	}
	private ExpressionSyntax CreateNotEqualWithComparerComment(SeparatedSyntaxList<ArgumentSyntax> arguments)
	{
	var result = CreateTUnitAssertion("IsNotEqualTo", arguments[1].Expression, arguments[0]);
	return result.WithLeadingTrivia(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using a custom condition."),
	SyntaxFactory.EndOfLine("\n"),
	SyntaxFactory.Whitespace(" "));
	var updatedTrivia = SyntaxFactory.TriviaList(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using Assert.That(...).IsEquivalentTo() or a custom condition."),
	SyntaxFactory.ElasticCarriageReturnLineFeed)
	.AddRange(result.GetLeadingTrivia());
	return result.WithLeadingTrivia(updatedTrivia);
	}
	private ExpressionSyntax CreateNotEqualWithComparerComment(SeparatedSyntaxList<ArgumentSyntax> arguments)
	{
	var result = CreateTUnitAssertion("IsNotEqualTo", arguments[1].Expression, arguments[0]);
	var updatedTrivia = SyntaxFactory.TriviaList(
	SyntaxFactory.Comment("// TODO: TUnit migration - custom comparer was used. Consider using a custom condition."),
	SyntaxFactory.ElasticCarriageReturnLineFeed)
	.AddRange(result.GetLeadingTrivia());
	return result.WithLeadingTrivia(updatedTrivia);

[Copilot]

The hardcoded whitespace string with 16 spaces creates fragile indentation that will break when placed at different nesting levels. The leading trivia from the original node should be preserved instead to maintain consistent indentation in the migrated code.

[Copilot]

The hardcoded whitespace string with 16 spaces creates fragile indentation that will break when placed at different nesting levels. The leading trivia from the original node should be preserved instead to maintain consistent indentation in the migrated code.

[Copilot]

The comment says "Same as ThrowsAny but for async", but ConvertThrowsAny already converts to ThrowsAsync. This means ThrowsAnyAsync is being converted to ThrowsAsync (correct), but ThrowsAny is also being converted to ThrowsAsync (incorrect - synchronous xUnit ThrowsAny should convert to synchronous TUnit Throws, not async ThrowsAsync). The ConvertThrowsAny method name and implementation are misleading and create a synchronous/async mismatch.

[Copilot]

The conversion logic incorrectly wraps any non-void return type in Task, which will break methods that already return Task or Task<T>. For example, a method returning Task<int> would become async Task<Task<int>>. The method should check if the return type is already a Task or Task<T> before wrapping it.

[Copilot]

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

[Copilot]

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.