Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Closed
ElektroKill opened this issue Jul 9, 2022 · 2 comments · Fixed by #2789
Closed

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

ElektroKill opened this issue Jul 9, 2022 · 2 comments · Fixed by #2789
Labels
Bug Decompiler The decompiler engine itself

Comments

@ElektroKill
Copy link
Contributor

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 ElektroKill added Bug Decompiler The decompiler engine itself labels Jul 9, 2022
@ElektroKill
Copy link
Contributor Author

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

@ElektroKill
Copy link
Contributor Author

ElektroKill commented Sep 18, 2022

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:

// then look for a type
ITypeDefinition def = n.GetTypeDefinition(identifier, k);
if (def != null)
{

The if condition here could be updated to if (def != null && TopLevelTypeDefinitionIsAccessible(def)).
Is this an appropriate fix?

ElektroKill added a commit to ElektroKill/ILSpy that referenced this issue Sep 18, 2022
dgrunwald added a commit that referenced this issue Sep 23, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 23, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Bug Decompiler The decompiler engine itself
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant