feat: properly migrate NUnit TestCase properties to TUnit attributes

[thomhurst]

Summary

Instead of adding TODO comments for unsupported NUnit TestCase properties, this PR properly converts them to their TUnit equivalents:

NUnit TestCase Property	TUnit Attribute
TestName = "..."	[DisplayName("...")]
Category = "..."	[Category("...")]
Description = "..."	[Property("Description", "...")]
Author = "..."	[Property("Author", "...")]
Explicit = true	[Explicit]
ExplicitReason = "..."	[Explicit] + [Property("ExplicitReason", "...")]

Changes

• New NUnitTestCasePropertyRewriter: Method-level rewriter that extracts properties from all TestCase attributes on a method and generates corresponding TUnit attributes
• Fix ConvertCategoryArguments: Keep [Category] as-is since TUnit has a native Category attribute (was incorrectly converting to [Property("Category", "...")])
• Reorder rewriters: Property extraction runs BEFORE ExpectedResult conversion so properties are captured from original TestCase attributes
• Add 8 new tests covering property mapping scenarios including combined ExpectedResult + properties

Example

Before (NUnit):

[TestCase(1, TestName = "Test One", Category = "Unit", Description = "First test")]
public void MyTest(int value) { }

After (TUnit):

[Test]
[Arguments(1)]
[DisplayName("Test One")]
[Category("Unit")]
[Property("Description", "First test")]
public void MyTest(int value) { }

Test plan

• All 39 NUnit migration tests pass
• All 21 MSTest migration tests pass
• All 33 xUnit migration tests pass
• Full analyzer test suite (410 tests) passes

🤖 Generated with Claude Code

[thomhurst]

Summary

Improves NUnit→TUnit migration by properly converting TestCase properties (TestName, Category, Description, Author, Explicit) to their TUnit equivalents instead of leaving TODO comments.

Critical Issues

1. Potential String Interpolation Parsing Bug (NUnitTestCasePropertyRewriter.cs:138)

The GetStringValue method has a fallback that may produce incorrect results for non-literal strings. The expression.ToString().Trim('"') will produce invalid results for interpolated strings, string concatenation, or const references.

Impact: When users have TestCase properties with non-literal strings, the migration will produce incorrect DisplayName/Category/Property values that may not compile or have wrong runtime values.

Recommendation: Return null for non-literal expressions and document that only literal strings are migrated automatically.

---

2. Missing Snapshot Test Verification

Per TUnit Rule 2 (Snapshot Testing), changes to analyzer output require verifying snapshot tests. This PR modifies code fixer output significantly. Need to verify whether snapshots were updated if the test framework uses them.

Suggestions

1. Performance: Unnecessary Allocations (NUnitTestCasePropertyRewriter.cs:60-66)

Multiple HashSet allocations per method even when no properties exist. Suggest early return if no named arguments exist before allocating TestCaseProperties.

2. DisplayName Conflict Handling (NUnitTestCasePropertyRewriter.cs:208-221)

When multiple TestCases have different TestNames, the code uses .First() which may be arbitrary. Consider not generating DisplayName if TestNames differ.

3. Code Clarity: Comment Accuracy (NUnitMigrationCodeFixProvider.cs:40-43)

The old comment at line 40 should be removed to avoid confusion with the new ordering.

Verdict

⚠️ REQUEST CHANGES - Critical string interpolation bug must be fixed

[Copilot]

Pull request overview

This PR improves NUnit to TUnit migration by properly converting TestCase properties to their TUnit equivalents instead of leaving TODO comments. The implementation adds a new NUnitTestCasePropertyRewriter that extracts properties from TestCase attributes and generates corresponding TUnit attributes (TestName → DisplayName, Category → Category, Description/Author → Property, Explicit → Explicit). The rewriter runs before ExpectedResult conversion to capture properties from original attributes.

Key changes:

• New property extraction rewriter that consolidates properties from all TestCase attributes on a method
• Fixed Category conversion to preserve native TUnit Category attribute instead of incorrectly converting to Property
• Execution order ensures properties are extracted before TestCase attributes are transformed

Reviewed changes

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

File	Description
TUnit.Analyzers.CodeFixers/NUnitTestCasePropertyRewriter.cs	New rewriter that extracts TestCase properties and generates TUnit attributes
TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs	Integrates property rewriter and fixes Category conversion
TUnit.Analyzers.CodeFixers/NUnitExpectedResultRewriter.cs	Updates to skip properties now handled by property rewriter
TUnit.Analyzers.Tests/NUnitMigrationAnalyzerTests.cs	Adds 8 tests covering property conversion scenarios

[Copilot]

According to TUnit coding guidelines, use collection expressions (C# 12+). This should be:

var newLists = new List<AttributeListSyntax>(existingAttributeLists);

should be:

List<AttributeListSyntax> newLists = [..existingAttributeLists];

[Copilot]

According to TUnit coding guidelines, use collection expressions (C# 12+). These HashSet initializations in the TestCaseProperties class should use collection expression syntax:

public HashSet<string> TestNames { get; } = new();

should be:

public HashSet<string> TestNames { get; } = [];

Apply the same change to all other HashSet properties (Categories, Descriptions, Authors, ExplicitReasons).

[Copilot]

This test has a logical issue: when there are two TestCase attributes with different TestName values ("Addition of one" and "Addition of two"), only one DisplayName attribute is generated, using the first value. This means the second test case loses its custom name.

In NUnit, each TestCase with TestName creates a separate test with that name. In TUnit, each Arguments attribute creates a separate test, but there's only one DisplayName that applies to all of them. The expected output should either:

1. Not generate a DisplayName when TestNames differ across TestCase attributes (preserving default behavior)
2. Generate multiple test methods instead of one parameterized method
3. Document that this is a known limitation

The current behavior silently drops "Addition of two" which could be unexpected.

Suggested change

[DisplayName("Addition of one")]

[Copilot]

The fallback logic here is problematic. When the expression is not a string literal (e.g., a constant, interpolated string, or complex expression), calling ToString().Trim('"') on the syntax node is unreliable:

1. Interpolated strings like $"Test {value}" will have their syntax representation trimmed, not their actual value
2. Constants or other expressions won't have quotes to trim
3. This could produce invalid attribute syntax

Consider either:

1. Only handling string literals and returning null for other cases (safest)
2. Using the semantic model to evaluate compile-time constants
3. Preserving the expression as-is without string manipulation

The current implementation risks generating malformed code for non-literal expressions.

Suggested change

	// Handle interpolated strings or other expressions by getting the text
	return expression.ToString().Trim('"');
	// For non-literal expressions, preserve the expression text as-is.
	// This avoids trying to interpret or partially trim the syntax representation.
	return expression.ToString();

[Copilot]

Missing using directive for System.Collections.Generic. The code uses List and HashSet but only includes System.Linq. While this might compile due to implicit global usings in newer .NET versions, it's better to be explicit for clarity and compatibility.

[Copilot]

This test expects both "Unit" and "Integration" categories to be applied to the method, but this doesn't match NUnit semantics. In NUnit, each TestCase attribute creates a separate test instance, so TestCase(1, Category = "Unit") creates one test in the "Unit" category, and TestCase(2, Category = "Integration") creates a different test in the "Integration" category.

The converted TUnit code applies BOTH categories to the same method, meaning both Arguments(1) and Arguments(2) will be tagged with both categories, which is incorrect. Each test variant should only have its corresponding category.

TUnit's architecture may not support per-argument categorization (since [Category] is a method-level attribute), making this an architectural limitation that should be documented rather than silently producing incorrect behavior.

Suggested change

	[Category("Unit")]
	[Category("Integration")]
	// NOTE: Per-argument categories (Unit/Integration) from NUnit TestCase attributes
	// are not preserved because TUnit only supports method-level categories.

[Copilot]

According to TUnit coding guidelines, use collection expressions (C# 12+) instead of the old initialization syntax. This should be:

var result = new List<AttributeSyntax>();

should be:

List<AttributeSyntax> result = [];

[Copilot]

According to TUnit coding guidelines, use collection expressions (C# 12+). This should be:

var newArgs = new List<AttributeArgumentSyntax>();

should be:

List<AttributeArgumentSyntax> newArgs = [];

[Copilot]

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Suggested change

	foreach (var trivia in triviaList)
	{
	if (trivia.IsKind(SyntaxKind.WhitespaceTrivia))
	{
	return trivia;
	}
	var whitespaceTrivia = triviaList.FirstOrDefault(t => t.IsKind(SyntaxKind.WhitespaceTrivia));
	if (whitespaceTrivia.IsKind(SyntaxKind.WhitespaceTrivia))
	{
	return whitespaceTrivia;

[Copilot]

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

Suggested change

	foreach (var category in properties.Categories.Distinct())
	{
	var categoryAttr = SyntaxFactory.Attribute(
	SyntaxFactory.IdentifierName("Category"),
	SyntaxFactory.AttributeArgumentList(
	SyntaxFactory.SingletonSeparatedList(
	SyntaxFactory.AttributeArgument(
	SyntaxFactory.LiteralExpression(
	SyntaxKind.StringLiteralExpression,
	SyntaxFactory.Literal(category))))));
	newLists.Add(CreateAttributeList(categoryAttr, indentation));
	}
	var categoryAttributeLists = properties.Categories
	.Distinct()
	.Select(category =>
	CreateAttributeList(
	SyntaxFactory.Attribute(
	SyntaxFactory.IdentifierName("Category"),
	SyntaxFactory.AttributeArgumentList(
	SyntaxFactory.SingletonSeparatedList(
	SyntaxFactory.AttributeArgument(
	SyntaxFactory.LiteralExpression(
	SyntaxKind.StringLiteralExpression,
	SyntaxFactory.Literal(category)))))),
	indentation));
	newLists = newLists.AddRange(categoryAttributeLists);

[thomhurst]

Summary

This PR properly migrates NUnit TestCase properties (TestName, Category, Description, Author, Explicit, ExplicitReason) to their TUnit equivalents instead of leaving TODO comments.

Critical Issues

None found ✅

Suggestions

Code Quality - All Good

The implementation is well-structured with:

• Early optimization (line 60-70 in NUnitTestCasePropertyRewriter.cs): Checks for named arguments before processing
• Correct rewriter ordering: Property extraction runs BEFORE ExpectedResult conversion to capture original attributes
• Smart DisplayName handling: Only generates [DisplayName] when all TestNames are identical across multiple [TestCase] attributes
• Proper Category fix: Keeps [Category] as-is since TUnit has native Category support

Test Coverage - Comprehensive

All 8 new tests cover the property mapping scenarios well:

• Same vs different TestNames
• All property types (Category, Description, Author, Explicit, ExplicitReason)
• Combined properties
• ExpectedResult + properties interaction

TUnit Rules Compliance

✅ Dual-Mode: Not applicable (analyzer code, not core engine)
✅ Snapshot Testing: Not applicable (no source generator output or public API changes)
✅ No VSTest: No VSTest references
✅ Performance: Appropriate for code fixer context (not hot path)
✅ AOT: Not applicable (compile-time only)

Previous Review Status

Unable to retrieve previous comments due to token scope limitations, but all code changes appear clean and well-tested.

Verdict

✅ APPROVE - No critical issues. This is a solid improvement that properly handles NUnit TestCase properties instead of leaving TODOs. The implementation is correct, well-tested, and follows TUnit best practices.