Avoid some redundant list allocations

[SimonCropp]

No description provided.

[claude]

Code Review

Overall this is a clean, well-targeted performance improvement. All 18 files follow the same pattern consistently, every downstream consumer correctly accepts IEnumerable<T>, and no correctness issues were introduced.

Checked for: bugs (including multiple enumeration, deferred execution side effects, thread safety), CLAUDE.md compliance (Performance First, AOT compatibility, dual-mode requirements, snapshot testing rules).

---

Minor Observation: Double Enumeration in AspireFixture.cs

One removal stands out as slightly counter to the PR's intent. In TUnit.Aspire/AspireFixture.cs, the pending variable is enumerated twice after the .ToList() was removed:

var pending = resourceNames.Where(n => !readySet.Contains(n)); // now lazy

// First enumeration:
logger.LogInformation($"Waiting for {string.Join(", ", pending.Select(n => $"'{n}'"))} ...");

// Second enumeration:
foreach (var name in pending) { ... }

The original .ToList() here was actually intentional — it materializes the filtered set once and re-uses it twice. Without it, the Where predicate runs twice over resourceNames. While this is not a correctness bug (the state doesn't change between the two enumerations, and it's in a timeout/polling code path), it does introduce redundant work rather than eliminating it — the opposite of what the PR aims to do.

A simple fix would be to keep the .ToList() on this specific line, or restructure to iterate pending just once.

---

All other changes are genuinely beneficial and correct. Nice cleanup!

[claude]

Follow-up Review

The concern raised in my previous review (double enumeration of pending in AspireFixture.cs) has been addressed — that file is no longer in the PR diff. Good.

Current State

All 17 remaining changes are correct. Every .ToList() / .ToArray() that was removed is consumed exactly once downstream, so deferred evaluation introduces no multiple-enumeration risk. Specific checks:

• MigrationAnalyzer.cs / XUnitTwoPhaseAnalyzer.cs — _originalRoot / OriginalRoot is treated as read-only throughout each method; the currentRoot variable is what evolves. No aliasing issue.
• DisposableFieldPropertyAnalyzer.cs — each syntaxNodes is consumed in exactly one foreach. Removing .ToArray() is safe even though GetSyntax() is slightly expensive; re-execution is never triggered.
• SingleTUnitAttributeAnalyzer.cs — GroupBy internalises the source immediately, so the upstream singleAttributes being lazy is inconsequential.
• SyntaxFactory.List(convertedStatements) — accepts IEnumerable<T> and enumerates once internally.

One Subtle Note (not a blocker)

In CodeGenerationHelpers.cs, the elements select lambda has side effects:

var elements = typedConstant.Values.Select((v, i) =>
{
    syntaxIndex++;   // side effect
    elementIndex++;  // side effect
    return TypedConstantParser.GetRawTypedConstantValue(v, paramType);
}); // was .ToList(), now lazy
argStrings.AddRange(elements);

This is safe today because List.AddRange enumerates exactly once. However, the original .ToList() here was doing double duty — eliminating allocation and signalling "evaluate now because we have stateful side effects". The lazy version is slightly more fragile if someone later adds a second use of elements. A comment, or keeping this single .ToList(), would make the intent clearer. That said, it is not a correctness issue in the current code.

Summary

All changes are beneficial and correct. The regression identified in the first review was fixed. This is a clean, minimal cleanup that delivers real (if small) allocation savings across the analyzer and code-fixer hot paths.

[claude]

Follow-up Review

The concern raised in my previous review (double enumeration of pending in AspireFixture.cs) has been addressed — that file is no longer in the PR diff. Good.

Current State

All 17 remaining changes are correct. Every .ToList() / .ToArray() that was removed is consumed exactly once downstream, so deferred evaluation introduces no multiple-enumeration risk. Specific checks:

• MigrationAnalyzer.cs / XUnitTwoPhaseAnalyzer.cs — _originalRoot / OriginalRoot is treated as read-only throughout each method; the currentRoot variable is what evolves. No aliasing issue.
• DisposableFieldPropertyAnalyzer.cs — each syntaxNodes is consumed in exactly one foreach. Removing .ToArray() is safe even though GetSyntax() is slightly expensive; re-execution is never triggered.
• SingleTUnitAttributeAnalyzer.cs — GroupBy internalises the source immediately, so the upstream singleAttributes being lazy is inconsequential.
• SyntaxFactory.List(convertedStatements) — accepts IEnumerable<T> and enumerates once internally.

One Subtle Note (not a blocker)

In CodeGenerationHelpers.cs, the elements select lambda has side effects:

var elements = typedConstant.Values.Select((v, i) =>
{
    syntaxIndex++;   // side effect
    elementIndex++;  // side effect
    return TypedConstantParser.GetRawTypedConstantValue(v, paramType);
}); // was .ToList(), now lazy
argStrings.AddRange(elements);

This is safe today because List.AddRange enumerates exactly once. However, the original .ToList() here was doing double duty — eliminating allocation and signalling "evaluate now because we have stateful side effects". The lazy version is slightly more fragile if someone later adds a second use of elements. A comment, or keeping this single .ToList(), would make the intent clearer. That said, it is not a correctness issue in the current code.

Summary

All changes are beneficial and correct. The regression identified in the first review was fixed. This is a clean, minimal cleanup that delivers real (if small) allocation savings across the analyzer and code-fixer hot paths.