Removing TypeDefinition from a module will throw MemberNotImportedException on save if PreserveAll is used

[xXTurnerLP]

AsmResolver Version

6.0.0-beta.1

.NET Version

.NET 8.0

Operating System

Windows

Describe the Bug

When removing a typedef from this particular assembly and using the PreserveAll flags it will throw a bunch of MemberNotImportedException exceptions but if that flag is not used it will work as expected and the modified assembly will have the typedef removed

How To Reproduce

To reproduce it, I've managed to create this minimal example for .NET 8.0 console project on windows

static void Main(string[] args)
{
	var module = ModuleDefinition.FromFile(args[0]);

	ManagedPEImageBuilder builder = new();
	DotNetDirectoryFactory factory = new();
	builder.DotNetDirectoryFactory = factory;
	//factory.MetadataBuilderFlags = MetadataBuilderFlags.None; // uncomment this to have it working as expected
	factory.MetadataBuilderFlags = MetadataBuilderFlags.PreserveAll; // uncomment this to trigger the issue

	var a = module.TopLevelTypes.First(x => x.Name == "CopyPasteEntityInfo");
	module.TopLevelTypes.Remove(a);

	module.Write("out_Rust.Data.dll", builder);
}

After building the app drag & drop this provided dll onto the .exe or pass it as the first argument if invoking from a console or a debugger.

For completeness sake, I will provide a working compiled example:
CompiledExample.zip (needs .NET 8.0 runtime to run)

Expected Behavior

Save the module without issues

Actual Behavior

Throws:

Unhandled exception. System.AggregateException: Construction of the PE image failed with one or more errors. (Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.) (Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.) (Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.) (Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.)
 ---> AsmResolver.DotNet.Builder.Metadata.MemberNotImportedException: Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.
   --- End of inner exception stack trace ---
   at AsmResolver.DotNet.ModuleDefinition.ToPEImage(IPEImageBuilder imageBuilder, Boolean throwOnNonFatalError)
   at AsmResolver.DotNet.ModuleDefinition.ToPEImage(IPEImageBuilder imageBuilder)
   at AsmResolver.DotNet.ModuleDefinition.Write(BinaryStreamWriter writer, IPEImageBuilder imageBuilder, IPEFileBuilder fileBuilder)
   at AsmResolver.DotNet.ModuleDefinition.Write(Stream outputStream, IPEImageBuilder imageBuilder, IPEFileBuilder fileBuilder)
   at AsmResolver.DotNet.ModuleDefinition.Write(String filePath, IPEImageBuilder imageBuilder, IPEFileBuilder fileBuilder)
   at AsmResolver.DotNet.ModuleDefinition.Write(String filePath, IPEImageBuilder imageBuilder)
   at Rust.Data_Processor.src.Program.Main(String[] args) in C:\Users\xxtur\Desktop\UKNDev\ukn-anticheat-tools\Rust.Data-Processor\src\Program.cs:line 36
 ---> (Inner Exception #1) AsmResolver.DotNet.Builder.Metadata.MemberNotImportedException: Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.<---

 ---> (Inner Exception #2) AsmResolver.DotNet.Builder.Metadata.MemberNotImportedException: Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.<---

 ---> (Inner Exception #3) AsmResolver.DotNet.Builder.Metadata.MemberNotImportedException: Member ProtoBuf.CopyPasteEntityInfo (0x020000E3) was not imported into the module.<---

Additional Context

No response

[Washi1337]

This is an interesting edge-case that can be reduced to preserving StandAloneSignatures with local variable signatures referencing the removed type.

Removing a type results in this type becoming "unimported" because it is no longer present in the module. However, preserving a standalone signature that still references this type will therefore mean preserving a signature with an invalid reference to a non-existing metadata member:

AsmResolver/src/AsmResolver.DotNet/Builder/DotNetDirectoryBuffer.cs

Lines 118 to 122 in bb04d6b

	if (member.Module != Module)
	{
	ErrorListener.RegisterException(new MemberNotImportedException((IMetadataMember) member));
	return false;
	}

This issue highlights the asymmetry that exists within what it means to "preserve indices" for different metadata tables. Preserving TypeDef indices preserves RIDs by replacing removed types with inaccessible empty dummy types. Preserving standalone signatures does not replace removed signatures with dummy metadata, but preserves the unreferenced signature as a whole including its contents. This was a conscious decision to make it easier to add "floating" signatures to modules that are not directly referenced by metadata but by code (e.g., using Module::ResolveSignature).

Since we are preserving invalid signatures, should we consider this an error? If so, how should a user deal with this (other than simply ignoring the error)? We also want the user to create invalid .NET modules. Should there be an extra builder flag for this? Should the signature keep referencing the original metadata token (and thus reference the new placeholder type), even if the target type is removed?

Open to suggestions.