Importing a nested type loses declaring type information

[SamboyCoding]

Describe the bug

Calling ReferenceImporter::ImportType on a type definition with a DeclaringType set results in a TypeReference with the resolution scope set to the module, not to the declaring type, so the TypeReference does not inherit the declaring type and an invalid type reference is emitted to the assembly.

To Reproduce

1. Set the base type of an type to be an imported reference to a nested type in another assembly
2. Attempt to open the type in ILSpy
3. ILSpy throws an exception (at least in C# mode, IL mode is fine)

Expected behavior

The type can be loaded successfully by ILSpy or Mono.Cecil, or indeed AsmResolver, and its base type can be resolved.

Screenshots

(Excuse the obfuscation)

ILSpy isn't loading these nested types because they have unresolvable base types

You can see the type is nested (hence the +) but it's being imported with the scope = module

Resulting type reference with no declaring type and scope = assembly reference.

Platform

• OS: Windows 11
• AsmResolver Version: 4.9.0

Additional context
N/A

[SamboyCoding]

I should add that the net result of this is that, when trying to resolve the type using e.g. Cecil, it looks for a top-level type with the name of the nested type, which obviously doesn't exist.

[Washi1337]

Interesting, seems nobody has ever tried to import a nested type definition (including myself and the unit tests). Potential fix is in #275.

[SamboyCoding]

Testing now

[SamboyCoding]

Yes I can confirm this fixes the issue. Thanks for the quick turnaround!

[SamboyCoding]

Any chance this could possibly become a 4.9.1 bugfix release? It's kind of a major blocker for a release I was hoping to get out today.

Edit: Or alternatively, if 4.10.0 is going to be reasonably soon, don't worry about it, I can wait a couple days.

[Washi1337]

I typically only reserve hotfixes for critical bugs that were introduced by changes in the previous release. I wouldn't classify this issue as one; it hasn't been reported until now despite being present since probably the beginning of 4.0.0, and it can be relatively easily worked around.

I also typically don't try to merge my own PRs until I looked over the code at least one more time with a fresh pair of eyes (typically with at least a night sleep in between). As you have seen with this issue, while there are many unit tests in AsmResolver, they unfortunately still do not cover every use case there is.

Besides, I have a feeling you will be finding more bugs hidden features relatively soon anyways, which we may be able to batch into the same release ;)

Your bughunting is greatly appreciated btw!

[SamboyCoding]

Well I can at least confirm that with the combination of all the fixes you've done so wonderfully over the last day-and-a-half, I'm getting the same output now as I was under cecil - or at least close enough that downstream tools don't pick up any difference - so I'm not expecting to find any more 'hidden features' in the next few days. We'll see how that opinion changes once I start actually populating method bodies haha (though I don't expect this to be for several weeks at the very least, yet)