fix(source-gen): jagged array data fails to compile (#6150)#6152
Conversation
Jagged-array test data such as `IEnumerable<Func<byte[][]>>` feeding a
`byte[][]` parameter produced uncompilable generated code (CS1586 + CS0178).
`CollectsTrailingArguments()` treats every array-typed parameter as a
trailing/collecting array, so `TupleArgumentHelper` built the empty-array
fallback as `new {elementType}[0]`. For a `byte[][]` parameter the element
type is `byte[]`, yielding `new byte[][0]` - the `[0]` rank specifier cannot
follow an array element type. The runtime arm compiled fine; the break was in
the unreachable case 0 / default arms that still must compile.
Emit `global::System.Array.Empty<T>()` instead, which is valid for both scalar
and array element types and is allocation-free.
Adds a compile/runtime guard (TUnit.TestProject/Bugs/6150) passing in both
source-gen and reflection modes, plus a source-gen snapshot test. Regenerates
the affected snapshots (new X[0] -> Array.Empty<X>()).
There was a problem hiding this comment.
Code Review: fix(source-gen): jagged array data fails to compile (#6150)
This is a clean, minimal, and well-reasoned fix. Here's my analysis:
Fix Quality
The 2-line change in TupleArgumentHelper.cs is correct and strictly better than the code it replaces:
Array.Empty<T>()vsnew T[0]: Thenew T[0]idiom only produces valid C# whenTis a non-array type. WhenTis itself an array (e.g.,byte[]for abyte[][]parameter), the emittednew byte[][0]parses asnew byte[](no size → CS1586) then[0](invalid rank → CS0178).Array.Empty<T>()is valid for anyT, including array element types.- The change applies consistently to both the static-count (
remainingArgCount == 0) and dynamic-count (ternary fallback) code paths — no partial fix. - Bonus:
Array.Empty<T>()returns a cached singleton, so it also eliminates unnecessary allocations in both paths across the board.
Test Coverage
Coverage is thorough:
- Runtime test (
TUnit.TestProject/Bugs/6150/Tests.cs): reproduces the exact failing pattern —[MethodDataSource]returningIEnumerable<Func<byte[][]>>feeding abyte[][]parameter, verified in both source-gen and reflection modes. - Snapshot test (
Tests6150.cs+Tests6150.Test.verified.txt): locks in the correctedArray.Empty<byte[]>()emission and shows thecase 1handling correctly passes the wholebyte[][]through via thearridentity cast. - Mechanical snapshot updates: ~11 existing
.verified.txtfiles updated uniformly. These are correct and expected.
Minor Observation: Snapshot test has empty assertion lambda
// Tests6150.cs
async generatedFiles =>
{
// empty body
});The snapshot comparison via Verify.NET handles the regression guard, so this isn't wrong — but other bug-fix snapshot tests in the project (e.g., Tests2112.cs) sometimes add an explicit Assert.That(generatedFiles).HasCount().EqualTo(...) inside the lambda as an extra signal of intent. Adding something like:
async generatedFiles =>
{
await Assert.That(generatedFiles).HasCount().EqualTo(1);
});...would make the test slightly more self-documenting. Not a blocker — the snapshot file itself is the authoritative guard.
Design Note (pre-existing, not introduced here)
CollectsTrailingArguments() returns true for any array-typed parameter, which is why byte[][] gets the collecting-parameter treatment and why the case 0 / default arms exist at all. This is pre-existing behavior from issue #6120 and is out of scope here. The fix is correct within that design — patching the codegen rather than the detection logic is the right scoped approach.
Summary
The root cause is correctly identified, the fix is minimal and universally safer than the code it replaces, and regression coverage is solid. LGTM.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
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.
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.48.6 to 1.49.0. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.49.0 <!-- Release notes generated using configuration in .github/release.yml at v1.49.0 --> ## What's Changed ### Other Changes * docs: benchmark page descriptions + promote Benchmarks in sidebar by @thomhurst in thomhurst/TUnit#6143 * feat(mocks): discriminate generic-method mocks by type argument by @thomhurst in thomhurst/TUnit#6153 * fix(source-gen): jagged array data fails to compile (#6150) by @thomhurst in thomhurst/TUnit#6152 * fix: dispose shared fixtures when only a subset of consuming tests runs by @thomhurst in thomhurst/TUnit#6156 ### Dependencies * chore(deps): update tunit to 1.48.6 by @thomhurst in thomhurst/TUnit#6142 * chore(deps): update react to ^19.2.7 by @thomhurst in thomhurst/TUnit#6144 * chore(deps): update aspire to 13.4.0 by @thomhurst in thomhurst/TUnit#6145 * chore(deps): update dependency nunit.analyzers to 4.14.0 by @thomhurst in thomhurst/TUnit#6146 * chore(deps): update dependency polyfill to 10.7.2 by @thomhurst in thomhurst/TUnit#6148 * chore(deps): update dependency polyfill to 10.7.2 by @thomhurst in thomhurst/TUnit#6149 * chore(deps): update dependency dompurify to v3.4.8 by @thomhurst in thomhurst/TUnit#6155 **Full Changelog**: thomhurst/TUnit@v1.48.6...v1.49.0 Commits viewable in [compare view](thomhurst/TUnit@v1.48.6...v1.49.0). </details> Updated [TUnit.AspNetCore](https://github.com/thomhurst/TUnit) from 1.48.6 to 1.49.0. <details> <summary>Release notes</summary> _Sourced from [TUnit.AspNetCore's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.49.0 <!-- Release notes generated using configuration in .github/release.yml at v1.49.0 --> ## What's Changed ### Other Changes * docs: benchmark page descriptions + promote Benchmarks in sidebar by @thomhurst in thomhurst/TUnit#6143 * feat(mocks): discriminate generic-method mocks by type argument by @thomhurst in thomhurst/TUnit#6153 * fix(source-gen): jagged array data fails to compile (#6150) by @thomhurst in thomhurst/TUnit#6152 * fix: dispose shared fixtures when only a subset of consuming tests runs by @thomhurst in thomhurst/TUnit#6156 ### Dependencies * chore(deps): update tunit to 1.48.6 by @thomhurst in thomhurst/TUnit#6142 * chore(deps): update react to ^19.2.7 by @thomhurst in thomhurst/TUnit#6144 * chore(deps): update aspire to 13.4.0 by @thomhurst in thomhurst/TUnit#6145 * chore(deps): update dependency nunit.analyzers to 4.14.0 by @thomhurst in thomhurst/TUnit#6146 * chore(deps): update dependency polyfill to 10.7.2 by @thomhurst in thomhurst/TUnit#6148 * chore(deps): update dependency polyfill to 10.7.2 by @thomhurst in thomhurst/TUnit#6149 * chore(deps): update dependency dompurify to v3.4.8 by @thomhurst in thomhurst/TUnit#6155 **Full Changelog**: thomhurst/TUnit@v1.48.6...v1.49.0 Commits viewable in [compare view](thomhurst/TUnit@v1.48.6...v1.49.0). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Fixes #6150.
Problem
A
MethodDataSourcereturningIEnumerable<Func<byte[][]>>(jagged-array data) feeding abyte[][]parameter made the source-generated code fail to compile:Switching the parameter to
List<byte[]>compiled fine, confirming the break is specific to jagged array parameters.Root cause
IParameterSymbol.CollectsTrailingArguments()returnstruefor any array-typed parameter, so abyte[][]test parameter is treated as a trailing/collecting array. The collecting path inTupleArgumentHelper.GenerateArgumentAccessWithParamsbuilt the empty-array fallback as:For a
byte[][]parameter the element type isbyte[], so this emitsnew global::System.Byte[][0]— C# parsesnew byte[](no size → CS1586) followed by[0](invalid rank specifier → CS0178). Thenew T[0]idiom only works whenTis not itself an array.The actual runtime arm (
case 1) already emitted valid code, which is why this surfaced as a compile error rather than a runtime one — the unreachablecase 0/defaultarms still have to compile.Fix
Replace both
new T[0]sites withglobal::System.Array.Empty<T>(), which is valid for scalar and array element types and is allocation-free. (TupleArgumentHelper.cs, 2 sites.)Tests
TUnit.TestProject/Bugs/6150/Tests.csreproduces the issue; it fails to build before the fix and passes after. Verified passing in both source-gen and reflection (--reflection) modes (3/3 each).TUnit.Core.SourceGenerator.Tests/Bugs/6150/Tests6150.cslocks in the correctedArray.Empty<global::System.Byte[]>()output..verified.txtsnapshots (mechanicalnew X[0]→Array.Empty<X>()), including the per-TFMConflictingNamespaceTestsvariants (net472/8/9/10). Full snapshot suite green across all four TFMs.