perf: shrink generated TestEntry builder IL via shared TUnit.Core factory helpers

[thomhurst]

Closes #6227

What

Two emit-shape changes in TestMetadataGenerator move repeated construction boilerplate out of generated code and into shared TUnit.Core helpers, so the code is JIT-compiled once instead of once per call site. Discovery-time tier-0 JIT cost is roughly linear in generated IL size, so this directly shrinks the per-class Entries builder cost.

1. DataSourceFactories (TUnit.Core/Helpers/DataSourceFactories.cs)

Every [MethodDataSource] call site previously inlined an async IAsyncEnumerable local function — a full compiler-generated state machine class per data source, differing only in one invocation expression. Generated code now emits a single expression:

Factory = static dataGeneratorMetadata => DataSourceFactories.FromEnumerable(static () => Cls.DataMethod()),

Helper selection mirrors the old compile-time branch order exactly (IAsyncEnumerable / Task<T> / ValueTask<T> / IEnumerable / single value, × static/instance). The single-value variant intentionally keeps no runtime IEnumerable check, and the instance variants keep the exact TestClassInstance error message. Helpers are async iterators, so the data-method invocation stays deferred to first enumeration — same laziness and exception surface as before.

2. TestEntryFactory.Create<T> (TUnit.Core/TestEntryFactory.cs)

TestEntry<T> construction is now one factory call with named arguments instead of a ~17-property object initializer (one call token vs newobj + a member token per property; T : class means one shared canonical JIT for the whole assembly). Default-valued arguments — empty arrays, zero column spans, repeatCount: 0, hasDataSource: false — are omitted entirely.

Net effect on this repo's own snapshots: -11,295 / +6,960 lines of generated code.

Constraints honored

• AOT-safe: plain static helpers, no reflection; [DynamicallyAccessedMembers] mirrored from TestEntry<T> onto Create<T>; user data members stay directly referenced in generated lambdas so trimming roots are unchanged.
• Emit-string-only generator change — no incremental pipeline shape changes.
• Reflection mode untouched (Factory is generator-only).

Test updates

• All snapshot .verified.txt files re-verified across net472/net8/net9/net10.
• TestsBase/Bugs2971NullableTypeTest scrub lists extended with the named-argument source-location markers (startColumnNumber: etc.) — these lines are also omitted when zero, keeping snapshots stable across Roslyn versions.
• BasicTests location assertions updated to the named-argument form; column assertions made conditional since spans are environment-sensitive and omitted when zero.
• PublicAPI snapshots updated for the two new public types.

Verification

• Snapshot tests: green on all 4 TFMs (119 passed each)
• TUnit.TestProject full [EngineTest=Pass] suite, source-gen mode: 5477 passed / 0 failed
• Reflection mode (run-reflection-tests.ps1 -Framework net10.0): passed
• TUnit.UnitTests: 219 passed; TUnit.Engine.Tests: green incl. FSharp/VB
• All 3 Roslyn generator variants build clean

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 56 complexity

Metric	Results
Complexity	56

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: perf: shrink generated TestEntry builder IL via shared TUnit.Core factory helpers

Overall: This is a well-designed, well-executed performance optimization. Approved with minor notes.

The design is sound — two complementary changes that reduce generated IL substantially (-11k/+7k lines) by:

1. Moving per-call-site async iterator state machines into shared DataSourceFactories helpers
2. Replacing 17-property object initializers with a single TestEntryFactory.Create<T> factory call using named arguments

Both strategies are coherent and the AOT/trim constraints are respected throughout.

---

Issue 1: #pragma warning disable CS1998 at file scope (minor)

In DataSourceFactories.cs, the CS1998 suppression covers the entire file:

// top of file
#pragma warning disable CS1998

Only the four non-awaiting methods (FromValue, FromEnumerable, FromInstanceValue, FromInstanceEnumerable) need this. If a future method is added to this file with a genuine missing await, the warning would be silently suppressed. Prefer per-method suppressions:

#pragma warning disable CS1998 // intentionally sync-only async iterator
public static async IAsyncEnumerable<...> FromValue(...)
#pragma warning restore CS1998

Why it matters: This file is the canonical home for data source iteration logic. Suppressing the whole file risks hiding real async mistakes as the class grows.

---

Issue 2: ValueTask detection via string prefix inside IsTask branch (minor design smell)

IsTask() already returns true for both Task<T> and ValueTask<T> (by checking string prefixes). Inside its branch, a second StartsWith check distinguishes them:

else if (IsTask(returnType))
{
    kind = returnType.ToDisplayString().StartsWith("System.Threading.Tasks.ValueTask")
        ? "ValueTask"
        : "Task";
}

This is consistent with the existing IsTask implementation, but the pattern exposes that IsTask is doing double duty. A cleaner approach:

// Split the check
else if (IsValueTask(returnType))
{
    kind = "ValueTask";
}
else if (IsTask(returnType))
{
    kind = "Task";
}

where IsValueTask does the StartsWith("System.Threading.Tasks.ValueTask") check and IsTask is updated to only match Task<T>. This eliminates the double-check without changing behavior, and makes the dispatch table read like a flat decision tree rather than a nested override.

Not a blocker — the current code is correct — but worth a note since EmitDataSourceFactoryInvocation is now the only caller that needs to distinguish them, and refactoring IsTask fixes it permanently.

---

Things done well

• Factory argument order in TestEntryFactory.Create is excellent: required high-frequency args first, optional defaults last. The default array parameters using null (instead of Array.Empty<T>()) is the right choice — it avoids allocations at the generated call sites while the factory consolidates the null-coalescing in one place.
• #if !DEBUG [EditorBrowsable(EditorBrowsableState.Never)] #endif pattern on both new public types keeps IntelliSense clean without hiding helpers during debugging.
• GetRequiredInstance extraction in DataSourceFactories avoids repeating the null-check + error message across all instance variants — clean.
• Test coverage is thorough: all 4 TFMs re-verified, snapshot scrub lists extended for the new named-argument forms, and BasicTests column assertions correctly made conditional since column spans are environment-sensitive.
• EmitDataSourceFactoryInvocation correctly handles the block-body form for argument-bearing data sources (where arguments must be bound before the invocation), and the single-expression form for the common no-argument case — the generated code is readable in both paths.

---

The two issues above are improvements worth making but neither is a blocker. The performance motivation is solid and the implementation is correct across all documented test paths.

[claude]

Follow-up review: issues from prior review already addressed

Looking at the actual patch in this PR, both concerns raised in the previous review comment are already handled in the submitted code:

Issue 1 (CS1998 suppression scope): The patch uses targeted per-block suppressions rather than file-scope. The #pragma warning disable CS1998 / #pragma warning restore CS1998 pairs wrap only the four sync-only async iterator methods (FromValue, FromEnumerable, FromInstanceValue, FromInstanceEnumerable). The concern about silently masking future async mistakes does not apply here.

Issue 2 (IsTask/IsValueTask dispatch): The patch already introduces a dedicated IsValueTask method and restricts IsTask to only System.Threading.Tasks.Task prefix. The generator's type dispatch is now a clean flat decision tree: IsAsyncEnumerable → IsValueTask → IsTask → IsEnumerable → single value. The double-check concern is fully resolved.

---

No blocking issues. The two new public types (DataSourceFactories, TestEntryFactory) are well-designed: they are static, AOT-safe, correctly annotated with [DynamicallyAccessedMembers], and hidden from IntelliSense in non-debug builds. The IL reduction (~-11k / +7k generated lines) is a genuine win. Approving.

[claude]

The implementation is correct and the two concerns from the previous review comment are already addressed in this PR. Approving.