fix: invoke inner Func for TestDataRow<Func<T>> data sources (#6161)

[thomhurst]

Problem

Fixes #6161.

A [MethodDataSource] returning IEnumerable<Func<T>> (or IEnumerable<Func<(int,int,int)>>) already invokes the Func and spreads the tuple result across the test method parameters. The same source wrapped as IEnumerable<TestDataRow<Func<T>>> did not — the inner Func was passed through as a single argument. Consequences:

• The test could only be consumed as TestMethod(Func<...> f), not TestMethod(int a, int b, int c).
• Parameterized DisplayName placeholders ($arg1, $First, …) couldn't bind to the real values.
• Because every row's display name collapsed to the same string, the runner treated the rows as duplicates ("runs the first one as many times as the DisplayName is the same").

The behaviour was already implied by the docs (TestDataRow<Func<HttpClient>> "to ensure fresh instances"), so this aligns the code with the documented intent.

Changes

• TestDataRowUnwrapper.UnwrapArray — invokes the inner Func after unwrapping the TestDataRow, so TestDataRow<Func<T>> behaves identically to a bare Func<T> source (invoke, then spread tuples). This is the single convergence point shared by both source-generated and reflection modes, so there is no codegen change and no snapshot churn.
• TestDataAnalyzer — strips Func<> and TestDataRow<> wrapper layers in a loop, so TestDataRow<Func<T>> (and Func<TestDataRow<T>>) reduce to the inner T before the tuple check. Prevents a false parameter-count diagnostic that would otherwise block the new pattern at compile time.
• DisplayNameSubstitutor — word-boundary-aware placeholder replacement, resolving positional $argN before $paramName. Fixes a pre-existing prefix-collision bug where a parameter named a was matched inside $arg1 (rendering 1rg1 instead of 1). This is the bug already documented by CustomDisplayNameTests.TestParameterNamePrefixBug.

Tests

• TUnit.TestProject/Bugs/6161/Tests.cs — TestDataRow<Func<(int,int,int)>> → (int a, int b, int c) with "$arg1 + $arg2 = $arg3", plus TestDataRow<Func<record>> → single-param. Passes in source-gen and reflection modes; display names render 1 + 1 = 2, 1 + 2 = 3, 2 + 3 = 5. An [After(Class)] hook asserts all three distinct rows ran (dedup regression).
• MethodDataSourceAnalyzerTests.Method_Data_Source_With_TestDataRow_Func_Tuple_No_Error — no-diagnostic coverage for the inner-Func nesting order.

Verification

• Source-generator snapshot suite: 118 pass, 0 received-file churn.
• Public API snapshot: no surface change.
• Analyzer tests: 47/47. CustomDisplayNameTests: 12/12 (incl. the now-fixed prefix bug).

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

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]

Code Review

This is a well-targeted fix for issue #6161. The three changes are cohesive and the root-cause reasoning is sound.

TestDataRowUnwrapper.cs — Core fix

data = DataSourceHelpers.InvokeIfFunc(data);

Placing this right after TryUnwrap is exactly the right spot. The comment correctly identifies why InvokeIfFunc doesn't help for TestDataRow<Func<T>> in DataSourceHelpers.ToObjectArray — the Func is opaque there, only surfacing after the TryUnwrap boundary. Simple and correct.

TestDataAnalyzer.cs — do/while loop

The loop approach is elegant. Because both if blocks can fire in the same iteration, Func<TestDataRow<T>> collapses to T in a single pass rather than requiring a second loop. Safe against infinite iteration since C# generics cannot form cycles — each iteration necessarily moves inward.

One observation: isFunc is set to true on any Func<> peel, including a second peel in a subsequent iteration (e.g., Func<Func<T>>). This is consistent with the existing semantics of isFunc — it signals that tuple-spread applies — which is correct for any Func in the chain. Not a bug, just worth being aware of.

DisplayNameSubstitutor.cs — ReplacePlaceholder + ordering fix

Ordering change (positional before named): Correct. Without this, $a (named param) matches the $a prefix inside $arg1, corrupting the output before the positional pass ever runs.

ReplacePlaceholder boundary check: The word-boundary logic is correct. I traced through the $arg1 / $arg10 interaction manually — $arg1 inside $arg10 is correctly skipped (the 0 after $arg1 is an identifier char), while standalone $arg1 is replaced.

One minor edge case to be aware of: since IsIdentifierChar includes _, a display name template like "$a_description" where a is a parameter name will not substitute $a there (because _ counts as an identifier char following $a). This is actually the correct/expected behavior in C# identifier semantics — a_description would be a different name — but users who write $a_suffix expecting $a to be replaced will be surprised. Not a regression from this PR, and the behavior is defensible; just worth documenting if it surfaces as a user complaint.

Potential substitution-order collision: If a user names a parameter arg1, the positional pass resolves $arg1 first (as the first argument's value), and the named pass for parameters[0] (named arg1) then searches for $arg1 in the already-substituted string. In practice this won't matter because the positional value is unlikely to re-introduce $arg1, but it's a latent edge case in any linear-substitution scheme.

Test coverage

The [After(Class)] deduplication guard in Tests.cs is a particularly good regression anchor — it directly tests the symptom that motivated the bug report (multiple rows collapsing to a single execution). The combination of tuple and reference-type test cases (TupleFuncIsUnwrapped / ReferenceFuncIsUnwrapped) is thorough.

Documentation

The doc addition accurately describes the behavior and the code example matches the regression test, which is a nice consistency detail.

---

Overall: The fix is correct, well-tested, and the implementation choices are sound. No blocking concerns — the edge cases noted above are pre-existing design tradeoffs rather than regressions. Good work.