Skip to content

fix(generic): infer object when a type parameter gets null and a value-type value - #7198

Merged
thomhurst merged 6 commits into
mainfrom
fix/issue-7155-null-value-type-generic-inference
Oct 20, 2026
Merged

thomhurst merged 6 commits into
mainfrom
fix/issue-7155-null-value-type-generic-inference

Conversation

@thomhurst

@thomhurst thomhurst commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Fixes #7155

Stacked on #7142 (base branch fix/issue-7120-null-arguments-generic-method). That PR adds the null-row handling this change extends: in reflection mode a type parameter that gets only nulls maps to object, and in the generator a later null no longer replaces a type that an earlier value inferred. Merge #7142 first; this PR then retargets to main.

Summary

When one type parameter gets a null and a value-type value, both modes now run the test as Run<object>, whichever order the values come in.

[Test]
[Arguments(1, null)]
[Arguments(null, 1)]
public void Run<T>(T first, T second) { }
Source-gen Reflection
main Run<object> for both rows Run<int> for both rows, null passed as 0
#7142 Run<int> for both rows, null passed as 0 Run<int> for both rows, null passed as 0
This PR Run<object>(1, null), Run<object>(null, 1) same

A non-null reference-type value still wins over the null, as #7142 set up: [Arguments("a", null)] runs as Run<string>, since a string can hold the null.

Cause

Both modes inferred T from the non-null value alone and ignored the null. A value type cannot hold null, so the invocation passed default(int) instead. Nothing reported that the row's null had been changed to 0.

Fix

The same rule is applied in each place that infers method type arguments from an [Arguments] row: if a type parameter gets a null and its inferred type is a (non-Nullable<T>) value type, it infers object.

  • Source generator, TestMetadataGenerator.InferTypesFromArgumentsAttribute: records which type parameters got a null and, after the loop, replaces a value-type inference for those with object.
  • Reflection, TestGenericTypeResolver.ResolveMethodGenericArguments (test name and metadata): the null-only list from fix(engine): infer object for null rows on generic test methods in reflection mode #7142 now also overrides a value-type mapping. A boxed value's run-time type is never Nullable<T>, so IsValueType is enough.
  • Reflection invoker, ReflectionTestDataCollector.CreateReflectionTestInvoker: it re-infers type arguments from the values on every call, so it needs the same rule, or the name would say Run<object> while the body ran as Run<int>. The null scan (ReceivesNullArgument) only runs when the inferred type is a value type, and allocates nothing.
  • Reflection [GenerateGenericTest] row inference, ReflectionTestDataCollector.InferMethodTypeArguments: it used last-value-wins, nulls included, so [Arguments(null, 1)] inferred int and [Arguments("a", null)] inferred object. It now matches the generator: the last non-null value wins, a null infers object only when nothing else does, and a value type plus a null infers object.
  • Separate change, reflection [GenerateGenericTest] row inference: InferMethodTypeArguments now ignores CancellationToken parameters when it pairs row values with parameters, because the engine supplies the token. Before, a token declared between data parameters shifted the values onto the wrong parameters. The source generator already skipped tokens. NullAndValueTypeWithTokenGenerateGenericTestMethod covers it.
  • The "value type other than Nullable<T>" check is shared by the three reflection paths through TestGenericTypeResolver.CannotHoldNull.
  • where T : struct: a null row infers object, which breaks the constraint. Each such row fails with Type 'Object' does not satisfy the 'struct' constraint for generic parameter 'T' in both modes, instead of running with the null turned into 0. The generated code compiles. NullAndValueTypeStructConstraintMethod pins this.

Tests

  • New fixtures tests/TUnit.TestProject/Bugs/_7155/NullAndValueTypeGenericMethodTests.cs, run directly (Release, net10.0):
    • NullAndValueTypeGenericMethod (both orders): before, 2/2 failed with Run<int> in both modes. After, 2/2 pass with Run<object> in both modes.
    • NullAndValueTypeGenerateGenericTestMethod ([GenerateGenericTest(typeof(object))] + [Arguments(null, 1)]): before, both modes produced Run<object> and a failing Run<int>. After, both modes produce one passing Run<object>.
  • New engine test Issue7155Tests: reflection mode passes. The AOT/source-gen variant runs only in CI.
  • New generator snapshot Tests7155, verified on net10.0, net9.0, net8.0 and net472. Full TUnit.Core.SourceGenerator.Tests on net10.0: 194 passed, 1 skipped, and no other snapshot changed.
  • TUnit.Engine.Tests (net10.0): Issue7120Tests, Issue7106Tests, Issue7107Tests, Issue7121Tests and GenericMethodWithDataSourceTests pass.
  • TUnit.TestProject namespaces Bugs._7120, _7106, _7099, _7092, _7096, _7121, _7107, _6150 and GenericMethodTests, in source-gen and reflection modes: the counts are the same as without this change. The only failures are the intentional constraint-violation tests in _7107, which fail the same way before and after.

Found along the way

Summary by CodeRabbit

  • Bug Fixes

    • Improved generic test argument inference when null values are combined with value types.
    • Prevented null arguments from producing invalid non-nullable value-type instantiations.
    • Preserved nullable and reference-type inference where supported.
    • Correctly excludes injected cancellation tokens from data-row argument inference.
    • Provides consistent behavior for dynamically invoked generic tests.
  • Tests

    • Added regression coverage for null/value-type combinations, nullable collections, cancellation tokens, and struct constraints.

…flection mode

[Arguments(null)] on a generic test method, with no [GenerateGenericTest],
failed in reflection mode with GenericTypeResolutionException: the generic
method definition reaches TestGenericTypeResolver, which skipped null
arguments and so could not resolve T. The source generator already infers
object for the row (the TUNIT0999 crash from the issue was fixed by #7111),
and the reflection invoker defaults an uninferred type argument to object.

TestGenericTypeResolver now maps a method type parameter that only received
null values to object, so the row runs as Run<object>(null) in both modes.
A non-null value for the same type parameter still takes precedence, as in
the invoker.

Fixes #7120
Reflection mode treated a declared object parameter as a placeholder for
the method's type parameter, so Run<T>(object context, T value) with
[Arguments(null, null)] failed and Run<T>(T value, object other) took T
from other. The object fast paths now apply only when the parameter types
are placeholders, not the method's declared types.

Source-gen mode let a later null value override an earlier inferred
type, so [Arguments("a", null)] on Run<T>(T first, T second) ran as
Run<object> while [Arguments(null, "a")] ran as Run<string>. A null value
now infers object only when no other value infers the type parameter,
matching reflection mode.
…e-type value

[Arguments(1, null)] on Run<T>(T a, T b) ran as Run<int> with the null turned
into 0. A value type cannot hold the null, so the type parameter now infers
object in the source generator, TestGenericTypeResolver, the reflection
invoker and the [GenerateGenericTest] row inference, in either order.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T05:46:18.103549Z e3a5270 PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b8f4f613-cb6c-4802-87d9-22f281581475
📥 Commits

Reviewing files that changed from the base of the PR and between 65f8504 and 08b1166.

📒 Files selected for processing: 3
  • src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs
  • src/TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
  • src/TUnit.Engine/Services/TestGenericTypeResolver.cs

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

📝 Walkthrough

Generic method inference now accounts for null arguments across source generation and reflection. When a null argument cannot be held by the inferred type, inference uses object. Nullable-compatible inferred types remain unchanged. Regression tests cover mixed arguments and related binding cases.

Changes: Generic null argument inference

Layer / File(s) Summary
Track nulls during generic inference
src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs, src/TUnit.Engine/Services/TestGenericTypeResolver.cs
Source generation and generic type resolution track null arguments. They use object when the inferred type is a non-nullable value type, while preserving nullable-compatible types.
Align reflection binding and invocation
src/TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
Reflection inference excludes CancellationToken parameters from row binding. It accounts for null arguments during inference and generic method invocation.
Cover generic null inference cases
tests/TUnit.TestProject/Bugs/_7155/*, tests/TUnit.Core.SourceGenerator.Tests/Bugs/7155/*, tests/TUnit.Core.SourceGenerator.Tests/Tests7155.Test.verified.txt, tests/TUnit.Engine.Tests/Issue7155Tests.cs
Regression tests cover mixed null and value arguments, nullable inference, CancellationToken binding, and struct constraints. Engine tests check pass counts and constraint errors. The generated snapshot records registrations for the source-generated cases.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 08b11

The change makes generic rows that mix null and value-type arguments infer object consistently in both modes. No merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Out of Scope Changes check Warning The pull request also changes behavior and project content that have no demonstrated connection to issue #7155. Examples include AWSSDK.SQS and StreamJsonRpc package upgrades in Directory.Packages.pro… Remove the unrelated package, solution, documentation, navigation, and TUnit0076 analyzer changes from this pull request, or move them to separate pull requests. Keep only changes required to implement and test issue #7155.
Docstring Coverage Warning Docstring coverage is 12.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 11 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: inferring object when a generic type parameter receives both null and a value-type value.
Linked Issues check Passed Issue #7155 requires consistent handling of a null and a value-type value for one generic type parameter. The source generator and reflection paths now infer object instead of a non-nullable value typ…
Full details: Linked Issues check

Explanation

Issue #7155 requires consistent handling of a null and a value-type value for one generic type parameter. The source generator and reflection paths now infer object instead of a non-nullable value type, for both argument orders. The reflection invoker applies the same rule to the invoked values. The added _7155 fixtures, engine tests, and source-generator snapshot cover the required behavior and the struct-constraint result.

Full details: Out of Scope Changes check

Explanation

The pull request also changes behavior and project content that have no demonstrated connection to issue #7155. Examples include AWSSDK.SQS and StreamJsonRpc package upgrades in Directory.Packages.props, multiple solution membership changes, new documentation and navigation content, and the unrelated TUnit0076 analyzer rule and documentation.

Resolution

Remove the unrelated package, solution, documentation, navigation, and TUnit0076 analyzer changes from this pull request, or move them to separate pull requests. Keep only changes required to implement and test issue #7155.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 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

A rabbit checks each row with care,
A null, a value, both held there.
If int cannot take null’s place,
object gives the row its space.
Nullable types keep their name,
And test rows run without the same old game.

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3a5270cb0

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

Comment thread src/TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Updates generic method inference so a null and a non-nullable value for the same type parameter infer object.

  • Generic test rows keep nulls when values infer a value type.
  • Generated generic-test rows infer types from their data values.

No new actionable issues found. Review was based on code inspection; no tests were run.

Reviews (4) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile

Comment thread src/TUnit.Engine/Services/TestGenericTypeResolver.cs Outdated
…ference

Two reflection-mode gaps in the null and value-type rule:

- A Nullable<T> inferred from a nested type, such as List<int?>, can hold
  a null. The resolver and the reflection invoker replaced it with
  object, so Run<T>(List<T> values, T missing) with a List<int?> row
  failed to bind List<int?> to List<object>. Both now keep Nullable<T>,
  as the source generator does.
- InferMethodTypeArguments matched [Arguments] values to parameters by
  position, including an injected CancellationToken. With a token
  between two T parameters, the null was never seen, so a
  [GenerateGenericTest] method also ran the row as Run<int>. It now skips
  token parameters, as the generator does.

Adds fixtures for both cases in Bugs/_7155, covered by Issue7155Tests.
@github-actions

Copy link
Copy Markdown
Contributor

Review of #7198

The rule is applied consistently in all four places that infer method type arguments: the generator, TestGenericTypeResolver, the reflection invoker, and InferMethodTypeArguments. The Nullable<T> carve-out for nested types such as List<int?> is handled the same way in each. The new tests cover both orderings, [GenerateGenericTest], and the nested-nullable case. I found no blocking issues.

Suggestions:

  1. Duplicated rule. The "null + value type => object" rule is now written four times, once as symbols and three times as Type. The reflection copies already differ slightly (IsValueType alone, versus IsValueType plus a Nullable.GetUnderlyingType check). A small shared helper such as ShouldWidenToObjectForNull(Type inferred) in the engine would stop them drifting apart.
  2. struct constraint. Widening to object for where T : struct (or unmanaged) produces an invalid instantiation. For example, [Arguments(1, null)] on Run<T>(T a, T b) where T : struct would make MakeGenericMethod(typeof(object)) throw in reflection mode and fail to compile in the generated code. Previously this silently ran as Run<int>. Skipping the widening, or emitting a clear diagnostic, when the type parameter has a value-type constraint would be safer. I'd add a test for it.
  3. Scope creep. The CancellationToken filtering in InferMethodTypeArguments is a separate behaviour change from Generic test method with a null and a non-null value for one type parameter infers object in source-gen but int in reflection (null becomes 0) #7155. It is covered by a test, but it deserves its own line in the PR description, or its own PR.
  4. Allocation. Array.FindAll allocates on the discovery path, but only when a token parameter is present, so this is fine.

Overall this is a sound fix, pending #7142 merging first.

…rence paths; pin the struct-constraint case

TestGenericTypeResolver.CannotHoldNull now holds the 'value type other than Nullable<T>' check
that the resolver, the reflection invoker and InferMethodTypeArguments each wrote out.
NullAndValueTypeStructConstraintMethod pins that a null row on a 'where T : struct' method
fails with the constraint message in both modes, and that the generated code compiles.
@thomhurst

Copy link
Copy Markdown
Owner Author

Dispositions for the github-actions review (07:45), as of 65f8504:

  1. Duplicated rule: done in 65f8504. TestGenericTypeResolver.CannotHoldNull(Type) now holds the "value type other than Nullable<T>" check. The resolver, the reflection invoker and InferMethodTypeArguments all call it. The three copies did not actually disagree: InferMethodTypeArguments only sees boxed values, whose run-time type is never Nullable<T>. They now share one definition, so they cannot drift. The generator applies the same rule to symbols.
  2. struct constraint: I checked it, and the generated code compiles. With [Arguments(1, null)] and [Arguments(null, 1)] on Run<T>(T first, T second) where T : struct, both rows fail in both modes with Type 'Object' does not satisfy the 'struct' constraint for generic parameter 'T'. That is the existing constraint-violation path from Generic class type arguments that violate constraints are dropped in source-gen and fail the whole assembly scan in reflection mode #7107. Before this PR the rows ran as Run<int> with the null silently turned into 0, which is the bug Generic test method with a null and a non-null value for one type parameter infers object in source-gen but int in reflection (null becomes 0) #7155 fixes. A null can never reach a struct-constrained parameter, so a failure that names the constraint is the right outcome. I kept the behaviour and pinned it in 65f8504: NullAndValueTypeStructConstraintMethod and Issue7155Tests.Null_And_Value_Type_Row_With_Struct_Constraint_Fails_With_The_Constraint_Message.
  3. CancellationToken filtering: it now has its own bullet in the PR description.
  4. Allocation: agreed, no change.

Verified locally (net10.0, Release): Bugs._7155 (7 tests: 5 pass, plus the 2 intended constraint failures), _7120 8/8, _7121 1/1, and _7107 (same 20 passed / 26 intended failures in each mode), in both source-gen and reflection modes. Issue7155Tests, Issue7120Tests and Issue7107Tests pass in reflection mode (AOT runs in CI). Generator output did not change.

Base automatically changed from fix/issue-7120-null-arguments-generic-method to main October 20, 2026 09:04
…alue-type-generic-inference

# Conflicts:
#	src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs
#	src/TUnit.Engine/Services/TestGenericTypeResolver.cs
@github-actions

Copy link
Copy Markdown
Contributor

Review of #7198

The rule is consistent across the three inference paths: the source generator, TestGenericTypeResolver, and the reflection collector and invoker. A null plus a value type now infers object. A non-null reference type still wins, and Nullable<T> is left alone. Pulling the check into TestGenericTypeResolver.CannotHoldNull stops the reflection paths drifting apart. The new test and the verified snapshot cover both argument orders. I only read the diff and did not run the tests.

Issue: misplaced doc comment (TestGenericTypeResolver.cs)

The new CannotHoldNull was inserted between the existing <summary> for HasGenericParamsArray ('parameter: a params T[]...') and that method. Two consequences:

  • CannotHoldNull now carries two stacked <summary> blocks, and the first one is a fragment of the old text.
  • HasGenericParamsArray has lost its documentation.

Move CannotHoldNull (with its own summary) above the old summary, or below HasGenericParamsArray.

Design notes (non-blocking)

  • InferMethodTypeArguments and the invoker's ReceivesNullArgument each re-scan the arguments to find nulls. The generator and TestGenericTypeResolver do the same with their own null tracking. If a fourth path appears, consider one shared 'collect null-receiving type parameters' helper, so the nested-Nullable<T> and CancellationToken-skipping subtleties live in one place.
  • The CancellationToken filtering in InferMethodTypeArguments is a separate behaviour change that rides along in this PR. It looks right and it matches the generator, but it deserves its own test, or at least a line in the description.

Otherwise this looks good once the doc comment is fixed.

@thomhurst
thomhurst merged commit 8e5f145 into main Oct 20, 2026
22 of 23 checks passed
@thomhurst
thomhurst deleted the fix/issue-7155-null-value-type-generic-inference branch October 20, 2026 10:22

This branch was successfully deployed

1 active deployment
Pull Requests — 08b11660 Deployed Oct 20, 2026 by thomhurst via modularpipeline (ubuntu-latest) #20330
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generic test method with a null and a non-null value for one type parameter infers object in source-gen but int in reflection (null becomes 0)

1 participant