ILSpy icon indicating copy to clipboard operation
ILSpy copied to clipboard

ILSpy decides to fully qualify type name when it is not necessary

Open ElektroKill opened this issue 3 years ago • 2 comments

Input code

Input assembly with dependencies: sample.zip

Source code:

using System.Collections.Generic;
using System.Linq;
using dnlib.DotNet;

namespace ILSpyFullyQualifyTestCase {
	public class Class1 {
		public static void TestMethod(ModuleDefMD md) {
			List<AssemblyRef> list = md.GetAssemblyRefs().ToList();
		}
	}
}

With dnlib 3.5.0 as nuget package.

Erroneous output

ILSpy fully qualifies the AssemblyRef type reference when it is not necessary. This adds clutter to the decompiled code.

using System.Collections.Generic;
using System.Linq;
using dnlib.DotNet;

public class Class1
{
	public static void TestMethod(ModuleDefMD md)
	{
		List<dnlib.DotNet.AssemblyRef> list = md.GetAssemblyRefs().ToList();
	}
}

Details

  • Product in use: ILSpy
  • Version in use: ILSpy version 8.0.0.7064-preview1

ElektroKill avatar Jul 09 '22 19:07 ElektroKill

Is there a chance for this issue to be fixed in the upcoming previews for ILSpy 8?

ElektroKill avatar Sep 17 '22 12:09 ElektroKill

I did a bit of debugging on this issue. It looks like the problem lies in the CSharpResolver.LookupSimpleNameOrTypeName method which incorrectly identifies the internal AssemblyRef class in System.Core as a candidate when resolving the AssemblyRef identifier. The precise method where this happens is LookInUsingScopeNamespace. This lookup causes ILSpy to generate a fully qualified name. This conclusion is incorrect as the AssemblyRef class in System.Core is inaccessible from Class1.TestMethod in compiled assembly as the class is marked internal and System.Core does not have any InternalsVisibleTo attributes defined.

Is CSharpRessolver supposed to return inaccessible types when instructed to resolve an identifier? I think an appropriate fix for this issue would be to ignore inaccessible types when determining whether a type reference is ambiguous.

A potential solution I came up with: https://github.com/icsharpcode/ILSpy/blob/5ac135095cf91287c3568881b777c45284cda784/ICSharpCode.Decompiler/CSharp/Resolver/CSharpResolver.cs#L1761-L1764 The if condition here could be updated to if (def != null && TopLevelTypeDefinitionIsAccessible(def)). Is this an appropriate fix?

ElektroKill avatar Sep 18 '22 16:09 ElektroKill