search

kekekeks / XamlX

General purpose pluggable XAML compiler with no runtime dependencies.
MIT License
319 stars 55 forks source link

IXamlType.FullName Inconsistency #119

Open workgroupengineering opened 6 months ago

workgroupengineering commented 6 months ago

While devlop test for #112, i found inconsistency when i get FullName of GenericType.

Sre

TypeSystemTest.Models.GenericType`1[[System.String, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]

Cecil

TypeSystemTest.Models.GenericType`1<System.String>

Which of both is the expected behavior?

maxkatz6 commented 6 months ago

Likely, Cecil one is expected.

Sre is a mix of Type.AssemblyQualifiedName (for generics) and FullName for the main type name itself.

Also, anything that compatible with .NET TypeNameParser is a bonus: https://github.com/dotnet/runtime/issues/97566

maxkatz6 commented 6 months ago

Looks like it's an expected behavior for Type.FullName to return assembly qualified names for generics. For some reason. https://learn.microsoft.com/en-us/dotnet/api/system.type.assemblyqualifiedname?view=net-8.0

maxkatz6 commented 6 months ago

For your tests, you might want to use these extensions probably: https://github.com/kekekeks/XamlX/blob/941dafce490ffd21178f4493a7f0425e9d478e60/src/XamlX/TypeSystem/TypeSystem.cs#L341-L351

Unless our GetFullName extension is missing generics (which it might do).

workgroupengineering commented 6 months ago

What do you think if I normalized the Sre FullName like in Cecil instead of using an extension method?

maxkatz6 commented 6 months ago

I don't know if it will break anything. It would require double checking any FullName usage. Also, it's still not clear which variant is more standard.

kasperk81 commented 6 months ago

Also, it's still not clear which variant is more standard.

there is no spec: https://github.com/dotnet/runtime/issues/97566#issuecomment-1921766972

in the future, more systems are likely to depend on TypeNameParser's behavior. @adamsitnik may able to clarify some confusion. cecil should probably switch to TNP for net9-and-above

adamsitnik commented 6 months ago

cecil should probably switch to TNP for net9-and-above

The new TypeName API is part of System.Reflection.Metadata package, which supports older monikers too:

image

Looks like it's an expected behavior for Type.FullName to return assembly qualified names for generics

Yes, but only for the generic arguments.

> typeof(List<int>).FullName
"System.Collections.Generic.List`1[[System.Int32, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]"
> typeof(List<int>).AssemblyQualifiedName
"System.Collections.Generic.List`1[[System.Int32, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e"