Skip to content

fix(mocks): strip nullable annotation from constructor dispatch patterns - #6498

Merged
thomhurst merged 1 commit into
mainfrom
fix/6492-nullable-ctor-pattern
Jul 28, 2026
Merged

fix(mocks): strip nullable annotation from constructor dispatch patterns#6498
thomhurst merged 1 commit into
mainfrom
fix/6492-nullable-ctor-pattern

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Fixes #6492

Problem

T.Mock() failed to compile for any class with a constructor parameter of a nullable type, e.g. Elastic.Clients.Elasticsearch.ElasticsearchClientSettings:

MockImplFactory.g.cs(121,52): error CS8116: It is not legal to use nullable type 'Uri?' in a
pattern; use the underlying type 'Uri' instead.

MockFactoryBuilder.GenerateConstructorDispatch builds an is pattern per parameter from MockParameterModel.FullyQualifiedType, which carries the nullable annotation. x is Uri? / x is int? are never legal C#.

Fix

Strip a single trailing ? before emitting the pattern type. A trailing ? on a value type means Nullable<T> — which boxes to either null or a boxed T — so those parameters now take the null-accepting (arg is null or T) branch alongside reference types, instead of the value-type-only arg is T branch that would have rejected a legitimate null.

The cast used to invoke the constructor keeps the annotated type, so nullability of the actual argument is unchanged.

Tests

tests/TUnit.Mocks.Tests/Issue6492Tests.cs — a class with same-arity constructor overloads (forcing the type-check dispatch path) taking Uri?, string?, a nullable delegate + nullable interface, and int? + nullable interface. Covers compilation, correct overload dispatch, boxed Nullable<T> dispatch, and null arguments.

Full TUnit.Mocks.Tests suite: 1166 passed. TUnit.Mocks.SourceGenerator.Tests snapshots: 80 passed, unchanged.

The generated mock factory dispatches between same-arity constructor
overloads with `is` patterns built from each parameter's fully qualified
type. A nullable type is never legal as the type of an `is` pattern
(CS8116), so any constructor taking `Uri?`, a nullable delegate, `int?`,
etc. produced uncompilable code — mocking the type was impossible.

Match against the underlying type instead. A trailing `?` on a value type
means `Nullable<T>`, which boxes to either null or a boxed `T`, so those
parameters now take the null-accepting branch alongside reference types.

Fixes #6492
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes generated constructor dispatch for nullable parameters.

  • Removes trailing nullable annotations from emitted C# type patterns.
  • Allows null when dispatching constructors with nullable reference or value-type parameters while preserving annotated constructor casts.
  • Adds regression coverage for nullable references, delegates, interfaces, boxed nullable values, null arguments, overload selection, and mocked members.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable defects identified in the changed constructor-dispatch behavior.

The generator now emits legal underlying types in nullable constructor patterns, preserves annotated casts, and adds coverage for the affected reference-type, delegate, interface, value-type, overload, and null scenarios.

Important Files Changed

Filename Overview
src/TUnit.Mocks.SourceGenerator/Builders/MockFactoryBuilder.cs Normalizes nullable pattern types and updates null acceptance during same-arity constructor dispatch without changing invocation casts.
tests/TUnit.Mocks.Tests/Issue6492Tests.cs Adds focused regression tests covering compilation, overload dispatch, nullable value boxing, null arguments, and normal mock behavior.

Reviews (1): Last reviewed commit: "fix(mocks): strip nullable annotation fr..." | Re-trigger Greptile

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review

Verified the fix by checking out the branch, building, and running the new tests directly (dotnet test --treenode-filter "/*/*/Issue6492Tests/*") — all 18 cases (6 tests × net8/9/10) pass.

Root-cause analysis is correct and slightly broader than the PR description suggests. The original code only null-checked non-value-type parameters:

p.IsValueType
    ? $"constructorArgs[{i}] is {p.FullyQualifiedType}"
    : $"(constructorArgs[{i}] is null or {p.FullyQualifiedType})"

But FullyQualifiedType comes from GetFullyQualifiedNameWithNullability(), which appends ? for annotated reference types too (e.g. Uri?). So the pre-existing is null or {FullyQualifiedType} branch for reference types was already emitting is null or global::System.Uri?, which is just as illegal (CS8116) as the value-type case. The new code fixes both paths uniformly by stripping the trailing ? before building either branch, and computing acceptsNull independently of where the ? came from. That's why the repro test uses Uri? (a reference type) rather than only int? — good instinct to cover the actual bug surface, not just the literal symptom from the issue title.

Test coverage is solid: same-arity overload dispatch, nullable delegate/interface, boxed Nullable<T>, and an explicit null-argument case are all exercised, and the fix is confirmed to not touch reflection-mode (T.Mock() is source-gen only, so no dual-mode counterpart needed here).

Minor, non-blocking observations (not requesting changes):

  • StripNullableAnnotation is a static, pure string transform — could be a [Pure]-style local helper or even inlined, but pulling it out as its own method with the XML doc explaining why (CS8116, both nullable-reference and Nullable<T> cases) is good for future maintainers touching this dispatch code. No change needed.
  • acceptsNull is derived from patternType.Length != p.FullyQualifiedType.Length rather than a direct EndsWith("?") check on the original — a little indirect, but correct and avoids computing the strip twice. Fine as-is.

No architectural concerns — this is a narrowly-scoped, well-tested bug fix in the constructor-dispatch codegen. Nice work isolating the actual predicate (ends with a lone trailing '?') rather than special-casing Uri/int? specifically, which keeps this correct for any nullable-annotated parameter type.

@thomhurst
thomhurst merged commit 8c98f36 into main Jul 28, 2026
13 checks passed
@thomhurst
thomhurst deleted the fix/6492-nullable-ctor-pattern branch July 28, 2026 10:55
This was referenced Jul 28, 2026
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.

[TUnit.Mocks] Mock() fails for ElasticsearchClientSettings: invalid nullable-type pattern CS8116

1 participant