[thomhurst]

Extract the analyzer's closed-signature typed-row binding calculation into TypedRowParameterBinding, next to the linked inference helper. Keep the existing conversion, optional omission and declared-position token rules unchanged.

Add compiled parity coverage that checks expected validation types, inference success versus null in both discovery modes, and concrete results from generated argument expressions and the reflection invoker. Cover omitted optional arguments and trailing arrays, packed and nested arrays, element conversions, loose trailing values, injected versus inferred CancellationTokens, and all five supported params collection interfaces. Compare collection support with both invocation implementations, including unsupported concrete collections and spans.

Closes #7465

Validation (SDK 10.0.403, net10.0, macOS arm64):

• 30 TypedRowParameterBindingTests passed.
• 17 TypedRowInferenceParityTests passed.
• 41 ArgumentsAnalyzerTests passed, including invalid-element and generic-constraint diagnostics.
• git diff --check passed.

No generator output, shipped public API, reflection path or discovery behavior changes. Existing build warnings are tracked in #7410 and #7412.

Summary by CodeRabbit

• Bug Fixes
  • Improved type validation for typed data rows with optional parameters, default values, and trailing params or array arguments.
  • Corrected handling of supported collection types and cancellation-token parameters when determining expected argument types.
  • Added coverage for typed-row behavior across generated and reflection-based invocation, including tuple rows.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The analyzer now uses TypedRowParameterBinding.GetExpectedTypes to calculate effective parameter types for inferred generic typed rows. New tests compare those types with generated and reflection-based invocation across optional parameters, arrays, params collections, tuples, and cancellation tokens.

Changes: Typed-row binding

Layer / File(s)	Summary
Derive effective parameter types src/TUnit.Analyzers/Extensions/TypedRowParameterBinding.cs	Adds a helper that excludes declared cancellation-token parameters, drops eligible omitted trailing parameters, and handles trailing params or SZ-array arguments as elements when they do not bind as a whole.
Integrate and verify typed-row binding src/TUnit.Analyzers/TestDataAnalyzer.cs, tests/TUnit.Core.SourceGenerator.Tests/TypedRowParameterBindingTests.cs	CheckDataGenerator uses the helper instead of inline parameter-type construction. Tests compare inferred types and expected row argument types with generated and reflection-based invocation, and check supported params element types.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related issues

• Typed tuple rows with multiple trailing values skip argument type validation #7463 — Tests cover loose trailing values and expected element types for expanded params rows.
• test: parity test for typed-row type inference across source-gen and reflection #7386 — Tests compare typed-row inference behavior across generated and reflection modes.
• Generic method inference from [Arguments<T>] rows: match array element types and add a generator/reflection parity test #7399 — Tests cover generic typed-row array-element inference and discovery-mode parity.

Possibly related PRs

• thomhurst/TUnit#7460 — Introduced the typed-row inference and inline closed-signature validation that this PR centralizes in a helper.
• thomhurst/TUnit#7343 — Introduced typed argument-row inference and analyzer validation for optional and params parameters.

Merge Risk: Minimal · up to 3c52f

No merge-blocking issue was identified in the typed-row binding change. It is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 12.50% 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. Documenta…	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 "Extract typed-row binding and verify invocation parity" matches the main change. The PR moves the analyzer's typed-row parameter binding calculation into a new TypedRowParameterBinding help…
Linked Issues check		Issue #7465 requires a pure helper for the closed parameter-type calculation, regression coverage, and preserved diagnostics and execution behavior. Helper: The new file src/TUnit.Analyzers/Extension…
Out of Scope Changes check		The whole-PR diff against the merge base b5561fb changes three files: TestDataAnalyzer.cs (extraction and call-site replacement), the new TypedRowParameterBinding.cs (extracted helper), and the new T…

Full details: Title check

Explanation

The title "Extract typed-row binding and verify invocation parity" matches the main change. The PR moves the analyzer's typed-row parameter binding calculation into a new TypedRowParameterBinding helper, and adds tests that compare expected validation types with generated and reflection-based invocation results. The title is specific and concise, and it names both the refactor and the added parity coverage.

Full details: Linked Issues check

Explanation

Issue #7465 requires a pure helper for the closed parameter-type calculation, regression coverage, and preserved diagnostics and execution behavior.

Helper: The new file src/TUnit.Analyzers/Extensions/TypedRowParameterBinding.cs adds GetExpectedTypes. Its logic matches the inline code removed from TestDataAnalyzer.CheckDataGenerator. It covers optional and defaulted trailing parameters, trailing params and SZ-array elements, the single-array-as-element rule, and CancellationToken removal by declared signature. It also adds GetParamsElementType for the five supported collection interfaces and SZ arrays. The helper only reads symbols and queries conversions, so it is pure in the sense the issue requires.

Analyzer call site: TestDataAnalyzer.CheckDataGenerator now calls TypedRowParameterBinding.GetExpectedTypes with the same arguments. CheckInferredTypeArgumentConstraints still runs before this call, so constraint diagnostics are unchanged. The diff does not touch invalid-element diagnostics.

Coverage: TypedRowParameterBindingTests covers the requested cases. These are omitted optional and trailing arrays, packed and nested arrays, element conversions, loose trailing values, injected and inferred CancellationTokens, and all five params collection interfaces, gated to NET9_0_OR_GREATER. Generated invocation and the reflection invoker are checked against the expected types. Collection support is compared with both invocation implementations, including unsupported List, HashSet, Span, and ReadOnlySpan inputs.

Limits: I did not run the tests. The reported counts (30, 17, 41 passed) are author-reported. The new tests call private helpers by reflection. This coupling is a maintenance risk, but it does not affect this verdict.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base b5561fb changes three files: TestDataAnalyzer.cs (extraction and call-site replacement), the new TypedRowParameterBinding.cs (extracted helper), and the new TypedRowParameterBindingTests.cs (parity tests). Each change supports issue #7465. The extraction removes inline logic and adds a call to the helper, with no behavior change. The tests cover the objectives. The PR does not change generator output, public API, reflection discovery, or diagnostic messages. No change lacks a connection to the issue. Related issues #7386, #7371, #7410, and #7412 are context only and do not add requirements.

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% 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 8 functions across 3 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 row with care
Optional values vanish there
Arrays keep their shape or spread
Tokens find their proper thread
Tests compare each route ahead
And nibble on the code instead}

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

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: a7c41ba9-7882-4d4d-913b-c890a6bfca35

Commits

Reviewing files that changed from the base of the PR and between b5561fb and 3c52f4e.

Files selected for processing: 3

• src/TUnit.Analyzers/Extensions/TypedRowParameterBinding.cs
• src/TUnit.Analyzers/TestDataAnalyzer.cs
• tests/TUnit.Core.SourceGenerator.Tests/TypedRowParameterBindingTests.cs

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

[thomhurst]

@codex review

[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-03T14:53:26.426366Z	3c52f4e	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

This is a behaviour-preserving extraction. I compared the diff to the code removed from TestDataAnalyzer.CheckDataGenerator: GetExpectedTypes and GetParamsElementType are moved verbatim, with context.Compilation and typeArguments replaced by parameters. The call site still calls method.Construct(inferred) only after CheckInferredTypeArgumentConstraints, so the ordering is unchanged. I didn't build or run the tests.

The new parity coverage is valuable. It compiles real code, emits it, and compares the analyzer's expected types with what the generated argument expressions and the reflection invoker actually do.

Suggestions, none of them blocking:

1. Reflection lookups by string name. The tests find internal members with GetType("...") and GetMethod("..."): GetExpectedTypes, GetParamsElementType, GetTrailingElementType, InferTypesFromTypedRow, CreateReflectionTestInvoker, InferMethodTypeArgumentsFromRowType and GetRowArgumentTypes. A rename will fail at runtime with a null-forgiving NullReferenceException instead of a compile error. If the test project can get InternalsVisibleTo for the analyzer, call the analyzer helpers directly (the test project already references TUnit.Analyzers, since it uses TestDataAnalyzer). For the engine, add one small helper that wraps the lookup and throws a message naming the missing member, so a failure says what was renamed.
2. Helper visibility. GetParamsElementType is private and exists in three copies (analyzer, TupleArgumentHelper.GetTrailingElementType, and the engine's reflection collector). The Collecting_element_types_match_both_invocation_implementations test guards against drift between them, which is good. Longer term, consider whether the analyzer and generator can share one implementation, since they already share the linked inference helper. That would make the parity test unnecessary for those two.
3. Test readability. The Cases() rows carry eight positional strings and a bool. A small record or named-tuple type would make the table easier to review and extend. The #if NET9_0_OR_GREATER branch also means the params-collection cases only run on some target frameworks. Please confirm CI covers one of them.

Overall this looks good to merge once CI is green.

[chatgpt-codex-connector]

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

Reviewed commit: 86118dbdf6

ℹ️ 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; the previous test weakness is fixed and no new actionable issues were found.

Summary

The PR extracts the analyzer’s existing typed-row binding logic into TypedRowParameterBinding and adds compiled checks against generated and reflection calls.

• Typed-row checks now use a dedicated binding helper.

_{Reviews (2) · Last reviewed commit: "Strengthen typed-row token identity asse..." · Reviewed by Greptile}

[thomhurst]

@codex review

Pushed 3c52f4e to address the token identity coverage finding: generated and reflection invocation now distinguish injected, scalar-row and array-element tokens by CancellationToken equality. All 30 TypedRowParameterBindingTests passed on net10.0; git diff --check passed.

Disposition of Claude's nonblocking suggestions:

1. Keeping reflection for this extraction: the tests exercise internal/private helpers across assemblies, including the engine. Broadening visibility or refactoring lookup diagnostics is optional test maintenance and does not affect the asserted binding results.
2. Keeping the three collection-type implementations separate here preserves the existing production behavior. This PR adds parity coverage specifically to detect drift; sharing more production code is outside this behavior-preserving extraction.
3. Keeping the existing MethodDataSource rows avoids a separate test-data refactor. CI coverage is confirmed: RunSourceGeneratorTestsModule uses ModernAndNetFrameworks in TestBaseModule, which includes net10.0 and also net9.0 on Linux. The conditional params-collection cases therefore run in CI; they also passed locally on net10.0.

The draft remains awaiting fresh reviews for the new head.

[github-actions]

Review

The extraction in TypedRowParameterBinding is a faithful move of the analyzer logic. I compared the removed block with the new class line by line, including the declared-position trailing-parameter handling and the GetParamsElementType collection set. I found no behavior change. Having TestDataAnalyzer call one helper next to TypedDataSourceInference is a clear improvement.

The parity tests cover the cases the PR lists, and they compare analyzer, generated-code and reflection-invoker results against each other.

Suggestions, none blocking:

• Reflection into internals. The tests look up GetExpectedTypes, GetParamsElementType, InferTypesFromTypedRow, GetTrailingElementType and CreateReflectionTestInvoker by string name. A rename turns these into runtime NullReferenceExceptions instead of compile errors. InternalsVisibleTo for the test assembly would let the analyzer helper be called directly. The engine types are probably harder to reach, so a shared nameof-based accessor or a helper that fails with a clear message when a lookup is null would at least make failures readable.
• GetParamsElementType is private but tested. The test reaches it with BindingFlags.NonPublic. Making it internal documents that it is part of the parity contract.
• Long test method. Expected_types_agree_with_generated_and_reflection_invocation runs analyzer, source-gen and reflection checks in one body. Splitting it into a shared fixture plus one assertion group per path would make a failure point to the diverging implementation sooner.
• #if NET9_0_OR_GREATER cases. The five params collection interface cases only run on net9.0 and later. Check CI still exercises them on a TFM that satisfies this.

Overall this is good to merge once CI is green.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 3c52f4e3cf

ℹ️ 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".

[thomhurst]

Promoted to ready at unchanged head 3c52f4e after positive Claude and Codex verdicts and resolution of the token-identity finding.

Disposition of the current Claude suggestions: retain the existing reflection-based test access and private helper visibility for this behavior-preserving extraction; these helpers are implementation details, and changing their visibility is unnecessary for parity coverage. Retain the combined parity test so each case compares all implementations together; splitting assertion groups is optional readability work. The net9.0/net10.0 CI coverage for the conditional collection cases is confirmed in the earlier disposition above.

Promotion triggers a fresh Claude review and full CI. Auto-merge must wait for the promotion Claude run to pass and its comment to be read; this single unit does not wait for runs.

[github-actions]

Review

This is a behaviour-preserving extraction: the closed-signature binding logic moves from TestDataAnalyzer.CheckDataGenerator into TypedRowParameterBinding.GetExpectedTypes, and GetParamsElementType moves with it. I compared the moved code line by line with the removed code. The only change is that context.Compilation and typeArguments become the compilation and argumentTypes parameters. The call site passes method.Construct(inferred) as before, so the diagnostics should not change.

The parity tests are a real improvement. They emit the generated argument expressions and run them, then run the reflection invoker, and compare both against the analyzer's expected types. That is the right way to keep the three implementations (analyzer, generator, reflection) in sync.

Suggestions, none blocking:

• The tests reach private and internal members through string-based reflection (GetMethod("GetExpectedTypes"), GetTrailingElementType, InferTypesFromTypedRow and similar). A rename will fail at runtime with a null-reference error, not at compile time. If TUnit.Core.SourceGenerator.Tests can see the analyzer's internals (for example via InternalsVisibleTo), calling TypedRowParameterBinding.GetExpectedTypes directly would be sturdier. You could also give the GetMethod calls a shared helper that asserts non-null with a clear message.
• Expected_types_agree_with_generated_and_reflection_invocation is long and mixes four concerns: inference, analyzer expected types, generated invocation and reflection invocation. Splitting it into helpers, or into separate assertions per mode, would make a failing case easier to diagnose. The case tuples are also positional (eight untyped fields), so a small record or named type would be more readable.
• The NET9_0_OR_GREATER guard around the params collection cases means the five-interface coverage only runs on net9 and later. That seems intended, but please confirm CI actually exercises it.

I did not run the tests locally. Overall this looks good to merge, assuming CI passes.

[thomhurst]

Run 41657938516 completed successfully on head 3c52f4e; the earlier wait has ended. All current CI checks, including the required OS-specific engine and AOT checks, pass. Codex reports a completed review with no findings for this same head, and the promotion Claude review is positive.

The promotion review repeats the already-dispositioned nonblocking suggestions: retain private reflection-based test access and the combined per-case parity assertions for this behavior-preserving extraction. The net9/net10 CI coverage of conditional params-collection tests is confirmed in the previous disposition. CodeRabbit's docstring-coverage warning is nonblocking: the extracted helper already has an XML summary and explanatory comments, and the remaining touched functions are test coverage or existing code. Adding blanket docstrings is unnecessary for this extraction. CodeRabbit and Greptile report no remaining actionable findings; the token-identity thread is fixed and resolved with no later reviewer reply.

Proceeding through the repository merge gate on the unchanged head.