Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

TypeName parsing API #100094

Merged
merged 52 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
22d131e
TypeNameParser first ref and configurable input validation
adamsitnik Jan 23, 2024
5de9526
assembly name parsing
adamsitnik Jan 24, 2024
6dbcd59
initial generic type info parsing
adamsitnik Jan 25, 2024
6f1f161
decorator parsing
adamsitnik Jan 26, 2024
4ec4ea5
Handle single dimensional array indexing
adamsitnik Jan 29, 2024
f5a8716
implement TypeName.GetType
adamsitnik Jan 29, 2024
52cb351
nested types support
adamsitnik Jan 30, 2024
60ad6f0
support ignore case
adamsitnik Jan 31, 2024
b5349bd
integrate with System.Private.CoreLib:
adamsitnik Jan 31, 2024
2dbd091
integrate with System.Private.CoreLib:
adamsitnik Feb 1, 2024
bd78637
integrate with System.Private.CoreLib for Mono and clr tools, improve…
adamsitnik Feb 2, 2024
8fda94a
build fix 1/n
adamsitnik Feb 2, 2024
745e7bb
make TypeNameParser internal, extend TypeName with Parse and TryParse…
adamsitnik Feb 7, 2024
ee43e71
introduce TotalComplexity
adamsitnik Feb 7, 2024
a3a7f26
introduce FullName, so we have Name, FullName and AssemblyQualifiedNa…
adamsitnik Feb 7, 2024
a2296d0
back tick error handling
adamsitnik Feb 9, 2024
4c8a6f7
move helper methods to a standalone helper type, include it as a link…
adamsitnik Feb 9, 2024
eadf970
increase test coverage, improve edge case handling
adamsitnik Feb 12, 2024
abf7543
sample SerializationBinder that uses the new APIs
adamsitnik Feb 12, 2024
281c4f3
cover more serialization binder scenarios with the tests to ensure th…
adamsitnik Feb 13, 2024
dc58cce
strict mode parsing: type names
adamsitnik Feb 19, 2024
b947977
strict mode parsing: assembly names
adamsitnik Feb 20, 2024
4c9a7d5
Merge remote-tracking branch 'upstream/main' into typeNameParser
adamsitnik Feb 22, 2024
c63f3a8
fix the build and apply some design changes
adamsitnik Feb 22, 2024
83cdd1b
add escaping support
adamsitnik Feb 22, 2024
23ad44f
fix the last failing tests, increase test coverage, fix the perf, fix…
adamsitnik Feb 23, 2024
96b04f4
Merge remote-tracking branch 'upstream/main' into typeNameParser
adamsitnik Mar 19, 2024
c8014fd
apply changes based on the 1st API Design Review
adamsitnik Mar 19, 2024
1c76366
apply changes based on the final API Design Review
adamsitnik Mar 20, 2024
8cd205a
solve the TODOs
adamsitnik Mar 20, 2024
ae514ec
implement IEquatable, add missing exception messages, set default Max…
adamsitnik Mar 21, 2024
c7c67c8
address code review feedback, make some tests conditional, remove inv…
adamsitnik Mar 22, 2024
87804e9
Apply suggestions from code review
adamsitnik Mar 22, 2024
97aad27
supress IL3050:RequiresDynamicCode
adamsitnik Mar 22, 2024
f940723
remove everything related to strict parsing (it will come back in a s…
adamsitnik Mar 22, 2024
b1ed250
remove special handling for Array.MaxLength-many generic args
adamsitnik Mar 22, 2024
90a5582
remove the unused property, use the new names for non-public APIs as …
adamsitnik Mar 22, 2024
9a5d01b
make the allocations happen when they are actually needed for the fir…
adamsitnik Apr 2, 2024
cfb2216
don't pre-allocate full names for all declaring types, just store the…
adamsitnik Apr 3, 2024
d63365f
build fix
adamsitnik Apr 3, 2024
7d685c8
Merge remote-tracking branch 'upstream/main' into typeNameParser
adamsitnik Apr 9, 2024
04731fb
address part of the code review feedback
adamsitnik Apr 9, 2024
113a87f
fix the build
adamsitnik Apr 11, 2024
7005164
AssemblyNameInfo
adamsitnik Apr 15, 2024
cd8a9c2
don't enforce the back tick convention
adamsitnik Apr 16, 2024
1143a3e
Merge remote-tracking branch 'upstream/main' into typeNameParser
adamsitnik Apr 17, 2024
afdbc5f
address API and code review feedback:
adamsitnik Apr 17, 2024
413be9e
minor tweaks after reading the code again
adamsitnik Apr 17, 2024
b424d76
try to improve the escaping handling
adamsitnik Apr 18, 2024
da203c0
address code review feedback:
adamsitnik Apr 23, 2024
7979e27
address code review feedback:
adamsitnik Apr 24, 2024
22de761
Apply suggestions from code review
adamsitnik Apr 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
don't enforce the back tick convention
  • Loading branch information
adamsitnik committed Apr 16, 2024
commit cd8a9c29a1e589ab417ebc8dd055fa48fd184c29
55 changes: 35 additions & 20 deletions src/libraries/Common/src/System/Reflection/Metadata/TypeName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@

#nullable enable

using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Text;

#if !SYSTEM_PRIVATE_CORELIB
using System.Collections.Immutable;
#endif

namespace System.Reflection.Metadata
{
[DebuggerDisplay("{AssemblyQualifiedName}")]
Expand All @@ -28,16 +33,24 @@ sealed class TypeName : IEquatable<TypeName>
/// So when the name is needed, a substring is being performed.
/// </summary>
private readonly int _nestedNameLength;
private readonly TypeName[]? _genericArguments;
private readonly TypeName? _elementOrGenericType;
private readonly TypeName? _declaringType;
#if SYSTEM_PRIVATE_CORELIB
private readonly IReadOnlyList<TypeName> _genericArguments;
#else
private readonly ImmutableArray<TypeName> _genericArguments;
#endif
private string? _name, _fullName, _assemblyQualifiedName;

internal TypeName(string? fullName,
AssemblyNameInfo? assemblyName,
TypeName? elementOrGenericType = default,
TypeName? declaringType = default,
TypeName[]? genericTypeArguments = default,
#if SYSTEM_PRIVATE_CORELIB
List<TypeName>? genericTypeArguments = default,
#else
ImmutableArray<TypeName>.Builder? genericTypeArguments = default,
#endif
sbyte rankOrModifier = default,
int nestedNameLength = -1)
{
Expand All @@ -46,11 +59,15 @@ internal TypeName(string? fullName,
_rankOrModifier = rankOrModifier;
_elementOrGenericType = elementOrGenericType;
_declaringType = declaringType;
_genericArguments = genericTypeArguments;
_nestedNameLength = nestedNameLength;

Debug.Assert(!(IsArray || IsPointer || IsByRef) || _elementOrGenericType is not null);
Debug.Assert(_genericArguments is null || _elementOrGenericType is not null);
#if SYSTEM_PRIVATE_CORELIB
_genericArguments = genericTypeArguments is not null ? genericTypeArguments : Array.Empty<TypeName>();
#else
_genericArguments = genericTypeArguments is null
? ImmutableArray<TypeName>.Empty
: genericTypeArguments.Count == genericTypeArguments.Capacity ? genericTypeArguments.MoveToImmutable() : genericTypeArguments.ToImmutableArray();
#endif
}

/// <summary>
Expand Down Expand Up @@ -100,7 +117,7 @@ public string FullName
{
if (_fullName is null)
{
if (_genericArguments is not null)
if (IsConstructedGenericType)
{
_fullName = TypeNameParserHelpers.GetGenericTypeFullName(GetGenericTypeDefinition().FullName.AsSpan(), _genericArguments);
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -138,7 +155,12 @@ public string FullName
/// <remarks>
/// Returns false for open generic types (e.g., "Dictionary&lt;,&gt;").
/// </remarks>
public bool IsConstructedGenericType => _genericArguments is not null;
public bool IsConstructedGenericType =>
#if SYSTEM_PRIVATE_CORELIB
_genericArguments.Count > 0;
#else
_genericArguments.Length > 0;
#endif

/// <summary>
/// Returns true if this is a "plain" type; that is, not an array, not a pointer, not a reference, and
Expand Down Expand Up @@ -274,12 +296,9 @@ public int GetNodeCount()
result = checked(result + GetElementType().GetNodeCount());
}

if (_genericArguments is not null)
foreach (TypeName genericArgument in _genericArguments)
{
foreach (TypeName genericArgument in _genericArguments)
{
result = checked(result + genericArgument.GetNodeCount());
}
result = checked(result + genericArgument.GetNodeCount());
}

return result;
Expand Down Expand Up @@ -358,16 +377,12 @@ public int GetArrayRank()
/// <para>For example, given "Dictionary&lt;string, int&gt;", returns a 2-element array containing
/// string and int.</para>
/// </remarks>
public
#if SYSTEM_PRIVATE_CORELIB
public TypeName[] GetGenericArguments() => _genericArguments ?? Array.Empty<TypeName>();
IReadOnlyList<TypeName>
#else
public Collections.Immutable.ImmutableArray<TypeName> GetGenericArguments()
=> _genericArguments is null ? Collections.Immutable.ImmutableArray<TypeName>.Empty :
#if NET8_0_OR_GREATER
Runtime.InteropServices.ImmutableCollectionsMarshal.AsImmutableArray(_genericArguments);
#else
Collections.Immutable.ImmutableArray.Create(_genericArguments);
#endif
ImmutableArray<TypeName>
#endif
GetGenericArguments() => _genericArguments;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
using System.Diagnostics;
using System.Text;

#if !SYSTEM_PRIVATE_CORELIB
using System.Collections.Immutable;
#endif

using static System.Reflection.Metadata.TypeNameParserHelpers;

#nullable enable
Expand Down Expand Up @@ -66,17 +70,20 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
}

List<int>? nestedNameLengths = null;
if (!TryGetTypeNameInfo(ref _inputString, ref nestedNameLengths, out int fullTypeNameLength, out int genericArgCount))
if (!TryGetTypeNameInfo(ref _inputString, ref nestedNameLengths, out int fullTypeNameLength))
{
return null;
}

ReadOnlySpan<char> fullTypeName = _inputString.Slice(0, fullTypeNameLength);
_inputString = _inputString.Slice(fullTypeNameLength);

int genericArgIndex = 0;
// Don't allocate now, as it may be an open generic type like "List`1"
TypeName[]? genericArgs = null;
// Don't allocate now, as it may be an open generic type like "Name`1"
#if SYSTEM_PRIVATE_CORELIB
List<TypeName>? genericArgs = null;
#else
ImmutableArray<TypeName>.Builder? genericArgs = null;
#endif

// Are there any captured generic args? We'll look for "[[" and "[".
// There are no spaces allowed before the first '[', but spaces are allowed
Expand All @@ -85,36 +92,18 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
ReadOnlySpan<char> capturedBeforeProcessing = _inputString;
if (IsBeginningOfGenericArgs(ref _inputString, out bool doubleBrackets))
{
int startingRecursionCheck = recursiveDepth;
int maxObservedRecursionCheck = recursiveDepth;

ParseAnotherGenericArg:

// Invalid generic argument count provided after backtick.
// Examples:
// - too many: List`1[[a], [b]]
// - not expected: NoBacktick[[a]]
if (genericArgIndex >= genericArgCount)
{
return null;
}

recursiveDepth = startingRecursionCheck;
// Namespace.Type`1[[GenericArgument1, AssemblyName1],[GenericArgument2, AssemblyName2]] - double square bracket syntax allows for fully qualified type names
// Namespace.Type`1[GenericArgument1,GenericArgument2] - single square bracket syntax is legal only for non-fully qualified type names
// Namespace.Type`1[[GenericArgument1, AssemblyName1], GenericArgument2] - mixed mode
// Namespace.Type`1[GenericArgument1, [GenericArgument2, AssemblyName2]] - mixed mode
// Namespace.Type`2[[GenericArgument1, AssemblyName1],[GenericArgument2, AssemblyName2]] - double square bracket syntax allows for fully qualified type names
// Namespace.Type`2[GenericArgument1,GenericArgument2] - single square bracket syntax is legal only for non-fully qualified type names
// Namespace.Type`2[[GenericArgument1, AssemblyName1], GenericArgument2] - mixed mode
// Namespace.Type`2[GenericArgument1, [GenericArgument2, AssemblyName2]] - mixed mode
TypeName? genericArg = ParseNextTypeName(allowFullyQualifiedName: doubleBrackets, ref recursiveDepth);
if (genericArg is null) // parsing failed
{
return null;
}

if (recursiveDepth > maxObservedRecursionCheck)
{
maxObservedRecursionCheck = recursiveDepth;
}

// For [[, there had better be a ']' after the type name.
if (doubleBrackets && !TryStripFirstCharAndTrailingSpaces(ref _inputString, ']'))
{
Expand All @@ -123,17 +112,13 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse

if (genericArgs is null)
{
// Parsing the rest would hit the limit.
// -1 because the first generic arg has been already parsed.
if (maxObservedRecursionCheck + genericArgCount - 1 > _parseOptions.MaxNodes)
{
recursiveDepth = _parseOptions.MaxNodes;
return null;
}

genericArgs = new TypeName[genericArgCount];
#if SYSTEM_PRIVATE_CORELIB
genericArgs = new List<TypeName>(2);
#else
genericArgs = ImmutableArray.CreateBuilder<TypeName>(2);
#endif
}
genericArgs[genericArgIndex++] = genericArg;
genericArgs.Add(genericArg);

// Is there a ',[' indicating fully qualified generic type arg?
// Is there a ',' indicating non-fully qualified generic type arg?
Expand All @@ -150,16 +135,6 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
{
return null;
}

// We have reached the end of generic arguments, but parsed fewer than expected.
// Example: A`2[[b]]
if (genericArgIndex != genericArgCount)
{
return null;
}

// And now that we're at the end, restore the max observed recursion count.
recursiveDepth = maxObservedRecursionCheck;
}

// If there was an error stripping the generic args, back up to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,54 +24,7 @@ internal static class TypeNameParserHelpers
private static readonly SearchValues<char> _endOfFullTypeNameDelimitersSearchValues = SearchValues.Create("[]&*,+\\");
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
#endif

/// <returns>
/// <para>Negative value for invalid type names.</para>
/// <para>Zero for valid non-generic type names.</para>
/// <para>Positive value for valid generic type names.</para>
/// </returns>
internal static int GetGenericArgumentCount(ReadOnlySpan<char> fullTypeName)
{
const int ShortestInvalidTypeName = 2; // Back tick and one digit. Example: "`1"
if (fullTypeName.Length < ShortestInvalidTypeName || !IsAsciiDigit(fullTypeName[fullTypeName.Length - 1]))
{
return 0;
}

int backtickIndex = fullTypeName.Length - 2; // we already know it's true for the last one
for (; backtickIndex >= 0; backtickIndex--)
{
if (fullTypeName[backtickIndex] == '`')
{
if (backtickIndex == 0)
{
return -1; // illegal name, example "`1"
}
else if (fullTypeName[backtickIndex - 1] == EscapeCharacter)
{
return 0; // legal name, but not a generic type definition. Example: "Escaped\\`1"
}
else if (TryParse(fullTypeName.Slice(backtickIndex + 1), out int value))
{
// From C# 2.0 language spec: 8.16.3 Multiple type parameters Generic type declarations can have any number of type parameters.
// There is no special treatment for values larger than Array.MaxLength,
// as the parser should simply prevent from parsing that many nodes.
// The value can still be negative, but it's fine as the caller should treat that as an error.
return value;
}

// most likely the value was too large to be parsed as an int
return -1;
}
else if (!IsAsciiDigit(fullTypeName[backtickIndex]) && fullTypeName[backtickIndex] != '-')
{
break;
}
}

return 0;
}

internal static string GetGenericTypeFullName(ReadOnlySpan<char> fullTypeName, TypeName[] genericArgs)
internal static string GetGenericTypeFullName(ReadOnlySpan<char> fullTypeName, IReadOnlyList<TypeName> genericArgs)
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
{
ValueStringBuilder result = new(stackalloc char[128]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be made more efficient by creating one top level ValueStringBuilder instead of creating ValueStringBuilder at each level of the generic type recursion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. But after I've addressed #100094 (comment) FullName is very unlikely to be used for generic types. Since it should be rare I am not going to implement it for now.

result.Append(fullTypeName);
Expand Down Expand Up @@ -239,12 +192,10 @@ internal static bool IsBeginningOfGenericArgs(ref ReadOnlySpan<char> span, out b
return false;
}

internal static bool TryGetTypeNameInfo(ref ReadOnlySpan<char> input, ref List<int>? nestedNameLengths,
out int totalLength, out int genericArgCount)
internal static bool TryGetTypeNameInfo(ref ReadOnlySpan<char> input, ref List<int>? nestedNameLengths, out int totalLength)
{
bool isNestedType;
totalLength = 0;
genericArgCount = 0;
do
{
int length = GetFullTypeNameLength(input.Slice(totalLength), out isNestedType);
Expand All @@ -267,14 +218,6 @@ internal static bool TryGetTypeNameInfo(ref ReadOnlySpan<char> input, ref List<i
length--;
}
#endif

int generics = GetGenericArgumentCount(input.Slice(totalLength, length));
if (generics < 0 || (generics > 0 && ((long)genericArgCount + generics > int.MaxValue)))
{
return false; // invalid type name detected!
}
genericArgCount += generics;

if (isNestedType)
{
// do not validate the type name now, it will be validated as a whole nested type name later
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,13 @@ private static (string typeNamespace, string name) SplitFullTypeName(string type
else if (typeName.IsConstructedGenericType)
{
var genericArgs = typeName.GetGenericArguments();
Type[] genericTypes = new Type[genericArgs.Length];
for (int i = 0; i < genericArgs.Length; i++)
#if SYSTEM_PRIVATE_CORELIB
int size = genericArgs.Count;
#else
int size = genericArgs.Length;
#endif
Type[] genericTypes = new Type[size];
for (int i = 0; i < size; i++)
{
Type? genericArg = Resolve(genericArgs[i]);
if (genericArg is null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2429,7 +2429,7 @@ public sealed partial class TypeName : System.IEquatable<System.Reflection.Metad
{
internal TypeName() { }
public string AssemblyQualifiedName { get { throw null; } }
public string? AssemblySimpleName { get { throw null; } }
public AssemblyNameInfo? AssemblyName { get { throw null; } }
public System.Reflection.Metadata.TypeName? DeclaringType { get { throw null; } }
public string FullName { get { throw null; } }
public bool IsArray { get { throw null; } }
Expand All @@ -2447,7 +2447,6 @@ internal TypeName() { }
public bool Equals(System.Reflection.Metadata.TypeName? other) { throw null; }
public override int GetHashCode() { throw null; }
public int GetArrayRank() { throw null; }
public System.Reflection.AssemblyName? GetAssemblyName() { throw null; }
public System.Collections.Immutable.ImmutableArray<System.Reflection.Metadata.TypeName> GetGenericArguments() { throw null; }
public System.Reflection.Metadata.TypeName GetGenericTypeDefinition() { throw null; }
public System.Reflection.Metadata.TypeName GetElementType() { throw null; }
Expand Down
Loading