fix(mocks): skip inaccessible internal accessors when mocking Azure.Response

[thomhurst]

Summary

• Fixes [Bug]: Issue mocking azure.response #5455. Mocking Azure.Response failed with CS0115: 'ResponseMockImpl.IsError.set': no suitable method found to override because the generator emitted an override setter for the base IsError property whose setter is internal (invisible to external assemblies).
• MemberDiscovery now checks accessor-level accessibility via a new IsAccessorAccessible helper, and HasGetter/HasSetter/SetterMemberId on the property model are computed from that.
• Threaded IAssemblySymbol? compilationAssembly through CreatePropertyModel / CreateIndexerModel / MergePropertyAccessors at every class- and interface-discovery call site so the behaviour is consistent (previously only the class-walk path had the context).

Test plan

• New regression test TUnit.Mocks.Tests/Issue5455Tests.cs::Mocking_Response_With_Internal_Setter_Compiles — fails to compile on main, passes after the fix
• TUnit.Mocks.Tests full suite: 773/773 pass (net10.0)
• TUnit.Mocks.SourceGenerator.Tests snapshot suite: 35/35 pass, no snapshot diffs

[claude]

Code Review

Good targeted fix for the setter accessibility issue. The root cause analysis is correct: was derived from symbol existence () rather than accessibility, which meant inaccessible internal accessors were being included in generated overrides.

What's working well

• Correct abstraction: cleanly separates the two concerns (exists + is accessible) and reuses the existing logic consistently.
• Consistent propagation: Threading compilationAssembly through all four call sites (class walk, interface walk ×2, merge path) was the right approach. Previously the interface walk paths were missing this context entirely — a latent inconsistency that this PR resolves.
• Counter hygiene: setterId is now only incremented when hasSetter is true, which is correct. The getter ID counter always increments regardless of hasGetter, matching the original behaviour — harmless but worth being aware of if counter-stability ever becomes important.

Potential follow-up: private accessor gap in IsMemberAccessible

IsMemberAccessible (line ~381) checks explicitly for Internal and ProtectedAndInternal but not for Private:

var accessibility = member.DeclaredAccessibility;
if (accessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!memberAssembly.GivesAccessTo(compilationAssembly))
        return false;
}
// Private falls through to AreMemberSignatureTypesAccessible which only checks types, not the member itself
return AreMemberSignatureTypesAccessible(member, compilationAssembly);

For a defined in an external assembly, IsAccessorAccessible on the setter would return true because Private isn't caught and the signature types are accessible. This would cause the same class of CS0115 error for properties with private set on external types. ProcessClassMembers already skips Private members (line 259), but that guards the property symbol itself, not its individual accessors.

Consider adding a Private check to IsMemberAccessible when memberAssembly != compilationAssembly:

if (accessibility is Accessibility.Private or Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!memberAssembly.GivesAccessTo(compilationAssembly))
        return false;
}

Note that Private members are never GivesAccessTo-eligible, so this would always return false for private accessors from external assemblies — which is the correct behaviour.

This isn't introduced by this PR (pre-existing gap) and private set on externally-defined virtual properties is much rarer than internal set, so a follow-up issue is reasonable rather than blocking this fix.

Test

The regression test is appropriate — compile-time failure is exactly the right thing to guard against here. One minor note: the test doesn't assert any behaviour (no call on the mock), but that's fine for a compilation regression test.

[claude]

Code Review

Good targeted fix for the internal setter accessibility issue. The root cause analysis is correct: HasSetter was derived from symbol existence (SetMethod is not null) rather than accessibility, which meant inaccessible internal accessors were being included in generated overrides.

What's working well

• Correct abstraction: IsAccessorAccessible cleanly separates the two concerns (exists + is accessible) and reuses the existing IsMemberAccessible logic consistently.
• Consistent propagation: Threading compilationAssembly through all four call sites (class walk, interface walk ×2, merge path) was the right approach. Previously the interface walk paths were missing this context entirely — a latent inconsistency this PR resolves.
• Counter hygiene: setterId is now only incremented when hasSetter is true. The getter ID counter always increments regardless of hasGetter, matching original behaviour — harmless, but worth knowing if counter-stability ever matters.

Potential follow-up: private accessor gap in IsMemberAccessible

IsMemberAccessible (line ~381) checks explicitly for Internal and ProtectedAndInternal but not Private:

var accessibility = member.DeclaredAccessibility;
if (accessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!memberAssembly.GivesAccessTo(compilationAssembly))
        return false;
}
// Private falls through to AreMemberSignatureTypesAccessible which checks types, not the member itself
return AreMemberSignatureTypesAccessible(member, compilationAssembly);

For a public virtual bool Foo { get; private set; } defined in an external assembly, IsAccessorAccessible on the setter would return true because Private isn't caught and the signature types are accessible. This would cause the same class of CS0115 error for properties with private set on external types. ProcessClassMembers already skips Private members (line 259), but that guards the property symbol itself — not its individual accessors.

Consider adding Private to the accessibility guard in IsMemberAccessible when memberAssembly != compilationAssembly:

if (accessibility is Accessibility.Private or Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!memberAssembly.GivesAccessTo(compilationAssembly))
        return false;
}

Since Private members are never GivesAccessTo-eligible, this always returns false for private accessors from external assemblies — the correct behaviour.

This gap is pre-existing (not introduced here) and private set on externally-defined virtual properties is rarer than internal set, so a follow-up issue is fine rather than blocking this fix.

Test

The regression test is appropriate — a compile-time failure is exactly the right thing to guard against here. The test doesn't assert any runtime behaviour, but that's fine for a compilation regression.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 minor

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

Results:
1 new issue

Category	Results
CodeStyle	1 minor

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

View in Codacy

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

[claude]

Follow-up Review

Previous feedback about the private accessor gap has been addressed in commit a56589d1. The PR is now in good shape.

Previous feedback — resolved ✓

The IsMemberAccessible function now has an explicit Private guard in both paths:

if (accessibility == Accessibility.Private)
    return false;

This prevents public virtual bool Foo { get; private set; } on external types from emitting an override setter — which was the same class of CS0115 error as the original internal set bug. The snapshot test was also expanded to cover all three cases (internal set, private set, protected internal set), so the regression guard is now comprehensive.

Minor observation on style

The Private check is added as a separate if statement before the combined Internal or ProtectedAndInternal check in both methods, rather than merged into the existing condition:

// Current
if (accessibility == Accessibility.Private)
    return false;
if (accessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal)
{ ... }

// Alternative (more compact)
if (accessibility is Accessibility.Private or Accessibility.Internal or Accessibility.ProtectedAndInternal)
{
    if (!memberAssembly.GivesAccessTo(compilationAssembly))
        return false;
}

The alternative works because Private can never satisfy GivesAccessTo (private access is never cross-assembly). However, the current two-statement form is equally correct and arguably more explicit about intent — Private doesn't need the GivesAccessTo call at all, so separating it is defensible. Not a blocking concern.

Conclusion

The fix is correct and complete. Both internal and private inaccessible accessors on externally-defined virtual properties are now properly excluded from generated overrides. The snapshot suite confirms the generator output is correct for all three accessor combinations in the test.

[claude]

Code Review — Final Pass

Previous feedback has all been addressed. Here's a summary of the full review cycle:

Previous feedback — all resolved ✓

private accessor gap in IsMemberAccessible (raised in prior review): Added the Accessibility.Private early-return guard in both IsMemberAccessibleFromExternal and IsMemberAccessible. A public virtual bool Foo { get; private set; } on an external type now correctly produces a getter-only override.

Comment/test cleanup (latest commit 7c6f44ea): The leaky IsAccessorAccessible cross-reference was dropped from the Private guard comment, ephemeral "reviewer-flagged" narration was removed from the snapshot test, and the test was renamed to the more precise Partial_Mock_Omits_Inaccessible_Property_Setters. All clean.

What's good about this PR

• Correct root cause: HasSetter was derived from SetMethod is not null (symbol existence) rather than accessibility. The new IsAccessorAccessible helper cleanly separates exists from is accessible from this assembly.
• Complete propagation: compilationAssembly is now threaded through all four paths (class walk, interface walk ×2, merge path). The interface-walk paths were missing this context before — a latent inconsistency now resolved.
• Counter hygiene: setterId is only incremented when hasSetter is true. The getter ID always increments, matching original behaviour.
• Comprehensive snapshot: The snapshot covers all three cases — internal set (skipped), private set (skipped), protected internal set (included via inheritance). Good regression guard.
• Real-world regression test: Issue5455Tests.cs tests against the actual Azure.Response type, which is the right level of confidence for a compile-time fix.

The fix is correct and complete. Approving.