Skip to content

Commit

Permalink
[X] simpilfy and avoid duplication in conversion (#7143)
Browse files Browse the repository at this point in the history
Simplify, and unify in a single place, the way we look for
TypeConverters on BP, properties, and type.

Remove a unused IValueProvider, fix code here and there

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
  • Loading branch information
StephaneDelcroix and mattleibow authored Aug 5, 2022
1 parent 22a6e00 commit b8ec9fb
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 255 deletions.
41 changes: 41 additions & 0 deletions src/Controls/src/Core/BindableProperty.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
Expand Down Expand Up @@ -240,5 +241,45 @@ internal bool TryConvert(ref object value)
}

internal delegate void BindablePropertyBindingChanging(BindableObject bindable, BindingBase oldValue, BindingBase newValue);

static readonly ConcurrentDictionary<BindableProperty, TypeConverter> s_converterCache = new();

internal TypeConverter GetBindablePropertyTypeConverter()
=> GetBindablePropertyTypeConverter(isAttached: null);

internal TypeConverter GetBindablePropertyTypeConverter(bool? isAttached)
{
if (s_converterCache.TryGetValue(this, out var converter))
return converter;

Type converterType = null;
if (!isAttached.HasValue || !isAttached.Value)
try
{
converterType = ((MemberInfo)DeclaringType.GetRuntimeProperty(PropertyName))?.GetTypeConverterType();
}
catch (AmbiguousMatchException e) //GetRuntimeProperty can throw, we'd like to catch that and rephrase
{
throw new AmbiguousMatchException($"Multiple properties with name '{DeclaringType}.{PropertyName}' found.", e);
}

if ((!isAttached.HasValue && converterType == null) || (isAttached.HasValue && isAttached.Value))
try
{
converterType = (DeclaringType.GetRuntimeMethod("Get" + PropertyName, new[] { typeof(BindableObject) }))?.GetTypeConverterType();
}
catch (AmbiguousMatchException e) //GetRuntimeProperty can throw, we'd like to catch that and rephrase
{
throw new AmbiguousMatchException($"Multiple properties with name '{DeclaringType}.Get{PropertyName}' found.", e);
}

if (converterType == null)
converterType = ReturnType.GetTypeConverterType();

converter = converterType == null ? null : (TypeConverter)Activator.CreateInstance(converterType);

// cache the result, even if it is null
return (s_converterCache[this] = converter);
}
}
}
3 changes: 2 additions & 1 deletion src/Controls/src/Core/IValueConverterProvider.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
using System;
using System.ComponentModel;
using System.Reflection;

namespace Microsoft.Maui.Controls.Xaml
{
interface IValueConverterProvider
{
object Convert(object value, Type toType, Func<MemberInfo> minfoRetriever, IServiceProvider serviceProvider);
object Convert(object value, Type toType, Func<TypeConverter> getTypeConverter, IServiceProvider serviceProvider);
}
}
6 changes: 5 additions & 1 deletion src/Controls/src/Core/Interactivity/BindingCondition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ bool EqualsToValue(object other)
return true;

object converted = null;

if (s_valueConverter != null)
converted = s_valueConverter.Convert(Value, other != null ? other.GetType() : typeof(object), null, null);
converted = s_valueConverter.Convert(Value,
other != null ? other.GetType() : typeof(object),
other != null ? other.GetType().GetTypeConverter : null,
null);
else
return false;

Expand Down
47 changes: 7 additions & 40 deletions src/Controls/src/Core/Interactivity/PropertyCondition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
namespace Microsoft.Maui.Controls
{
/// <include file="../../../docs/Microsoft.Maui.Controls/PropertyCondition.xml" path="Type[@FullName='Microsoft.Maui.Controls.PropertyCondition']/Docs" />
[ProvideCompiled("Microsoft.Maui.Controls.XamlC.PassthroughValueProvider")]
[AcceptEmptyServiceProvider]
public sealed class PropertyCondition : Condition, IValueProvider
public sealed class PropertyCondition : Condition
{
readonly BindableProperty _stateProperty;

Expand All @@ -24,7 +22,7 @@ public PropertyCondition()
/// <include file="../../../docs/Microsoft.Maui.Controls/PropertyCondition.xml" path="//Member[@MemberName='Property']/Docs" />
public BindableProperty Property
{
get { return _property; }
get => _property;
set
{
if (_property == value)
Expand All @@ -35,27 +33,14 @@ public BindableProperty Property

//convert the value
if (_property != null && s_valueConverter != null)
{
Func<MemberInfo> minforetriever = () =>
{
try
{
return Property.DeclaringType.GetRuntimeProperty(Property.PropertyName);
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple properties with name '{Property.DeclaringType}.{Property.PropertyName}' found.", new XmlLineInfo(), innerException: e);
}
};
Value = s_valueConverter.Convert(Value, Property.ReturnType, minforetriever, null);
}
_triggerValue = s_valueConverter.Convert(Value, _property.ReturnType, _property.GetBindablePropertyTypeConverter, null);
}
}

/// <include file="../../../docs/Microsoft.Maui.Controls/PropertyCondition.xml" path="//Member[@MemberName='Value']/Docs" />
public object Value
{
get { return _triggerValue; }
get => _triggerValue;
set
{
if (_triggerValue == value)
Expand All @@ -65,30 +50,12 @@ public object Value

//convert the value
if (_property != null && s_valueConverter != null)
{
Func<MemberInfo> minforetriever = () =>
{
try
{
return Property.DeclaringType.GetRuntimeProperty(Property.PropertyName);
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple properties with name '{Property.DeclaringType}.{Property.PropertyName}' found.", new XmlLineInfo(), innerException: e);
}
};
value = s_valueConverter.Convert(value, Property.ReturnType, minforetriever, null);
}
_triggerValue = value;
_triggerValue = s_valueConverter.Convert(value, _property.ReturnType, _property.GetBindablePropertyTypeConverter, null);
else
_triggerValue = value;
}
}

object IValueProvider.ProvideValue(IServiceProvider serviceProvider)
{
//This is no longer required
return this;
}

internal override bool GetState(BindableObject bindable)
{
return (bool)bindable.GetValue(_stateProperty);
Expand Down
4 changes: 2 additions & 2 deletions src/Controls/src/Core/OnPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ public static implicit operator T(OnPlatform<T> onPlatform)
continue;
if (!onPlat.Platform.Contains(DeviceInfo.Platform.ToString()))
continue;
return (T)s_valueConverter.Convert(onPlat.Value, typeof(T), null, null);
return (T)s_valueConverter.Convert(onPlat.Value, typeof(T), typeof(T).GetTypeConverter, null);
}

// fallback for UWP
foreach (var onPlat in onPlatform.Platforms)
{
if (onPlat.Platform != null && onPlat.Platform.Contains("UWP") && DeviceInfo.Platform == DevicePlatform.WinUI)
return (T)s_valueConverter.Convert(onPlat.Value, typeof(T), null, null);
return (T)s_valueConverter.Convert(onPlat.Value, typeof(T), typeof(T).GetTypeConverter, null);
}
}

Expand Down
25 changes: 1 addition & 24 deletions src/Controls/src/Core/Setter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,8 @@ object IValueProvider.ProvideValue(IServiceProvider serviceProvider)
throw new XamlParseException("Property not set", serviceProvider);
var valueconverter = serviceProvider.GetService(typeof(IValueConverterProvider)) as IValueConverterProvider;

MemberInfo minforetriever()
{
MemberInfo minfo = null;
try
{
minfo = Property.DeclaringType.GetRuntimeProperty(Property.PropertyName);
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple properties with name '{Property.DeclaringType}.{Property.PropertyName}' found.", serviceProvider, innerException: e);
}
if (minfo != null)
return minfo;
try
{
return Property.DeclaringType.GetRuntimeMethod("Get" + Property.PropertyName, new[] { typeof(BindableObject) });
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple methods with name '{Property.DeclaringType}.Get{Property.PropertyName}' found.", serviceProvider, innerException: e);
}
}
Value = valueconverter.Convert(Value, Property.ReturnType, Property.GetBindablePropertyTypeConverter, serviceProvider);

object value = valueconverter.Convert(Value, Property.ReturnType, minforetriever, serviceProvider);
Value = value;
return this;
}

Expand Down
27 changes: 2 additions & 25 deletions src/Controls/src/Core/StyleSheets/Style.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,33 +92,10 @@ public void Apply(VisualElement styleable, bool inheriting = false)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
static object Convert(object target, object value, BindableProperty property)
{
var serviceProvider = new StyleSheetServiceProvider(target, property);
Func<MemberInfo> minforetriever =
() =>
{
MemberInfo minfo = null;
try
{
minfo = property.DeclaringType.GetRuntimeProperty(property.PropertyName);
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple properties with name '{property.DeclaringType}.{property.PropertyName}' found.", serviceProvider, innerException: e);
}
if (minfo != null)
return minfo;
try
{
return property.DeclaringType.GetRuntimeMethod("Get" + property.PropertyName, new[] { typeof(BindableObject) });
}
catch (AmbiguousMatchException e)
{
throw new XamlParseException($"Multiple methods with name '{property.DeclaringType}.Get{property.PropertyName}' found.", serviceProvider, innerException: e);
}
};
var ret = value.ConvertTo(property.ReturnType, minforetriever, serviceProvider, out Exception exception);
var ret = value.ConvertTo(property.ReturnType, property.GetBindablePropertyTypeConverter, new StyleSheetServiceProvider(target, property), out var exception);
if (exception != null)
throw exception;

return ret;
}

Expand Down
77 changes: 54 additions & 23 deletions src/Controls/src/Core/Xaml/TypeConversionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ internal static object ConvertTo(this object value, Type toType, Func<ParameterI
if (pinfoRetriever == null || pinfoRetriever() is not ParameterInfo pInfo)
return null;
var convertertype = pInfo.CustomAttributes.GetTypeConverterType();
var convertertype = pInfo.GetTypeConverterType();
if (convertertype == null)
return null;
return (TypeConverter)Activator.CreateInstance(convertertype);
Expand All @@ -68,10 +68,8 @@ internal static object ConvertTo(this object value, Type toType, Func<MemberInfo
{
if (!s_converterCache.TryGetValue(memberInfo, out converter))
{
if (memberInfo.CustomAttributes.GetTypeConverterType() is Type converterType)
{
if (memberInfo.GetTypeConverterType() is Type converterType)
converter = (TypeConverter)Activator.CreateInstance(converterType);
}
// cache the result, even if it is null
s_converterCache[memberInfo] = converter;
Expand All @@ -85,7 +83,7 @@ internal static object ConvertTo(this object value, Type toType, Func<MemberInfo
if (!s_converterCache.TryGetValue(toType, out converter))
{
if (toType.CustomAttributes.GetTypeConverterType() is Type converterType)
if (toType.GetTypeConverterType() is Type converterType)
{
converter = (TypeConverter)Activator.CreateInstance(converterType);
}
Expand All @@ -100,18 +98,64 @@ internal static object ConvertTo(this object value, Type toType, Func<MemberInfo
return ConvertTo(value, toType, getConverter, serviceProvider, out exception);
}

static Type GetTypeConverterType(this IEnumerable<CustomAttributeData> attributes)
internal static TypeConverter GetTypeConverter(this MemberInfo memberInfo)
{
foreach (var converterAttribute in attributes)
if (s_converterCache.TryGetValue(memberInfo, out var converter))
return converter;

var converterType = memberInfo.GetTypeConverterType();
if (converterType == null && memberInfo is PropertyInfo propertyInfo)
converterType = propertyInfo.PropertyType.GetTypeConverterType();

converter = converterType == null ? null : (TypeConverter)Activator.CreateInstance(converterType);

// cache the result, even if it is null
return (s_converterCache[memberInfo] = converter);
}

internal static Type GetTypeConverterType(this MemberInfo memberInfo)
{
if (memberInfo == null)
return null;

var attributes = memberInfo.CustomAttributes;
if (attributes == null)
return null;

foreach (var attribute in attributes)
{
if (converterAttribute.AttributeType != typeof(System.ComponentModel.TypeConverterAttribute))
if (attribute.AttributeType != typeof(System.ComponentModel.TypeConverterAttribute))
continue;
var ctor = converterAttribute.ConstructorArguments[0];
var ctor = attribute.ConstructorArguments[0];
if (ctor.ArgumentType == typeof(string))
return Type.GetType((string)ctor.Value);
if (ctor.ArgumentType == typeof(Type))
return (Type)ctor.Value;
}

return null;
}

internal static Type GetTypeConverterType(this ParameterInfo parameterInfo)
{
if (parameterInfo == null)
return null;

var attributes = parameterInfo.CustomAttributes;
if (attributes == null)
return null;

foreach (var attribute in attributes)
{
if (attribute.AttributeType != typeof(System.ComponentModel.TypeConverterAttribute))
continue;
var ctor = attribute.ConstructorArguments[0];
if (ctor.ArgumentType == typeof(string))
return Type.GetType((string)ctor.Value);
if (ctor.ArgumentType == typeof(Type))
return (Type)ctor.Value;
}

return null;
}

Expand Down Expand Up @@ -155,7 +199,7 @@ internal static object ConvertTo(this object value, Type toType, Func<TypeConver
}
catch (Exception e)
{
exception = e;
exception = e as XamlParseException ?? new XamlParseException($"Failed to retrieve TypeConverter: {e.Message}", serviceProvider, e);
return null;
}
try
Expand All @@ -171,19 +215,6 @@ internal static object ConvertTo(this object value, Type toType, Func<TypeConver
return null;
}

if (converter != null)
{
try
{
return converter.ConvertFromInvariantString(str);
}
catch (Exception e)
{
exception = new XamlParseException("Type conversion failed", serviceProvider, e);
return null;
}
}

var ignoreCase = (serviceProvider?.GetService(typeof(IConverterOptions)) as IConverterOptions)?.IgnoreCase ?? false;

//If the type is nullable, as the value is not null, it's safe to assume we want the built-in conversion
Expand Down
6 changes: 4 additions & 2 deletions src/Controls/src/Core/Xaml/ValueConverterProvider.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.ComponentModel;

using System.Reflection;

using Microsoft.Maui.Controls;
Expand All @@ -9,9 +11,9 @@ namespace Microsoft.Maui.Controls.Xaml
{
class ValueConverterProvider : IValueConverterProvider
{
public object Convert(object value, Type toType, Func<MemberInfo> minfoRetriever, IServiceProvider serviceProvider)
public object Convert(object value, Type toType, Func<TypeConverter> getTypeConverter, IServiceProvider serviceProvider)
{
var ret = value.ConvertTo(toType, minfoRetriever, serviceProvider, out Exception exception);
var ret = value.ConvertTo(toType, getTypeConverter, serviceProvider, out Exception exception);
if (exception != null)
{
var lineInfo = (serviceProvider.GetService(typeof(IXmlLineInfoProvider)) is IXmlLineInfoProvider lineInfoProvider) ? lineInfoProvider.XmlLineInfo : new XmlLineInfo();
Expand Down
Loading

0 comments on commit b8ec9fb

Please sign in to comment.