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.

Sign up for GitHub

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

Jump to bottom

Improve DefaultTypeConverter #1461

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Improve DefaultTypeConverter #1461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
103 changes: 49 additions & 54 deletions Jint/Runtime/Interop/DefaultTypeConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public class DefaultTypeConverter : ITypeConverter

private readonly record struct TypeConversionKey(Type Source, Type Target);

private static readonly ConcurrentDictionary<TypeConversionKey, bool> _knownConversions = new();
private static readonly ConcurrentDictionary<TypeConversionKey, MethodInfo?> _knownCastOperators = new();

private static readonly Type intType = typeof(int);
Expand All @@ -39,28 +38,48 @@ public DefaultTypeConverter(Engine engine)

public virtual object? Convert(object? value, Type type, IFormatProvider formatProvider)
{
if (!TryConvert(value, type, formatProvider, propagateException: true, out var converted, out var problemMessage))
{
ExceptionHelper.ThrowError(_engine, problemMessage ?? $"Unable to convert {value} to type {type}");
}
return converted;
}

public virtual bool TryConvert(object? value, Type type, IFormatProvider formatProvider, [NotNullWhen(true)] out object? converted)
{
return TryConvert(value, type, formatProvider, propagateException: false, out converted, out _);
}

private bool TryConvert(object? value, Type type, IFormatProvider formatProvider, bool propagateException, out object? converted, out string? problemMessage)
{
converted = null;
problemMessage = null;

if (value is null)
{
if (TypeConverter.TypeIsNullable(type))
{
return null;
return true;
}

ExceptionHelper.ThrowNotSupportedException($"Unable to convert null to '{type.FullName}'");
problemMessage = $"Unable to convert null to '{type.FullName}'";
return false;
}

// don't try to convert if value is derived from type
if (type.IsInstanceOfType(value))
{
return value;
converted = value;
return true;
}

if (type.IsGenericType)
{
var result = TypeConverter.IsAssignableToGenericType(value.GetType(), type);
if (result.IsAssignable)
{
return value;
converted = value;
return true;
}
}

Expand All @@ -77,7 +96,8 @@ public DefaultTypeConverter(Engine engine)
ExceptionHelper.ThrowArgumentOutOfRangeException();
}

return Enum.ToObject(type, integer);
converted = Enum.ToObject(type, integer);
return true;
}

var valueType = value.GetType();
Expand All @@ -90,7 +110,8 @@ public DefaultTypeConverter(Engine engine)
&& (type == typeof(long) || type == typeof(int) || type == typeof(short) || type == typeof(byte) || type == typeof(ulong) || type == typeof(uint) || type == typeof(ushort) || type == typeof(sbyte)))
{
// this is not safe
ExceptionHelper.ThrowArgumentOutOfRangeException(nameof(value) , "Cannot convert floating point number with decimals to integral type");
problemMessage = $"Cannot convert floating point number {doubleValue} with decimals to integral type {type}";
return false;
}
}

Expand All @@ -113,7 +134,8 @@ public DefaultTypeConverter(Engine engine)
functionInstance?.SetHiddenClrObjectProperty(delegatePropertyKey, d);
}

return d;
converted = d;
return true;
}
}

Expand All @@ -122,7 +144,8 @@ public DefaultTypeConverter(Engine engine)
var source = value as object[];
if (source == null)
{
ExceptionHelper.ThrowArgumentException($"Value of object[] type is expected, but actual type is {value.GetType()}.");
problemMessage = $"Value of object[] type is expected, but actual type is {value.GetType()}";
return false;
}

var targetElementType = type.GetElementType();
Expand All @@ -133,7 +156,9 @@ public DefaultTypeConverter(Engine engine)
}
var result = Array.CreateInstance(targetElementType, source.Length);
itemsConverted.CopyTo(result, 0);
return result;

converted = result;
return true;
}

var typeDescriptor = TypeDescriptor.Get(valueType);
Expand All @@ -144,7 +169,8 @@ public DefaultTypeConverter(Engine engine)
// value types
if (type.IsValueType && constructors.Length > 0)
{
ExceptionHelper.ThrowArgumentException("No valid constructors found");
problemMessage = $"No valid constructors found for {type}";
return false;
}

var constructorParameters = Array.Empty<object>();
Expand All @@ -167,7 +193,7 @@ public DefaultTypeConverter(Engine engine)
foreach (var constructor in constructors)
{
var parameterInfos = constructor.GetParameters();
if (parameterInfos.All(p => p.IsOptional) && constructor.IsPublic)
if (parameterInfos.All(static p => p.IsOptional) && constructor.IsPublic)
{
constructorParameters = new object[parameterInfos.Length];
found = true;
Expand All @@ -178,7 +204,8 @@ public DefaultTypeConverter(Engine engine)

if (!found)
{
ExceptionHelper.ThrowArgumentException("No valid constructors found");
problemMessage = $"No valid constructors found for type {type}";
return false;
}
}

Expand All @@ -202,28 +229,31 @@ public DefaultTypeConverter(Engine engine)
}
}

return obj;
converted = obj;
return true;
}

try
{
return System.Convert.ChangeType(value, type, formatProvider);
converted = System.Convert.ChangeType(value, type, formatProvider);
return true;
}
catch (Exception e)
{
// check if we can do a cast with operator overloading
if (TryCastWithOperators(value, type, valueType, out var invoke))
{
return invoke;
converted = invoke;
return true;
}

if (!_engine.Options.Interop.ExceptionHandler(e))
if (propagateException && !_engine.Options.Interop.ExceptionHandler(e))
{
throw;
}

ExceptionHelper.ThrowError(_engine, e.Message);
return null;
problemMessage = e.Message;
return false;
}
}

Expand Down Expand Up @@ -331,41 +361,6 @@ private static bool TryCastWithOperators(object value, Type type, Type valueType
return false;
}

public virtual bool TryConvert(object? value, Type type, IFormatProvider formatProvider, out object? converted)
{
var key = new TypeConversionKey(value?.GetType() ?? typeof(void), type);

// string conversion is not stable, "filter" -> int is invalid, "0" -> int is valid
var canTryConvert = value is string || _knownConversions.GetOrAdd(key, _ =>
{
try
{
Convert(value, type, formatProvider);
return true;
}
catch
{
return false;
}
});

if (canTryConvert)
{
try
{
converted = Convert(value, type, formatProvider);
return true;
}
catch
{
converted = null;
return false;
}
}

converted = null;
return false;
}
}

internal static class ObjectExtensions
Expand Down
23 changes: 17 additions & 6 deletions Jint/Runtime/Interop/ITypeConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
namespace Jint.Runtime.Interop
using System.Diagnostics.CodeAnalysis;

namespace Jint.Runtime.Interop;

/// <summary>
/// Handles conversions between CLR types.
/// </summary>
public interface ITypeConverter
{
public interface ITypeConverter
{
object? Convert(object? value, Type type, IFormatProvider formatProvider);
bool TryConvert(object? value, Type type, IFormatProvider formatProvider, out object? converted);
}
/// <summary>
/// Converts value to to type. Throws exception if cannot be done.
/// </summary>
object? Convert(object? value, Type type, IFormatProvider formatProvider);

/// <summary>
/// Converts value to to type. Returns false if cannot be done.
/// </summary>
bool TryConvert(object? value, Type type, IFormatProvider formatProvider, [NotNullWhen(true)] out object? converted);
}