#110: Made type lookup more robust. Fixed exception on .NET Standard …

[mstyura]

…2.0 libraries.

[AppVeyorBot]

✅ Build Anotar 3.3.2-PullRequest.111+2.build.31 completed (commit 50c5bd688d by @mstyura)

[tpluscode]

Hey @mstyura. Thank you so much for taking the time to add this fix.

Could you please make the tests run also against .net core 2.0? You'll have to add a netcoreapp20 build target to the XAssemblyToProcess and have tests code run the weaver against it. Here's where I set it for Serilog. Similarly for Custom.

[mstyura]

@tpluscode I can do this (and already roughly tried), but what I can say is that issue is only reproducible during actual build. But not via test. So there is some critical different between test and actual build. But I'l add netstandard2.0 target to project and test (for other tests I guess it would be valuable).

[tpluscode]

Interesting. Are you saying that current tests ran against netcoreapp20 would not fail? I'm going to try that myself right now

[tpluscode]

These sets of FirstOrDefault are repeated a few times. Please extract these fragments including the null check and Debug.Write into a method. Maybe something like GetTypeFromFirst(params AssemblyReference[] candidateAssemblies)

[mstyura]

No problem, will do

[mstyura]

@tpluscode either I did wrong changes in my tests for .netstandard2.0 so the did not fail, or the test environment is different from build environment.

[mstyura]

@tpluscode have you managed to reproduce the original issue in your environment during build and test?

[tpluscode]

Unfortunately not. That shouldn't stop me from merging this.

[tpluscode]

Actually, you wouldn't have a repro of this, would you?

[mstyura]

@tpluscode I'm going to dive deep into how the current implementation of unit tests works to find out why they does not detect problem

[mstyura]

So the key difference is different AssemblyResolver used during unit test and during MSBuild execution.
When weaver is executed under MSBuild the AssemblyResolver has type AssemblyResover from assembly FodeIsoleated. Unit tests have its own relatively simple MockAssemlbyResolver.
So the key thing is that during unit tests mscorlib is resolved to assembly from CLR under which test is executed. On my machine it is mscorlib is come from regular desktop .NET Framework and has all types like System.Object, System.String and etc in it.
When weaver is executed under MSBuild mscorlib is resolved to %USERPOFILE% .nuget\packages\netstandard.library\2.0.0\build\netstandard2.0\\ref\mscorlib.dll. This assembly doesn't have any type definition except type forwarding like [assembly: TypeForwardedTo(typeof(Action))].
So, there are three ways I see to fix the issue:

1. use the same resolver as Fody itself when it execute weavers (tests will reveal such problems in future)
2. add handling of forwarded types (will fix execution on existing and possibly new runtimes)
3. implement both 1 & 2 solutions.

I think the best option would be 3, but I'll start with 2 since it looks like the easiest one.

[tpluscode]

Great findings. Do you think it will be a lot of code? Maybe let's have it in a new branch?

[mstyura]

@tpluscode don't think that solution 1 would be easy, so dedicated branch (and probably github issue) is good way to tackle the problem. The solution 2: type lookup among type-forwarded types already implemented in this merge request and ready to be merged.

[mstyura]

@tpluscode would you mind publishing beta package to nuget with merged changes?

[tpluscode]

Try the latest prerelease