Weaver is changing signature of overwritten method

[mkaring]

Hello,

your weaver is working nicely all in all. How ever I encountered an problem where the weaver is changing the signature of an method that is declared in another assembly.

For example: Assembly A declares:

public Class1
{
    protected virtual ResourceManager GetResourceManager() { }
}

This is an API not developed by me and so I can't change it.

My assembly B overwrites this method:

public Class2 : Class1
{
    protected overrides ResourceManager GetResourceManager() {
        return …
    }
}

Now the ResourceManager class is weaved in assembly B to MyCustomResourceManager, so I can exchange the ResourceManager created from the *.resx files.

The problem is that the weaver changes the method mentioned above to

public Class2 : Class1
{
    protected overrides MyCustomResourceManager GetResourceManager() {
        return …
    }
}

This results in the method not properly overwriting the method anymore and so the method is not called anymore by the external API.

There are multiple solution to this problem from my point of view.

1. Allow the SustitutionAttribute to be applied only to specific classes/methods/properties/fields.
2. Create a new attribute that allows disabling the substitution for specific classes/methods/properties/fields.
3. Implement a possibility to limit the substitution to internal API.
4. Detect automatically if the method/property that is about to be altered is overwriting or implementing an method/property that is not subject to the substitution and disable the substitution for the signature of this method/property.

What do you think is the most feasible way to support this case?

[tom-englert]

To me a combination of 2. and 4. would make the most sense. Can you provide a PR?

Just being curious: Where do you get the external API from? Is this a public package?

[mkaring]

It is the API of Autodesk AutoCAD Map 3D. So it's not free to use... but you can get your hands on it.

I have some ideas on how to implement this. I'll send a PR your way, but it will take a couple of days.

[mkaring]

@tom-englert Is there any trick in getting the unit tests to work?

[tom-englert]

@mkaring they all work for me local and on the build server - what is your problem?

[mkaring]

Nevermind. My PeVerifier was talking German. Fody does not like that.

[tom-englert]

@mkaring looking at your sample again: wouldn't it solve the problem already if you move Class2 into a different assembly?

[mkaring]

@tom-englert That would work in case Class2 would only contain this one method. But it contains many more methods. And many of them use the generated class Properties.Resources where I want to change the ResourceManager as well.

[tom-englert]

@mkaring hard to tell without seeing the code. I assume you will setup the tests for your PR to resemble exactly this scenario, so it will be much clearer then.

[tom-englert]

Looking at your sample the core problem is that you derive the substitute from the class to be substituted. This is not a good idea, both classes should be unrelated.

Doing it this way should work:

namespace AssemblyToProcess
{
    #region External Stuff
    public abstract class ExternalClass
    {
        public abstract ExternalDataProvider GetDataProvider();
        public string GetText() => GetDataProvider().GetText();
    }
    public class ExternalDataProvider
    {
        public virtual string GetText() => "Test";
    }
    #endregion

    public class CustomClass : ExternalClass
    {
        public override ExternalDataProvider GetDataProvider() => new QuotingDataProvider();
    }

    public class QuotingDataProvider : ExternalDataProvider
    {
        public override string GetText() => "\"" + base.GetText() + "\"";
    }

    public class SubstituteClass
    {
        private ExternalClass _inner = new CustomClass();

        public string GetText() => _inner.GetText();
    }

    [assembly: Substitute(typeof(ExternalClass), typeof(SubstituteClass))]
}

[mkaring]

This is impossible in my use case, because the “CustomClass“ has to inherit the “ExternalClass“, because it is created by reflection from the external API.

[mkaring]

Some more explanation. The specific use case is the following:

The "ExternalClass" is actually a "ResourceManager". The resource manager is provided by the class that is generated by Visual Studio. The external API requires an System.Resources.ResourceManager to be returned. So in this case the ResourceManager instance that is part of the generated code is usually returned. Now I have to apply a specific transformation (replacing some placeholders that are in the resource strings) to all resources that are returned by the ResourceManager. For this purpose I got a own implementation of the ResourceManager that applies the transformation to every resource string that is returned. So the substituted class is my TransformatingResourceManager. Since the returning the API requires to return a ResourceManager it has to extend this class.

I could just create the correct instance of the of the resource manager without using the Substitution, but in the DLL in question there are hundreds of other invocations of the generated Resource class, that contains the default ResourceManager. I need to call the function that applies the transformation every time I fetch a string from the generated class. My idea what to replace the ResourceManager in the generated Resources class, to avoid having to call this function every time.

But as it is right now the Substitution breaks the signature of the overwritten method and so the class becomes invalid, because the implementation does not override an abstract method.

Splitting the classes that can be Substituted and the ones that are broken by the Substitution does not work because those classes depend on each other and need to be in the same assembly.

[tom-englert]

The problem that Substitute.Fody is addressing is to intercept non-virtual methods.

As you see in the sample, method ApplyResources is not virtual, so to make existing code calling MyImplementation.ApplyResources, all calls to the method have to be replaced. If MyImplementation would derive from ComponentResourceManager, and some code would allow to cast MyImplementation to ComponentResourceManager, that code would then call ComponentResourceManager.ApplyResources instead of MyImplementation.ApplyResources, bypassing my custom implementation - and because of this it can't be allowed, since it would break everything.

If, as in your sample, the methods of interest are all virtual (e.g. ResourceManager.GetString(...)), you simply have to replace any call to = new ResourceManager() with = (ResourceManager) new MyResourceManager(), and it will start working - you don't have to tweak any code at all!

So Substitute.Fody is definitely not what you're looking for. You'd need something more simple.

[mkaring]

Not sure how that would work. The ResourceManager I want to replace is effectively this one. This way I want to have this property to return the modified resources. In the same assembly there is the implementation of the external api for has a ResourceManager as return value. And this one gets substituted as well. So I would be good with any way to stopping Substitute from modifying the ResourceManager in the return value of the overwritten API.

[tom-englert]

To solve this, it's easier to change the code generator, to get control of the created resource manager: https://github.com/tom-englert/ResXResourceManager/issues/165 If you translate the .cs templates to .vb templates, you can help solving the above issue. Substitute is only required when you deal with WinForms resources, where you can't get hold of the generated code.

[tom-englert]

See also https://github.com/tom-englert/ResXResourceManager/wiki/Custom-Code-Generator