As per https://github.com/dotnet/runtime/issues/29823, it is no longer reliable to directly check for TypeConverterAttribute
as some types are now handled intrinsically within TypeConverter
.
The following code needs to utilize TypeDescriptor.GetConverter
instead:
Related issues: #9815
@SeeminglyScience Thanks for great investigations! Do you ready to pull PR?
I found TypeConverterAttribute still in some places.
Do you ready to pull PR?
Nah you can mark this one up for grabs I think. It should be a simple change but it will need some tests that I don't have the bandwidth for atm.
It should be a simple change
It can be not trivial. The TypeConverterAttribute is used in PSMemberViewTypes.
@SteveL-MSFT It seems it is critical for 7.0.
@iSazonov we'd need to test to be sure, but from the referenced .NET Core issue it seems .NET Core team are saying that we should not need to be checking directly for TypeConverterAttribute
, and that the TypeDescriptor.GetConverter
method(s) will check that as well as the various other methods of registering type converters.
TypeConverterAttribute is not deprecated and we should keep current code. Also the attribute can be used in any custom code.
I tried direct fix and it doesn't work. Need more investigations.
To put it simple - the capability is required for PS 7 compatibility with WMF 5.1 and earlier code. How it is implemented is unimportant but the TypeConverter is what seems to be used in Net 4.x and earlier.
PowerShell has supported this since at least PS2. There are products that are dependent on this capability through PowerShell syntax support for:
[System.Drawing.Point]'10,20'
Now fix works. Will pull tomorrow with a test.
@iSazonov given the changes that .NET made in .NET 5, has this been fixed in 7.1? If so, can you please close it as fixed?
@joeyaiello .Net team changed the implementation some times for last 1-2 years because they want to reduce cross assembly dependencies. I think we need re-check that all converters we want to have work in current version (I think they is near RC and .Net 5 has stable package design).
Most helpful comment
Now fix works. Will pull tomorrow with a test.