[thomhurst]

TestDataAnalyzer looped over Func<T> and TestDataRow<T> wrappers until none were left. That reduced Func<Func<int>> and TestDataRow<TestDataRow<int>> to int, so TUnit0001 accepted them. At run time the test then received a Func<int> or a TestDataRow<int> and the cast failed.

Change

TUnit0001 now unwraps only the layers the runtime row reader removes. This matches GetMethodDataSourceRowType in the source generator (#7143):

1. one optional outer Func<T> (DataSourceHelpers.ToObjectArray)
2. one optional TestDataRow<T> (TestDataRowUnwrapper.UnwrapArray)
3. one optional Func<T> inside the row

Func<T>, TestDataRow<T>, Func<TestDataRow<T>>, TestDataRow<Func<T>> and Func<TestDataRow<Func<T>>> still reduce to T. Deeper nesting is now checked as the wrapper value the test actually receives.

Tests

New tests in MethodDataSourceAnalyzerTests:

• Func<Func<int>> for an int parameter reports TUnit0001.
• TestDataRow<TestDataRow<int>> for an int parameter reports TUnit0001.
• Func<Func<int>> for a Func<int> parameter reports nothing.
• Func<TestDataRow<Func<int>>> for an int parameter reports nothing.

dotnet test in tests/TUnit.Analyzers.Tests: 2754 total, 0 failed, 2751 succeeded.

This change is analyzer-only, so source-generated and reflection metadata collection are unaffected.

Closes #7227

Summary by CodeRabbit

• Bug Fixes
  • Improved validation of method data sources with nested Func<T> and TestDataRow<T> wrappers. The analyzer now unwraps only the supported outer layers, so remaining nested types are included when checking whether data source values match test parameters.
  • Added coverage for matching and mismatching parameter types across nested wrapper combinations.

[thomhurst]

@codex review

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 70e7a466-98d2-4aa0-8886-10cceef9eeda

Commits

Reviewing files that changed from the base of the PR and between 7e579da and 3cda39a.

Files selected for processing: 2

• src/TUnit.Analyzers/TestDataAnalyzer.cs
• tests/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.

Walkthrough

The analyzer now unwraps one outer Func<T>, one TestDataRow<T>, and one Func<T> inside the row. New tests cover nested wrappers and parameter type matches and mismatches.

Changes: Data-source type unwrapping

Layer / File(s)	Summary
Bound wrapper unwrapping and validate type matching src/TUnit.Analyzers/TestDataAnalyzer.cs, tests/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs	UnwrapTypes now removes at most one outer Func<T>, one TestDataRow<T>, and one inner Func<T>. Tests verify matching and mismatching parameter types for nested wrappers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 3cda3

The bounded wrapper handling and its new tests present no identified issue that needs resolution before merge.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the analyzer (TUnit0001) and the specific change: it unwraps only the Func/TestDataRow layers that the runtime removes. The raw summary says UnwrapTypes now removes at most one outer F…
Linked Issues check		[#7227] Requirement: TUnit0001 must unwrap only the layers the runtime removes: one optional outer Func, one optional TestDataRow, then one optional Func inside the row. Deeper nesting must b…
Out of Scope Changes check		Changed files at head 3cda39a, relative to merge base 7e579da: src/TUnit.Analyzers/TestDataAnalyzer.cs and tests/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs. - UnwrapTypes: the loop is rep…

Full details: Title check

Explanation

The title names the analyzer (TUnit0001) and the specific change: it unwraps only the Func/TestDataRow layers that the runtime removes. The raw summary says UnwrapTypes now removes at most one outer Func<T>, one TestDataRow<T>, and one inner Func<T>, and the objectives say this aligns with the runtime row reader. The title is specific and matches the main change. It uses the conventional fix(analyzers) prefix and is one sentence, though at 86 characters it is slightly long.

Full details: Linked Issues check

Explanation

[#7227] Requirement: TUnit0001 must unwrap only the layers the runtime removes: one optional outer Func<T>, one optional TestDataRow<T>, then one optional Func<T> inside the row. Deeper nesting must be reported.

Evidence at head 3cda39a:

• TestDataAnalyzer.cs UnwrapTypes replaces the former do/while loop with three single steps: TryUnwrapFunc (outer), a TestDataRow<T> check, then TryUnwrapFunc on the row value. Each layer is removed at most once.
• src/TUnit.Core/Helpers/DataSourceHelpers.cs ToObjectArray calls InvokeIfFunc once on the item. This matches the outer Func step. I did not read TestDataRowUnwrapper.UnwrapArray; its behavior comes from the issue text and PR description.
• Repro from the issue, IEnumerable<Func<Func<int>>> for an int parameter: the analyzer now stops at Func<int>, which differs from int, so it reports TUnit0001.
• Func<TestDataRow<Func<int>>> and TestDataRow<Func<int>> still reduce to int, as the runtime requires.
• New tests in MethodDataSourceAnalyzerTests.cs cover the mismatch cases (Func<Func<int>> and TestDataRow<TestDataRow<int>> for int), the matching Func<int> parameter case, and the valid Func<TestDataRow<Func<int>>> case.

Test results (2754 total, 0 failed) are author-reported. I did not run the test suite.

Full details: Out of Scope Changes check

Explanation

Changed files at head 3cda39a, relative to merge base 7e579da: src/TUnit.Analyzers/TestDataAnalyzer.cs and tests/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs.

• UnwrapTypes: the loop is replaced by bounded unwrapping steps. This directly implements [#7227]. The new private static TryUnwrapFunc helper contains the same display-string check as the original code, moved into one place. It supports the same objective.
• Tests: the four added tests cover the issue scenario and the valid boundary cases. They support the objective.

No unrelated behavior change is visible in the reviewed hunks. The PR is analyzer-only, and the diff does not touch runtime or source generator code.

The display-string match for System.Func is pre-existing behavior. The PR references follow-up issue #7380 for it. It is not an out-of-scope change in this PR.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each wrapper in a row,
One Func&lt;T&gt; is all it lets go.
A row unwraps once, then one more,
Nested types stay at the door.
Tests confirm each type in view.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-02T06:27:20.848712Z	3cda39a	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review: TUnit0001 unwrap depth

LGTM. Replacing the unbounded do/while with a fixed three-step peel (outer Func<T>, one TestDataRow<T>, inner Func<T>) matches the runtime row reader. That fixes the false negative for Func<Func<int>> and TestDataRow<TestDataRow<int>>.

• The early-return check before the peel still lets a Func<int> parameter match a Func<Func<int>> source. The new no-error test covers this.
• Extracting TryUnwrapFunc removes the duplicated Func detection.
• The four new tests cover both the mismatch cases and the full three-layer case.
• This is analyzer-only, so the dual-mode metadata constraint doesn't apply.

Optional, non-blocking: TryUnwrapFunc still matches by ToDisplayString().StartsWith("System.Func<"). That allocates a string on every call. It is also looser than the OriginalDefinition symbol comparison used for TestDataRow, because the prefix match would also accept other System.Func<...> types. Resolving System.Func1once viaGetTypeByMetadataName` and comparing symbols would be cheaper and stricter. It's pre-existing behaviour, so a follow-up is fine.

I did not run the tests myself; the PR reports 2754 analyzer tests passing.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: b5d08a1e29

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

TUnit0001 now stops unwrapping where the runtime stops.

• TUnit0001 checks the wrapper value that reaches the test.

No actionable issues found. Tests were not run during this review.

_{Reviews (2) · Last reviewed commit: "fix(analyzers): TUnit0001 unwraps only t..." · Reviewed by Greptile}

[github-actions]

Review: TUnit0001 unwrap depth

LGTM. Replacing the unbounded do/while with a fixed peel (one outer Func<T>, one TestDataRow<T>, one inner Func<T>) matches the runtime row reader. That closes the false negatives for Func<Func<int>> and TestDataRow<TestDataRow<int>>.

• Extracting TryUnwrapFunc removes the duplicated Func detection.
• The four new tests cover the full three-layer case, both mismatch cases, and the Func<int> parameter case that must stay valid.
• The change is analyzer-only, so the source-generator and reflection paths are unaffected.

Optional: TryUnwrapFunc matches on ToDisplayString().StartsWith("System.Func<"), which allocates a string per call. Comparing OriginalDefinition to GetTypeByMetadataName("System.Func1")` would be cheaper and more precise. The old code did the same, so this isn't a blocker.

No previous review findings are outstanding.

[thomhurst]

Disposition of the optional finding from the Claude reviews (display-string System.Func< match in TryUnwrapFunc): this pattern predates the PR and is also used at two other call sites in TestDataAnalyzer. Follow-up issue: #7380. It does not block this PR.

Head 3cda39a is a clean rebase of b5d08a1. git range-diff shows the patch is identical. Requesting a Codex review for the current head below.

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 3cda39aa75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Review

The change is correct and well scoped. Replacing the unbounded unwrap loop with a fixed Func → TestDataRow → Func sequence matches the runtime row reader (ToObjectArray and TestDataRowUnwrapper). That means Func<Func<int>> and TestDataRow<TestDataRow<int>> are now flagged instead of failing at run time.

• Extracting TryUnwrapFunc removes the duplicated Func check and keeps the behaviour of the original code.
• Tests cover the three-layer success case, both mismatch cases, and the Func<int> parameter that should be accepted.
• The analyzer is shared by both metadata-collection modes, so no duplicate implementation is needed.

Two optional suggestions:

1. TryUnwrapFunc matches on ToDisplayString().StartsWith("System.Func<"). This allocates, and it would also match nested types with a similar name. Comparing OriginalDefinition against GetTypeByMetadataName("System.Func1")` is cheaper and stricter. This was already the case before the PR, so I wouldn't block on it.
2. A test with a tuple inside the wrappers, such as Func<TestDataRow<(int,int)>>, would confirm the tuple path still works after the reordering.

LGTM.

[thomhurst]

Dispositions for the latest Claude review (comment 6570666000) on head 3cda39a:

1. Display-string System.Func< match: this behaviour predates the PR and is tracked in TestDataAnalyzer: detect Func<T> by symbol instead of display-string prefix #7380, as noted above. It does not block this PR.
2. Tuple-inside-wrappers test: this is optional coverage. The tuple expansion in UnwrapTypes still runs after the peel, unchanged from before. I added it to TestDataAnalyzer: detect Func<T> by symbol instead of display-string prefix #7380 so the coverage lands with the TryUnwrapFunc rework rather than restarting review on this head. It does not block this PR.

Both Claude and Codex (receipt for 3cda39aa75) are positive on the current head. Arming auto-merge.