From a573f96d83b5c2864810421eac7673d078d7570e Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 02:33:14 +0200 Subject: [PATCH 01/35] Add `TypeName.Namespace` and tests. --- .../ref/System.Reflection.Metadata.cs | 1 + .../System/Reflection/Metadata/TypeName.cs | 42 ++++++++++++++++++- .../Metadata/TypeNameParserHelpers.cs | 29 +++++++++++++ .../Metadata/TypeNameParserHelpersTests.cs | 12 ++++++ .../tests/Metadata/TypeNameTests.cs | 11 +++-- 5 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index a57eee34a15ab2..62c9cd41230aae 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2437,6 +2437,7 @@ internal TypeName() { } public bool IsSZArray { get { throw null; } } public bool IsVariableBoundArrayType { get { throw null; } } public string Name { get { throw null; } } + public string Namespace { get { throw null; } } public int GetArrayRank() { throw null; } public System.Reflection.Metadata.TypeName GetElementType() { throw null; } public System.Collections.Immutable.ImmutableArray GetGenericArguments() { throw null; } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index a631576843f90d..cb0116bd023dfe 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -39,7 +39,7 @@ sealed class TypeName #else private readonly ImmutableArray _genericArguments; #endif - private string? _name, _fullName, _assemblyQualifiedName; + private string? _name, _namespace, _fullName, _assemblyQualifiedName; internal TypeName(string? fullName, AssemblyNameInfo? assemblyName, @@ -284,6 +284,46 @@ public string Name } } + /// + /// The namespace of this type; e.g., "System". + /// + public string Namespace + { + get + { + if (_namespace is null) + { + TypeName rootTypeName = this; + while (true) + { + if (IsConstructedGenericType) + { + rootTypeName = GetGenericTypeDefinition(); + } + else if (IsPointer || IsByRef || IsArray) + { + rootTypeName = GetElementType(); + } + else + { + break; + } + } + + // At this point the type does not have a modifier applied to it, so it should have its full name initialized. + Debug.Assert(rootTypeName._fullName is not null); + ReadOnlySpan rootFullName = rootTypeName._fullName.AsSpan(); + if (rootTypeName._nestedNameLength > 0) + { + rootFullName = rootFullName.Slice(0, rootTypeName._nestedNameLength); + } + _namespace = TypeNameParserHelpers.GetNamespace(rootFullName).ToString(); + } + + return _namespace; + } + } + /// /// Represents the total number of instances that are used to describe /// this instance, including any generic arguments or underlying types. diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 7cafd746b7d176..69dfa7dac12bc0 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -102,6 +102,35 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; } + internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) + { + int offset = fullName.LastIndexOf('.'); + + if (offset > 0 && fullName[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) + { + offset = GetUnescapedOffset(fullName, startIndex: offset); + } + + return offset < 0 ? [] : fullName.Slice(0, offset); + + static int GetUnescapedOffset(ReadOnlySpan fullName, int startIndex) + { + int offset = startIndex; + for (; offset >= 0; offset--) + { + if (fullName[offset] == '.') + { + if (offset == 0 || fullName[offset - 1] != EscapeCharacter) + { + break; + } + offset--; // skip the escaping character + } + } + return offset; + } + } + internal static ReadOnlySpan GetName(ReadOnlySpan fullName) { // The two-value form of MemoryExtensions.LastIndexOfAny does not suffer diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 01ec45b50cc3e1..15ff2afe881106 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -37,6 +37,18 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected Assert.Equal(expectedIsNested, isNested); } + [Theory] + [InlineData("JustTypeName", "")] + [InlineData("Namespace.TypeName", "Namespace")] + [InlineData("Namespace1.Namespace2.TypeName", "Namespace1.Namespace2")] + [InlineData("Namespace.NotNamespace\\.TypeName", "Namespace")] + [InlineData("Namespace1.Namespace2.Containing+Nested", "Namespace1.Namespace2")] + [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] + [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "")] + [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "")] + public void GetNamespaceReturnsJustNamespace(string fullName, string expected) + => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); + [Theory] [InlineData("JustTypeName", "JustTypeName")] [InlineData("Namespace.TypeName", "TypeName")] diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index dcf46d54d2d958..3f7d18850091f1 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -13,13 +13,14 @@ namespace System.Reflection.Metadata.Tests public class TypeNameTests { [Theory] - [InlineData(" System.Int32", "System.Int32", "Int32")] - [InlineData(" MyNamespace.MyType+NestedType", "MyNamespace.MyType+NestedType", "NestedType")] - public void SpacesAtTheBeginningAreOK(string input, string expectedFullName, string expectedName) + [InlineData(" System.Int32", "System.Int32", "System", "Int32")] + [InlineData(" MyNamespace.MyType+NestedType", "MyNamespace.MyType+NestedType", "MyNamespace", "NestedType")] + public void SpacesAtTheBeginningAreOK(string input, string expectedFullName, string expectedNamespace, string expectedName) { TypeName parsed = TypeName.Parse(input.AsSpan()); Assert.Equal(expectedName, parsed.Name); + Assert.Equal(expectedNamespace, parsed.Namespace); Assert.Equal(expectedFullName, parsed.FullName); Assert.Equal(expectedFullName, parsed.AssemblyQualifiedName); } @@ -32,6 +33,7 @@ public void LeadingDotIsNotConsumedForFullTypeNamesWithoutNamespace() TypeName parsed = TypeName.Parse(".NoNamespace".AsSpan()); Assert.Equal("NoNamespace", parsed.Name); + Assert.Empty(parsed.Namespace); Assert.Equal(".NoNamespace", parsed.FullName); Assert.Equal(".NoNamespace", parsed.AssemblyQualifiedName); } @@ -698,6 +700,7 @@ private static void VerifyNestedNames(TypeName parsed, TypeName made, AssemblyNa while (true) { Assert.Equal(parsed.Name, made.Name); + Assert.Equal(parsed.Namespace, made.Namespace); Assert.Equal(parsed.FullName, made.FullName); Assert.Equal(assemblyName, made.AssemblyName); Assert.NotEqual(parsed.AssemblyQualifiedName, made.AssemblyQualifiedName); @@ -792,6 +795,7 @@ public void ParsedNamesMatchSystemTypeNames(Type type) Type genericType = type.GetGenericTypeDefinition(); TypeName genericTypeName = parsed.GetGenericTypeDefinition(); Assert.Equal(genericType.Name, genericTypeName.Name); + Assert.Equal(genericType.Namespace, genericTypeName.Namespace); Assert.Equal(genericType.FullName, genericTypeName.FullName); Assert.Equal(genericType.AssemblyQualifiedName, genericTypeName.AssemblyQualifiedName); } @@ -965,6 +969,7 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) { Assert.Equal(type.AssemblyQualifiedName, typeName.AssemblyQualifiedName); Assert.Equal(type.FullName, typeName.FullName); + Assert.Equal(type.Namespace, typeName.Namespace); Assert.Equal(type.Name, typeName.Name); #if NET From dfe213ff490bd998f0e072c9d3a4650bf1295a37 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 03:11:28 +0200 Subject: [PATCH 02/35] Add `TypeName.Unescape`. --- .../System/Reflection/Metadata/TypeName.cs | 16 ++++++ .../Metadata/TypeNameParserHelpers.cs | 57 +++++++++++++++++-- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index cb0116bd023dfe..a125e9752ccf9f 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -441,6 +441,22 @@ public static bool TryParse(ReadOnlySpan typeName, [NotNullWhen(true)] out return result is not null; } + /// + /// Converts any escaped characters in the input type name or namespace. + /// + /// The input string containing the name to convert. + /// A string of characters with any escaped characters converted to their unescaped form. + /// The unescaped string can be used for looking up the type name or namespace in metadata. + public static string Unescape(string name) + { + if (name is null) + { + TypeNameParserHelpers.ThrowArgumentNullException(nameof(name)); + } + + return TypeNameParserHelpers.Unescape(name); + } + /// /// Gets the number of dimensions in an array. /// diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 69dfa7dac12bc0..866e5dfb3d5ba4 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -16,8 +16,10 @@ internal static class TypeNameParserHelpers internal const int ByRef = -3; private const char EscapeCharacter = '\\'; #if NET8_0_OR_GREATER - // Keep this in sync with GetFullTypeNameLength/NeedsEscaping private static readonly SearchValues s_endOfFullTypeNameDelimitersSearchValues = SearchValues.Create("[]&*,+\\"); + private static bool NeedsEscaping(char c) => s_endOfFullTypeNameDelimitersSearchValues.Contains(c); +#else + private static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; #endif internal static string GetGenericTypeFullName(ReadOnlySpan fullTypeName, ReadOnlySpan genericArgs) @@ -97,9 +99,6 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) } return offset; } - - // Keep this in sync with s_endOfFullTypeNameDelimitersSearchValues - static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; } internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) @@ -164,6 +163,50 @@ static int GetUnescapedOffset(ReadOnlySpan fullName, int startIndex) } } + internal static string Unescape(string input) + { + int indexOfEscapeChar = input.IndexOf(EscapeCharacter); + if (indexOfEscapeChar < 0) + { + // Nothing to escape, just return the original value. + return input; + } + + ValueStringBuilder builder = new(stackalloc char[128]); + builder.EnsureCapacity(input.Length); + + UnescapeToBuilder(input.AsSpan(), ref builder); + + string result = builder.ToString(); + builder.Dispose(); + return result; + + static void UnescapeToBuilder(ReadOnlySpan input, ref ValueStringBuilder builder) + { + while (!input.IsEmpty) + { + int indexOfEscapeChar = input.IndexOf(EscapeCharacter); + if (indexOfEscapeChar < 0) + { + builder.Append(input); + break; + } + builder.Append(input.Slice(0, indexOfEscapeChar)); + int indexOfNextChar = indexOfEscapeChar + 1; + if (indexOfNextChar < input.Length && input[indexOfNextChar] is char c && NeedsEscaping(c)) + { + builder.Append(c); + indexOfNextChar++; + } + else + { + builder.Append(EscapeCharacter); + } + input = input.Slice(indexOfNextChar); + } + } + } + // this method handles escaping of the ] just to let the AssemblyNameParser fail for the right input internal static ReadOnlySpan GetAssemblyNameCandidate(ReadOnlySpan input) { @@ -379,6 +422,12 @@ internal static bool TryStripFirstCharAndTrailingSpaces(ref ReadOnlySpan s return false; } + [DoesNotReturn] + internal static void ThrowArgumentNullException(string paramName) + { + throw new ArgumentNullException(paramName); + } + [DoesNotReturn] internal static void ThrowArgumentException_InvalidTypeName(int errorIndex) { From 2037805d391e4f302c45db45b893bec940c6ca6e Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 04:09:33 +0200 Subject: [PATCH 03/35] Fix infinite loops. --- .../src/System/Reflection/Metadata/TypeName.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index a125e9752ccf9f..311512f6cd0bee 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -296,13 +296,13 @@ public string Namespace TypeName rootTypeName = this; while (true) { - if (IsConstructedGenericType) + if (rootTypeName.IsConstructedGenericType) { - rootTypeName = GetGenericTypeDefinition(); + rootTypeName = rootTypeName.GetGenericTypeDefinition(); } - else if (IsPointer || IsByRef || IsArray) + else if (rootTypeName.IsPointer || rootTypeName.IsByRef || rootTypeName.IsArray) { - rootTypeName = GetElementType(); + rootTypeName = rootTypeName.GetElementType(); } else { From d0c0e021a76419083050261213dfcf42409f3106 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 04:16:55 +0200 Subject: [PATCH 04/35] Simplify loop. --- .../src/System/Reflection/Metadata/TypeName.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 311512f6cd0bee..085ff73c75c1da 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -217,6 +217,7 @@ public string FullName /// This is because determining whether a type truly is a generic type requires loading the type /// and performing a runtime check. /// + [MemberNotNullWhen(false, nameof(_elementOrGenericType))] public bool IsSimple => _elementOrGenericType is null; /// @@ -294,20 +295,9 @@ public string Namespace if (_namespace is null) { TypeName rootTypeName = this; - while (true) + while (!rootTypeName.IsSimple) { - if (rootTypeName.IsConstructedGenericType) - { - rootTypeName = rootTypeName.GetGenericTypeDefinition(); - } - else if (rootTypeName.IsPointer || rootTypeName.IsByRef || rootTypeName.IsArray) - { - rootTypeName = rootTypeName.GetElementType(); - } - else - { - break; - } + rootTypeName = rootTypeName._elementOrGenericType; } // At this point the type does not have a modifier applied to it, so it should have its full name initialized. From 3567d78d4aaf38e3f9caf555bf285ff5642edbb0 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 04:18:21 +0200 Subject: [PATCH 05/35] Update reference assembly. --- .../System.Reflection.Metadata/ref/System.Reflection.Metadata.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs index 62c9cd41230aae..7490b2c7dbc85f 100644 --- a/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs +++ b/src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs @@ -2450,6 +2450,7 @@ internal TypeName() { } public System.Reflection.Metadata.TypeName MakeSZArrayTypeName() { throw null; } public static System.Reflection.Metadata.TypeName Parse(System.ReadOnlySpan typeName, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } public static bool TryParse(System.ReadOnlySpan typeName, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Reflection.Metadata.TypeName? result, System.Reflection.Metadata.TypeNameParseOptions? options = null) { throw null; } + public static string Unescape(string name) { throw null; } public System.Reflection.Metadata.TypeName WithAssemblyName(System.Reflection.Metadata.AssemblyNameInfo? assemblyName) { throw null; } } public sealed partial class TypeNameParseOptions From afc4cb18e906c03e992850ccba7ff1635ed8594b Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 21:33:48 +0200 Subject: [PATCH 06/35] Address PR feedback around `Unescape`. --- .../System/Reflection/Metadata/TypeName.cs | 5 +- .../Metadata/TypeNameParserHelpers.cs | 56 +++++++++---------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 085ff73c75c1da..1fa8ce78091b42 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -436,7 +436,10 @@ public static bool TryParse(ReadOnlySpan typeName, [NotNullWhen(true)] out /// /// The input string containing the name to convert. /// A string of characters with any escaped characters converted to their unescaped form. - /// The unescaped string can be used for looking up the type name or namespace in metadata. + /// + /// The unescaped string can be used for looking up the type name or namespace in metadata. + /// This method removes escape characters even if they precede a character that does not require escaping. + /// public static string Unescape(string name) { if (name is null) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 866e5dfb3d5ba4..d85dad4ef26162 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -16,10 +16,8 @@ internal static class TypeNameParserHelpers internal const int ByRef = -3; private const char EscapeCharacter = '\\'; #if NET8_0_OR_GREATER + // Keep this in sync with GetFullTypeNameLength/NeedsEscaping private static readonly SearchValues s_endOfFullTypeNameDelimitersSearchValues = SearchValues.Create("[]&*,+\\"); - private static bool NeedsEscaping(char c) => s_endOfFullTypeNameDelimitersSearchValues.Contains(c); -#else - private static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; #endif internal static string GetGenericTypeFullName(ReadOnlySpan fullTypeName, ReadOnlySpan genericArgs) @@ -99,6 +97,9 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) } return offset; } + + // Keep this in sync with s_endOfFullTypeNameDelimitersSearchValues + static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; } internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) @@ -165,45 +166,42 @@ static int GetUnescapedOffset(ReadOnlySpan fullName, int startIndex) internal static string Unescape(string input) { - int indexOfEscapeChar = input.IndexOf(EscapeCharacter); - if (indexOfEscapeChar < 0) + int indexOfEscapeCharacter = input.IndexOf(EscapeCharacter); + if (indexOfEscapeCharacter < 0) { // Nothing to escape, just return the original value. return input; } - ValueStringBuilder builder = new(stackalloc char[128]); - builder.EnsureCapacity(input.Length); - - UnescapeToBuilder(input.AsSpan(), ref builder); + return UnescapeToBuilder(input, indexOfEscapeCharacter); - string result = builder.ToString(); - builder.Dispose(); - return result; - - static void UnescapeToBuilder(ReadOnlySpan input, ref ValueStringBuilder builder) + static string UnescapeToBuilder(string name, int indexOfEscapeCharacter) { - while (!input.IsEmpty) + // this code path is executed very rarely (IL Emit or pure IL with chars not allowed in C# or F#) + var sb = new ValueStringBuilder(stackalloc char[64]); + sb.EnsureCapacity(name.Length); + sb.Append(name.AsSpan(0, indexOfEscapeCharacter)); + + for (int i = indexOfEscapeCharacter; i < name.Length;) { - int indexOfEscapeChar = input.IndexOf(EscapeCharacter); - if (indexOfEscapeChar < 0) - { - builder.Append(input); - break; - } - builder.Append(input.Slice(0, indexOfEscapeChar)); - int indexOfNextChar = indexOfEscapeChar + 1; - if (indexOfNextChar < input.Length && input[indexOfNextChar] is char c && NeedsEscaping(c)) + char c = name[i++]; + + if (c != EscapeCharacter) { - builder.Append(c); - indexOfNextChar++; + sb.Append(c); } - else + else if (i < name.Length && name[i] == EscapeCharacter) // escaped escape character ;) { - builder.Append(EscapeCharacter); + sb.Append(c); + // Consume the escaped escape character, it's important for edge cases + // like escaped escape character followed by another escaped char (example: "\\\\\\+") + i++; } - input = input.Slice(indexOfNextChar); } + + string result = sb.ToString(); + sb.Dispose(); + return result; } } From dde6edad5b903cb561f59b08fb70811a2d2cce39 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 21:53:24 +0200 Subject: [PATCH 07/35] Remove file with duplicate `Unescape` method. --- .../Reflection/TypeNameResolver.CoreCLR.cs | 6 +- .../Reflection/TypeNameResolver.NativeAot.cs | 6 +- .../CustomAttributeTypeNameParser.cs | 4 +- .../ILVerification/ILVerification.projitems | 3 - .../ILCompiler.Compiler.csproj | 3 - .../ILCompiler.TypeSystem.csproj | 3 - .../Reflection/Metadata/TypeNameHelpers.cs | 79 ------------------- .../System.Private.CoreLib.Shared.projitems | 3 - .../src/System/Reflection/TypeNameResolver.cs | 2 +- .../Reflection/TypeNameResolver.Mono.cs | 2 +- .../src/linker/Linker/TypeNameResolver.cs | 4 +- .../illink/src/linker/Mono.Linker.csproj | 1 - 12 files changed, 12 insertions(+), 104 deletions(-) delete mode 100644 src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs index 70b88afc0726bb..c438aeb1d9a057 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs @@ -231,7 +231,7 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, } return null; } - return GetTypeFromDefaultAssemblies(TypeNameHelpers.Unescape(escapedTypeName), nestedTypeNames, parsedName); + return GetTypeFromDefaultAssemblies(TypeName.Unescape(escapedTypeName), nestedTypeNames, parsedName); } if (assembly is RuntimeAssembly runtimeAssembly) @@ -239,7 +239,7 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, // Compat: Non-extensible parser allows ambiguous matches with ignore case lookup bool useReflectionForNestedTypes = _extensibleParser && _ignoreCase; - type = runtimeAssembly.GetTypeCore(TypeNameHelpers.Unescape(escapedTypeName), useReflectionForNestedTypes ? default : nestedTypeNames, + type = runtimeAssembly.GetTypeCore(TypeName.Unescape(escapedTypeName), useReflectionForNestedTypes ? default : nestedTypeNames, throwOnFileNotFound: _throwOnError, ignoreCase: _ignoreCase); if (type is null) @@ -282,7 +282,7 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeNameHelpers.Unescape(escapedTypeName)), + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), typeName: parsedName.FullName); } return null; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs index e512d04c3ac099..4c9356e0ad533b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs @@ -158,7 +158,7 @@ internal partial struct TypeNameResolver { if (assembly is RuntimeAssemblyInfo runtimeAssembly) { - type = runtimeAssembly.GetTypeCore(TypeNameHelpers.Unescape(escapedTypeName), throwOnError: _throwOnError, ignoreCase: _ignoreCase); + type = runtimeAssembly.GetTypeCore(TypeName.Unescape(escapedTypeName), throwOnError: _throwOnError, ignoreCase: _ignoreCase); } else { @@ -173,7 +173,7 @@ internal partial struct TypeNameResolver } else { - string? unescapedTypeName = TypeNameHelpers.Unescape(escapedTypeName); + string? unescapedTypeName = TypeName.Unescape(escapedTypeName); RuntimeAssemblyInfo? defaultAssembly = null; if (_defaultAssemblyName != null) @@ -235,7 +235,7 @@ internal partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeNameHelpers.Unescape(escapedTypeName)), + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), typeName: parsedName.FullName); } return null; diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs index 83e74b02dd9f0c..2ddd6171a957e4 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs @@ -179,10 +179,10 @@ private TypeDesc GetSimpleTypeFromModule(TypeName typeName, ModuleDesc module) TypeDesc type = GetSimpleTypeFromModule(typeName.DeclaringType, module); if (type == null) return null; - return ((MetadataType)type).GetNestedType(TypeNameHelpers.Unescape(typeName.Name)); + return ((MetadataType)type).GetNestedType(TypeName.Unescape(typeName.Name)); } - string fullName = TypeNameHelpers.Unescape(typeName.FullName); + string fullName = TypeName.Unescape(typeName.FullName); if (_canonResolver != null) { diff --git a/src/coreclr/tools/ILVerification/ILVerification.projitems b/src/coreclr/tools/ILVerification/ILVerification.projitems index fc02753d601044..9fa519f1a3ca25 100644 --- a/src/coreclr/tools/ILVerification/ILVerification.projitems +++ b/src/coreclr/tools/ILVerification/ILVerification.projitems @@ -66,9 +66,6 @@ Utilities\CustomAttributeTypeNameParser.cs - - Utilities\TypeNameHelpers.cs - Utilities\ValueStringBuilder.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 954ae4124058ac..3076638131ea89 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -397,9 +397,6 @@ - - Utilities\TypeNameHelpers.cs - Utilities\ValueStringBuilder.cs diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj index cafa4952376f5c..38ace04a3a964f 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj @@ -198,9 +198,6 @@ Utilities\CustomAttributeTypeNameParser.cs - - Utilities\TypeNameHelpers.cs - Utilities\ValueStringBuilder.cs diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs deleted file mode 100644 index f43a04fadfd595..00000000000000 --- a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs +++ /dev/null @@ -1,79 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Reflection.Metadata; -using System.Text; - -namespace System.Reflection.Metadata -{ - internal static class TypeNameHelpers - { - private const char EscapeCharacter = '\\'; - - /// - /// Removes escape characters from the string (if there were any found). - /// - internal static string Unescape(string name) - { - int indexOfEscapeCharacter = name.IndexOf(EscapeCharacter); - if (indexOfEscapeCharacter < 0) - { - return name; - } - - return Unescape(name, indexOfEscapeCharacter); - - static string Unescape(string name, int indexOfEscapeCharacter) - { - // this code path is executed very rarely (IL Emit or pure IL with chars not allowed in C# or F#) - var sb = new ValueStringBuilder(stackalloc char[64]); - sb.EnsureCapacity(name.Length); - sb.Append(name.AsSpan(0, indexOfEscapeCharacter)); - - for (int i = indexOfEscapeCharacter; i < name.Length;) - { - char c = name[i++]; - - if (c != EscapeCharacter) - { - sb.Append(c); - } - else if (i < name.Length && name[i] == EscapeCharacter) // escaped escape character ;) - { - sb.Append(c); - // Consume the escaped escape character, it's important for edge cases - // like escaped escape character followed by another escaped char (example: "\\\\\\+") - i++; - } - } - - return sb.ToString(); - } - } - - 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); - } - } -} diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 61d304184b2865..a8658090059db9 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1489,9 +1489,6 @@ Common\System\Reflection\Metadata\TypeName.cs - - Common\System\Reflection\Metadata\TypeNameHelpers.cs - Common\System\Reflection\Metadata\TypeNameParser.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs index 04ab8e7ee08598..fd9cb2ecd3e5e0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs @@ -80,7 +80,7 @@ internal partial struct TypeNameResolver current = typeName; while (current.IsNested) { - nestedTypeNames[--nestingDepth] = TypeNameHelpers.Unescape(current.Name); + nestedTypeNames[--nestingDepth] = TypeName.Unescape(current.Name); current = current.DeclaringType!; } diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs index ef5d63ec28074d..964335bd412053 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs @@ -151,7 +151,7 @@ internal unsafe ref partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeNameHelpers.Unescape(escapedTypeName))); + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))); } return null; } diff --git a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs index aa56f99a2a4f31..b15e77272ede77 100644 --- a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs +++ b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs @@ -129,10 +129,10 @@ public bool TryResolveTypeName ( TypeDefinition? type = GetSimpleTypeFromModule (typeName.DeclaringType!, module); if (type == null) return null; - return GetNestedType (type, TypeNameHelpers.Unescape (typeName.Name)); + return GetNestedType (type, TypeName.Unescape (typeName.Name)); } - return module.ResolveType (TypeNameHelpers.Unescape (typeName.FullName), _metadataResolver); + return module.ResolveType (TypeName.Unescape (typeName.FullName), _metadataResolver); } TypeDefinition? GetNestedType (TypeDefinition type, string nestedTypeName) diff --git a/src/tools/illink/src/linker/Mono.Linker.csproj b/src/tools/illink/src/linker/Mono.Linker.csproj index 8f1f1e58fcbeae..af73b43ef4cdcf 100644 --- a/src/tools/illink/src/linker/Mono.Linker.csproj +++ b/src/tools/illink/src/linker/Mono.Linker.csproj @@ -74,7 +74,6 @@ - From 1da808a79d09ab37670d5bfe4f7768fb8b2e7197 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 22:13:58 +0200 Subject: [PATCH 08/35] Fix tests. --- .../tests/Metadata/TypeNameTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 3f7d18850091f1..342fb3891a6d9c 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -795,7 +795,7 @@ public void ParsedNamesMatchSystemTypeNames(Type type) Type genericType = type.GetGenericTypeDefinition(); TypeName genericTypeName = parsed.GetGenericTypeDefinition(); Assert.Equal(genericType.Name, genericTypeName.Name); - Assert.Equal(genericType.Namespace, genericTypeName.Namespace); + Assert.Equal(genericType.Namespace ?? "", genericTypeName.Namespace); Assert.Equal(genericType.FullName, genericTypeName.FullName); Assert.Equal(genericType.AssemblyQualifiedName, genericTypeName.AssemblyQualifiedName); } @@ -969,7 +969,7 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) { Assert.Equal(type.AssemblyQualifiedName, typeName.AssemblyQualifiedName); Assert.Equal(type.FullName, typeName.FullName); - Assert.Equal(type.Namespace, typeName.Namespace); + Assert.Equal(type.Namespace ?? "", typeName.Namespace); Assert.Equal(type.Name, typeName.Name); #if NET From ae14c6a06b33105ff4d09b5f492b539cc7e718ab Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 23:02:25 +0200 Subject: [PATCH 09/35] Reduce allocations when calling `Namespace` across a type name hierachy. --- .../src/System/Reflection/Metadata/TypeName.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 1fa8ce78091b42..167481cf32461b 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -300,14 +300,19 @@ public string Namespace rootTypeName = rootTypeName._elementOrGenericType; } - // At this point the type does not have a modifier applied to it, so it should have its full name initialized. - Debug.Assert(rootTypeName._fullName is not null); - ReadOnlySpan rootFullName = rootTypeName._fullName.AsSpan(); - if (rootTypeName._nestedNameLength > 0) + // By setting the namespace field at the root type name, we avoid recomputing it for all derived names. + if (rootTypeName._namespace is null) { - rootFullName = rootFullName.Slice(0, rootTypeName._nestedNameLength); + // At this point the type does not have a modifier applied to it, so it should have its full name set. + Debug.Assert(rootTypeName._fullName is not null); + ReadOnlySpan rootFullName = rootTypeName._fullName.AsSpan(); + if (rootTypeName._nestedNameLength > 0) + { + rootFullName = rootFullName.Slice(0, rootTypeName._nestedNameLength); + } + rootTypeName._namespace = TypeNameParserHelpers.GetNamespace(rootFullName).ToString(); } - _namespace = TypeNameParserHelpers.GetNamespace(rootFullName).ToString(); + _namespace = rootTypeName._namespace; } return _namespace; From 0b033532f02b8dabb26ac8ebba3271f29ee40276 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 23:23:08 +0200 Subject: [PATCH 10/35] Fix nested types with namespaces. --- .../Metadata/TypeNameParserHelpers.cs | 58 +++++++++++++++++-- .../Metadata/TypeNameParserHelpersTests.cs | 4 ++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index d85dad4ef26162..c6133c01d156e0 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -104,23 +104,69 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) { - int offset = fullName.LastIndexOf('.'); + // A type name of a nested type can also contain a namespace. + // We want to return the namespace of the outermost type, so we need to split before the first unescaped '+', + // and then before the last unescaped '.' after that. + int offset = IndexOfAnyUnescaped(fullName, '+'); - if (offset > 0 && fullName[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) + if (offset > 0) { - offset = GetUnescapedOffset(fullName, startIndex: offset); + fullName = fullName.Slice(0, offset); } + offset = LastIndexOfAnyUnescaped(fullName, '.'); + return offset < 0 ? [] : fullName.Slice(0, offset); + } - static int GetUnescapedOffset(ReadOnlySpan fullName, int startIndex) + private static int IndexOfAnyUnescaped(ReadOnlySpan str, char c) + { + int offset = str.IndexOf(c); + + if (offset > 0 && str[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) + { + offset = IndexOfAnyUnescapedSlow(str, c, startIndex: offset); + } + + return offset; + + static int IndexOfAnyUnescapedSlow(ReadOnlySpan str, char c, int startIndex) + { + int offset = startIndex; + for (; offset < str.Length; offset++) + { + if (str[offset] == c) + { + if (offset == 0 || str[offset - 1] != EscapeCharacter) + { + break; + } + offset++; // skip the escaping character + } + } + return offset; + } + } + + private static int LastIndexOfAnyUnescaped(ReadOnlySpan str, char c) + { + int offset = str.LastIndexOf(c); + + if (offset > 0 && str[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) + { + offset = LastIndexOfAnyUnescapedSlow(str, c, startIndex: offset); + } + + return offset; + + static int LastIndexOfAnyUnescapedSlow(ReadOnlySpan str, char c, int startIndex) { int offset = startIndex; for (; offset >= 0; offset--) { - if (fullName[offset] == '.') + if (str[offset] == c) { - if (offset == 0 || fullName[offset - 1] != EscapeCharacter) + if (offset == 0 || str[offset - 1] != EscapeCharacter) { break; } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 15ff2afe881106..149c541cd2b2be 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -46,6 +46,10 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "")] [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType", "Namespace1")] + [InlineData("Namespace1.Not\\+NestedNamespace.MyInnerType", "Namespace1.Not\\+NestedNamespace")] + [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType", "")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+NestedNamespace2.MyInnermostType", "Namespace1")] public void GetNamespaceReturnsJustNamespace(string fullName, string expected) => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); From 41d2f7cb7ccb823bda23acf34e651599b06c6ab6 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 20 Jan 2025 23:27:35 +0200 Subject: [PATCH 11/35] Add tests for `Unescape`. --- .../tests/Metadata/TypeNameTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 342fb3891a6d9c..792a39bac87df1 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -89,6 +89,16 @@ public void ParserIsNotEnforcingRuntimeSpecificRules(string input) } } + [Theory] + [InlineData("Int32", "Int32")] + [InlineData("System.Int32", "System.Int32")] + [InlineData("System.Int32[]", "System.Int32[]")] + [InlineData("System.Int32\\[\\]", "System.Int32[]")] + [InlineData("System.Int32\\", "System.Int32\\")] + [InlineData("System.Int32\\\\[]", "System.Int32\\[]")] + public void Unescape(string input, string expectedUnescaped) + => Assert.Equal(expectedUnescaped, TypeName.Unescape(input)); + [Theory] [InlineData("Namespace.Kość", "Namespace.Kość")] public void UnicodeCharactersAreAllowedByDefault(string input, string expectedFullName) From 1e3f969dd3ed300f89045b29e3cfa0c1e1f80b59 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 21 Jan 2025 00:30:25 +0200 Subject: [PATCH 12/35] Fix compile errors. --- .../Common/Utilities/CustomAttributeTypeNameParser.cs | 2 +- src/tools/illink/src/linker/Linker/TypeNameResolver.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs index 2ddd6171a957e4..8af22916a3bef4 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs @@ -191,7 +191,7 @@ private TypeDesc GetSimpleTypeFromModule(TypeName typeName, ModuleDesc module) return canonType; } - (string typeNamespace, string name) = TypeNameHelpers.Split(fullName); + (string typeNamespace, string name) = TypeName.Split(fullName); return module.GetType(typeNamespace, name, throwIfNotFound: false); } diff --git a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs index b15e77272ede77..f46907151c0bec 100644 --- a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs +++ b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs @@ -9,7 +9,6 @@ using TypeName = System.Reflection.Metadata.TypeName; using TypeNameParseOptions = System.Reflection.Metadata.TypeNameParseOptions; using AssemblyNameInfo = System.Reflection.Metadata.AssemblyNameInfo; -using TypeNameHelpers = System.Reflection.Metadata.TypeNameHelpers; #nullable enable From 269d0cc126195737925feb483f8996bd8ec2237e Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 21 Jan 2025 00:57:11 +0200 Subject: [PATCH 13/35] Restore `TypeNameHelpers` and use it in all places except CoreLib. --- .../CustomAttributeTypeNameParser.cs | 6 +- .../ILVerification/ILVerification.projitems | 3 + .../ILCompiler.Compiler.csproj | 3 + .../ILCompiler.TypeSystem.csproj | 3 + .../Reflection/Metadata/TypeNameHelpers.cs | 79 +++++++++++++++++++ .../src/linker/Linker/TypeNameResolver.cs | 5 +- .../illink/src/linker/Mono.Linker.csproj | 1 + 7 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs index 8af22916a3bef4..83e74b02dd9f0c 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs @@ -179,10 +179,10 @@ private TypeDesc GetSimpleTypeFromModule(TypeName typeName, ModuleDesc module) TypeDesc type = GetSimpleTypeFromModule(typeName.DeclaringType, module); if (type == null) return null; - return ((MetadataType)type).GetNestedType(TypeName.Unescape(typeName.Name)); + return ((MetadataType)type).GetNestedType(TypeNameHelpers.Unescape(typeName.Name)); } - string fullName = TypeName.Unescape(typeName.FullName); + string fullName = TypeNameHelpers.Unescape(typeName.FullName); if (_canonResolver != null) { @@ -191,7 +191,7 @@ private TypeDesc GetSimpleTypeFromModule(TypeName typeName, ModuleDesc module) return canonType; } - (string typeNamespace, string name) = TypeName.Split(fullName); + (string typeNamespace, string name) = TypeNameHelpers.Split(fullName); return module.GetType(typeNamespace, name, throwIfNotFound: false); } diff --git a/src/coreclr/tools/ILVerification/ILVerification.projitems b/src/coreclr/tools/ILVerification/ILVerification.projitems index 9fa519f1a3ca25..fc02753d601044 100644 --- a/src/coreclr/tools/ILVerification/ILVerification.projitems +++ b/src/coreclr/tools/ILVerification/ILVerification.projitems @@ -66,6 +66,9 @@ Utilities\CustomAttributeTypeNameParser.cs + + Utilities\TypeNameHelpers.cs + Utilities\ValueStringBuilder.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 3076638131ea89..954ae4124058ac 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -397,6 +397,9 @@ + + Utilities\TypeNameHelpers.cs + Utilities\ValueStringBuilder.cs diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj index 38ace04a3a964f..cafa4952376f5c 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj @@ -198,6 +198,9 @@ Utilities\CustomAttributeTypeNameParser.cs + + Utilities\TypeNameHelpers.cs + Utilities\ValueStringBuilder.cs diff --git a/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs new file mode 100644 index 00000000000000..f43a04fadfd595 --- /dev/null +++ b/src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Reflection.Metadata; +using System.Text; + +namespace System.Reflection.Metadata +{ + internal static class TypeNameHelpers + { + private const char EscapeCharacter = '\\'; + + /// + /// Removes escape characters from the string (if there were any found). + /// + internal static string Unescape(string name) + { + int indexOfEscapeCharacter = name.IndexOf(EscapeCharacter); + if (indexOfEscapeCharacter < 0) + { + return name; + } + + return Unescape(name, indexOfEscapeCharacter); + + static string Unescape(string name, int indexOfEscapeCharacter) + { + // this code path is executed very rarely (IL Emit or pure IL with chars not allowed in C# or F#) + var sb = new ValueStringBuilder(stackalloc char[64]); + sb.EnsureCapacity(name.Length); + sb.Append(name.AsSpan(0, indexOfEscapeCharacter)); + + for (int i = indexOfEscapeCharacter; i < name.Length;) + { + char c = name[i++]; + + if (c != EscapeCharacter) + { + sb.Append(c); + } + else if (i < name.Length && name[i] == EscapeCharacter) // escaped escape character ;) + { + sb.Append(c); + // Consume the escaped escape character, it's important for edge cases + // like escaped escape character followed by another escaped char (example: "\\\\\\+") + i++; + } + } + + return sb.ToString(); + } + } + + 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); + } + } +} diff --git a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs index f46907151c0bec..aa56f99a2a4f31 100644 --- a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs +++ b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs @@ -9,6 +9,7 @@ using TypeName = System.Reflection.Metadata.TypeName; using TypeNameParseOptions = System.Reflection.Metadata.TypeNameParseOptions; using AssemblyNameInfo = System.Reflection.Metadata.AssemblyNameInfo; +using TypeNameHelpers = System.Reflection.Metadata.TypeNameHelpers; #nullable enable @@ -128,10 +129,10 @@ public bool TryResolveTypeName ( TypeDefinition? type = GetSimpleTypeFromModule (typeName.DeclaringType!, module); if (type == null) return null; - return GetNestedType (type, TypeName.Unescape (typeName.Name)); + return GetNestedType (type, TypeNameHelpers.Unescape (typeName.Name)); } - return module.ResolveType (TypeName.Unescape (typeName.FullName), _metadataResolver); + return module.ResolveType (TypeNameHelpers.Unescape (typeName.FullName), _metadataResolver); } TypeDefinition? GetNestedType (TypeDefinition type, string nestedTypeName) diff --git a/src/tools/illink/src/linker/Mono.Linker.csproj b/src/tools/illink/src/linker/Mono.Linker.csproj index af73b43ef4cdcf..8f1f1e58fcbeae 100644 --- a/src/tools/illink/src/linker/Mono.Linker.csproj +++ b/src/tools/illink/src/linker/Mono.Linker.csproj @@ -74,6 +74,7 @@ + From 94441e784b88e29a78c217059eb5d771defdaa02 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 01:24:35 +0200 Subject: [PATCH 14/35] Do not omit escape character at the end when unescaping. --- .../src/System/Reflection/Metadata/TypeNameParserHelpers.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index c6133c01d156e0..a489f105c3a985 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -232,11 +232,11 @@ static string UnescapeToBuilder(string name, int indexOfEscapeCharacter) { char c = name[i++]; - if (c != EscapeCharacter) + if (c != EscapeCharacter || i == name.Length) { sb.Append(c); } - else if (i < name.Length && name[i] == EscapeCharacter) // escaped escape character ;) + else if (name[i] == EscapeCharacter) // escaped escape character ;) { sb.Append(c); // Consume the escaped escape character, it's important for edge cases From c98beecd81a8ee99363233dc0413e912df48cad1 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 02:13:59 +0200 Subject: [PATCH 15/35] Return the namespace of the innermost nested type that has one. --- .../Metadata/TypeNameParserHelpers.cs | 51 +++++-------------- .../Metadata/TypeNameParserHelpersTests.cs | 7 +-- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index a489f105c3a985..a49bd6cbdcc70d 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -104,51 +104,24 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) { - // A type name of a nested type can also contain a namespace. - // We want to return the namespace of the outermost type, so we need to split before the first unescaped '+', - // and then before the last unescaped '.' after that. - int offset = IndexOfAnyUnescaped(fullName, '+'); - - if (offset > 0) - { - fullName = fullName.Slice(0, offset); - } - - offset = LastIndexOfAnyUnescaped(fullName, '.'); - - return offset < 0 ? [] : fullName.Slice(0, offset); - } - - private static int IndexOfAnyUnescaped(ReadOnlySpan str, char c) - { - int offset = str.IndexOf(c); - - if (offset > 0 && str[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) - { - offset = IndexOfAnyUnescapedSlow(str, c, startIndex: offset); - } - - return offset; - - static int IndexOfAnyUnescapedSlow(ReadOnlySpan str, char c, int startIndex) + // A nested type name can have a namespace itself. Iterate from the innermost type outwards, + // and return the first namespace we find. + int offset; + while (LastIndexOfUnescaped(fullName, '+') is int nestingBoundaryPos && nestingBoundaryPos > 0) { - int offset = startIndex; - for (; offset < str.Length; offset++) + ReadOnlySpan nestedFullName = fullName.Slice(nestingBoundaryPos + 1); + offset = LastIndexOfUnescaped(nestedFullName, '.'); + if (offset > 0) { - if (str[offset] == c) - { - if (offset == 0 || str[offset - 1] != EscapeCharacter) - { - break; - } - offset++; // skip the escaping character - } + return nestedFullName.Slice(0, offset); } - return offset; + fullName = fullName.Slice(0, nestingBoundaryPos); } + offset = LastIndexOfUnescaped(fullName, '.'); + return offset < 0 ? [] : fullName.Slice(0, offset); } - private static int LastIndexOfAnyUnescaped(ReadOnlySpan str, char c) + private static int LastIndexOfUnescaped(ReadOnlySpan str, char c) { int offset = str.LastIndexOf(c); diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 149c541cd2b2be..532e6c4b61fd14 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -46,10 +46,11 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "")] [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType", "Namespace1")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType", "NestedNamespace")] [InlineData("Namespace1.Not\\+NestedNamespace.MyInnerType", "Namespace1.Not\\+NestedNamespace")] - [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType", "")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+NestedNamespace2.MyInnermostType", "Namespace1")] + [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType", "NestedNamespace")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+NestedNamespace2.MyInnermostType", "NestedNamespace2")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+MyInnermostType", "NestedNamespace")] public void GetNamespaceReturnsJustNamespace(string fullName, string expected) => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); From 3763dbeffc2748a1392b4475a72422970710b7aa Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 13:17:33 +0200 Subject: [PATCH 16/35] Update `TypeName.Name` to return the whole name of nested types. --- .../Metadata/TypeNameParserHelpers.cs | 49 ++++++------------- .../Metadata/TypeNameParserHelpersTests.cs | 3 +- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index a49bd6cbdcc70d..39aafba008503c 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; @@ -121,6 +121,20 @@ internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) return offset < 0 ? [] : fullName.Slice(0, offset); } + internal static ReadOnlySpan GetName(ReadOnlySpan fullName) + { + // If the type is nested, return the whole name after the plus sign. + int offset = LastIndexOfUnescaped(fullName, '+'); + + if (offset < 0) + { + // Look for dots only if the type is not nested. + offset = LastIndexOfUnescaped(fullName, '.'); + } + + return offset < 0 ? fullName : fullName.Slice(offset + 1); + } + private static int LastIndexOfUnescaped(ReadOnlySpan str, char c) { int offset = str.LastIndexOf(c); @@ -150,39 +164,6 @@ static int LastIndexOfAnyUnescapedSlow(ReadOnlySpan str, char c, int start } } - internal static ReadOnlySpan GetName(ReadOnlySpan fullName) - { - // The two-value form of MemoryExtensions.LastIndexOfAny does not suffer - // from the behavior mentioned in the comment at the top of GetFullTypeNameLength. - // It always takes O(m * i) worst-case time and is safe to use here. - - int offset = fullName.LastIndexOfAny('.', '+'); - - if (offset > 0 && fullName[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) - { - offset = GetUnescapedOffset(fullName, startIndex: offset); - } - - return offset < 0 ? fullName : fullName.Slice(offset + 1); - - static int GetUnescapedOffset(ReadOnlySpan fullName, int startIndex) - { - int offset = startIndex; - for (; offset >= 0; offset--) - { - if (fullName[offset] is '.' or '+') - { - if (offset == 0 || fullName[offset - 1] != EscapeCharacter) - { - break; - } - offset--; // skip the escaping character - } - } - return offset; - } - } - internal static string Unescape(string input) { int indexOfEscapeCharacter = input.IndexOf(EscapeCharacter); diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 532e6c4b61fd14..bf3beed41098ee 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -61,6 +61,7 @@ public void GetNamespaceReturnsJustNamespace(string fullName, string expected) [InlineData("Namespace.NotNamespace\\.TypeName", "NotNamespace\\.TypeName")] [InlineData("Namespace1.Namespace2.Containing+Nested", "Nested")] [InlineData("Namespace1.Namespace2.Not\\+Nested", "Not\\+Nested")] + [InlineData("Namespace1+Dotted.Name", "Dotted.Name")] [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "NotNamespace1\\.NotNamespace2\\.TypeName")] [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "NotNamespace1\\.NotNamespace2\\.Not\\+Nested")] public void GetNameReturnsJustName(string fullName, string expected) From 00c0ab742695f9abe2a13ff98f372db5d7d4ce92 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 13:22:32 +0200 Subject: [PATCH 17/35] Remove unnecessary `ValueStringBuilder.Dispose`. --- .../src/System/Reflection/Metadata/TypeNameParserHelpers.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 39aafba008503c..2dd92b53262e08 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -199,9 +199,7 @@ static string UnescapeToBuilder(string name, int indexOfEscapeCharacter) } } - string result = sb.ToString(); - sb.Dispose(); - return result; + return sb.ToString(); } } From 999d4e43f652edae9a2a11e4620e68a2b377a98f Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 13:27:33 +0200 Subject: [PATCH 18/35] Update `GetNamespace` to fail if a nested type has a namespace. --- .../src/Resources/Strings.resx | 3 +++ .../Metadata/TypeNameParserHelpers.cs | 25 ++++++++++++------- .../Metadata/TypeNameParserHelpersTests.cs | 13 ++++++---- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx index 21bab756dd6a89..d9ce464a1ebedb 100644 --- a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx @@ -429,6 +429,9 @@ This operation is only valid on arrays, pointers and references. + + Nested type name cannot have a namespace. + The given assembly name was invalid. diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 2dd92b53262e08..128d65ea8dc75e 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; @@ -104,20 +104,16 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) { - // A nested type name can have a namespace itself. Iterate from the innermost type outwards, - // and return the first namespace we find. - int offset; + // Detect if a nested type name has a namespace (e.g. "a.b+c") and throw if it does. while (LastIndexOfUnescaped(fullName, '+') is int nestingBoundaryPos && nestingBoundaryPos > 0) { - ReadOnlySpan nestedFullName = fullName.Slice(nestingBoundaryPos + 1); - offset = LastIndexOfUnescaped(nestedFullName, '.'); - if (offset > 0) + if (LastIndexOfUnescaped(fullName.Slice(nestingBoundaryPos + 1), '.') > 0) { - return nestedFullName.Slice(0, offset); + ThrowInvalidOperation_NestedTypeNamespace(); } fullName = fullName.Slice(0, nestingBoundaryPos); } - offset = LastIndexOfUnescaped(fullName, '.'); + int offset = LastIndexOfUnescaped(fullName, '.'); return offset < 0 ? [] : fullName.Slice(0, offset); } @@ -485,6 +481,17 @@ internal static void ThrowInvalidOperation_HasToBeArrayClass() #endif } + [DoesNotReturn] + internal static void ThrowInvalidOperation_NestedTypeNamespace() + { +#if SYSTEM_REFLECTION_METADATA + throw new InvalidOperationException(SR.InvalidOperation_NestedTypeNamespace); +#else + Debug.Fail("Expected to be unreachable"); + throw new InvalidOperationException(); +#endif + } + internal static bool IsMaxDepthExceeded(TypeNameParseOptions options, int depth) #if SYSTEM_PRIVATE_CORELIB => false; // CoreLib does not enforce any limits diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index bf3beed41098ee..b010efbcdcebfa 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -46,14 +46,17 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "")] [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType", "NestedNamespace")] [InlineData("Namespace1.Not\\+NestedNamespace.MyInnerType", "Namespace1.Not\\+NestedNamespace")] - [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType", "NestedNamespace")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+NestedNamespace2.MyInnermostType", "NestedNamespace2")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+MyInnermostType", "NestedNamespace")] public void GetNamespaceReturnsJustNamespace(string fullName, string expected) => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); + [Theory] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType")] + [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType")] + [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+MyInnermostType")] + public void GetNamespaceThrowsOnNestedNamespace(string fullName) + => Assert.Throws(() => TypeNameParserHelpers.GetNamespace(fullName.AsSpan())); + [Theory] [InlineData("JustTypeName", "JustTypeName")] [InlineData("Namespace.TypeName", "TypeName")] From 2885d4c79032deb7c936f6a1399d74cecf31bc64 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 17:22:02 +0200 Subject: [PATCH 19/35] Remove support for nested types and escaped dots in namespaces. --- .../src/Resources/Strings.resx | 2 +- .../System/Reflection/Metadata/TypeName.cs | 8 +++- .../Metadata/TypeNameParserHelpers.cs | 13 +----- .../Metadata/TypeNameParserHelpersTests.cs | 14 +------ .../tests/Metadata/TypeNameTests.cs | 42 +++++++++++++++---- 5 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx index d9ce464a1ebedb..b832902510c241 100644 --- a/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx @@ -430,7 +430,7 @@ This operation is only valid on arrays, pointers and references. - Nested type name cannot have a namespace. + Cannot retrieve the namespace of a nested type. The given assembly name was invalid. diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 167481cf32461b..7aaec9e7507583 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -288,10 +288,16 @@ public string Name /// /// The namespace of this type; e.g., "System". /// + /// This instance is a nested type. public string Namespace { get { + if (IsNested) + { + TypeNameParserHelpers.ThrowInvalidOperation_NestedTypeNamespace(); + } + if (_namespace is null) { TypeName rootTypeName = this; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 128d65ea8dc75e..78e799e884e0a0 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers; @@ -104,16 +104,7 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) { - // Detect if a nested type name has a namespace (e.g. "a.b+c") and throw if it does. - while (LastIndexOfUnescaped(fullName, '+') is int nestingBoundaryPos && nestingBoundaryPos > 0) - { - if (LastIndexOfUnescaped(fullName.Slice(nestingBoundaryPos + 1), '.') > 0) - { - ThrowInvalidOperation_NestedTypeNamespace(); - } - fullName = fullName.Slice(0, nestingBoundaryPos); - } - int offset = LastIndexOfUnescaped(fullName, '.'); + int offset = fullName.LastIndexOf('.'); return offset < 0 ? [] : fullName.Slice(0, offset); } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index b010efbcdcebfa..18cd209512223f 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -41,22 +41,10 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected [InlineData("JustTypeName", "")] [InlineData("Namespace.TypeName", "Namespace")] [InlineData("Namespace1.Namespace2.TypeName", "Namespace1.Namespace2")] - [InlineData("Namespace.NotNamespace\\.TypeName", "Namespace")] - [InlineData("Namespace1.Namespace2.Containing+Nested", "Namespace1.Namespace2")] [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] - [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "")] - [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "")] - [InlineData("Namespace1.Not\\+NestedNamespace.MyInnerType", "Namespace1.Not\\+NestedNamespace")] public void GetNamespaceReturnsJustNamespace(string fullName, string expected) => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); - [Theory] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType")] - [InlineData("Namespace1\\.MyOuterType+NestedNamespace.MyInnerType")] - [InlineData("Namespace1.MyOuterType+NestedNamespace.MyInnerType+MyInnermostType")] - public void GetNamespaceThrowsOnNestedNamespace(string fullName) - => Assert.Throws(() => TypeNameParserHelpers.GetNamespace(fullName.AsSpan())); - [Theory] [InlineData("JustTypeName", "JustTypeName")] [InlineData("Namespace.TypeName", "TypeName")] diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 792a39bac87df1..d234ecf5ed8d6c 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -14,13 +14,20 @@ public class TypeNameTests { [Theory] [InlineData(" System.Int32", "System.Int32", "System", "Int32")] - [InlineData(" MyNamespace.MyType+NestedType", "MyNamespace.MyType+NestedType", "MyNamespace", "NestedType")] - public void SpacesAtTheBeginningAreOK(string input, string expectedFullName, string expectedNamespace, string expectedName) + [InlineData(" MyNamespace.MyType+NestedType", "MyNamespace.MyType+NestedType", null, "NestedType")] + public void SpacesAtTheBeginningAreOK(string input, string expectedFullName, string? expectedNamespace, string expectedName) { TypeName parsed = TypeName.Parse(input.AsSpan()); Assert.Equal(expectedName, parsed.Name); - Assert.Equal(expectedNamespace, parsed.Namespace); + if (expectedNamespace is null) + { + Assert.Throws(() => parsed.Namespace); + } + else + { + Assert.Equal(expectedNamespace, parsed.Namespace); + } Assert.Equal(expectedFullName, parsed.FullName); Assert.Equal(expectedFullName, parsed.AssemblyQualifiedName); } @@ -710,7 +717,14 @@ private static void VerifyNestedNames(TypeName parsed, TypeName made, AssemblyNa while (true) { Assert.Equal(parsed.Name, made.Name); - Assert.Equal(parsed.Namespace, made.Namespace); + if (parsed.IsNested) + { + Assert.Throws(() => made.Namespace); + } + else + { + Assert.Equal(parsed.Namespace, made.Namespace); + } Assert.Equal(parsed.FullName, made.FullName); Assert.Equal(assemblyName, made.AssemblyName); Assert.NotEqual(parsed.AssemblyQualifiedName, made.AssemblyQualifiedName); @@ -805,7 +819,14 @@ public void ParsedNamesMatchSystemTypeNames(Type type) Type genericType = type.GetGenericTypeDefinition(); TypeName genericTypeName = parsed.GetGenericTypeDefinition(); Assert.Equal(genericType.Name, genericTypeName.Name); - Assert.Equal(genericType.Namespace ?? "", genericTypeName.Namespace); + if (genericType.IsNested) + { + Assert.Throws(() => genericTypeName.Namespace); + } + else + { + Assert.Equal(genericType.Namespace ?? "", genericTypeName.Namespace); + } Assert.Equal(genericType.FullName, genericTypeName.FullName); Assert.Equal(genericType.AssemblyQualifiedName, genericTypeName.AssemblyQualifiedName); } @@ -914,7 +935,7 @@ static void Verify(Type type, TypeName typeName, bool ignoreCase) { return Make(GetType(typeName.GetGenericTypeDefinition(), throwOnError, ignoreCase)); } - else if(typeName.IsArray || typeName.IsPointer || typeName.IsByRef) + else if (typeName.IsArray || typeName.IsPointer || typeName.IsByRef) { return Make(GetType(typeName.GetElementType(), throwOnError, ignoreCase)); } @@ -979,7 +1000,14 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) { Assert.Equal(type.AssemblyQualifiedName, typeName.AssemblyQualifiedName); Assert.Equal(type.FullName, typeName.FullName); - Assert.Equal(type.Namespace ?? "", typeName.Namespace); + if (type.IsNested) + { + Assert.Throws(() => typeName.Namespace); + } + else + { + Assert.Equal(type.Namespace ?? "", typeName.Namespace); + } Assert.Equal(type.Name, typeName.Name); #if NET From c84c41edfdbbf56af50f9fc6a8b204a4913f3f5b Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 26 Jan 2025 17:29:47 +0200 Subject: [PATCH 20/35] Remove support for escaped dots in `GetName`, and optimize it if the type is not nested. --- .../src/System/Reflection/Metadata/TypeName.cs | 7 ++++--- .../Reflection/Metadata/TypeNameParserHelpers.cs | 12 ++++-------- .../tests/Metadata/TypeNameParserHelpersTests.cs | 11 ++++------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 7aaec9e7507583..9c580c1fb2cb37 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -263,7 +263,8 @@ public string Name { if (IsConstructedGenericType) { - _name = TypeNameParserHelpers.GetName(GetGenericTypeDefinition().FullName.AsSpan()).ToString(); + TypeName genericTypeDef = GetGenericTypeDefinition(); + _name = TypeNameParserHelpers.GetName(genericTypeDef.FullName.AsSpan(), genericTypeDef.IsNested).ToString(); } else if (IsPointer || IsByRef || IsArray) { @@ -273,11 +274,11 @@ public string Name } else if (_nestedNameLength > 0 && _fullName is not null) { - _name = TypeNameParserHelpers.GetName(_fullName.AsSpan(0, _nestedNameLength)).ToString(); + _name = TypeNameParserHelpers.GetName(_fullName.AsSpan(0, _nestedNameLength), IsNested).ToString(); } else { - _name = TypeNameParserHelpers.GetName(FullName.AsSpan()).ToString(); + _name = TypeNameParserHelpers.GetName(FullName.AsSpan(), IsNested).ToString(); } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 78e799e884e0a0..dcbbad1c297e37 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -108,16 +108,12 @@ internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) return offset < 0 ? [] : fullName.Slice(0, offset); } - internal static ReadOnlySpan GetName(ReadOnlySpan fullName) + internal static ReadOnlySpan GetName(ReadOnlySpan fullName, bool isNested) { // If the type is nested, return the whole name after the plus sign. - int offset = LastIndexOfUnescaped(fullName, '+'); - - if (offset < 0) - { - // Look for dots only if the type is not nested. - offset = LastIndexOfUnescaped(fullName, '.'); - } + int offset = isNested + ? LastIndexOfUnescaped(fullName, '+') + : fullName.LastIndexOf('.'); return offset < 0 ? fullName : fullName.Slice(offset + 1); } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 18cd209512223f..4963015501c051 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -49,14 +49,11 @@ public void GetNamespaceReturnsJustNamespace(string fullName, string expected) [InlineData("JustTypeName", "JustTypeName")] [InlineData("Namespace.TypeName", "TypeName")] [InlineData("Namespace1.Namespace2.TypeName", "TypeName")] - [InlineData("Namespace.NotNamespace\\.TypeName", "NotNamespace\\.TypeName")] - [InlineData("Namespace1.Namespace2.Containing+Nested", "Nested")] + [InlineData("Namespace1.Namespace2.Containing+Nested", "Nested", true)] [InlineData("Namespace1.Namespace2.Not\\+Nested", "Not\\+Nested")] - [InlineData("Namespace1+Dotted.Name", "Dotted.Name")] - [InlineData("NotNamespace1\\.NotNamespace2\\.TypeName", "NotNamespace1\\.NotNamespace2\\.TypeName")] - [InlineData("NotNamespace1\\.NotNamespace2\\.Not\\+Nested", "NotNamespace1\\.NotNamespace2\\.Not\\+Nested")] - public void GetNameReturnsJustName(string fullName, string expected) - => Assert.Equal(expected, TypeNameParserHelpers.GetName(fullName.AsSpan()).ToString()); + [InlineData("Namespace1+Dotted.Name", "Dotted.Name", true)] + public void GetNameReturnsJustName(string fullName, string expected, bool isNested = false) + => Assert.Equal(expected, TypeNameParserHelpers.GetName(fullName.AsSpan(), isNested).ToString()); [Theory] [InlineData("simple", "simple")] From fa7b4598c57981b94849bf30884687033b8ce11e Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 27 Jan 2025 02:50:04 +0200 Subject: [PATCH 21/35] Update tests. --- .../tests/Metadata/TypeNameTests.cs | 5 +++-- .../tests/System.Runtime.Tests/System/Type/TypeTests.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index d234ecf5ed8d6c..df24b6bfba3e3b 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -103,6 +103,7 @@ public void ParserIsNotEnforcingRuntimeSpecificRules(string input) [InlineData("System.Int32\\[\\]", "System.Int32[]")] [InlineData("System.Int32\\", "System.Int32\\")] [InlineData("System.Int32\\\\[]", "System.Int32\\[]")] + [InlineData("System\\.Int32", "System.Int32")] public void Unescape(string input, string expectedUnescaped) => Assert.Equal(expectedUnescaped, TypeName.Unescape(input)); @@ -155,7 +156,7 @@ private static void EnsureMaxNodesIsRespected(string typeName, int expectedNodeC Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); validate(parsed); - // Specified MaxNodes is less than the actual node count + // Specified MaxNodes is less than the actual node count TypeNameParseOptions less = new() { MaxNodes = expectedNodeCount - 1 diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs index 4344f5dc55b8d8..490ba6dd1abc20 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs @@ -503,7 +503,6 @@ public void MakePointerType_ByRef_ThrowsTypeLoadException() [InlineData("Outside[][]", typeof(Outside[][]))] [InlineData("Outside`1[System.Nullable`1[System.Boolean]]", typeof(Outside))] [InlineData(".Outside`1", typeof(Outside<>))] - [InlineData(".Outside`1+.Inside`1", typeof(Outside<>.Inside<>))] public void GetTypeByName_ValidType_ReturnsExpected(string typeName, Type expectedType) { Assert.Equal(expectedType, Type.GetType(typeName, throwOnError: false, ignoreCase: false)); @@ -539,6 +538,7 @@ public static IEnumerable GetTypeByName_InvalidElementType() [InlineData("Outside`1[System.Boolean, System.Int32]", typeof(ArgumentException), true)] [InlineData(".System.Int32", typeof(TypeLoadException), false)] [InlineData("..Outside`1", typeof(TypeLoadException), false)] + [InlineData(".Outside`1+.Inside`1", typeof(TypeLoadException), false)] [MemberData(nameof(GetTypeByName_InvalidElementType))] public void GetTypeByName_Invalid(string typeName, Type expectedException, bool alwaysThrowsException) { From a6100e7a2e34e7baf33fa151a364170ee5ce5087 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 27 Jan 2025 03:01:48 +0200 Subject: [PATCH 22/35] Simplify `Name` to avoid linear search of the full name in nested types. --- .../System/Reflection/Metadata/TypeName.cs | 28 +++++++++---- .../Metadata/TypeNameParserHelpers.cs | 39 ------------------- .../Metadata/TypeNameParserHelpersTests.cs | 10 ----- .../tests/Metadata/TypeNameTests.cs | 27 ++++++++++++- 4 files changed, 47 insertions(+), 57 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 9c580c1fb2cb37..145b899bee7bec 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -230,6 +230,7 @@ public string FullName /// Returns true if this is a nested type (e.g., "Namespace.Declaring+Nested"). /// For nested types returns their declaring type. /// + [MemberNotNullWhen(true, nameof(_declaringType))] public bool IsNested => _declaringType is not null; /// @@ -263,8 +264,7 @@ public string Name { if (IsConstructedGenericType) { - TypeName genericTypeDef = GetGenericTypeDefinition(); - _name = TypeNameParserHelpers.GetName(genericTypeDef.FullName.AsSpan(), genericTypeDef.IsNested).ToString(); + _name = GetGenericTypeDefinition().Name; } else if (IsPointer || IsByRef || IsArray) { @@ -272,13 +272,27 @@ public string Name builder.Append(GetElementType().Name); _name = TypeNameParserHelpers.GetRankOrModifierStringRepresentation(_rankOrModifier, ref builder); } - else if (_nestedNameLength > 0 && _fullName is not null) - { - _name = TypeNameParserHelpers.GetName(_fullName.AsSpan(0, _nestedNameLength), IsNested).ToString(); - } else { - _name = TypeNameParserHelpers.GetName(FullName.AsSpan(), IsNested).ToString(); + // _fullName can be null only in constructed generic or modified types, which we handled above. + Debug.Assert(_fullName is not null); + ReadOnlySpan name = _fullName.AsSpan(); + if (_nestedNameLength > 0) + { + name = name.Slice(0, _nestedNameLength); + } + if (IsNested) + { + // If the type is nested, we know the length of the declaring type's full name. + // Get the characters after that plus one for the '+' separator. + name = name.Slice(_declaringType._nestedNameLength + 1); + } + else if (name.LastIndexOf('.') is int dotIndex && dotIndex >= 0) + { + // If the type is not nested, find the last dot in the full name and and return the substring after it. + name = name.Slice(dotIndex + 1); + } + _name = name.ToString(); } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index dcbbad1c297e37..ea5f099e4ddfba 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -108,45 +108,6 @@ internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) return offset < 0 ? [] : fullName.Slice(0, offset); } - internal static ReadOnlySpan GetName(ReadOnlySpan fullName, bool isNested) - { - // If the type is nested, return the whole name after the plus sign. - int offset = isNested - ? LastIndexOfUnescaped(fullName, '+') - : fullName.LastIndexOf('.'); - - return offset < 0 ? fullName : fullName.Slice(offset + 1); - } - - private static int LastIndexOfUnescaped(ReadOnlySpan str, char c) - { - int offset = str.LastIndexOf(c); - - if (offset > 0 && str[offset - 1] == EscapeCharacter) // this should be very rare (IL Emit & pure IL) - { - offset = LastIndexOfAnyUnescapedSlow(str, c, startIndex: offset); - } - - return offset; - - static int LastIndexOfAnyUnescapedSlow(ReadOnlySpan str, char c, int startIndex) - { - int offset = startIndex; - for (; offset >= 0; offset--) - { - if (str[offset] == c) - { - if (offset == 0 || str[offset - 1] != EscapeCharacter) - { - break; - } - offset--; // skip the escaping character - } - } - return offset; - } - } - internal static string Unescape(string input) { int indexOfEscapeCharacter = input.IndexOf(EscapeCharacter); diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 4963015501c051..9c11dec5a224d8 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -45,16 +45,6 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected public void GetNamespaceReturnsJustNamespace(string fullName, string expected) => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); - [Theory] - [InlineData("JustTypeName", "JustTypeName")] - [InlineData("Namespace.TypeName", "TypeName")] - [InlineData("Namespace1.Namespace2.TypeName", "TypeName")] - [InlineData("Namespace1.Namespace2.Containing+Nested", "Nested", true)] - [InlineData("Namespace1.Namespace2.Not\\+Nested", "Not\\+Nested")] - [InlineData("Namespace1+Dotted.Name", "Dotted.Name", true)] - public void GetNameReturnsJustName(string fullName, string expected, bool isNested = false) - => Assert.Equal(expected, TypeNameParserHelpers.GetName(fullName.AsSpan(), isNested).ToString()); - [Theory] [InlineData("simple", "simple")] [InlineData("simple]", "simple")] diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index df24b6bfba3e3b..0f90fb798e604f 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -96,6 +96,31 @@ public void ParserIsNotEnforcingRuntimeSpecificRules(string input) } } + [Theory] + [InlineData("Type+InnerType", (string[])["Type", "InnerType"])] + [InlineData("Type+InnerType+InnermostType", (string[])["Type", "InnerType", "InnermostType"])] + [InlineData("NotNested\\+Name", (string[])["NotNested\\+Name"])] + [InlineData("NameEndingInBackSlash\\\\+NestedName", (string[])["NameEndingInBackSlash\\\\", "NestedName"])] + public void NestedName(string input, string[] expectedNames) + { + TypeName parsed = TypeName.Parse(input.AsSpan()); + int i = expectedNames.Length - 1; + while (true) + { + Assert.Equal(expectedNames[i], parsed.Name); + // Caling FullName trims the _fullName value of the instance to just this type's full name. + // Test calling Name again to ensure it's still correct. + _ = parsed.FullName; + Assert.Equal(expectedNames[i], parsed.Name); + if (!parsed.IsNested) + { + break; + } + parsed = parsed.DeclaringType; + i--; + } + } + [Theory] [InlineData("Int32", "Int32")] [InlineData("System.Int32", "System.Int32")] From ae3b7e2f604ee9aea34a72cb260467625a7eddd0 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 28 Jan 2025 00:02:07 +0200 Subject: [PATCH 23/35] [mono] Do not treat nested type names as full names. --- .../src/System/RuntimeType.Mono.cs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index 348125a734fb26..e95a8d9e4172ad 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -683,21 +683,17 @@ private ListBuilder GetFieldCandidates(string? name, BindingFlags bin return candidates; } - private ListBuilder GetNestedTypeCandidates(string? fullname, BindingFlags bindingAttr, bool allowPrefixLookup) + private ListBuilder GetNestedTypeCandidates(string? name, BindingFlags bindingAttr, bool allowPrefixLookup) { - bool prefixLookup; bindingAttr &= ~BindingFlags.Static; - string? name, ns; - MemberListType listType; - SplitName(fullname, out name, out ns); - FilterHelper(bindingAttr, ref name, allowPrefixLookup, out prefixLookup, out _, out listType); + FilterHelper(bindingAttr, ref name, allowPrefixLookup, out bool prefixLookup, out _, out MemberListType listType); RuntimeType[] cache = GetNestedTypes_internal(name, bindingAttr, listType); ListBuilder candidates = new ListBuilder(cache.Length); for (int i = 0; i < cache.Length; i++) { RuntimeType nestedClass = cache[i]; - if (FilterApplyType(nestedClass, bindingAttr, name, prefixLookup, ns)) + if (FilterApplyType(nestedClass, bindingAttr, name, prefixLookup, null)) { candidates.Add(nestedClass); } @@ -1008,22 +1004,19 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] - public override Type? GetNestedType(string fullname, BindingFlags bindingAttr) + public override Type? GetNestedType(string name, BindingFlags bindingAttr) { - ArgumentNullException.ThrowIfNull(fullname); + ArgumentNullException.ThrowIfNull(name); bindingAttr &= ~BindingFlags.Static; - string? name, ns; - MemberListType listType; - SplitName(fullname, out name, out ns); - FilterHelper(bindingAttr, ref name, out _, out listType); + FilterHelper(bindingAttr, ref name, out _, out MemberListType listType); RuntimeType[] cache = GetNestedTypes_internal(name, bindingAttr, listType); RuntimeType? match = null; for (int i = 0; i < cache.Length; i++) { RuntimeType nestedType = cache[i]; - if (FilterApplyType(nestedType, bindingAttr, name, false, ns)) + if (FilterApplyType(nestedType, bindingAttr, name, false, null)) { if (match != null) throw ThrowHelper.GetAmbiguousMatchException(match); From d6180935f136aa168c17def84165619875cf49ed Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 28 Jan 2025 01:31:22 +0200 Subject: [PATCH 24/35] Fix compile errors. --- .../src/System/RuntimeType.Mono.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index e95a8d9e4172ad..e05abd346ed20f 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -883,12 +883,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents)] - public override EventInfo? GetEvent(string name, BindingFlags bindingAttr) + public override EventInfo? GetEvent([MaybeNull] string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); MemberListType listType; - FilterHelper(bindingAttr, ref name!, out _, out listType); + FilterHelper(bindingAttr, ref name, out _, out listType); RuntimeEventInfo[] cache = GetEvents_internal(name, listType, this); EventInfo? match = null; @@ -911,12 +911,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] - public override FieldInfo? GetField(string name, BindingFlags bindingAttr) + public override FieldInfo? GetField([MaybeNull] string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); MemberListType listType; - FilterHelper(bindingAttr, ref name!, out _, out listType); + FilterHelper(bindingAttr, ref name, out _, out listType); RuntimeFieldInfo[] cache = GetFields_internal(name, bindingAttr, listType, this); FieldInfo? match = null; @@ -1004,7 +1004,7 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] - public override Type? GetNestedType(string name, BindingFlags bindingAttr) + public override Type? GetNestedType([MaybeNull] string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); From 82e156f8638c15495b3e664ef7b005ca70483d32 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 3 Feb 2025 02:47:38 +0200 Subject: [PATCH 25/35] Update algorithm to find namespace delimiter. --- .../src/System/Reflection/Metadata/TypeName.cs | 15 +++++++++++---- .../Reflection/Metadata/TypeNameParserHelpers.cs | 14 +++++++++++--- .../tests/Metadata/TypeNameParserHelpersTests.cs | 14 ++++++++------ .../tests/Metadata/TypeNameTests.cs | 1 + .../src/System/RuntimeType.Mono.cs | 15 ++++++--------- 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 145b899bee7bec..0a1198ac6e6cf0 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -287,10 +287,10 @@ public string Name // Get the characters after that plus one for the '+' separator. name = name.Slice(_declaringType._nestedNameLength + 1); } - else if (name.LastIndexOf('.') is int dotIndex && dotIndex >= 0) + else if (TypeNameParserHelpers.IndexOfNamespaceDelimiter(name) is int idx && idx >= 0) { - // If the type is not nested, find the last dot in the full name and and return the substring after it. - name = name.Slice(dotIndex + 1); + // If the type is not nested, find the namespace delimiter in the full name and and return the substring after it. + name = name.Slice(idx + 1); } _name = name.ToString(); } @@ -331,7 +331,14 @@ public string Namespace { rootFullName = rootFullName.Slice(0, rootTypeName._nestedNameLength); } - rootTypeName._namespace = TypeNameParserHelpers.GetNamespace(rootFullName).ToString(); + if (TypeNameParserHelpers.IndexOfNamespaceDelimiter(rootFullName) is int idx && idx >= 0) + { + rootTypeName._namespace = rootFullName.Slice(0, idx).ToString(); + } + else + { + rootTypeName._namespace = string.Empty; + } } _namespace = rootTypeName._namespace; } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index ea5f099e4ddfba..8884c8397728bb 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -102,10 +102,18 @@ static int GetUnescapedOffset(ReadOnlySpan input, int startOffset) static bool NeedsEscaping(char c) => c is '[' or ']' or '&' or '*' or ',' or '+' or EscapeCharacter; } - internal static ReadOnlySpan GetNamespace(ReadOnlySpan fullName) + internal static int IndexOfNamespaceDelimiter(ReadOnlySpan fullName) { - int offset = fullName.LastIndexOf('.'); - return offset < 0 ? [] : fullName.Slice(0, offset); + // Matches algorithm from ns::FindSep in src\coreclr\utilcode\namespaceutil.cpp + // This could result in the type name beginning with a '.' character. + int index = fullName.LastIndexOf('.'); + + if (index > 0 && fullName[index - 1] == '.') + { + index--; + } + + return index; } internal static string Unescape(string input) diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index 9c11dec5a224d8..080510a3eb83a8 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -38,12 +38,14 @@ public void GetFullTypeNameLengthReturnsExpectedValue(string input, int expected } [Theory] - [InlineData("JustTypeName", "")] - [InlineData("Namespace.TypeName", "Namespace")] - [InlineData("Namespace1.Namespace2.TypeName", "Namespace1.Namespace2")] - [InlineData("Namespace1.Namespace2.Not\\+Nested", "Namespace1.Namespace2")] - public void GetNamespaceReturnsJustNamespace(string fullName, string expected) - => Assert.Equal(expected, TypeNameParserHelpers.GetNamespace(fullName.AsSpan()).ToString()); + [InlineData("JustTypeName", -1)] + [InlineData("Namespace.TypeName", 9)] + [InlineData("Namespace1.Namespace2.TypeName", 21)] + [InlineData("Namespace..Name", 9)] + [InlineData("Namespace...Name", 10)] + [InlineData("Namespace..Name.", 15)] + public void IndexOfNamespaceDelimiter(string fullName, int expected) + => Assert.Equal(expected, TypeNameParserHelpers.IndexOfNamespaceDelimiter(fullName.AsSpan())); [Theory] [InlineData("simple", "simple")] diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 0f90fb798e604f..934aabe30994d1 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -119,6 +119,7 @@ public void NestedName(string input, string[] expectedNames) parsed = parsed.DeclaringType; i--; } + Assert.Equal(0, i); } [Theory] diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index e05abd346ed20f..e44077ed24b5cb 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection; +using System.Reflection.Metadata; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Serialization; @@ -175,22 +176,17 @@ private static void SplitName(string? fullname, out string? name, out string? ns return; // Get namespace - int nsDelimiter = fullname.LastIndexOf('.'); - if (nsDelimiter != -1) + int nsDelimiter = TypeNameParserHelpers.IndexOfNamespaceDelimiter(fullname); + if (nsDelimiter > 0) { ns = fullname.Substring(0, nsDelimiter); - int nameLength = fullname.Length - ns.Length - 1; - if (nameLength != 0) - name = fullname.Substring(nsDelimiter + 1, nameLength); - else - name = ""; + name = fullname.Substring(nsDelimiter + 1); Debug.Assert(fullname.Equals(ns + "." + name)); } else { name = fullname; } - } #endregion @@ -1676,7 +1672,8 @@ internal void CallDefaultStructConstructor(ref byte value) [Flags] // Types of entries cached in TypeCache - private enum TypeCacheEntries { + private enum TypeCacheEntries + { IsGenericTypeDef = 1, IsDelegate = 2, IsValueType = 4, From 0d220f2bf8a9cdaf457177f30d722f2a29b0e53a Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 3 Feb 2025 03:23:16 +0200 Subject: [PATCH 26/35] Revert nullable annotations changes. --- .../src/System/RuntimeType.Mono.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index e44077ed24b5cb..6c91827270602a 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -879,12 +879,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicEvents | DynamicallyAccessedMemberTypes.NonPublicEvents)] - public override EventInfo? GetEvent([MaybeNull] string name, BindingFlags bindingAttr) + public override EventInfo? GetEvent(string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); MemberListType listType; - FilterHelper(bindingAttr, ref name, out _, out listType); + FilterHelper(bindingAttr, ref name!, out _, out listType); RuntimeEventInfo[] cache = GetEvents_internal(name, listType, this); EventInfo? match = null; @@ -907,12 +907,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields)] - public override FieldInfo? GetField([MaybeNull] string name, BindingFlags bindingAttr) + public override FieldInfo? GetField(string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); MemberListType listType; - FilterHelper(bindingAttr, ref name, out _, out listType); + FilterHelper(bindingAttr, ref name!, out _, out listType); RuntimeFieldInfo[] cache = GetFields_internal(name, bindingAttr, listType, this); FieldInfo? match = null; @@ -1000,12 +1000,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] - public override Type? GetNestedType([MaybeNull] string name, BindingFlags bindingAttr) + public override Type? GetNestedType(string name, BindingFlags bindingAttr) { ArgumentNullException.ThrowIfNull(name); bindingAttr &= ~BindingFlags.Static; - FilterHelper(bindingAttr, ref name, out _, out MemberListType listType); + FilterHelper(bindingAttr, ref name!, out _, out MemberListType listType); RuntimeType[] cache = GetNestedTypes_internal(name, bindingAttr, listType); RuntimeType? match = null; From e1f044c1d02d9690a8ea4868c780c21aec0d206a Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 4 Feb 2025 22:11:56 +0200 Subject: [PATCH 27/35] Redirect all `Type.GetType` overloads in Mono through `TypeNameResolver`. --- src/mono/System.Private.CoreLib/src/System/Type.Mono.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs index 3fc4f6082c9b28..1d7fb8c11fd372 100644 --- a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs @@ -30,7 +30,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, bool throwOnError, bool ignoreCase) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return RuntimeType.GetType(typeName, throwOnError, ignoreCase, ref stackMark); + return GetType(typeName, null, null, throwOnError, ignoreCase, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -38,7 +38,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, bool throwOnError) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return RuntimeType.GetType(typeName, throwOnError, false, ref stackMark); + return GetType(typeName, null, null, throwOnError, false, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -46,7 +46,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return RuntimeType.GetType(typeName, false, false, ref stackMark); + return GetType(typeName, null, null, false, false, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] From a3bf79d280816e6d17ae73d17a5510957b755bdb Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 4 Feb 2025 23:45:03 +0200 Subject: [PATCH 28/35] Update tests to account for Mono using the managed type parser in `Type.GetType`. --- .../System/Type/TypeTests.cs | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs index 490ba6dd1abc20..221b03f1050ead 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs @@ -509,26 +509,6 @@ public void GetTypeByName_ValidType_ReturnsExpected(string typeName, Type expect Assert.Equal(expectedType, Type.GetType(typeName.ToLower(), throwOnError: false, ignoreCase: true)); } - public static IEnumerable GetTypeByName_InvalidElementType() - { - Type expectedException = PlatformDetection.IsMonoRuntime - ? typeof(ArgumentException) // https://github.com/dotnet/runtime/issues/45033 - : typeof(TypeLoadException); - - yield return new object[] { "System.Int32&&", expectedException, true }; - yield return new object[] { "System.Int32&*", expectedException, true }; - yield return new object[] { "System.Int32&[]", expectedException, true }; - yield return new object[] { "System.Int32&[*]", expectedException, true }; - yield return new object[] { "System.Int32&[,]", expectedException, true }; - - // https://github.com/dotnet/runtime/issues/45033 - if (!PlatformDetection.IsMonoRuntime) - { - yield return new object[] { "System.Void[]", expectedException, true }; - yield return new object[] { "System.TypedReference[]", expectedException, true }; - } - } - [Theory] [InlineData("system.nullable`1[system.int32]", typeof(TypeLoadException), false)] [InlineData("System.NonExistingType", typeof(TypeLoadException), false)] @@ -539,7 +519,13 @@ public static IEnumerable GetTypeByName_InvalidElementType() [InlineData(".System.Int32", typeof(TypeLoadException), false)] [InlineData("..Outside`1", typeof(TypeLoadException), false)] [InlineData(".Outside`1+.Inside`1", typeof(TypeLoadException), false)] - [MemberData(nameof(GetTypeByName_InvalidElementType))] + [InlineData("System.Int32&&", typeof(TypeLoadException), true)] + [InlineData("System.Int32&*", typeof(TypeLoadException), true)] + [InlineData("System.Int32&[]", typeof(TypeLoadException), true)] + [InlineData("System.Int32&[*]", typeof(TypeLoadException), true)] + [InlineData("System.Int32&[,]", typeof(TypeLoadException), true)] + [InlineData("System.Void[]", typeof(TypeLoadException), true)] + [InlineData("System.TypedReference[]", typeof(TypeLoadException), true)] public void GetTypeByName_Invalid(string typeName, Type expectedException, bool alwaysThrowsException) { if (!alwaysThrowsException) From c023336c89f458c66e17d31858dea4e992026e27 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Thu, 6 Feb 2025 01:57:11 +0200 Subject: [PATCH 29/35] Disallow getting the namespace of non-simple nested type names. --- .../src/System/Reflection/Metadata/TypeName.cs | 10 +++++----- .../tests/Metadata/TypeNameTests.cs | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 0a1198ac6e6cf0..8b6ea3696e4f02 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -308,11 +308,6 @@ public string Namespace { get { - if (IsNested) - { - TypeNameParserHelpers.ThrowInvalidOperation_NestedTypeNamespace(); - } - if (_namespace is null) { TypeName rootTypeName = this; @@ -321,6 +316,11 @@ public string Namespace rootTypeName = rootTypeName._elementOrGenericType; } + if (rootTypeName.IsNested) + { + TypeNameParserHelpers.ThrowInvalidOperation_NestedTypeNamespace(); + } + // By setting the namespace field at the root type name, we avoid recomputing it for all derived names. if (rootTypeName._namespace is null) { diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 934aabe30994d1..2fd1477709f19b 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -1027,7 +1027,7 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) { Assert.Equal(type.AssemblyQualifiedName, typeName.AssemblyQualifiedName); Assert.Equal(type.FullName, typeName.FullName); - if (type.IsNested) + if (GetSimpleAncestor(typeName).IsNested) { Assert.Throws(() => typeName.Namespace); } @@ -1045,6 +1045,15 @@ private static void EnsureBasicMatch(TypeName typeName, Type type) Assert.Equal(type.IsByRef, typeName.IsByRef); Assert.Equal(type.IsConstructedGenericType, typeName.IsConstructedGenericType); Assert.Equal(type.IsNested, typeName.IsNested); + + static TypeName GetSimpleAncestor(TypeName typeName) + { + while (!typeName.IsSimple) + { + typeName = typeName.IsConstructedGenericType ? typeName.GetGenericTypeDefinition() : typeName.GetElementType(); + } + return typeName; + } } public class NestedNonGeneric_0 From cb2bd2d0d93a940e16f2247fef8a16f5b8ed5d14 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Thu, 6 Feb 2025 02:32:37 +0200 Subject: [PATCH 30/35] Ignore ambiguous match exceptions in non-extensible `Type.GetType` overloads. This matches the behavior of CoreCLR and Native AOT. --- .../System/Reflection/TypeNameResolver.Mono.cs | 14 +++++++++++++- .../src/System/RuntimeType.Mono.cs | 13 +++++++++++-- .../src/System/Type.Mono.cs | 16 ++++++++-------- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs index 964335bd412053..565faeea7920b0 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs @@ -18,6 +18,7 @@ internal unsafe ref partial struct TypeNameResolver private Func? _typeResolver; private bool _throwOnError; private bool _ignoreCase; + private bool _extensibleParser; private void* _stackMark; [RequiresUnreferencedCode("The type might be removed")] @@ -27,6 +28,7 @@ internal unsafe ref partial struct TypeNameResolver Func? typeResolver, bool throwOnError, bool ignoreCase, + bool extensibleParser, ref StackCrawlMark stackMark) { ArgumentNullException.ThrowIfNull(typeName); @@ -52,6 +54,7 @@ internal unsafe ref partial struct TypeNameResolver _typeResolver = typeResolver, _throwOnError = throwOnError, _ignoreCase = ignoreCase, + _extensibleParser = extensibleParser, _stackMark = Unsafe.AsPointer(ref stackMark) }.Resolve(parsed); } @@ -144,7 +147,16 @@ internal unsafe ref partial struct TypeNameResolver if (_ignoreCase) bindingFlags |= BindingFlags.IgnoreCase; - type = type.GetNestedType(nestedTypeNames[i], bindingFlags); + if (type is RuntimeType rt) + { + // Compat: Non-extensible parser allows ambiguous matches with ignore case lookup + bool ignoreAmbiguousMatch = !_extensibleParser && _ignoreCase; + type = rt.GetNestedType(nestedTypeNames[i], bindingFlags, ignoreAmbiguousMatch); + } + else + { + type = type.GetNestedType(nestedTypeNames[i], bindingFlags); + } if (type is null) { diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index 6c91827270602a..b27924bcb1c7bd 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1000,12 +1000,12 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) } [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] - public override Type? GetNestedType(string name, BindingFlags bindingAttr) + internal Type? GetNestedType([MaybeNull] string name, BindingFlags bindingAttr, bool ignoreAmbiguousMatch) { ArgumentNullException.ThrowIfNull(name); bindingAttr &= ~BindingFlags.Static; - FilterHelper(bindingAttr, ref name!, out _, out MemberListType listType); + FilterHelper(bindingAttr, ref name, out _, out MemberListType listType); RuntimeType[] cache = GetNestedTypes_internal(name, bindingAttr, listType); RuntimeType? match = null; @@ -1018,12 +1018,21 @@ public override MemberInfo[] GetMembers(BindingFlags bindingAttr) throw ThrowHelper.GetAmbiguousMatchException(match); match = nestedType; + + if (ignoreAmbiguousMatch) + break; } } return match; } + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicNestedTypes | DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] + public override Type? GetNestedType(string name, BindingFlags bindingAttr) + { + return GetNestedType(name, bindingAttr, ignoreAmbiguousMatch: false); + } + [DynamicallyAccessedMembers(GetAllMembers)] public override MemberInfo[] GetMember(string name, MemberTypes type, BindingFlags bindingAttr) { diff --git a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs index 1d7fb8c11fd372..5740704b764651 100644 --- a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs @@ -30,7 +30,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, bool throwOnError, bool ignoreCase) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, null, null, throwOnError, ignoreCase, ref stackMark); + return GetType(typeName, null, null, throwOnError, ignoreCase, false, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -38,7 +38,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, bool throwOnError) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, null, null, throwOnError, false, ref stackMark); + return GetType(typeName, null, null, throwOnError, false, false, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -46,7 +46,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, null, null, false, false, ref stackMark); + return GetType(typeName, null, null, false, false, false, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -54,7 +54,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, Func? assemblyResolver, Func? typeResolver) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, assemblyResolver, typeResolver, false, false, ref stackMark); + return GetType(typeName, assemblyResolver, typeResolver, false, false, true, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -62,7 +62,7 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, Func? assemblyResolver, Func? typeResolver, bool throwOnError) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, assemblyResolver, typeResolver, throwOnError, false, ref stackMark); + return GetType(typeName, assemblyResolver, typeResolver, throwOnError, false, true, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] @@ -70,13 +70,13 @@ internal IntPtr GetUnderlyingNativeHandle() public static Type? GetType(string typeName, Func? assemblyResolver, Func? typeResolver, bool throwOnError, bool ignoreCase) { StackCrawlMark stackMark = StackCrawlMark.LookForMyCaller; - return GetType(typeName, assemblyResolver, typeResolver, throwOnError, ignoreCase, ref stackMark); + return GetType(typeName, assemblyResolver, typeResolver, throwOnError, ignoreCase, true, ref stackMark); } [RequiresUnreferencedCode("The type might be removed")] - private static Type? GetType(string typeName, Func? assemblyResolver, Func? typeResolver, bool throwOnError, bool ignoreCase, ref StackCrawlMark stackMark) + private static Type? GetType(string typeName, Func? assemblyResolver, Func? typeResolver, bool throwOnError, bool ignoreCase, bool extensibleParser, ref StackCrawlMark stackMark) { - return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver, throwOnError, ignoreCase, ref stackMark); + return TypeNameResolver.GetType(typeName, assemblyResolver, typeResolver, throwOnError, ignoreCase, extensibleParser, ref stackMark); } public static Type? GetTypeFromHandle(RuntimeTypeHandle handle) From 02c2efed541f9521e37b9349e83e179c8a75a03b Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Thu, 6 Feb 2025 02:58:02 +0200 Subject: [PATCH 31/35] Set `TypeLoadException.TypeName` in Mono as well. The property became init-only (internally) in order to avoid conflicts with runtime-specific constructor overloads. --- .../Reflection/TypeNameResolver.CoreCLR.cs | 16 ++++++++-------- .../Reflection/TypeNameResolver.NativeAot.cs | 8 ++++---- .../src/System/TypeLoadException.cs | 17 +++++++---------- .../System/Reflection/TypeNameResolver.Mono.cs | 6 ++++-- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs index c438aeb1d9a057..6b9058bee56d79 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs @@ -213,8 +213,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName), - typeName: escapedTypeName); + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) + { TypeName = escapedTypeName }; } return null; } @@ -246,8 +246,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, { if (_throwOnError) { - throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, runtimeAssembly.FullName), - typeName: parsedName.FullName); + throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, runtimeAssembly.FullName)) + { TypeName = parsedName.FullName }; } return null; } @@ -282,8 +282,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), - typeName: parsedName.FullName); + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))) + { TypeName = parsedName.FullName }; } return null; } @@ -320,8 +320,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, if (_throwOnError) { - throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, (requestingAssembly ?? coreLib).FullName), - typeName: parsedName.FullName); + throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, (requestingAssembly ?? coreLib).FullName)) + { TypeName = parsedName.FullName }; } return null; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs index 4c9356e0ad533b..dd88bbe4956241 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs @@ -146,8 +146,8 @@ internal partial struct TypeNameResolver { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName), - typeName: escapedTypeName); + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) + { TypeNameResolver = escapedTypeName }; } return null; } @@ -235,8 +235,8 @@ internal partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), - typeName: parsedName.FullName); + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))); + { TypeName = parsedName.FullName }; } return null; } diff --git a/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs b/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs index b269f391316250..7e93e952b43358 100644 --- a/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs +++ b/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs @@ -29,15 +29,6 @@ public TypeLoadException(string? message, Exception? inner) HResult = HResults.COR_E_TYPELOAD; } -#if !MONO - internal TypeLoadException(string message, string typeName) - : base(message) - { - HResult = HResults.COR_E_TYPELOAD; - _className = typeName; - } -#endif - public override string Message { get @@ -47,7 +38,13 @@ public override string Message } } - public string TypeName => _className ?? string.Empty; + public string TypeName + { + get => _className ?? string.Empty; + // This is an init-only property, because a (string message, string typeName) constructor + // would conflict with Mono's (string typeName, string assemblyName) constructor. + internal init => _className = value; + } [Obsolete(Obsoletions.LegacyFormatterImplMessage, DiagnosticId = Obsoletions.LegacyFormatterImplDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [EditorBrowsable(EditorBrowsableState.Never)] diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs index 565faeea7920b0..e280c44275afb9 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs @@ -117,7 +117,8 @@ internal unsafe ref partial struct TypeNameResolver { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)); + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) + { TypeName = escapedTypeName }; } return null; } @@ -163,7 +164,8 @@ internal unsafe ref partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))); + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))) + { TypeName = parsedName.FullName }; } return null; } From ed19a9734bc79f008c68bfc191bbb8c800e6e0fb Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Thu, 6 Feb 2025 03:02:39 +0200 Subject: [PATCH 32/35] Update src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs Co-authored-by: Adam Sitnik --- .../System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 2fd1477709f19b..abd0ec87059f7d 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -120,6 +120,7 @@ public void NestedName(string input, string[] expectedNames) i--; } Assert.Equal(0, i); + Assert.Equal(0, i); } [Theory] From b924a6f0f4c13cc28e0893b0acae868a6edf29cc Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sat, 8 Feb 2025 18:04:53 +0200 Subject: [PATCH 33/35] Use alternative strategy to provide a `(message, typeName)` overload in Mono, while still avoiding changes to the Mono runtime code. --- .../Reflection/TypeNameResolver.CoreCLR.cs | 16 ++++++++-------- .../Reflection/TypeNameResolver.NativeAot.cs | 8 ++++---- .../src/System/TypeLoadException.cs | 17 ++++++++++------- .../System/Reflection/TypeNameResolver.Mono.cs | 8 ++++---- .../src/System/TypeLoadException.Mono.cs | 12 ++++++++++++ 5 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs index 6b9058bee56d79..c438aeb1d9a057 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs @@ -213,8 +213,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) - { TypeName = escapedTypeName }; + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName), + typeName: escapedTypeName); } return null; } @@ -246,8 +246,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, { if (_throwOnError) { - throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, runtimeAssembly.FullName)) - { TypeName = parsedName.FullName }; + throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, runtimeAssembly.FullName), + typeName: parsedName.FullName); } return null; } @@ -282,8 +282,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))) - { TypeName = parsedName.FullName }; + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), + typeName: parsedName.FullName); } return null; } @@ -320,8 +320,8 @@ internal static RuntimeType GetTypeReferencedByCustomAttribute(string typeName, if (_throwOnError) { - throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, (requestingAssembly ?? coreLib).FullName)) - { TypeName = parsedName.FullName }; + throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, parsedName.FullName, (requestingAssembly ?? coreLib).FullName), + typeName: parsedName.FullName); } return null; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs index dd88bbe4956241..4c9356e0ad533b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs @@ -146,8 +146,8 @@ internal partial struct TypeNameResolver { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) - { TypeNameResolver = escapedTypeName }; + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName), + typeName: escapedTypeName); } return null; } @@ -235,8 +235,8 @@ internal partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))); - { TypeName = parsedName.FullName }; + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), + typeName: parsedName.FullName); } return null; } diff --git a/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs b/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs index 7e93e952b43358..b269f391316250 100644 --- a/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs +++ b/src/libraries/System.Private.CoreLib/src/System/TypeLoadException.cs @@ -29,6 +29,15 @@ public TypeLoadException(string? message, Exception? inner) HResult = HResults.COR_E_TYPELOAD; } +#if !MONO + internal TypeLoadException(string message, string typeName) + : base(message) + { + HResult = HResults.COR_E_TYPELOAD; + _className = typeName; + } +#endif + public override string Message { get @@ -38,13 +47,7 @@ public override string Message } } - public string TypeName - { - get => _className ?? string.Empty; - // This is an init-only property, because a (string message, string typeName) constructor - // would conflict with Mono's (string typeName, string assemblyName) constructor. - internal init => _className = value; - } + public string TypeName => _className ?? string.Empty; [Obsolete(Obsoletions.LegacyFormatterImplMessage, DiagnosticId = Obsoletions.LegacyFormatterImplDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [EditorBrowsable(EditorBrowsableState.Never)] diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs index 146389a2f21eb0..252ff469956a85 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs @@ -115,8 +115,8 @@ internal unsafe ref partial struct TypeNameResolver { throw new TypeLoadException(assembly is null ? SR.Format(SR.TypeLoad_ResolveType, escapedTypeName) : - SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName)) - { TypeName = escapedTypeName }; + SR.Format(SR.TypeLoad_ResolveTypeFromAssembly, escapedTypeName, assembly.FullName), + typeName: escapedTypeName); } return null; } @@ -160,8 +160,8 @@ internal unsafe ref partial struct TypeNameResolver if (_throwOnError) { throw new TypeLoadException(SR.Format(SR.TypeLoad_ResolveNestedType, - nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName))) - { TypeName = parsedName.FullName }; + nestedTypeNames[i], (i > 0) ? nestedTypeNames[i - 1] : TypeName.Unescape(escapedTypeName)), + typeName: escapedTypeName); } return null; } diff --git a/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs b/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs index 9f4f4ced458b04..74bb386164d9fd 100644 --- a/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs @@ -13,6 +13,18 @@ internal TypeLoadException(string className, string assemblyName) _assemblyName = assemblyName; } + // Because the Mono runtime has a dependency to a (string, string) constructor overload, + // we add a dummy parameter with a default value to minimize native code changes. + // In order to use this overload, callers should either pass three arguments, or specify + // one of the parameters by name. + internal TypeLoadException(string message, string typeName, bool useMessageTypeNameOverload = true) + : base(message) + { + _ = useMessageTypeNameOverload; + HResult = HResults.COR_E_TYPELOAD; + _className = typeName; + } + private void SetMessageField() { if (_message != null) From c55b4871b4bc52a69030af6cd7a1da68566e0864 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sun, 9 Feb 2025 04:19:13 +0200 Subject: [PATCH 34/35] Address PR feedback. --- .../Reflection/Metadata/TypeNameParserHelpers.cs | 2 +- .../src/System/TypeLoadException.Mono.cs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index 8884c8397728bb..c44ad7920a94a2 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -129,7 +129,7 @@ internal static string Unescape(string input) static string UnescapeToBuilder(string name, int indexOfEscapeCharacter) { - // this code path is executed very rarely (IL Emit or pure IL with chars not allowed in C# or F#) + // This code path is executed very rarely (IL Emit or pure IL with chars not allowed in C# or F#). var sb = new ValueStringBuilder(stackalloc char[64]); sb.EnsureCapacity(name.Length); sb.Append(name.AsSpan(0, indexOfEscapeCharacter)); diff --git a/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs b/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs index 74bb386164d9fd..78670d58c3237c 100644 --- a/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/TypeLoadException.Mono.cs @@ -1,26 +1,26 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; + namespace System { public partial class TypeLoadException { - // Called by runtime - internal TypeLoadException(string className, string assemblyName) + [SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Called by runtime")] + private TypeLoadException(string className, string assemblyName) : this(null) { _className = className; _assemblyName = assemblyName; } - // Because the Mono runtime has a dependency to a (string, string) constructor overload, - // we add a dummy parameter with a default value to minimize native code changes. - // In order to use this overload, callers should either pass three arguments, or specify - // one of the parameters by name. - internal TypeLoadException(string message, string typeName, bool useMessageTypeNameOverload = true) + // Because the Mono runtime has a dependency on (string, string) constructors of exception types, + // we add a dummy parameter with a default value. + // In order to use this overload, callers should specify the typeName parameter by name. + internal TypeLoadException(string message, string typeName, bool _ = true) : base(message) { - _ = useMessageTypeNameOverload; HResult = HResults.COR_E_TYPELOAD; _className = typeName; } From 7378b4a8ba76551b93419e54e4d965dd75884b94 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Sun, 9 Feb 2025 16:54:37 -0800 Subject: [PATCH 35/35] Update src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs --- .../src/System/Reflection/Metadata/TypeName.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 8b6ea3696e4f02..783f90c5475acc 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -289,7 +289,7 @@ public string Name } else if (TypeNameParserHelpers.IndexOfNamespaceDelimiter(name) is int idx && idx >= 0) { - // If the type is not nested, find the namespace delimiter in the full name and and return the substring after it. + // If the type is not nested, find the namespace delimiter in the full name and return the substring after it. name = name.Slice(idx + 1); } _name = name.ToString();