Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with member hiding while mocking interfaces. #40

Open
HugoX104447 opened this issue Jan 24, 2025 · 1 comment
Open

Issue with member hiding while mocking interfaces. #40

HugoX104447 opened this issue Jan 24, 2025 · 1 comment

Comments

@HugoX104447
Copy link
Contributor

HugoX104447 commented Jan 24, 2025

Hi,
Thank you for providing this excellent mocking framework! While using it, I encountered an issue related to member hiding when mocking interfaces. Please bear with me if this report gets a bit lengthy or detailed. To help clarify the problem, I've created a Pull Request (#41) that illustrates the issue and provides a partial solution.

Problem Description

When attempting to mock IDerivedInterface with the current framework, a compile-time error occurs due to non-unique generated names for the "store" variables. Both interfaces generate the same member name get_MyProperty_BagStore, causing symbol duplication.

public interface IBaseInterface
{
    string MyProperty { get; set; }  // Generates: get_MyProperty_BagStore
}

public interface IDerivedInterface : IBaseInterface
{
    new int MyProperty { get; set; }  // Generates: get_MyProperty_BagStore
}

Example of Generated Code

In IDerivedInterfaceMockSetup, the following problematic code is generated:

private global::MockMe.Mocks.ClassMemberMocks.MemberMock<global::System.String>? get_MyProperty_BagStore;
private List<ArgBagWithMock<PropertySetterArgs<global::System.String>>>? set_MyProperty_StringBagStore;
public global::MockMe.Mocks.ClassMemberMocks.GetSetPropertyMock<global::System.String> MyProperty =>
    new(get_MyProperty_BagStore ??= new(), set_MyProperty_StringBagStore ??= new());

private global::MockMe.Mocks.ClassMemberMocks.MemberMock<global::System.Int32>? get_MyProperty_BagStore;
private List<ArgBagWithMock<PropertySetterArgs<global::System.Int32>>>? set_MyProperty_Int32BagStore;
public global::MockMe.Mocks.ClassMemberMocks.GetSetPropertyMock<global::System.Int32> MyProperty =>
    new(get_MyProperty_BagStore ??= new(), set_MyProperty_Int32BagStore ??= new());

The duplicated get_MyProperty_BagStore symbol leads to compilation failure. Additionally:

  • Both MyProperty members are generated, but explicit member implementations are missing.
  • In IDerivedInterfaceMockCallTracker, the implementation for global::System.Int32 MyProperty {...} is absent.
  • In IDerivedInterfaceMockAsserter, explicit member implementation issues persist.

Partial Solution in Pull Request (#38)

To address the duplicate symbol issue, I introduced a method GetUniqueStoreName in Pull Request (#38), which included the enclosing type in the generated store name:

public interface IBaseInterface
{
    string MyProperty { get; set; }  // Generates: IBaseInterface_get_MyProperty_BagStore
}

public interface IDerivedInterface : IBaseInterface
{
    new int MyProperty { get; set; }  // Generates: IDerivedInterface_get_MyProperty_BagStore
}

This change resolves the duplication and moves the framework closer to handling these scenarios. However, further adjustments are required.

Remaining Issues

  1. Explicit Member Implementation:
    The IDerivedInterfaceMockCallTracker class does not implement global::System.Int32 MyProperty {...}. Additional changes are needed to ensure unique dictionary entries for callTrackerMeta, similar to what is already done for setupMeta and assertMeta.

    Relevant sections of the codebase that need modification:

  1. Full Compilation:
    The explicit member implementations for properties in the IDerivedInterface mock are still missing, preventing full compilation. Unfortunately, I couldn’t delve deeper into the Roslyn APIs to resolve this completely.

Additional Resources

Conclusion

The PR addresses a partial part of the issue, but further adjustments are required to generate explicit implementations and ensure complete compilation.

Thank you for your time and consideration! 😊

@connorivy
Copy link
Owner

@HugoX104447

This is a very interesting edge case. Thanks for finding and reporting it.

I now understand why including the type name in the store makes sense. Apologies, I did not think about a case such as this.

I think I understand the use case for this, but to be sure, could you share some example code? It would be helpful for me to see an example method that you would want to test with this type of mock.

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

No branches or pull requests

2 participants