fix(mocks): resolve build errors when mocking Azure SDK clients

[thomhurst]

Summary

Fixes #5434 — TUnit.Mocks failed to compile generated code for Azure.Storage.Blobs.BlobClient and Azure.Data.Tables.TableClient.

• CS0411: generic override bodies now emit explicit type arguments on base.Method<T>(...) calls, so T can be inferred when it only appears in the return type (e.g. TableClient.GetEntity<T>).
• CS0111: the extension-method generator now keeps out parameters on methods where stripping them would collide with another overload (e.g. BlobClient.GenerateSasUri with/without out string stringToSign). Detection is per-overload-group so existing tests using out-stripped signatures are unaffected.

Test plan

• New Issue5434Tests mocks both BlobClient and TableClient
• All 763 TUnit.Mocks tests pass on net10.0

[codacy-production]

Not up to standards ⛔

🔴 Issues 2 minor

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

Results:
2 new issues

Category	Results
CodeStyle	2 minor

View in Codacy

🟢 Metrics 15 complexity

Metric	Results
Complexity	15

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Code Review — fix(mocks): resolve build errors when mocking Azure SDK clients

The two-bug fix is well-scoped and the root causes are correctly diagnosed. Here are my findings, ordered by severity.

---

CS0411 Fix () — Clean and Correct

Adding to all 6 base.Method(...) call sites is minimal and correct. For non-generic methods the call is unchanged; for generic methods with a return-only type parameter it bypasses the compiler's failing inference. No concerns here.

---

CS0111 Fix () — Correct, but with a few actionable concerns

1. threaded through 5 method signatures

The disambiguation flag is added as a parameter to , , , , and . Threading a contextual boolean through five call sites is a maintenance burden — the call sites don't communicate why the flag exists.

A better approach is to compute the disambiguation set at model build time and encode the decision into itself (e.g., add a property). Generation logic then reads from the model rather than accepting a runtime decision as a parameter. This also makes the flag visible to any future builder that needs it, without further signature changes.

2. for parameter initialization may silently produce

In , parameters are initialized with . For reference types (e.g., the in ), compiles to with the nullable warning suppressed. Any test code that reads after calling the mock will get a rather than a compile-time warning.

Consider using (without ) so callers at least get nullable analysis warnings, or emit an explicit with a comment explaining the intentional behavior. Either way, a code comment is needed.

3. Disambiguation changes the public extension-method signature silently

When keepOutParams is true, the emitted extension method now has an parameter that callers must satisfy with . This is a user-experience regression for the disambiguated overloads — code that previously compiled with now requires . There is no comment, warning, or documentation noting this. At minimum a comment in would help future maintainers understand the tradeoff.

4. Disambiguation key does not account for parameters

builds its collision key by excluding only parameters. If two overloads differ only by a parameter, they produce the same key and could trigger false-positive disambiguation. Unlikely in practice, but the algorithm isn't fully general. Consider documenting the known limitation.

5. Test coverage is compile-only

verifies that and construct without throwing — this directly validates the bug fix. However, neither test:

• Calls to confirm the disambiguated extension method signature is actually usable
• Calls to exercise the generic override path

Without at least one call-site test, a future regression that breaks the extension method body (rather than its signature) would not be caught.

---

Summary

Area	Assessment
CS0411 fix	Correct and minimal — no concerns
CS0111 fix correctness	Correct for the Azure SDK cases reported
design	Leaks emit-time decision into 5 signatures; should belong to the model layer
initialization	Silently produces for reference-type params; needs a comment or removal of
API impact on callers	Disambiguation forces at call sites; no documentation of this change
Edge cases	params not excluded from the collision key
Test coverage	Compilation-only; no behavioral coverage of the fixed call paths

The fix is correct and the approach is sound. The main actionable items are: (a) document or reconsider for initialization, (b) add at least one call-site test for both fixed paths, and (c) consider moving the disambiguation decision into the model layer to avoid the 5-method boolean threading.

[claude]

Code Review — fix(mocks): resolve build errors when mocking Azure SDK clients

The two-bug fix is well-scoped and the root causes are correctly diagnosed. Here are my findings, ordered by severity.

---

CS0411 Fix (MockImplBuilder.cs) — Clean and Correct

Adding GetTypeParameterList(method) to all 6 base.Method(...) call sites is minimal and correct. For non-generic methods the call is unchanged; for generic methods with a return-only type parameter it bypasses the compiler's failing inference. No concerns here.

---

CS0111 Fix (MockMembersBuilder.cs) — Correct, but with a few actionable concerns

1. keepOutParams bool threaded through 5 method signatures

The disambiguation flag is added as a parameter to GenerateMemberMethod, EmitMemberMethodBody, EmitFuncOverloads, EmitSingleFuncOverload, and GetArgParameterList. Threading a contextual boolean through five call sites is a maintenance burden — the call sites don't communicate why the flag exists.

A better approach is to compute the disambiguation set at model build time and encode the decision into MockMemberModel itself (e.g., add a bool KeepOutParamsInSignature property). Generation logic then reads from the model rather than accepting a runtime decision as a parameter. This also makes the flag visible to any future builder that needs it, without further signature changes.

2. default! for out parameter initialization may silently produce null

In EmitOutParamDefaults, out parameters are initialized with default!. For reference types (e.g., out string stringToSign in BlobClient.GenerateSasUri), default! compiles to null with the nullable warning suppressed. Any test code that reads stringToSign after calling the mock will get a NullReferenceException rather than a compile-time warning.

Consider using default (without !) so callers at least get nullable analysis warnings, or emit an explicit null! with a comment explaining the intentional behavior. Either way, a code comment is needed.

3. Disambiguation changes the public extension-method signature silently

When keepOutParams is true, the emitted extension method now has an out T name parameter that callers must satisfy with out _. This is a user-experience regression for the disambiguated overloads — code that previously compiled as mock.GenerateSasUri(perms, expires) now requires mock.GenerateSasUri(perms, expires, out _). There is no comment, warning, or documentation noting this change. At minimum a comment in EmitSingleFuncOverload would help future maintainers understand the tradeoff.

4. Disambiguation key does not account for ref parameters

ComputeOutDisambiguationSet builds its collision key by excluding only ParameterDirection.Out parameters. If two overloads differ only by a ref parameter, they produce the same key and could trigger false-positive disambiguation. Unlikely in practice, but the algorithm is not fully general. Consider documenting the known limitation.

5. Test coverage is compile-only

Issue5434Tests.cs verifies that Mock.Of<BlobClient> and Mock.Of<TableClient> construct without throwing — this directly validates the bug fix. However, neither test:

• Calls mock.GenerateSasUri(..., out _) to confirm the disambiguated extension method signature is actually usable
• Calls mock.GetEntity<SomeType>(...) to exercise the generic override path

Without at least one call-site test, a future regression that breaks the extension method body (rather than its signature) would not be caught.

---

Summary

Area	Assessment
CS0411 fix	Correct and minimal — no concerns
CS0111 fix correctness	Correct for the Azure SDK cases reported
keepOutParams design	Leaks emit-time decision into 5 signatures; should belong to the model layer
default! initialization	Silently produces null for reference-type out params; needs a comment or removal of !
API impact on callers	Disambiguation forces out _ at call sites; no documentation of this change
Edge cases	ref params not excluded from the collision key
Test coverage	Compilation-only; no behavioral coverage of the fixed call paths

The fix is correct and the approach is sound. The main actionable items are: (a) document or reconsider default! for out initialization, (b) add at least one call-site test for both fixed paths, and (c) consider moving the disambiguation decision into the model layer to avoid threading a boolean through 5 method signatures.

[claude]

Updated Code Review — fix(mocks): resolve build errors when mocking Azure SDK clients

This is a follow-up review. I need to correct my two previous reviews on this PR — they contained inaccurate claims that I want to address.

---

Corrections to my previous reviews

My earlier reviews raised the concern that keepOutParams was being "threaded through 5 method signatures". That was wrong. Looking at the actual code, the PR already uses the model-layer approach I recommended: KeepOutParamsInExtensionSignature lives on MockMemberModel, and the generation methods read from method.KeepOutParamsInExtensionSignature directly. The boolean is not passed as a parameter anywhere.

My earlier reviews also flagged the tests as "compile-only" — also incorrect. Can_Configure_BlobClient_GenerateSasUri_OutOverload and Can_Configure_TableClient_GetEntity_Generic both call the setup methods, exercising both fixed code paths.

I apologize for the noise from those earlier reviews.

---

CS0411 Fix (MockImplBuilder.cs) — Correct

GetTypeParameterList(method) applied uniformly across all 6 base.Method(...) sites is minimal and correct. No concerns.

---

CS0111 Fix (MockMembersBuilder.cs) — Correct, with one genuine concern

KeepOutParamsInExtensionSignature in Equals/GetHashCode is semantically misleading

The property is computed ephemerally inside ApplyOutDisambiguation via m with { KeepOutParamsInExtensionSignature = true } and is never true on any model stored in the incremental pipeline. Including it in Equals/GetHashCode on MockMemberModel is harmless at runtime, but it creates a false impression: it looks like a stable data property that should affect model identity, when it's actually a transient computation result that lives only inside Build().

A cleaner alternative:

// In ApplyOutDisambiguation — return a set, don't mutate the model
private static HashSet<int> ComputeOutDisambiguationSet(EquatableArray<MockMemberModel> methods)
{
    // ... same grouping logic ...
    return flagged; // HashSet<int> of MemberIds
}

// In Build() — pass the set to generators that need it
var disambiguated = ComputeOutDisambiguationSet(model.Methods);
foreach (var method in model.Methods)
{
    GenerateMemberMethod(writer, method, disambiguated.Contains(method.MemberId), model, safeName);
}

This keeps the model clean, avoids polluting Equals/GetHashCode, and makes the scope of the computation obvious.

default! for out parameter initialization — Acceptable with the existing comment

The EmitOutParamDefaults comment explains the intent well. This is fine.

Disambiguation key correctness — Good

The key includes {p.Direction}:{p.FullyQualifiedType} for non-Out parameters, so ref int and int are treated as distinct. My previous review's concern about ref parameters was also incorrect.

---

Tests — Good

All four tests are well-scoped. The compilation tests catch the build errors, and the call-site tests (GenerateSasUri(... out _) and GetEntity<TableEntity>(...)) confirm the fixed paths are actually usable.

---

Summary

Area	Assessment
CS0411 fix	Correct and minimal
CS0111 fix correctness	Correct for the reported cases
KeepOutParamsInExtensionSignature in Equals/GetHashCode	Harmless but architecturally misleading — prefer returning a HashSet<int> from the disambiguation helper
default! initialization	Acceptable; comment explains the intent
Test coverage	Good — both compile-time and call-site coverage

The fix is solid and the root causes are correctly diagnosed. The one genuine suggestion is moving the disambiguation state out of the model and back into a locally scoped HashSet<int>, but this is not blocking.

[claude]

Follow-up Review — fix(mocks): resolve build errors when mocking Azure SDK clients

Reviewing the third commit (c6e63380) against the outstanding concern from my last review.

---

Previous concern addressed: KeepOutParamsInExtensionSignature in Equals/GetHashCode

The latest commit excludes KeepOutParamsInExtensionSignature from both Equals and GetHashCode. The property is present on the record but correctly not part of model identity, matching the documented intent in its XML doc comment. This was the one genuine remaining concern — it is now resolved.

---

Final assessment

Area	Status
CS0411 fix (GetTypeParameterList on all base.Method calls)	Correct and minimal
CS0111 fix (ApplyOutDisambiguation + KeepOutParamsInExtensionSignature)	Correct
KeepOutParamsInExtensionSignature excluded from model equality	Addressed in c6e63380
default! for out param initialization with explanatory comment	Acceptable
Test coverage (compile-time + call-site for both bug paths)	Good

The fix is well-scoped, correctly diagnosed, and the incremental-pipeline concern is properly handled. No remaining blockers.

[claude]

Final Review — All Concerns Addressed

This is a follow-up to my previous reviews. The last commit (7d08754) updates the snapshot for Partial_Mock_With_Generic_Constrained_Virtual_Methods, which correctly reflects the explicit type arguments (<T>, <TInput, TResult>) now emitted on base.Method() calls. The snapshot change is expected and correct per the project's snapshot testing policy.

Status of all previously raised concerns

Concern	Status
CS0411: explicit type arguments on base.Method<T>()	Fixed in commit ab1d2d74
CS0111: ApplyOutDisambiguation to avoid extension method collisions	Fixed in commit ab1d2d74
keepOutParams boolean threaded through signatures	Addressed in 2a828f9d — moved to KeepOutParamsInExtensionSignature on the model
KeepOutParamsInExtensionSignature polluting Equals/GetHashCode	Addressed in c6e63380 — excluded from model equality
default! for out-param initialization	Acceptable; explanatory comment present
Test coverage	Good — both compile-time (Can_Mock_BlobClient/Can_Mock_TableClient) and call-site tests (GenerateSasUri(..., out _) and GetEntity<TableEntity>(...)) present
Snapshot updated	Done in 7d087547

The PR is well-scoped, correctly fixes both root causes, and all previously raised concerns have been addressed. Ready to merge.