fix: expand migration code fixer attribute support

[thomhurst]

Summary

Expands the migration code fixers with additional attribute conversions for NUnit and MSTest frameworks.

NUnit Improvements

• [Values] → [Matrix] - Parameter-level discrete values
• [Range] → [MatrixRange] - Parameter-level numeric ranges
• [ValueSource] → [MatrixSourceMethod] - Parameter-level method data source
• [NonParallelizable] → [NotInParallel] - Disable parallel execution
• [Parallelizable(ParallelScope.None)] → [NotInParallel] - Disable parallel execution
• [Parallelizable] (other scopes) → Removed (TUnit is parallel by default)
• [Repeat] → [Repeat] - Same name, preserved
• Bug fix: Fixed ShouldRemoveAttribute prematurely removing [Parallelizable] before ConvertAttribute could handle ParallelScope.None

MSTest Improvements

• [Priority(1)] → [Property("Priority", "1")] - Fixed attribute name conversion (was keeping "Priority" instead of "Property")
• [TestCategory("X")] → [Property("Category", "X")] - Fixed attribute name conversion
• [Owner("Name")] → [Property("Owner", "Name")] - New conversion
• [ExpectedException(typeof(T))] → Method body wrapped in Assert.ThrowsAsync<T>() - Complex transformation

Test plan

• All 316 NUnit migration tests pass
• All 108 MSTest migration tests pass
• New tests added for each attribute conversion

🤖 Generated with Claude Code

[thomhurst]

Summary

Expands migration code fixers to support additional MSTest and NUnit attributes, including parameter-level data attributes, parallelization control, and exception expectations.

Critical Issues

1. Async lambda bug in ExpectedException conversion (MSTestMigrationCodeFixProvider.cs:928-933)

The MSTestExpectedExceptionRewriter.WrapBodyInThrowsAsync method creates a non-async lambda, but if the original method body contains await expressions, this will cause a compilation error.

Problem code:

var lambda = SyntaxFactory.ParenthesizedLambdaExpression(
    SyntaxFactory.ParameterList(),
    SyntaxFactory.Block(originalStatements)
);

If originalStatements contains await, the lambda needs to be async:

var lambda = SyntaxFactory.ParenthesizedLambdaExpression(
    asyncKeyword: hasAwait ? SyntaxFactory.Token(SyntaxKind.AsyncKeyword) : default,
    SyntaxFactory.ParameterList(),
    SyntaxFactory.Block(originalStatements)
);

Where hasAwait is:

bool hasAwait = originalStatements.Any(s => s.DescendantNodes().OfType<AwaitExpressionSyntax>().Any());

Test coverage needed: Add a test case with an async method:

[TestMethod]
[ExpectedException(typeof(ArgumentException))]
public async Task TestMethodAsync()
{
    await Task.Delay(1);
    throw new ArgumentException("test");
}

Suggestions

None - the rest of the implementation looks solid.

Verdict

⚠️ REQUEST CHANGES - Critical issue with async lambda generation that would break migration of async test methods with [ExpectedException].

[thomhurst]

Summary

Expands migration code fixers with additional attribute conversions for NUnit and MSTest frameworks, including parameter-level data attributes, parallelization control, and exception expectations.

Critical Issues

1. Async lambda bug in ExpectedException conversion remains unresolved (MSTestMigrationCodeFixProvider.cs)

The previous review identified a critical bug where the lambda created in WrapBodyInThrowsAsync is not marked as async when the original method body contains await expressions. This issue has NOT been fixed in the current PR.

Problem: When converting an async method with [ExpectedException], the generated code will have a non-async lambda containing await expressions, causing a compiler error.

Required fix: The code must check if originalStatements contain await expressions, and if so, add the async modifier to the lambda using SyntaxFactory.Token(SyntaxKind.AsyncKeyword).

Missing test coverage: The PR lacks a test case for async methods with [ExpectedException]. The only test is for a synchronous method.

Suggestions

None - the other conversions (Values→Matrix, Range→MatrixRange, NonParallelizable→NotInParallel, etc.) look correct and follow TUnit patterns.

Previous Review Status

The critical async lambda issue from the previous review (2026-01-13T01:23:26Z) remains unaddressed. While the PR was updated 10 minutes after the review, the fix was not applied.

Verdict

⚠️ REQUEST CHANGES - The critical async lambda bug must be fixed before merging. This will cause compilation errors when migrating async test methods that use [ExpectedException].

[thomhurst]

Summary

Expands migration code fixers with additional attribute conversions for NUnit and MSTest frameworks, including parameter-level data attributes, parallelization control, and exception expectations.

Critical Issues

1. Operator precedence bug in InferRangeType (NUnitMigrationCodeFixProvider.cs, line ~187 in diff)

The condition for detecting 'd'/'D' suffix has an operator precedence issue:

if (tokenText.EndsWith("d", StringComparison.OrdinalIgnoreCase) ||
    tokenText.EndsWith("D", StringComparison.OrdinalIgnoreCase) && \!tokenText.StartsWith("0x", StringComparison.OrdinalIgnoreCase))

The && binds tighter than ||, so this is parsed as:

if (tokenText.EndsWith("d", StringComparison.OrdinalIgnoreCase) ||
    (tokenText.EndsWith("D", StringComparison.OrdinalIgnoreCase) && \!tokenText.StartsWith("0x")))

This means lowercase 'd' suffix would match even for hex literals (though unlikely in practice). Should be:

if ((tokenText.EndsWith("d", StringComparison.OrdinalIgnoreCase) ||
     tokenText.EndsWith("D", StringComparison.OrdinalIgnoreCase)) && 
    \!tokenText.StartsWith("0x", StringComparison.OrdinalIgnoreCase))

However, on reflection, the hex check may be unnecessary - C# doesn't allow 'd' suffix on hex literals, so this would be caught as invalid syntax before the migration tool runs. Consider removing the hex check entirely or adding parentheses for clarity.

Suggestions

None - the rest of the implementation is solid. Good work on:

• ✅ Fixing the async lambda issue from the previous review
• ✅ Adding comprehensive test coverage (60+ new tests)
• ✅ Handling edge cases like DirectoryInfo vs string paths
• ✅ Clear comments explaining design decisions

Previous Review Status

The critical async lambda bug has been successfully fixed. The code now:

1. Detects await expressions in the original method body
2. Adds async keyword to the lambda when needed
3. Has proper test coverage for both sync and async cases

Verdict

✅ APPROVE - The operator precedence issue is minor and unlikely to cause real-world problems since invalid syntax would be caught before migration. Everything else looks excellent.