Add TypeName APIs to simplify metadata lookup.

[teo-tsirpanis]

Fixes #101774.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[jkotas]

Could you please delete

runtime/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

Lines 1492 to 1494 in 29013d8

	<Compile Include="$(CommonPath)System\Reflection\Metadata\TypeNameHelpers.cs">
	<Link>Common\System\Reflection\Metadata\TypeNameHelpers.cs</Link>
	</Compile>

and replace the calls to it with this API?

(Other uses of TypeNameHelpers.cs should be delete too once these APIs are available in the LKG runtime.)

Copilot reviewed 5 out of 17 changed files in this pull request and generated no comments.

Files not reviewed (12)

• src/coreclr/tools/ILVerification/ILVerification.projitems: Language not supported
• src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj: Language not supported
• src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj: Language not supported
• src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems: Language not supported
• src/tools/illink/src/linker/Mono.Linker.csproj: Language not supported
• src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs: Evaluated as low risk
• src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs: Evaluated as low risk
• src/tools/illink/src/linker/Linker/TypeNameResolver.cs: Evaluated as low risk
• src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs: Evaluated as low risk
• src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs: Evaluated as low risk
• src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs: Evaluated as low risk
• src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs: Evaluated as low risk

[teo-tsirpanis]

There are some failing tests that needs updating. This change is fixing .NET Framework -> .NET Core regression in type name parsing. This change is going back to .NET Framework behavior that made more sense.

We fail to parse .Outside`1+.Inside`1, which looks related to this comment. I don't understand what do I need to change.

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs

Lines 253 to 256 in 843f798

	// Compat: Ignore leading '.' for type names without namespace. .NET Framework historically ignored leading '.' here. It is likely
	// that code out there depends on this behavior. For example, type names formed by concatenating namespace and name, without checking for
	// empty namespace (bug), are going to have superfluous leading '.'.
	// This behavior means that types that start with '.' are not round-trippable via type name.

[jkotas]

We fail to parse .Outside1+.Inside1, which looks related to this comment. I don't understand what do I need to change.

I think we should match the .NET Framework behavior for these corner cases. For example:

Type.GetType("A+.B", throwOnError: true);
// Type.GetType("A+Whatever.B", throwOnError: true); - this should fail too, just like it failed in .NET Framework

public class A
{
    public class B
    {
    }
}

failed in .NET Framework. It "works" in .NET Core today, but I do not think that it should. I would make it fail again and update the tests as necessary.

[teo-tsirpanis]

Updated tests to account for .Outside`1+.Inside`1 failing to parse.

[jkotas]

Could you please take a look at the Mono test failures?

[jkotas]

Looks good to me. @adamsitnik Could you please review this change as well? We are changing the type name parsing behavior for nested types with its own namespaces to fix .NET Framework -> .NET Core type name parsing regression and to better match reflection behavior.

[teo-tsirpanis]

    System.Tests.TypeTests.GetTypeByName_Invalid(typeName: ".Outside`1+.Inside`1", expectedException: typeof(System.TypeLoadException), alwaysThrowsException: False) [FAIL]
      Assert.Null() Failure: Value is not null
      Expected: null
      Actual:   typeof(Outside<,>)
      Stack Trace:
        /_/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs(547,0): at System.Tests.TypeTests.GetTypeByName_Invalid(String typeName, Type expectedException, Boolean alwaysThrowsException)
           at System.Object.InvokeStub_TypeTests.GetTypeByName_Invalid(Object , Span`1 )
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(136,0): at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

[teo-tsirpanis]

Mono thinks GetNestedType accepts a full name, while it does not per the documentation.

runtime/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs

Lines 1011 to 1018 in 8eab62c

	public override Type? GetNestedType(string fullname, BindingFlags bindingAttr)
	{
	ArgumentNullException.ThrowIfNull(fullname);
	bindingAttr &= ~BindingFlags.Static;
	string? name, ns;
	MemberListType listType;
	SplitName(fullname, out name, out ns);

[jkotas]

Mono thinks GetNestedType accepts a full name, while it does not per the documentation.

Looks like something that should be cleaned up. Do you have a good idea what it needs to be done?

[teo-tsirpanis]

I tried something. Let's see.

[adamsitnik]

LGTM, thank you for your contribution @teo-tsirpanis !

And big thanks to @jkotas for the review!

[adamsitnik]

According to Copilot, IL allows for creating a type with a name that contains a dot that is not treated as a namespace separator. Example: Namespace.Type.Name

I've tried to repro it with ILEmit, but it treats dots as namespace separators:

using System.Reflection;
using System.Reflection.Emit;

AssemblyBuilder assembly = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("AssemblyName"), AssemblyBuilderAccess.Run);
ModuleBuilder module = assembly.DefineDynamicModule("ModuleName");

Type typeWithDot = module.DefineType("Namespace.Type.Name").CreateType();
Console.WriteLine($"FullName: {typeWithDot.FullName}");
Console.WriteLine($"Namespace: {typeWithDot.Namespace}");

FullName: Namespace.Type.Name
Namespace: Namespace.Type

Of course I am not trying to say that we should change the implementation. I just wonder if it's worth adding a remark in the xml docs? Or at least a comment (that would also mention that we don't need to worry about escape characters here?) cc @jkotas

[jkotas]

According to Copilot, IL allows for creating a type with a name that contains a dot that is not treated as a namespace separator.

That does not sound correct to me. The namespace/name splitting of the full name is really just a metadata encoding optimization to avoid storing namespaces repeatedly. The ECMA-335 says: "While some programming languages introduce the concept of a namespace, the only support in the CLI
for this concept is as a metadata encoding technique. Type names are always specified by their full
name relative to the assembly in which they are defined.".

Unfortunately, ECMA-335 does not specify the namespace/name splitting algorithm. It says that you can use any dot for the split, but that makes things complicated, so it is not used in practice.

The namespace/name splitting algorithm that has been used in .NET runtime since forever is:

The last dot in the name of non-nested type is always treated as a namespace separator, except when there is a dot immediately preceding the last dot - the second to last dot is treated as a namespace separator in that case instead.

If you use the reflection emit test above to explore various cases, you should see following behavior:
Namespace..Name -> namespace=Namespace, name = .Name
Namespace...Name -> namespace=Namespace., name = .Name
Namespace..Name. -> namespace=Namespace..Name, name = ``

@teo-tsirpanis Could you please add coverage for these cases?

I think we should fix the type name parser implementation to follow it to avoid inconsistencies. This algorithm is implemented by

runtime/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs

Lines 56 to 77 in d20c35e

	internal static (string typeNamespace, string name) Split(string typeName)
	{
	string typeNamespace, name;
	// Matches algorithm from ns::FindSep in src\coreclr\utilcode\namespaceutil.cpp
	// This could result in the type name beginning with a '.' character.
	int separator = typeName.LastIndexOf('.');
	if (separator <= 0)
	{
	typeNamespace = "";
	name = typeName;
	}
	else
	{
	if (typeName[separator - 1] == '.')
	separator--;
	typeNamespace = typeName.Substring(0, separator);
	name = typeName.Substring(separator + 1);
	}
	return (typeNamespace, name);
	}

that we hope to replace by the new APIs.

[teo-tsirpanis]

Done.

[jkotas]

Should this check be done on rootTypeName?

It does not make sense for "MyNamespace.A+B[]" to return MyNamespace when "MyNamespace.A+B" fails with InvalidOperationException.

[jkotas]

One test is still failing on Mono

[teo-tsirpanis]

One test is still failing on Mono

I took another look at the sources and have no idea on why it fails. Might try debugging it, once I manage to build Mono on Windows.