[API Proposal]: Simplify lookup of parsed TypeName in metadata

[jkotas]

Background and motivation

The recently introduced System.Reflection.Metadata.TypeName is likely to be frequently combined with lookup of the type in metadata. The information required to lookup a type in ECMA335 metadata tables is unescaped (namespace, name) tuple. This information is not provided by the TypeName type today. In order to lookup the parser type name in metadata, the consumers have to implement unescaping and full type name splitting helper methods to convert the information provided TypeName this tuple.

API Proposal

  namespace System.Reflection.Metadata;

  public class TypeName
  {
      // Existing APIs. Return escaped names (to match reflection), no API to get Namespace only.
      public string Name { get; }
      public string FullName { get; }
      
      // Returns escaped namespace. (Same API exists on System.Type today.)
+    public string Namespace { get; }

      // Unescapes 
+    public static string Unescape(string name);
  }

API Usage

CustomType ResolveType(TypeName typeName)
{
    if (typeName.IsSimple)
    {
        if (typeName.IsNested)
        {
            string nestedName = TypeName.Unescape(typeName.Name);
            ... use nestedName to lookup the type in metadata ...
        }
        else
        {
            string typeNamespace = TypeName.Unescape(typeName.Namespace);
            string typeName = TypeName.Unescape(typeName.Name);
            ... use typeNamespace and typeName to lookup the type in metadata ...
        }
    }

    ... handle other cases ...
}

Alternative Designs

Alt option 1: Introduce a method that returns an unescaped (namespace, name) tuple.

Alt option 2: Change the Namespace and Name methods to return unescaped names (different from reflection). It would avoid need to introduce Unescape method.

Risks

No response

[jkotas]

I have filled this proposal based on my experience with implementing lookup of parsed TypeName in tools #101767.

@adamsitnik Any thoughts about this API?

[KalleOlaviNiemitalo]

ECMA-335 6th edition §II.22.14 says

The full name of the type need not be stored directly. Instead, it can be split into two parts at any included “.” (although typically this is done at the last “.” in the full name). The part preceding the “.” is stored as the TypeNamespace and that following the “.” is stored as the TypeName. If there is no “.” in the full name, then the TypeNamespace shall be the index of the empty string.

The proposed GetMetadataNamespaceAndName() API looks like it could easily mislead callers into not recognizing types whose names have been split at a “.” other than the last one.

[jkotas]

Good point. .NET Framework and .NET Core never followed this rule in practice. The full type name is expected to be split in namespace + name using this algorithm. We may want to add a note to https://github.com/dotnet/runtime/blob/main/docs/design/specs/Ecma-335-Augments.md .

[adamsitnik]

Returns (unescaped namespace, unescaped name) tuple for simple types.

Does it always return Name or FullName? Or does it depend on the nested vs non-nested?

tuple

This particular library can be consumed by Full Framework apps and they often struggle to use ValueTuple (dotnet/BenchmarkDotNet#895, dotnet/BenchmarkDotNet#667)

Throws exception otherwise.

I see why low level metadata parsers need that, but I find it too specific to particular use case.

What would we benefit from exposing such public API?

[jkotas]

Does it always return Name or FullName? Or does it depend on the nested vs non-nested?

For non-nested types, it returns unescaped FullName split into name and namespace.
For nested types, it returns unescaped Name (nested types do not have namespace)

This particular library can be consumed by Full Framework apps and they often struggle to use ValueTuple

I do not mind changing this to whatever else API review prefers; or expose the static helper methods instead.

I see why low level metadata parsers need that, but I find it too specific to particular use case.

System.Reflection.Metadata is a library for low-level metadata readers. Every tool or library that reads metadata in its entirety using this library needs this functionality. Every shipping TypeName use in this repo needs it so far.

Also, I was looking at converting

runtime/src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/General/Helpers.cs

Lines 293 to 330 in 71abc26

	public static RoType? LoadTypeFromAssemblyQualifiedName(string name, RoAssembly defaultAssembly, bool ignoreCase, bool throwOnError)
	{
	if (!name.TypeNameContainsTypeParserMetacharacters())
	{
	// Fast-path: the type contains none of the parser metacharacters nor the escape character. Just treat as plain old type name.
	name.SplitTypeName(out string ns, out string simpleName);
	RoType? type = defaultAssembly.GetTypeCore(ns, simpleName, ignoreCase: ignoreCase, out Exception? e);
	if (type != null)
	return type;
	if (throwOnError)
	throw e!;
	}
	MetadataLoadContext loader = defaultAssembly.Loader;
	Func<AssemblyName, Assembly> assemblyResolver =
	loader.LoadFromAssemblyName;
	Func<Assembly?, string, bool, Type?> typeResolver =
	delegate (Assembly? assembly, string fullName, bool ignoreCase2)
	{
	assembly ??= defaultAssembly;
	Debug.Assert(assembly is RoAssembly);
	RoAssembly roAssembly = (RoAssembly)assembly;
	fullName = fullName.UnescapeTypeNameIdentifier();
	fullName.SplitTypeName(out string ns, out string simpleName);
	Type? type = roAssembly.GetTypeCore(ns, simpleName, ignoreCase: ignoreCase2, out Exception? e);
	if (type != null)
	return type;
	if (throwOnError)
	throw e!;
	return null;
	};
	return (RoType?)Type.GetType(name, assemblyResolver: assemblyResolver, typeResolver: typeResolver, throwOnError: throwOnError, ignoreCase: ignoreCase);
	}

to use TypeName. It would need to include the .cs files with the static helper methods in yet another place if I do that.

[adamsitnik]

System.Reflection.Metadata is a library for low-level metadata readers.

You have convinced me 👍

FWIW the alternative design for me:

namespace System.Reflection.Metadata;

public class TypeName
{
     public string Name { get; }
     public string FullName { get; }
    
+    public string Namespace { get; }
+    public static string Unescape(string name);
}

[jkotas]

Update the proposal at the top. I think it is a better starting point for the API review discussion.

[steveharter]

Moving to v10 since we have reached the "feature complete" milestone; this is still on the API review backlog.

[bartonjs]

Video

• System.Type.Namespace is a nullable string property, and seems to have distinction between null (Generic type parameters) and empty (global namespace types); it was asked if we need the same semantics here. The answer was "no, this will always return empty".
• It was asked if we need a static string Escape(string name) for symmetry, and the answer was "no, call the CreateSimpleTypeName API and then read the appropriate name property from the result".

namespace System.Reflection.Metadata;

public partial class TypeName
{      
    public string Namespace { get; }

    public static string Unescape(string name);
}