[thomhurst]

Closes #7197

Summary

A generic test method with a T? parameter, where T : struct, made the source generator emit code that did not compile:

[Test]
[Arguments(1, null)]
public async Task Run<T>(T value, T? other) where T : struct { ... }

error CS0246: The type or namespace name 'T' could not be found

Cause

CodeGenerationHelpers.GenerateTypeInfo describes a parameter type that contains a type parameter as a ConstructedGeneric, using typeof(<unbound definition>). It prints that definition with DisplayFormats.FullyQualifiedGenericWithGlobalPrefix, which shortens an unbound Nullable<> to T?. The result, typeof(T?), is emitted outside the method, where T doesn't exist.

Fix

For a Nullable<T> definition, emit typeof(global::System.Nullable<>). I left the shared display format as it is because many other call sites use it.

The generated parameter metadata is now:

new global::TUnit.Core.ConstructedGeneric(typeof(global::System.Nullable<>), [new global::TUnit.Core.GenericParameter(0, true, "T")])

Tests

• tests/TUnit.TestProject/Bugs/_7197/NullableValueTypeGenericMethodTests.cs ([EngineTest(ExpectedResult.Pass)]) is the repro from the issue. Before the fix the test project did not build. After the fix the test passes as Run<int>(1, null) in both source-gen and --reflection modes (net10.0, Release).
• Added the Tests7197 generator snapshot, which I reviewed. All 863 TUnit.Core.SourceGenerator.Tests pass on net10.0, net9.0, net8.0 and net472.

Also fixed (red main)

The net472 AotConverterScanTests fixture fix that was here is now on main via #7330, so after rebasing this PR no longer touches that file.

Summary by CodeRabbit

• Bug Fixes
  • Fixed test generation for generic methods that use nullable value-type parameters. Tests can now correctly pass a value and null to the same method and verify the nullable parameter is empty.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

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: 0bd6a5c3-8c15-4e55-a0d7-41f97774ab31

Commits

Reviewing files that changed from the base of the PR and between a8474e3 and b782124.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

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: e7bc088a-d357-452d-b180-4bcb394432aa

Commits

Reviewing files that changed from the base of the PR and between d43977c and a8474e3.

Files selected for processing: 5

• src/TUnit.Core.SourceGenerator/CodeGenerationHelpers.cs
• tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs
• tests/TUnit.Core.SourceGenerator.Tests/Bugs/7197/Tests7197.cs
• tests/TUnit.Core.SourceGenerator.Tests/Tests7197.Test.verified.txt
• tests/TUnit.TestProject/Bugs/_7197/NullableValueTypeGenericMethodTests.cs

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

Walkthrough

The source generator now emits an open Nullable<> definition for nullable generic types that contain generic parameters. The PR adds regression coverage and changes a test fixture from a record struct to a readonly struct.

Changes: Nullable generic metadata

Layer / File(s)	Summary
Nullable generic metadata and regression coverage src/TUnit.Core.SourceGenerator/CodeGenerationHelpers.cs, tests/TUnit.TestProject/Bugs/_7197/NullableValueTypeGenericMethodTests.cs, tests/TUnit.Core.SourceGenerator.Tests/Bugs/7197/Tests7197.cs, tests/TUnit.Core.SourceGenerator.Tests/Tests7197.Test.verified.txt	The generator emits typeof(global::System.Nullable<>) when a constructed generic type contains generic parameters. The regression test covers Run<int>(1, null) and its generated test registration.

Test fixture compatibility

Layer / File(s)	Summary
Readonly struct fixture tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs	The fixture replaces its positional record struct with a readonly struct and explicit constructor.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• Red main: AotConverterScanTests.SkipsConversionsBlockedByObsoleteError fails on net472 (IsExternalInit) #7329 — The fixture change removes the record struct that required IsExternalInit, matching the reported .NET Framework test build failure.

Possibly related PRs

• thomhurst/TUnit#7319 — Both changes address the IsExternalInit requirement in the same test fixture; this PR replaces the record struct, while #7319 adds polyfills.
• thomhurst/TUnit#7317 — Both changes modify the same obsolete-conversion test fixture; this PR changes the struct form used by that regression test.

Suggested labels: PATCH

Merge Risk: Minimal · previous review a8474

The nullable generic metadata fix and net472-compatible fixture change present no identified merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The change to tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs has no connection to #7197. It replaces the record struct Token in the SkipsConversionsBlockedByObsoleteError fixture with…	Remove the AotConverterScanTests.cs change from this PR and submit it in a separate pull request that references the failing test (AotConverterScanTests.SkipsConversionsBlockedByObsoleteError) and the red modularpipeline (windows-latest) ru…

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main source generator fix for generic test methods with a T? parameter.
Linked Issues check		Objective [#7197]: generated code for a generic test method with a T? (Nullable) parameter, where T : struct, must compile and run as Run(1, null). The diff at src/TUnit.Core.SourceGenerator/C…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Linked Issues check

Explanation

Objective [#7197]: generated code for a generic test method with a T? (Nullable<T>) parameter, where T : struct, must compile and run as Run<int>(1, null). The diff at src/TUnit.Core.SourceGenerator/CodeGenerationHelpers.cs (GenerateTypeInfo, hasGenericParameters branch) emits typeof(global::System.Nullable<>) when namedType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T. Other generic definitions keep GetTypeOfExpression(ConstructUnboundGenericType()). This removes the typeof(T?) expression that referenced T outside the method scope. The PR adds the repro test at tests/TUnit.TestProject/Bugs/_7197/NullableValueTypeGenericMethodTests.cs and the Tests7197 snapshot (Tests7197.Test.verified.txt). The snapshot shows the registered metadata and an invocation that casts arguments to int and int? and calls Run<int>. The build and test results (net10.0 source-gen and --reflection modes; 863 generator tests on four frameworks) are author-reported. I did not rebuild or rerun them. The code evidence supports the requirement.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. 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 1 functions across 4 files. (1 skipped: 1 unsupported.)

Full details: Out of Scope Changes check

Explanation

The change to tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs has no connection to #7197. It replaces the record struct Token in the SkipsConversionsBlockedByObsoleteError fixture with a plain readonly struct, to fix a net472 CS0518 IsExternalInit build failure. That failure concerns a different test fixture and a different target framework. It does not concern nullable parameter metadata generation. The PR description states that this change fixes a red main build. That is a separate concern from the linked issue. The source generator fix, the repro test, and the Tests7197 snapshot are in scope and are connected to the objective.

Resolution

Remove the AotConverterScanTests.cs change from this PR and submit it in a separate pull request that references the failing test (AotConverterScanTests.SkipsConversionsBlockedByObsoleteError) and the red modularpipeline (windows-latest) run. Keep the CodeGenerationHelpers.cs fix, the Tests7197 snapshot, and the Bugs/_7197 repro test in this PR.

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 the generic type,
Where nullable values take their shape.
The tests now follow int and null,
And structs compile without record rules.
The source is ready, neat and bright.}

[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-01T15:40:25.616024Z	b782124	New commits

ℹ️ 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: LGTM.

• Fix: Special-casing Nullable<> in GenerateTypeInfo is minimal and targeted. It correctly avoids changing the shared display format that other call sites rely on. Using OriginalDefinition.SpecialType == System_Nullable_T is robust. The generated ConstructedGeneric(typeof(global::System.Nullable<>), [GenericParameter(0, true, "T")]) looks right, and the repro covers source-gen and reflection modes.
• Tests: The engine repro and the generator snapshot follow repo conventions. The .verified.txt is placed alongside the other TestsNNNN snapshots.
• net472 fix: Replacing the record struct with a plain readonly struct in AotConverterScanTests is sensible and preserves coverage.

Optional: the typeof(global::System.Nullable<>) string could be a named constant if it's needed elsewhere. Otherwise this is good to merge.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: a8474e359c

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

[Medium impact] The PR appears safe to merge.

Summary

Fixes generated metadata for T? parameters where T : struct.

• Generic test methods can use nullable value-type parameters in generated tests.

No actionable issues were found.

_{Reviews (2) · Last reviewed commit: "fix(sourcegen): emit typeof(Nullable<>) ..." · Reviewed by Greptile}

[github-actions]

Review

The fix is correct and small. Checking OriginalDefinition.SpecialType == System_Nullable_T and emitting typeof(global::System.Nullable<>) avoids the T? shortening in FullyQualifiedGenericWithGlobalPrefix. Leaving the shared display format alone is the right call, since many other call sites use it.

• The repro test ([EngineTest(ExpectedResult.Pass)]) and the Tests7197 snapshot cover the regression. The snapshot shows the expected ConstructedGeneric(typeof(global::System.Nullable<>), ...).
• The AotConverterScanTests fixture change (plain readonly struct instead of a record struct, to avoid IsExternalInit on net472) keeps the same conversions, so it still tests the same thing. It also unblocks main.

Optional follow-up: the special case sits at one call site. If other places build typeof(...) from an unbound generic with the same display format, a GetTypeOfExpression helper that handles Nullable<> would fix them all at once. I didn't find another affected path in this diff, so this shouldn't block merge.

No blocking issues. LGTM.

[thomhurst]

I rebased onto main at 62db448 to fix a merge conflict. The only conflict was in AotConverterScanTests.cs, where #7330 had already landed the same net472 fixture fix, so I took main's version. The new head is b782124. The diff is now limited to the #7197 fix, its snapshot and the TestProject repro.

On the new base, the Tests7197 snapshot test and AotConverterScanTests pass on net10.0.

I turned auto-merge off before force-pushing. It should be re-armed once claude-review and Codex have reviewed b782124.

[github-actions]

Review

The fix is correct and minimal. Nullable<> is special-cased in GenerateTypeInfo and emitted as typeof(global::System.Nullable<>). This avoids typeof(T?), which doesn't compile outside the method's T scope. The SpecialType.System_Nullable_T check on OriginalDefinition is the right way to detect it. The generator-only change is fine because the later execution path is shared.

Tests: there is an engine repro in TestProject/Bugs/_7197 and a generator snapshot (Tests7197). The snapshot shows the expected ConstructedGeneric(typeof(global::System.Nullable<>), [...]).

Non-blocking suggestion: the same problem could affect other open generics that the display format abbreviates. Putting the open-definition printing in one helper, instead of special-casing at this call site, would make that harder to miss. I found no other affected case here.

LGTM.