Skip to content

fix(mocks): wrap a real object whose class has no parameterless ctor (#6253)#6255

Merged
thomhurst merged 1 commit into
mainfrom
fix/6253-wrap-no-parameterless-ctor
Jun 15, 2026
Merged

fix(mocks): wrap a real object whose class has no parameterless ctor (#6253)#6255
thomhurst merged 1 commit into
mainfrom
fix/6253-wrap-no-parameterless-ctor

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Problem

Fixes #6253. Mock.Wrap(realObject) on a class whose only constructor takes parameters fails to compile:

error CS7036: There is no argument given that corresponds to the required parameter 'param'
of 'RealClassWrapMockImpl.RealClassWrapMockImpl(MockEngine<RealClass>, RealClass, string)'
public class RealClass
{
    public RealClass(string param) { }
}

var mock = Mock.Wrap(new RealClass("value")); // did not compile

Root cause

The two halves of the wrap generator disagreed on the wrapper's constructor signature:

  • MockFactoryBuilder.BuildWrapFactory always emits new XxxWrapMockImpl(engine, instance) — exactly two args, never forwarding constructor parameters (the wrapped instance is already built).
  • MockImplBuilder.GenerateWrapConstructors emitted one wrapper ctor per base ctor, each demanding that base ctor's parameters (WrapMockImpl(engine, instance, string param) : base(param)).

When the class had no parameterless base ctor, the only generated overload needed a third argument, so the factory's two-arg call had no match. (Those per-ctor wrapper ctors were effectively dead code — the factory never passed those args.)

Fix

Collapse to a single (engine, wrappedInstance) wrapper ctor whose base initializer is computed by a new GetWrapBaseInitializer:

  • no constructors → implicit parameterless base()
  • a parameterless ctor exists → : base() (unchanged — existing snapshots stay byte-identical)
  • otherwise → chain to the fewest-parameter accessible ctor with default(T)! arguments (out _ for out params; ref-bearing ctors deprioritized)

The wrapper derives from the real type only for type-compatibility and delegates every member to _wrappedInstance, so its own (default-constructed) base sub-object is never observed — the default args only satisfy C#'s base-call requirement.

Known limitation

The wrapper's base ctor still runs with default args (source-gen can't skip it), so a base ctor that validates/dereferences its arguments would throw at Mock.Wrap. This is inherent to source-generated wrappers and matches the existing partial-mock constructor-dispatch behavior; it's documented in the test header.

Tests

  • Runtime (WrapNoParameterlessCtorTests, 15 tests): single ref/value param, multiple params, params, leading-then-params, multiple overloads (none parameterless), optional, nullable, in, reference-type, struct, closed generics (ref + value), and verification-while-delegating. Each asserts the real instance's data is returned, proving the default base args don't leak.
  • Generator snapshot (Wrap_Mock_Without_Parameterless_Constructor) locks in the emitted : base(default(string)!) and the single two-arg ctor.

Full TUnit.Mocks.Tests (1121) and generator snapshot suite (69 + 1 new) pass.

…6253)

Mock.Wrap(realObject) on a class with only a parameterized constructor
failed to compile with CS7036. The wrap factory always constructs via
`new XxxWrapMockImpl(engine, instance)` (two args, never forwarding ctor
params), but the generator emitted one wrapper ctor per base ctor — each
requiring that base ctor's parameters. With no parameterless base ctor,
there was no matching two-arg overload.

Collapse to a single (engine, wrappedInstance) wrapper ctor whose base
initializer is computed by GetWrapBaseInitializer: implicit base() when
no ctors, `: base()` when a parameterless one exists, otherwise chain to
the fewest-parameter accessible ctor with default arguments. The wrapper
delegates every member to the wrapped instance, so its own base sub-object
is never observed — the default args only satisfy C#'s base-call rule.

Add runtime coverage (WrapNoParameterlessCtorTests: single/multi/params/
optional/nullable/in/ref-type/struct params, multiple overloads, closed
generics, verification) and a generator snapshot test.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

Summary

This is a clean, well-targeted fix for a clear bug. The root cause analysis is accurate: the wrap factory always called with exactly two args, but was emitting one wrapper ctor per base ctor (each forwarding those ctor's parameters as additional args). When no parameterless base ctor existed, the only generated overloads required extra arguments, creating an unresolvable mismatch. The fix is conceptually correct — collapse to a single two-arg ctor and only vary the base-call initializer.


What's well done

  • Root cause correctly identified. The old multi-ctor approach was generating dead code (the factory never passed those extra args). Collapsing to one ctor is the right design, not just a workaround.
  • ** is clean.** Three-case logic (no ctors / parameterless exists / fallback to fewest-param) is easy to follow and matches the invariant that the base sub-object is never observed.
  • Test coverage is thorough. 15 runtime tests cover value types, reference types, , leading+, multiple overloads, optional, nullable, , structs, generics, and verification. The snapshot test locks in the emitted so regressions are caught at the generator level.
  • Known limitation is explicitly documented. The test file header and PR description both call out that a validating base ctor will throw — this is the right level of honesty.

Concerns

1. Unhandled edge case: all accessible constructors have parameters

The code deprioritizes -bearing constructors, but if all accessible constructors have at least one parameter, it still selects one and emits for the ref params:

requires an lvalue; is an rvalue — that's a CS1510 compile error in the generated file. The heuristic avoids this when a non-ref alternative exists, but not when there isn't one.

Suggested improvement: Either (a) add a case that emits a local variable ( or a local default), or (b) emit a Roslyn diagnostic at source-gen time when no suitable base ctor can be found, matching how the rest of the generator surfaces unsupported configurations. A test case with a single -only constructor would expose this gap.

2. on value types is noise (minor)

is valid and compiles, but the null-forgiving operator is a no-op for value types. Since the generated file has at the top, this causes no practical problem — but a reader of the generated output might find it odd. If you want to be precise: only emit when . Not blocking.


Verdict

Approve — the fix is correct for the targeted scenario and the test suite confirms it. The -only edge case is worth a follow-up issue or a quick guard before merging, but it doesn't regress any existing behavior.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

Summary

This is a clean, well-targeted fix for a clear bug. The root cause analysis is accurate: the wrap factory always called new XxxWrapMockImpl(engine, instance) with exactly two args, but GenerateWrapConstructors was emitting one wrapper ctor per base ctor (each forwarding those base ctor parameters as additional args). When no parameterless base ctor existed, the only generated overloads required extra arguments, creating an unresolvable mismatch. The fix is conceptually correct — collapse to a single two-arg ctor and only vary the base-call initializer.


What's well done

  • Root cause correctly identified. The old multi-ctor approach was generating dead code (the factory never passed those extra args). Collapsing to one ctor is the right design, not just a workaround.
  • GetWrapBaseInitializer is clean. Three-case logic (no ctors / parameterless exists / fallback to fewest-param) is easy to follow and matches the invariant that the base sub-object is never observed.
  • Test coverage is thorough. 15 runtime tests cover value types, reference types, params, leading+params, multiple overloads, optional, nullable, in, structs, generics, and verification. The snapshot test locks in the emitted : base(default(string)!) so regressions are caught at the generator level.
  • Known limitation is explicitly documented. The test file header and PR description both call out that a validating base ctor will throw — this is the right level of honesty.

Concerns

1. Unhandled edge case: all accessible constructors have ref parameters

The code deprioritizes ref-bearing constructors, but if all accessible constructors have at least one ref parameter, it still selects one and emits default(T)! for those ref params:

private static string GetWrapBaseArgument(MockParameterModel p) => p.Direction == ParameterDirection.Out
    ? "out _"
    : $"default({p.FullyQualifiedType})!";  // ParameterDirection.Ref also ends up here

A ref int x parameter requires an lvalue; default(int)! is an rvalue — that produces CS1510 in the emitted file. The OrderBy heuristic avoids this when a non-ref alternative exists, but not when there isn't one.

Suggested improvement: Either (a) add a ParameterDirection.Ref branch that emits a temp local (e.g. a ref-returning call or a stack-allocated default), or (b) emit a Roslyn diagnostic when no suitable base ctor can be found, matching how the rest of the generator surfaces unsupported configurations. A test covering a class whose only constructor takes a ref parameter would expose this gap.

2. Null-forgiving ! on value type args (minor)

default(int)! is valid and compiles, but the null-forgiving operator is a no-op for value types. The generated file has #pragma warning disable so there is no practical impact — but a reader of the generated output might find it odd. Emitting ! only when !p.IsValueType would produce cleaner generated code. Not blocking.


Verdict

Approve — the fix is correct for the targeted scenario and the test suite confirms it. The ref-only constructor edge case is worth a follow-up issue or a small guard before merging, but it does not regress any existing behavior.

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 medium · 2 minor

Alerts:
⚠ 3 issues (≤ 0 issues of at least minor severity)

Results:
3 new issues

Category Results
CodeStyle 2 minor
Performance 1 medium

View in Codacy

🟢 Metrics 43 complexity

Metric Results
Complexity 43

View in Codacy

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.

@thomhurst
thomhurst merged commit 3b14cbf into main Jun 15, 2026
14 of 15 checks passed
@thomhurst
thomhurst deleted the fix/6253-wrap-no-parameterless-ctor branch June 15, 2026 16:21
This was referenced Jun 15, 2026
This was referenced Jul 13, 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.

[Bug]: Wrapping a real object without an empty constructor in a mock doesn't build

1 participant