Repository navigation
add TypeName.CreateSimpleTypeName - #136275
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/area-system-reflection-metadata |
|
Yes, specifying both now throws
|
|
It is just to make sure the nested type's short name comes out right,
without it the nested type's Name comes out wrong. Name is cut using the
declaring type's nested name length, and that is -1 unless the type was
parsed as a declaring type
…On Thu, 22 Oct 2026, 16:09 Jan Kotas, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs
<#136275 (comment)>:
> + }
+
+ if (!declaringType.IsSimple)
+ {
+ throw new ArgumentException(SR.Format(SR.Arg_NotSimpleTypeName, declaringType.FullName), nameof(declaringType));
+ }
+
+ if (assemblyInfo is not null)
+ {
+ throw new ArgumentException(SR.Arg_NestedTypeAssemblyInfo, nameof(assemblyInfo));
+ }
+
+ string declaringFullName = declaringType.FullName;
+ if (declaringType._nestedNameLength != declaringFullName.Length)
+ {
+ declaringType = new TypeName(declaringFullName, declaringType.AssemblyName, declaringType: declaringType._declaringType, nestedNameLength: declaringFullName.Length);
What is this line trying to achieve?
—
Reply to this email directly, view it on GitHub
<#136275?email_source=notifications&email_token=CLD57VKTABKP5AEE7I7YAH35VIPMHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRGY2DANZTHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5616407385>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CLD57VJUWE576QZT24CAV7T5VIPMHAVCNFSNUABFKJSXA33TNF2G64TZHMZDCMBXGE3DAMBVHNEXG43VMU5TKOJXGM2TONJRGU4KC5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/CLD57VJD7AFTXVUWY7FB7C35VIPMHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRGY2DANZTHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/CLD57VPTRAAM7ZK43IT4YG35VIPMHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNRRGY2DANZTHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I do not see the problem. Can you elaborate? |
Sorry, I should have shown it, you see when the declaring type comes from the parser it knows where its name ends, so nothing goes wrong. But when it is made with CreateSimpleTypeName, it never went through the parser, so that length is -1. Then Name cuts the nested type's full name at position 0 and you get the whole thing back instead of just the nested name. That line rebuilds the declaring type with the right length so the cut lands after the +. var outer = TypeName.CreateSimpleTypeName("Outer"); Honestly, I didn't think of this until now, Name could use _declaringType.FullName.Length instead, and this line can go, should I do that instead? |
|
For performance, it would be best to as lazy as possible with any allocations. The current API implementation in the PR is potentially going to allocate two Instead, it can just allocate |
Done |
There was a problem hiding this comment.
🔵 Needs a closer look
Human review should confirm API-approval provenance and build/test results for the new public factory and shared name-generation changes.
No open issues 🎉
What changed in this PR
Adds TypeName.CreateSimpleTypeName to construct simple and nested type names without reparsing concatenated strings, completing the API work described in #102263.
Changes:
- Adds the public factory with metadata-name escaping and argument validation.
- Updates lazy name generation to support factory-created nested types.
- Adds tests for escaping, nesting, assembly handling, derived names, and invalid arguments.
| File | Description |
|---|---|
src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs |
Tests factory behavior and validation. |
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs |
Adds metadata-name escaping. |
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs |
Implements the factory and nested-name support. |
src/libraries/System.Reflection.Metadata/src/Resources/Strings.resx |
Adds an error message for conflicting assembly arguments. |
src/libraries/System.Reflection.Metadata/ref/System.Reflection.Metadata.cs |
Exposes the factory in the reference assembly. |
Review effort: Balanced
fixes #102263
adds the remaining approved API. the name is escaped, nested names are joined to the declaring type with
+. tests added.