Skip to content

Commit

Permalink
Typename parsing cleanup (dotnet#101767)
Browse files Browse the repository at this point in the history
- Remove unnecessary System.Type wrapper for type name parsing in tools.
  Create the type directly from the parsed name.
- Rename S.R.TypeNameParser to S.R.TypeNameResolver to avoid name collisions
  with S.R.M.TypeNameParser
- Move type name Unescape and Split helpers into dedicated helper type
- Other cleanup
  • Loading branch information
jkotas authored and Ruihan-Yin committed May 30, 2024
1 parent 06be071 commit c8b6779
Show file tree
Hide file tree
Showing 35 changed files with 412 additions and 497 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeModule.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeParameterInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimePropertyInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\TypeNameParser.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\TypeNameResolver.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\RuntimeTypeMetadataUpdateHandler.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\CastHelpers.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\ICastableHelpers.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,12 @@ internal static unsafe MarshalAsAttribute GetMarshalAs(ConstArray nativeType, Ru
: Text.Encoding.UTF8.GetString(MemoryMarshal.CreateReadOnlySpanFromNullTerminated(marshalCookieRaw));

RuntimeType? safeArrayUserDefinedType = string.IsNullOrEmpty(safeArrayUserDefinedTypeName) ? null :
TypeNameParser.GetTypeReferencedByCustomAttribute(safeArrayUserDefinedTypeName, scope);
TypeNameResolver.GetTypeReferencedByCustomAttribute(safeArrayUserDefinedTypeName, scope);
RuntimeType? marshalTypeRef = null;

try
{
marshalTypeRef = marshalTypeName is null ? null : TypeNameParser.GetTypeReferencedByCustomAttribute(marshalTypeName, scope);
marshalTypeRef = marshalTypeName is null ? null : TypeNameResolver.GetTypeReferencedByCustomAttribute(marshalTypeName, scope);
}
catch (TypeLoadException)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ private static partial void GetTypeCoreIgnoreCase(QCallAssembly assembly,
{
ArgumentException.ThrowIfNullOrEmpty(name);

return TypeNameParser.GetType(name, topLevelAssembly: this,
return TypeNameResolver.GetType(name, topLevelAssembly: this,
throwOnError: throwOnError, ignoreCase: ignoreCase);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ private static object EncodedValueToRawValue(PrimitiveValue val, CustomAttribute
}
private static RuntimeType ResolveType(RuntimeModule scope, string typeName)
{
RuntimeType type = TypeNameParser.GetTypeReferencedByCustomAttribute(typeName, scope);
RuntimeType type = TypeNameResolver.GetTypeReferencedByCustomAttribute(typeName, scope);
Debug.Assert(type is not null);
return type;
}
Expand Down Expand Up @@ -899,7 +899,7 @@ private static CustomAttributeType ParseCustomAttributeType(ref CustomAttributeD
throw new BadImageFormatException();
}

enumType = TypeNameParser.GetTypeReferencedByCustomAttribute(enumTypeMaybe, module);
enumType = TypeNameResolver.GetTypeReferencedByCustomAttribute(enumTypeMaybe, module);
if (!enumType.IsEnum)
{
throw new BadImageFormatException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
{
ArgumentException.ThrowIfNullOrEmpty(className);

return TypeNameParser.GetType(className, topLevelAssembly: Assembly,
return TypeNameResolver.GetType(className, topLevelAssembly: Assembly,
throwOnError: throwOnError, ignoreCase: ignoreCase);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Reflection.Metadata;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Loader;
using System.Threading;

namespace System.Reflection
{
internal partial struct TypeNameParser
internal partial struct TypeNameResolver
{
private Func<AssemblyName, Assembly?>? _assemblyResolver;
private Func<Assembly?, string, bool, Type?>? _typeResolver;
Expand Down Expand Up @@ -55,13 +56,13 @@ internal partial struct TypeNameParser
return null;
}

Metadata.TypeName? parsed = Metadata.TypeNameParser.Parse(typeName, throwOnError: throwOnError);
TypeName? parsed = TypeNameParser.Parse(typeName, throwOnError);
if (parsed is null)
{
return null;
}

return new TypeNameParser()
return new TypeNameResolver()
{
_assemblyResolver = assemblyResolver,
_typeResolver = typeResolver,
Expand All @@ -79,7 +80,7 @@ internal partial struct TypeNameParser
bool ignoreCase,
Assembly topLevelAssembly)
{
Metadata.TypeName? parsed = Metadata.TypeNameParser.Parse(typeName, throwOnError);
TypeName? parsed = TypeNameParser.Parse(typeName, throwOnError);

if (parsed is null)
{
Expand All @@ -90,7 +91,7 @@ internal partial struct TypeNameParser
return throwOnError ? throw new ArgumentException(SR.Argument_AssemblyGetTypeCannotSpecifyAssembly) : null;
}

return new TypeNameParser()
return new TypeNameResolver()
{
_throwOnError = throwOnError,
_ignoreCase = ignoreCase,
Expand All @@ -110,8 +111,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,

RuntimeAssembly requestingAssembly = scope.GetRuntimeAssembly();

Metadata.TypeName parsed = Metadata.TypeName.Parse(typeName);
RuntimeType? type = (RuntimeType?)new TypeNameParser()
TypeName parsed = TypeName.Parse(typeName);
RuntimeType? type = (RuntimeType?)new TypeNameResolver()
{
_throwOnError = true,
_suppressContextualReflectionContext = true,
Expand Down Expand Up @@ -140,13 +141,13 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,
return null;
}

Metadata.TypeName? parsed = Metadata.TypeNameParser.Parse(typeName, throwOnError);
TypeName? parsed = TypeNameParser.Parse(typeName, throwOnError);
if (parsed is null)
{
return null;
}

RuntimeType? type = (RuntimeType?)new TypeNameParser()
RuntimeType? type = (RuntimeType?)new TypeNameResolver()
{
_requestingAssembly = requestingAssembly,
_throwOnError = throwOnError,
Expand Down Expand Up @@ -181,11 +182,10 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "TypeNameParser.GetType is marked as RequiresUnreferencedCode.")]
Justification = "TypeNameResolver.GetType is marked as RequiresUnreferencedCode.")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "TypeNameParser.GetType is marked as RequiresUnreferencedCode.")]
private Type? GetType(string escapedTypeName, // For nested types, it's Name. For other types it's FullName
ReadOnlySpan<string> nestedTypeNames, Metadata.TypeName parsedName)
Justification = "TypeNameResolver.GetType is marked as RequiresUnreferencedCode.")]
private Type? GetType(string escapedTypeName, ReadOnlySpan<string> nestedTypeNames, TypeName parsedName)
{
Assembly? assembly;

Expand Down Expand Up @@ -230,12 +230,12 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,
}
return null;
}
return GetTypeFromDefaultAssemblies(UnescapeTypeName(escapedTypeName), nestedTypeNames, parsedName);
return GetTypeFromDefaultAssemblies(TypeNameHelpers.Unescape(escapedTypeName), nestedTypeNames, parsedName);
}

if (assembly is RuntimeAssembly runtimeAssembly)
{
string unescapedTypeName = UnescapeTypeName(escapedTypeName);
string unescapedTypeName = TypeNameHelpers.Unescape(escapedTypeName);
// Compat: Non-extensible parser allows ambiguous matches with ignore case lookup
if (!_extensibleParser || !_ignoreCase)
{
Expand Down Expand Up @@ -268,7 +268,7 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,
if (_throwOnError)
{
throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType,
nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : escapedTypeName));
nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeNameHelpers.Unescape(escapedTypeName)));
}
return null;
}
Expand All @@ -277,7 +277,7 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName,
return type;
}

private Type? GetTypeFromDefaultAssemblies(string typeName, ReadOnlySpan<string> nestedTypeNames, Metadata.TypeName parsedName)
private Type? GetTypeFromDefaultAssemblies(string typeName, ReadOnlySpan<string> nestedTypeNames, TypeName parsedName)
{
RuntimeAssembly? requestingAssembly = (RuntimeAssembly?)_requestingAssembly;
if (requestingAssembly is not null)
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/System.Private.CoreLib/src/System/Type.CoreCLR.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public abstract partial class Type : MemberInfo, IReflect
public static Type? GetType(string typeName, bool throwOnError, bool ignoreCase)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark),
return TypeNameResolver.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark),
throwOnError: throwOnError, ignoreCase: ignoreCase);
}

Expand All @@ -25,7 +25,7 @@ public abstract partial class Type : MemberInfo, IReflect
public static Type? GetType(string typeName, bool throwOnError)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark),
return TypeNameResolver.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark),
throwOnError: throwOnError);
}

Expand All @@ -34,7 +34,7 @@ public abstract partial class Type : MemberInfo, IReflect
public static Type? GetType(string typeName)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark));
return TypeNameResolver.GetType(typeName, Assembly.GetExecutingAssembly(ref stackMark));
}

[RequiresUnreferencedCode("The type might be removed")]
Expand All @@ -45,7 +45,7 @@ public abstract partial class Type : MemberInfo, IReflect
Func<Assembly?, string, bool, Type?>? typeResolver)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, assemblyResolver, typeResolver,
return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver,
((assemblyResolver != null) && (typeResolver != null)) ? null : Assembly.GetExecutingAssembly(ref stackMark));
}

Expand All @@ -58,7 +58,7 @@ public abstract partial class Type : MemberInfo, IReflect
bool throwOnError)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, assemblyResolver, typeResolver,
return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver,
((assemblyResolver != null) && (typeResolver != null)) ? null : Assembly.GetExecutingAssembly(ref stackMark),
throwOnError: throwOnError);
}
Expand All @@ -73,7 +73,7 @@ public abstract partial class Type : MemberInfo, IReflect
bool ignoreCase)
{
StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller;
return TypeNameParser.GetType(typeName, assemblyResolver, typeResolver,
return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver,
((assemblyResolver != null) && (typeResolver != null)) ? null : Assembly.GetExecutingAssembly(ref stackMark),
throwOnError: throwOnError, ignoreCase: ignoreCase);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ internal static class ReflectionHelpers
// a default assembly name.
public static Type GetType(string typeName, string callingAssemblyName, bool throwOnError, bool ignoreCase)
{
return TypeNameParser.GetType(typeName, throwOnError: throwOnError, ignoreCase: ignoreCase, defaultAssemblyName: callingAssemblyName);
return TypeNameResolver.GetType(typeName, throwOnError: throwOnError, ignoreCase: ignoreCase, defaultAssemblyName: callingAssemblyName);
}

// This entry is used to implement Type.GetType()'s ability to detect the calling assembly and use it as
// a default assembly name.
public static Type ExtensibleGetType(string typeName, string callingAssemblyName, Func<AssemblyName, Assembly?> assemblyResolver, Func<Assembly?, string, bool, Type?>? typeResolver, bool throwOnError, bool ignoreCase)
{
return TypeNameParser.GetType(typeName, assemblyResolver, typeResolver, throwOnError: throwOnError, ignoreCase: ignoreCase, defaultAssemblyName: callingAssemblyName);
return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver, throwOnError: throwOnError, ignoreCase: ignoreCase, defaultAssemblyName: callingAssemblyName);
}

// This supports Assembly.GetExecutingAssembly() intrinsic expansion in the compiler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@
<Compile Include="System\Reflection\Runtime\TypeInfos\RuntimeTypeInfo.CoreGetDeclared.cs" />
<Compile Include="System\Reflection\Runtime\TypeInfos\RuntimeTypeInfo.InvokeMember.cs" />
<Compile Include="System\Reflection\Runtime\TypeInfos\RuntimeTypeInfo.TypeComponentsCache.cs" />
<Compile Include="System\Reflection\TypeNameParser.NativeAot.cs" />
<Compile Include="System\Reflection\TypeNameResolver.NativeAot.cs" />
</ItemGroup>
<ItemGroup>
<Compile Include="Internal\Reflection\Core\AssemblyBinder.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public sealed override Type GetType(string name, bool throwOnError, bool ignoreC
{
ArgumentException.ThrowIfNullOrEmpty(name);

return TypeNameParser.GetType(name,
return TypeNameResolver.GetType(name,
throwOnError: throwOnError,
ignoreCase: ignoreCase,
topLevelAssembly: this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@

using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Reflection.Metadata;
using System.Reflection.Runtime.Assemblies;
using System.Reflection.Runtime.General;

namespace System.Reflection
{
//
// Parser for type names passed to GetType() apis.
// Resolver for type names passed to GetType() apis.
//
internal partial struct TypeNameParser
internal partial struct TypeNameResolver
{
private Func<AssemblyName, Assembly?>? _assemblyResolver;
private Func<Assembly?, string, bool, Type?>? _typeResolver;
Expand Down Expand Up @@ -51,13 +52,13 @@ internal partial struct TypeNameParser
return null;
}

Metadata.TypeName? parsed = Metadata.TypeNameParser.Parse(typeName, throwOnError);
TypeName? parsed = TypeNameParser.Parse(typeName, throwOnError);
if (parsed is null)
{
return null;
}

return new TypeNameParser()
return new TypeNameResolver()
{
_assemblyResolver = assemblyResolver,
_typeResolver = typeResolver,
Expand All @@ -74,7 +75,7 @@ internal partial struct TypeNameParser
bool ignoreCase,
Assembly topLevelAssembly)
{
Metadata.TypeName? parsed = Metadata.TypeNameParser.Parse(typeName, throwOnError);
TypeName? parsed = TypeNameParser.Parse(typeName, throwOnError);

if (parsed is null)
{
Expand All @@ -85,7 +86,7 @@ internal partial struct TypeNameParser
return throwOnError ? throw new ArgumentException(SR.Argument_AssemblyGetTypeCannotSpecifyAssembly) : null;
}

return new TypeNameParser()
return new TypeNameResolver()
{
_throwOnError = throwOnError,
_ignoreCase = ignoreCase,
Expand Down Expand Up @@ -117,7 +118,7 @@ internal partial struct TypeNameParser
Justification = "GetType APIs are marked as RequiresUnreferencedCode.")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "GetType APIs are marked as RequiresUnreferencedCode.")]
private Type? GetType(string escapedTypeName, ReadOnlySpan<string> nestedTypeNames, Metadata.TypeName parsedName)
private Type? GetType(string escapedTypeName, ReadOnlySpan<string> nestedTypeNames, TypeName parsedName)
{
Assembly? assembly;

Expand Down Expand Up @@ -156,7 +157,7 @@ internal partial struct TypeNameParser
{
if (assembly is RuntimeAssemblyInfo runtimeAssembly)
{
type = runtimeAssembly.GetTypeCore(UnescapeTypeName(escapedTypeName), throwOnError: _throwOnError, ignoreCase: _ignoreCase);
type = runtimeAssembly.GetTypeCore(TypeNameHelpers.Unescape(escapedTypeName), throwOnError: _throwOnError, ignoreCase: _ignoreCase);
}
else
{
Expand All @@ -171,7 +172,7 @@ internal partial struct TypeNameParser
}
else
{
string? unescapedTypeName = UnescapeTypeName(escapedTypeName);
string? unescapedTypeName = TypeNameHelpers.Unescape(escapedTypeName);

RuntimeAssemblyInfo? defaultAssembly = null;
if (_defaultAssemblyName != null)
Expand Down Expand Up @@ -233,7 +234,7 @@ internal partial struct TypeNameParser
if (_throwOnError)
{
throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType,
nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : escapedTypeName));
nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeNameHelpers.Unescape(escapedTypeName)));
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ internal string FormatTypeNameForReflection()
[RequiresUnreferencedCode("The type might be removed")]
public static Type GetType(string typeName, bool throwOnError, bool ignoreCase)
{
return TypeNameParser.GetType(typeName, throwOnError: throwOnError, ignoreCase: ignoreCase);
return TypeNameResolver.GetType(typeName, throwOnError: throwOnError, ignoreCase: ignoreCase);
}

[Intrinsic]
Expand All @@ -108,7 +108,7 @@ public static Type GetType(string typeName, bool throwOnError, bool ignoreCase)
[RequiresUnreferencedCode("The type might be removed")]
public static Type GetType(string typeName, Func<AssemblyName, Assembly?>? assemblyResolver, Func<Assembly?, string, bool, Type?>? typeResolver, bool throwOnError, bool ignoreCase)
{
return TypeNameParser.GetType(typeName, assemblyResolver, typeResolver, throwOnError: throwOnError, ignoreCase: ignoreCase);
return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver, throwOnError: throwOnError, ignoreCase: ignoreCase);
}
}
}
Loading

0 comments on commit c8b6779

Please sign in to comment.